On Fri, 8 Nov 2024 19:36:34 GMT, Artur Barashev <abaras...@openjdk.org> wrote:
>> The current syntax of the jdk.tls.disabledAlgorithms makes it difficult to >> disable algorithms that affect both the key exchange and authentication >> parts of a TLS cipher suite. For example, if you add "RSA" to the >> jdk.tls.disabledAlgorithms security property, it disables all cipher suites >> that use RSA, whether it is for key exchange or authentication. If you only >> want to disable cipher suites that use RSA for key exchange, the only >> workaround is to list the whole cipher suite name, so an exact match is >> done, but if there are many cipher suites that use that key exchange >> algorithm, this becomes cumbersome. > > Artur Barashev has updated the pull request incrementally with one additional > commit since the last revision: > > Ignore all regex characters but asterisk. Update test patterns. test/jdk/sun/security/ssl/CipherSuite/AbstractDisableCipherSuites.java line 81: > 79: return true; > 80: } > 81: } Not only check if there are disabled ciphers in the result but also check if there are any ciphers in the result. Otherwise an "overmotivated" filtering mechanism wipes away all ciphers undetected. test/jdk/sun/security/ssl/CipherSuite/AbstractDisableCipherSuites.java line 162: > 160: return true; > 161: } > 162: } (same as above) Not only check if there are disabled ciphers in the result but also check if there are any ciphers in the result. Otherwise an "overmotivated" filtering mechanism wipes away all ciphers undetected. test/jdk/sun/security/ssl/CipherSuite/TLSCipherSuiteWildCardMatchingDisablePartsOfCipherSuite.java line 44: > 42: private static final String SECURITY_PROPERTY = > "jdk.tls.disabledAlgorithms"; > 43: private static final String TEST_ALGORITHMS = > 44: "TLS_RSA_*, TLS_ECDH*WITH_AES_256_GCM_*, > TLS_*_anon_WITH_AES_*_SHA"; Maybe add another pattern that contains other regex-characters to test that it doesn't lead to an error and that the chars are handled as part of the pattern, e.g. `TLS_.*`. If it's interpreted as regex this should disable all ciphers (internally converted to `TLS_..*`. Handled "literally" it would only disable all ciphers starting with `TLS_.` (i.e. none). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1835031165 PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1835031450 PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1835031605
