On Wed, 30 Jul 2025 16:23:30 GMT, Sean Mullan <[email protected]> wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Address review comments from Sean and Tony.
>
> src/java.base/share/classes/sun/security/util/CryptoAlgorithmConstraints.java
> line 78:
>
>> 76: int idx = dk.indexOf(".");
>> 77: if (idx == -1) {
>> 78: debug("Remove invalid entry: " + dk);
>
> I think we should throw `IllegalArgumentException` on invalid syntax or
> algorithms that don't have an OID. The reason is that it could be very unsafe
> to ignore typos and such, because the user may still think that an algorithm
> is disabled when it is not.
Well, I see your concern and it's valid. However, quite a few algorithms do not
have OIDs as the java security standard names may not have an 1-to-1 mapping to
OID, or no OID defined at all. For example, none of `Keystore` type has a
corresponding OID. Also, in the case of `Cipher`, this is even more
complicated, e.g. `AES` OIDs are keysize-specific and `PBES2` cipher has one
OID but there are multiple algorithm names which includes additional
components/algorithms info (`PBEWithHmacSHA1AndAES_128`,
`PBEWithHmacSHA512/256AndAES_256`. Thus, we can't use whether there is an OID
to check for user typos. In addition, there could be algorithms which JDK does
not have an OID mapping as `KnownOIDs` usually doesn't cover algorithms that we
don't support. If we want to be stricter, I can change to error out if invalid
entry is detected instead of ignored. However, we can only validate against
syntax and perhaps reject unsupported services if desired. But the algorithm
part is really difficult
to validate.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26377#discussion_r2244590697
