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

Reply via email to