Enable standalone ACCP Ed25519 #438
Open
Conversation
This file contains 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
WillChilds-Klein
changed the title
WillChilds-Klein
force-pushed
the
eddsa-no-jdk-dep
branch
from
d60584a to
eb9a7d2
Compare
WillChilds-Klein
changed the title
WillChilds-Klein
changed the title
WillChilds-Klein
force-pushed
the
eddsa-no-jdk-dep
branch
from
b13f432 to
5b217fd
Compare
geedo0
previously approved these changes
Mar 7, 2025
| @@ -87,10 +87,12 @@ public static void setupParameters() throws Exception { | |||
|
|
|||
| for (String algorithm : ALGORITHMS) { | |||
| KeyPairGenerator kpg; | |||
| if (algorithm.startsWith("ML-DSA")) { | |||
| if (algorithm.startsWith("ML-DSA") | |||
| || (algorithm.startsWith("Ed") && TestUtil.getJavaVersion() < 15)) { | |||
There was a problem hiding this comment.
Bit of code smell here, I'm paranoid that matching on startsWith("Ed") is a bit too general.
There was a problem hiding this comment.
This is very fair. Since it's a maintainability issue and not a correctness issue, I'll do a follow-up PR to match algorithm more precisely.
alexw91
reviewed
Mar 7, 2025
tst/com/amazon/corretto/crypto/provider/test/RemoveDefaultProvidersTest.java
Outdated
Show resolved
Hide resolved
alexw91
approved these changes
Mar 7, 2025
brian-jarvis-aws
approved these changes
Mar 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue #, if available:
Description of changes:
Notes
PR #394 deliberately coupled ACCP's Ed25519 key generation to SunEC's KeyFactory in the hopes of increasing compatibility. However, this also prevents the use of Ed25519 in standalone ACCP. This PR provides a fallback mechanism to allow Ed25519 keys to be generated by standalone ACCP without relying on SunEC. If SunEC has a relevant KeyFactory registered, the old behavior is retained for backwards compatibility.
Conveniently, this fallback mechanism also allows us to provide Ed25519 support on JDK versions < 15.
Testing
We add a test that de- and re-registers default providers to prove Ed25519 signatures can be performed by standalone ACCP with an empty JCA registry.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.