On Sat, 9 Nov 2024 00:07:07 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:
>
> Set initial cache size
test/jdk/sun/security/ssl/CipherSuite/TLSCipherSuiteWildCardMatchingIllegalArgument.java
line 47:
> 45: * class. Thus, we need a separate test class each time we need to modify
> 46: * "jdk.tls.disabledAlgorithms" config value for testing.
> 47: */
Still nitpick-level and it might violate coding rules I'm not aware of:
Declaring `wildCardMatch` package visible should allow you to test that
methods's behaviour by simply calling it with fitting parameters in the other
test class. Pass the cache-Map as parameter instead of accessing the "global
variable" in the method and you can additionally test the caching mechanism.
test/jdk/sun/security/ssl/CipherSuite/TLSCipherSuiteWildCardMatchingIllegalArgument.java
line 71:
> 69: }
> 70: fail("No IllegalArgumentException was thrown");
> 71: }
And another nitpick ;-) Put the fail-call under `SSLContext.getInstance` within
the try/catch-block so you don't need to explictly return in the catch-block.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1835379521
PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1835381505
