From 0e369d21e25a6de07a5fb54b78dab0012add60c0 Mon Sep 17 00:00:00 2001
Date: Thu, 5 Nov 2020 19:36:14 +0800
Subject: [PATCH] 8223940: Private key not supported by chosen signature
 algorithm

Summary: TLS1.3: If a client prefers RSA-PSS, but the provider does not support it, it will fail as above rather than falling back on other providers.
LLT: N/A
Bug url: https://bugs.openjdk.java.net/browse/JDK-8223940
---
 .../sun/security/ssl/CertificateVerify.java        | 54 +++++++++---------
 .../sun/security/ssl/DHServerKeyExchange.java      | 30 +++++-----
 .../sun/security/ssl/ECDHServerKeyExchange.java    | 28 ++++------
 .../classes/sun/security/ssl/SignatureScheme.java  | 64 +++++++++++++++++-----
 4 files changed, 102 insertions(+), 74 deletions(-)

diff --git a/jdk/src/share/classes/sun/security/ssl/CertificateVerify.java b/jdk/src/share/classes/sun/security/ssl/CertificateVerify.java
index 323a7d995..409119c09 100644
--- a/jdk/src/share/classes/sun/security/ssl/CertificateVerify.java
+++ b/jdk/src/share/classes/sun/security/ssl/CertificateVerify.java
@@ -31,6 +31,7 @@ import java.security.*;
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.Locale;
+import java.util.Map;
 import sun.security.ssl.SSLHandshake.HandshakeMessage;
 import sun.security.ssl.X509Authentication.X509Credentials;
 import sun.security.ssl.X509Authentication.X509Possession;
@@ -585,30 +586,27 @@ final class CertificateVerify {
 
             // This happens in client side only.
             ClientHandshakeContext chc = (ClientHandshakeContext)context;
-            this.signatureScheme = SignatureScheme.getPreferableAlgorithm(
+            Map.Entry<SignatureScheme, Signature> schemeAndSigner =
+                    SignatureScheme.getSignerOfPreferableAlgorithm(
                     chc.peerRequestedSignatureSchemes,
                     x509Possession,
                     chc.negotiatedProtocol);
-            if (signatureScheme == null) {
+            if (schemeAndSigner == null) {
                 // Unlikely, the credentials generator should have
                 // selected the preferable signature algorithm properly.
                 throw chc.conContext.fatal(Alert.INTERNAL_ERROR,
-                    "No preferred signature algorithm for CertificateVerify");
+                    "No supported CertificateVerify signature algorithm for " +
+                    x509Possession.popPrivateKey.getAlgorithm() +
+                    "  key");
             }
 
+            this.signatureScheme = schemeAndSigner.getKey();
             byte[] temproary = null;
             try {
-                Signature signer =
-                    signatureScheme.getSignature(x509Possession.popPrivateKey);
+                Signature signer = schemeAndSigner.getValue();
                 signer.update(chc.handshakeHash.archived());
                 temproary = signer.sign();
-            } catch (NoSuchAlgorithmException |
-                    InvalidAlgorithmParameterException nsae) {
-                throw chc.conContext.fatal(Alert.INTERNAL_ERROR,
-                        "Unsupported signature algorithm (" +
-                        signatureScheme.name +
-                        ") used in CertificateVerify handshake message", nsae);
-            } catch (InvalidKeyException | SignatureException ikse) {
+            } catch (SignatureException ikse) {
                 throw chc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                         "Cannot produce CertificateVerify signature", ikse);
             }
@@ -668,7 +666,7 @@ final class CertificateVerify {
             this.signature = Record.getBytes16(m);
             try {
                 Signature signer =
-                    signatureScheme.getSignature(x509Credentials.popPublicKey);
+                    signatureScheme.getVerifier(x509Credentials.popPublicKey);
                 signer.update(shc.handshakeHash.archived());
                 if (!signer.verify(signature)) {
                     throw shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
@@ -897,17 +895,22 @@ final class CertificateVerify {
                 X509Possession x509Possession) throws IOException {
             super(context);
 
-            this.signatureScheme = SignatureScheme.getPreferableAlgorithm(
-                    context.peerRequestedSignatureSchemes,
-                    x509Possession,
-                    context.negotiatedProtocol);
-            if (signatureScheme == null) {
+            Map.Entry<SignatureScheme, Signature> schemeAndSigner =
+                     SignatureScheme.getSignerOfPreferableAlgorithm(
+                     context.peerRequestedSignatureSchemes,
+                     x509Possession,
+                     context.negotiatedProtocol);
+            if (schemeAndSigner == null) {
                 // Unlikely, the credentials generator should have
                 // selected the preferable signature algorithm properly.
                 throw context.conContext.fatal(Alert.INTERNAL_ERROR,
-                    "No preferred signature algorithm for CertificateVerify");
+                    "No supported CertificateVerify signature algorithm for " +
+                    x509Possession.popPrivateKey.getAlgorithm() +
+                    "  key");
             }
 
+            this.signatureScheme = schemeAndSigner.getKey();
+
             byte[] hashValue = context.handshakeHash.digest();
             byte[] contentCovered;
             if (context.sslConfig.isClientMode) {
@@ -924,17 +927,10 @@ final class CertificateVerify {
 
             byte[] temproary = null;
             try {
-                Signature signer =
-                    signatureScheme.getSignature(x509Possession.popPrivateKey);
+                Signature signer = schemeAndSigner.getValue();
                 signer.update(contentCovered);
                 temproary = signer.sign();
-            } catch (NoSuchAlgorithmException |
-                    InvalidAlgorithmParameterException nsae) {
-                throw context.conContext.fatal(Alert.INTERNAL_ERROR,
-                        "Unsupported signature algorithm (" +
-                        signatureScheme.name +
-                        ") used in CertificateVerify handshake message", nsae);
-            } catch (InvalidKeyException | SignatureException ikse) {
+            } catch (SignatureException ikse) {
                 throw context.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                         "Cannot produce CertificateVerify signature", ikse);
             }
@@ -1005,7 +1001,7 @@ final class CertificateVerify {
 
             try {
                 Signature signer =
-                    signatureScheme.getSignature(x509Credentials.popPublicKey);
+                    signatureScheme.getVerifier(x509Credentials.popPublicKey);
                 signer.update(contentCovered);
                 if (!signer.verify(signature)) {
                     throw context.conContext.fatal(Alert.HANDSHAKE_FAILURE,
diff --git a/jdk/src/share/classes/sun/security/ssl/DHServerKeyExchange.java b/jdk/src/share/classes/sun/security/ssl/DHServerKeyExchange.java
index 48ca870ed..3245482d7 100644
--- a/jdk/src/share/classes/sun/security/ssl/DHServerKeyExchange.java
+++ b/jdk/src/share/classes/sun/security/ssl/DHServerKeyExchange.java
@@ -42,6 +42,7 @@ import java.security.SignatureException;
 import java.text.MessageFormat;
 import java.util.EnumSet;
 import java.util.Locale;
+import java.util.Map;
 import javax.crypto.interfaces.DHPublicKey;
 import javax.crypto.spec.DHParameterSpec;
 import javax.crypto.spec.DHPublicKeySpec;
@@ -125,24 +126,21 @@ final class DHServerKeyExchange {
                         shc.negotiatedProtocol.useTLS12PlusSpec();
                 Signature signer = null;
                 if (useExplicitSigAlgorithm) {
-                    signatureScheme = SignatureScheme.getPreferableAlgorithm(
-                            shc.peerRequestedSignatureSchemes,
-                            x509Possession,
-                            shc.negotiatedProtocol);
-                    if (signatureScheme == null) {
+                    Map.Entry<SignatureScheme, Signature> schemeAndSigner =
+                            SignatureScheme.getSignerOfPreferableAlgorithm(
+                                    shc.peerRequestedSignatureSchemes,
+                                    x509Possession,
+                                    shc.negotiatedProtocol);
+                    if (schemeAndSigner == null) {
                         // Unlikely, the credentials generator should have
                         // selected the preferable signature algorithm properly.
                         throw shc.conContext.fatal(Alert.INTERNAL_ERROR,
-                            "No preferred signature algorithm");
-                    }
-                    try {
-                        signer = signatureScheme.getSignature(
-                                x509Possession.popPrivateKey);
-                    } catch (NoSuchAlgorithmException | InvalidKeyException |
-                            InvalidAlgorithmParameterException nsae) {
-                        throw shc.conContext.fatal(Alert.INTERNAL_ERROR,
-                            "Unsupported signature algorithm: " +
-                            signatureScheme.name, nsae);
+                                "No supported signature algorithm for " +
+                                x509Possession.popPrivateKey.getAlgorithm() +
+                                "  key");
+                    } else {
+                        signatureScheme = schemeAndSigner.getKey();
+                        signer = schemeAndSigner.getValue();
                     }
                 } else {
                     signatureScheme = null;
@@ -241,7 +239,7 @@ final class DHServerKeyExchange {
             Signature signer;
             if (useExplicitSigAlgorithm) {
                 try {
-                    signer = signatureScheme.getSignature(
+                    signer = signatureScheme.getVerifier(
                             x509Credentials.popPublicKey);
                 } catch (NoSuchAlgorithmException | InvalidKeyException |
                         InvalidAlgorithmParameterException nsae) {
diff --git a/jdk/src/share/classes/sun/security/ssl/ECDHServerKeyExchange.java b/jdk/src/share/classes/sun/security/ssl/ECDHServerKeyExchange.java
index 8c23e3546..2e1cc02c9 100644
--- a/jdk/src/share/classes/sun/security/ssl/ECDHServerKeyExchange.java
+++ b/jdk/src/share/classes/sun/security/ssl/ECDHServerKeyExchange.java
@@ -45,6 +45,7 @@ import java.security.spec.InvalidKeySpecException;
 import java.text.MessageFormat;
 import java.util.EnumSet;
 import java.util.Locale;
+import java.util.Map;
 import sun.security.ssl.ECDHKeyExchange.ECDHECredentials;
 import sun.security.ssl.ECDHKeyExchange.ECDHEPossession;
 import sun.security.ssl.SSLHandshake.HandshakeMessage;
@@ -139,26 +140,21 @@ final class ECDHServerKeyExchange {
                         shc.negotiatedProtocol.useTLS12PlusSpec();
                 Signature signer = null;
                 if (useExplicitSigAlgorithm) {
-                    signatureScheme = SignatureScheme.getPreferableAlgorithm(
-                            shc.peerRequestedSignatureSchemes,
-                            x509Possession,
-                            shc.negotiatedProtocol);
-                    if (signatureScheme == null) {
+                    Map.Entry<SignatureScheme, Signature> schemeAndSigner =
+                            SignatureScheme.getSignerOfPreferableAlgorithm(
+                                shc.peerRequestedSignatureSchemes,
+                                x509Possession,
+                                shc.negotiatedProtocol);
+                    if (schemeAndSigner == null) {
                         // Unlikely, the credentials generator should have
                         // selected the preferable signature algorithm properly.
                         throw shc.conContext.fatal(Alert.INTERNAL_ERROR,
-                                "No preferred signature algorithm for " +
+                                "No supported signature algorithm for " +
                                 x509Possession.popPrivateKey.getAlgorithm() +
                                 "  key");
-                    }
-                    try {
-                        signer = signatureScheme.getSignature(
-                                x509Possession.popPrivateKey);
-                    } catch (NoSuchAlgorithmException | InvalidKeyException |
-                            InvalidAlgorithmParameterException nsae) {
-                        throw shc.conContext.fatal(Alert.INTERNAL_ERROR,
-                            "Unsupported signature algorithm: " +
-                            signatureScheme.name, nsae);
+                    } else {
+                        signatureScheme = schemeAndSigner.getKey();
+                        signer = schemeAndSigner.getValue();
                     }
                 } else {
                     signatureScheme = null;
@@ -295,7 +291,7 @@ final class ECDHServerKeyExchange {
             Signature signer;
             if (useExplicitSigAlgorithm) {
                 try {
-                    signer = signatureScheme.getSignature(
+                    signer = signatureScheme.getVerifier(
                             x509Credentials.popPublicKey);
                 } catch (NoSuchAlgorithmException | InvalidKeyException |
                         InvalidAlgorithmParameterException nsae) {
diff --git a/jdk/src/share/classes/sun/security/ssl/SignatureScheme.java b/jdk/src/share/classes/sun/security/ssl/SignatureScheme.java
index 93f5473da..9fd9707ba 100644
--- a/jdk/src/share/classes/sun/security/ssl/SignatureScheme.java
+++ b/jdk/src/share/classes/sun/security/ssl/SignatureScheme.java
@@ -31,6 +31,7 @@ import java.security.spec.AlgorithmParameterSpec;
 import java.security.spec.ECParameterSpec;
 import java.security.spec.MGF1ParameterSpec;
 import java.security.spec.PSSParameterSpec;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -38,6 +39,7 @@ import java.util.Collections;
 import java.util.EnumSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import sun.security.ssl.SupportedGroupsExtension.NamedGroup;
 import sun.security.ssl.SupportedGroupsExtension.NamedGroupType;
@@ -427,7 +429,7 @@ enum SignatureScheme {
         return null;
     }
 
-    static SignatureScheme getPreferableAlgorithm(
+    static Map.Entry<SignatureScheme, Signature> getSignerOfPreferableAlgorithm(
             List<SignatureScheme> schemes,
             X509Possession x509Possession,
             ProtocolVersion version) {
@@ -452,7 +454,10 @@ enum SignatureScheme {
                             x509Possession.getECParameterSpec();
                     if (params != null &&
                             ss.namedGroup == NamedGroup.valueOf(params)) {
-                        return ss;
+                        Signature signer = ss.getSigner(signingKey);
+                        if (signer != null) {
+                            return new SimpleImmutableEntry<>(ss, signer);
+                        }
                     }
 
                     if (SSLLogger.isOn &&
@@ -477,7 +482,10 @@ enum SignatureScheme {
                         NamedGroup keyGroup = NamedGroup.valueOf(params);
                         if (keyGroup != null &&
                                 SupportedGroups.isSupported(keyGroup)) {
-                            return ss;
+                            Signature signer = ss.getSigner(signingKey);
+                            if (signer != null) {
+                                return new SimpleImmutableEntry<>(ss, signer);
+                            }
                         }
                     }
 
@@ -488,7 +496,10 @@ enum SignatureScheme {
                             "), unsupported EC parameter spec: " + params);
                     }
                 } else {
-                    return ss;
+                    Signature signer = ss.getSigner(signingKey);
+                    if (signer != null) {
+                        return new SimpleImmutableEntry<>(ss, signer);
+                    }
                 }
             }
         }
@@ -509,21 +520,48 @@ enum SignatureScheme {
         return new String[0];
     }
 
-    Signature getSignature(Key key) throws NoSuchAlgorithmException,
+    // This method is used to get the signature instance of this signature
+    // scheme for the specific public key.  Unlike getSigner(), the exception
+    // is bubbled up.  If the public key does not support this signature
+    // scheme, it normally means the TLS handshaking cannot continue and
+    // the connection should be terminated.
+    Signature getVerifier(PublicKey publicKey) throws NoSuchAlgorithmException,
             InvalidAlgorithmParameterException, InvalidKeyException {
         if (!isAvailable) {
             return null;
         }
 
-        Signature signer = JsseJce.getSignature(algorithm);
-        if (key instanceof PublicKey) {
-            SignatureUtil.initVerifyWithParam(signer, (PublicKey)key,
-                    signAlgParameter);
-        } else {
-            SignatureUtil.initSignWithParam(signer, (PrivateKey)key,
-                    signAlgParameter, null);
+        Signature verifier = Signature.getInstance(algorithm);
+        SignatureUtil.initVerifyWithParam(verifier, publicKey, signAlgParameter);
+
+        return verifier;
+    }
+
+    // This method is also used to choose preferable signature scheme for the
+    // specific private key.  If the private key does not support the signature
+    // scheme, {@code null} is returned, and the caller may fail back to next
+    // available signature scheme.
+    private Signature getSigner(PrivateKey privateKey) {
+        if (!isAvailable) {
+            return null;
         }
 
-        return signer;
+        try {
+            Signature signer = Signature.getInstance(algorithm);
+            SignatureUtil.initSignWithParam(signer, privateKey,
+                signAlgParameter,
+                null);
+            return signer;
+        } catch (NoSuchAlgorithmException | InvalidKeyException |
+                InvalidAlgorithmParameterException nsae) {
+            if (SSLLogger.isOn &&
+                    SSLLogger.isOn("ssl,handshake,verbose")) {
+                SSLLogger.finest(
+                    "Ignore unsupported signature algorithm (" +
+                    this.name + ")", nsae);
+            }
+        }
+
+        return null;
     }
 }
-- 
2.12.3

