On Thu, 10 Sep 2026 16:30:41 GMT, Artur Barashev <[email protected]> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updated with Artur's comment
>
> src/java.base/share/classes/sun/security/ssl/CertificateMessage.java line
> 1041:
>
>> 1039: SSLLogger.warning("No available authentication
>> scheme");
>> 1040: }
>> 1041: } else if (pos instanceof X509Possession x509Possession &&
>
> Why we fail earlier here now? How is it related to ML-DSA signature scheme?
In certificate selection path, after X509Authentication.createPossession()
returns a cert/key, this is to verify that it can actually produce a valid
CertificateVerify signer for the peer requested signature schemes (make sure
parameter set check after cert selection).
This change is validated by the new tests: MLDSACertSelection.java for
test.case=fail44, and MLDSAClientAuthMismatch.java for test
case=test.case=failClientAuthMismatch. Mach5 tier1-tier3 tests also passed.
> src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 553:
>
>> 551: keyAlgorithm.equalsIgnoreCase(ss.keyAlgorithm) &&
>> 552: ss.isAllowed(constraints, version,
>> HANDSHAKE_SCOPE)) {
>> 553: if (ss.hasNamedParam()) {
>
> I find correct solution somewhat fragile:
>
> - `AsymmetricKey.getParams()` that is being called in
> `KeyUtil.getAlgorithm()` is optional and defaults to `null` - that can be the
> case for 3rd party provider.
> - Hard-coding ML-DSA and EdDSA as the only algorithms that can have named
> parameters instead of testing the key directly.
>
> I suggest the following solution instead:
>
> Suggestion:
>
> if (signingKey.getParams() instanceof NamedParameterSpec nps
> && !ss.algorithm.equalsIgnoreCase(nps.getName())) {
> if (SSLLogger.isOn() &&
> SSLLogger.isOn(
> SSLLogger.Opt.HANDSHAKE_VERBOSE)) {
> SSLLogger.finest(
> "Ignore the signature algorithm (" + ss +
> "), unsupported named parameter: " +
> nps.getName());
> }
> continue;
> }
Updated. Thanks for the suggestion.
> src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 628:
>
>> 626: }
>> 627:
>> 628: private boolean hasNamedParam() {
>
> I suggest to remove this method.
Removed.
> src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 677:
>
>> 675: }
>> 676:
>> 677: if (hasNamedParam()) {
>
> Suggestion:
>
> if (publicKey.getParams() instanceof NamedParameterSpec nps
> && !algorithm.equalsIgnoreCase(nps.getName())) {
> throw new InvalidKeyException("Unsupported named parameter: " +
> nps.getName());
> }
Updated as suggested.
> src/java.base/share/classes/sun/security/ssl/X509Authentication.java line 367:
>
>> 365:
>> 366: PublicKey serverPublicKey = serverCerts[0].getPublicKey();
>> 367: if (!shc.negotiatedProtocol.useTLS13PlusSpec() &&
>
> This change looks redundant: restricting signature schemes by the protocol
> version is done in `SignatureScheme.java`
Removed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32529#discussion_r3983959624
PR Review Comment: https://git.openjdk.org/jdk/pull/32529#discussion_r3983958766
PR Review Comment: https://git.openjdk.org/jdk/pull/32529#discussion_r3983959046
PR Review Comment: https://git.openjdk.org/jdk/pull/32529#discussion_r3983959363
PR Review Comment: https://git.openjdk.org/jdk/pull/32529#discussion_r3983959863
