RFR: 8346129: Simplify EdDSA & XDH curve name usage [v4]

[Anthony Scarpino]

On Fri, 7 Mar 2025 19:07:16 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   rename getNamedCurveFromKey
>
> src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java line 274:
> 
>> 272:                     yield List.of();
>> 273:                 }
>> 274:                 yield List.of(nc.getNameAndAliases());
> 
> Do you want to add `EC` itself to the list? I am asking because for EdDSA you added both the algorithm name and the parameter set name.

No, "EC" isn't an alias for a particular curve.

> src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java line 276:
> 
>> 274:                 yield List.of(nc.getNameAndAliases());
>> 275:             }
>> 276:             default -> List.of(key.getAlgorithm(), KeyUtil.getAlgorithm(key));
> 
> What if these 2 are the same string?

Are you concerned about duplicate checks?

> test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java line 86:
> 
>> 84:                 Arrays.asList(
>> 85:                     new TestCase("EdDSA", false),
>> 86:                     new TestCase("Ed25519", true),
> 
> Why should the above pass? If you disable `EdDSA` and you are still allowed `Signature.getInstance("Ed25519")`? If this is because it will reject whatever EdDSA key later? Why both check `CryptoPrimitive.SIGNATURE` at all?

I'm confused by this comment.  With removing the hardcoded aliases in AbstractAlgorithmConstraints, which is what I thought you had suggested, EdDSA and Ed25519 are now separate as the check is effectively a string compare check against the disabledAlgorithm list

The second half of that case statement has a key that can check against both EdDSA and the NPS.

With respect to `CryptoPrimitive.SIGNATURE`, it just a value used in the test, it can't be null.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23647#discussion_r1985768201
PR Review Comment: https://git.openjdk.org/jdk/pull/23647#discussion_r1985768272
PR Review Comment: https://git.openjdk.org/jdk/pull/23647#discussion_r1985768304