On Wed, 30 Jul 2025 07:25:57 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> This enhancement introduces a new security property
>> "jdk.crypto.disabledAlgorithms" which can be leveraged to disable algorithms
>> for JCE/JCA crypto services. For now, only Cipher, KeyStore, MessageDigest,
>> and Signature services support this new security property. The support can
>> be expanded later to cover more services if needed. Note that this security
>> property is meant to disable algorithms irrespective of providers. If the
>> algorithm is found to be disabled, it will be rejected before reaching out
>> to provider(s) for the corresponding implementation(s).
>>
>> A few implementation notes:
>> 1) The specified security property value is lazily loaded and all changes
>> after it's been loaded are ignored. Invalid entries, e.g. wrong syntax, are
>> ignored and removed. The algorithm name check is case-insensitive. If a
>> disabled algorithm is known to has an object identifier (oid) by JDK, this
>> oid and its aliases is also added to the disabled services.
>> 2) The algorithm name checking impl is based on the
>> sun.security.util.AlgorithmConstraints class, but without the decomposing
>> and different constraints.
>> 3) The hardwiring of NONEwithRSA signature to RSA/ECB/PKCS1Padding cipher in
>> java.security.Signature class is removed. Instead, this is moved to the
>> provider level, i.e. SunJCE and SunPKCS11 provider are changed to claim the
>> NONEwithRSA signature support. Disabling one will not affect the other.
>>
>> CSR will be filed once the review is wrapping up.
>>
>> Thanks~
>> Valerie
>
> 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.
src/java.base/share/classes/sun/security/util/CryptoAlgorithmConstraints.java
line 83:
> 81: }
> 82: String service = dk.substring(0, idx);
> 83: String algo = dk.substring(idx + 1);
You should check for invalid syntax such as ".algo" or "service."
src/java.base/share/classes/sun/security/util/CryptoAlgorithmConstraints.java
line 110:
> 108: }
> 109:
> 110: return cachedCheckAlgorithm(serviceDesc);
Is this doing a case-insensitive check? I was trying to figure that out, but I
think "md2" should still match "MD2" for example.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26377#discussion_r2243237992
PR Review Comment: https://git.openjdk.org/jdk/pull/26377#discussion_r2243102379
PR Review Comment: https://git.openjdk.org/jdk/pull/26377#discussion_r2243757403
