On Fri, 21 Feb 2025 21:11:54 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:
>> src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java >> line 78: >> >>> 76: private static List<String> aliasEd25519 = null; >>> 77: private static List<String> aliasXDH = null; >>> 78: private static List<String> aliasX25519 = null; >> >> I am a little suspicious in this approach. At least this means for each >> "family" algorithm name like "EdDSA", we need to hardcode all its parameter >> set names here. Sounds not very sustainable. >> >> An EdDSA key always has its `getAlgorithm` being "EdDSA" (at least inside >> SunEC) and its `getParams()` being the parameter set name. So it looks like >> it's enough if we do a name comparison on both. >> >> Also, why no `aliasEd448` and `aliasX448` here? > > I have to give more thought on checking the algorithm and the `getParams()` > against the list. That may eliminate the need for the hardcoded list.. > > As to why 448 curves didn't need an alias, there is no other way to specify > those curves other than their given name, like mentioned with the KPG/Ed25519 > example in my comment to Sean So using the `getAlgorithm()` and `getParams().getName()` work because there is a Key, but not for non-key situation such as `permits(Set.of(CryptoPrimitive.SIGNATURE), "EdDSA", null)`. But this does bring up a point I had not considered. The `permits()` call does not specify any key details other than the family name. Perhaps it's ok for `permits()` to return a passing result with the information given. But for a `permit()` that had more detail, it could return a failing result. However, the KPG example does make me think that the consistency in the current PR is better. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23647#discussion_r1966265139