-
Notifications
You must be signed in to change notification settings - Fork 17
Added ACCP for ECDH for client generate #2276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
390e539
Added ACCP Provider and ecdh caching
RSam25 6139317
Moved crypto code to a service and updated ACCP
RSam25 7d2de01
[CI Pipeline] Released Snapshot version: 5.63.6-alpha-280-SNAPSHOT
e14fc39
Load in ACCP provider
RSam25 0afcc43
Fixed incorrect imports
RSam25 e530d6c
[CI Pipeline] Released Snapshot version: 5.63.7-alpha-281-SNAPSHOT
b3e8b8b
Reverted Dockerfile to remove JFR flags
RSam25 d210d41
Added macos binaries for ACCP
RSam25 0bb6890
Removed threadlocal caching and added CryptoProvider to tests
RSam25 3b284b8
Merge remote-tracking branch 'origin/main' into srm-UID2-6479-change-…
RSam25 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
src/main/java/com/uid2/operator/service/CryptoProviderService.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package com.uid2.operator.service; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import com.amazon.corretto.crypto.provider.AmazonCorrettoCryptoProvider; | ||
|
|
||
| import javax.crypto.KeyAgreement; | ||
| import java.security.NoSuchAlgorithmException; | ||
| import java.security.NoSuchProviderException; | ||
| import java.security.Security; | ||
|
|
||
| public class CryptoProviderService { | ||
| private static final Logger LOGGER = LoggerFactory.getLogger(CryptoProviderService.class); | ||
|
|
||
| // ECDH provider selection: tries ACCP first, falls back to default (SunEC) | ||
| private static final String ECDH_PROVIDER_NAME = initEcdhProvider(); | ||
|
|
||
| private static String initEcdhProvider() { | ||
| // Try ACCP (Amazon Corretto Crypto Provider) first | ||
| try { | ||
| // Add ACCP at lowest priority so it doesn't become default for other algorithms | ||
| Security.addProvider(AmazonCorrettoCryptoProvider.INSTANCE); | ||
|
|
||
| // Verify it works for ECDH | ||
| KeyAgreement ka = KeyAgreement.getInstance("ECDH", AmazonCorrettoCryptoProvider.PROVIDER_NAME); | ||
| LOGGER.info("ECDH using AmazonCorrettoCryptoProvider (added at lowest priority)"); | ||
| return AmazonCorrettoCryptoProvider.PROVIDER_NAME; | ||
| } catch (Throwable e) { | ||
| // ACCP not available | ||
| LOGGER.info("AmazonCorrettoCryptoProvider is not available: {}", e.getMessage()); | ||
| } | ||
|
|
||
| // Fall back to default provider | ||
| LOGGER.info("ECDH using default provider (SunEC)"); | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Create ECDH Key Agreement using ACCP if available, fall back to SunEC if not | ||
| * @return ECDH KeyAgreement | ||
| * @throws NoSuchAlgorithmException | ||
| */ | ||
| public static KeyAgreement createKeyAgreement() throws NoSuchAlgorithmException { | ||
| if (ECDH_PROVIDER_NAME != null) { | ||
| try { | ||
| return KeyAgreement.getInstance("ECDH", ECDH_PROVIDER_NAME); | ||
| } catch (NoSuchProviderException e) { | ||
| LOGGER.info("{} is not available: {}", ECDH_PROVIDER_NAME, e.getMessage()); | ||
| } | ||
| } | ||
| return KeyAgreement.getInstance("ECDH"); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just call initEcdhProvider in createKeyAgreement
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to ensure the provider is registered once globally. Calling initECDHProvider in createKeyAgreement would try to register it for every thread that tries to access it. While Security.addProvider is idempotent, I didn't want to repeat that operation unnecessarily, especially since if ACCP is not available for some reason, it would throw an exception (which is caught) that would unwind the stack and introduce some overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling initECDHProvider once vs 10 times is a bit of a microoptimisation which sacrifices readability, for example, I had to read twice to understand why this was a static variable, if it was used anywhere else, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. However, I removed ThreadLocal caching due to the issue Andrei pointed out and now it is important we don't throw an exception every time if ACCP doesn't load for some reason.