8225766: Curve in certificate should not affect signature scheme when using TLSv1.3

Reviewed-by: ascarpino

src/java.base/share/classes/sun/security/ssl/SignatureScheme.java

@@ -39,6 +39,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 import sun.security.ssl.NamedGroup.NamedGroupType;
+import sun.security.ssl.SupportedGroupsExtension.SupportedGroups;
 import sun.security.ssl.X509Authentication.X509Possession;
 import sun.security.util.KeyUtil;
 import sun.security.util.SignatureUtil;
@@ -440,6 +441,39 @@ enum SignatureScheme {
                             ss.namedGroup == NamedGroup.valueOf(params)) {
                         return ss;
                     }
+
+                    if (SSLLogger.isOn &&
+                            SSLLogger.isOn("ssl,handshake,verbose")) {
+                        SSLLogger.finest(
+                            "Ignore the signature algorithm (" + ss +
+                            "), unsupported EC parameter spec: " + params);
+                    }
+                } else if ("EC".equals(ss.keyAlgorithm)) {
+                    // Must be a legacy signature algorithm, which does not
+                    // specify the associated named groups.  The connection
+                    // cannot be established if the peer cannot recognize
+                    // the named group used for the signature.  RFC 8446
+                    // does not define countermeasures for the corner cases.
+                    // In order to mitigate the impact, we choose to check
+                    // against the local supported named groups.  The risk
+                    // should be minimal as applications should not use
+                    // unsupported named groups for its certificates.
+                    ECParameterSpec params =
+                            x509Possession.getECParameterSpec();
+                    if (params != null) {
+                        NamedGroup keyGroup = NamedGroup.valueOf(params);
+                        if (keyGroup != null &&
+                                SupportedGroups.isSupported(keyGroup)) {
+                            return ss;
+                        }
+                    }
+
+                    if (SSLLogger.isOn &&
+                            SSLLogger.isOn("ssl,handshake,verbose")) {
+                        SSLLogger.finest(
+                            "Ignore the legacy signature algorithm (" + ss +
+                            "), unsupported EC parameter spec: " + params);
+                    }
                 } else {
                     return ss;
                 }

src/java.base/share/classes/sun/security/ssl/X509Authentication.java

@@ -69,7 +69,7 @@ enum X509Authentication implements SSLAuthentication {
     final String keyType;
     final SSLPossessionGenerator possessionGenerator;
 
-    X509Authentication(String keyType,
+    private X509Authentication(String keyType,
             SSLPossessionGenerator possessionGenerator) {
         this.keyType = keyType;
         this.possessionGenerator = possessionGenerator;
@@ -326,10 +326,12 @@ enum X509Authentication implements SSLAuthentication {
                 return null;
             }
 
-            // For ECC certs, check whether we support the EC domain
+            // For TLS 1.2 and prior versions, the public key of a EC cert
-            // parameters.  If the client sent a SupportedEllipticCurves
+            // MUST use a curve and point format supported by the client.
-            // ClientHello extension, check against that too.
+            // But for TLS 1.3, signature algorithms are negotiated
-            if (keyType.equals("EC")) {
+            // independently via the "signature_algorithms" extension.
+            if (!shc.negotiatedProtocol.useTLS13PlusSpec() &&
+                    keyType.equals("EC")) {
                 if (!(serverPublicKey instanceof ECPublicKey)) {
                     if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
                         SSLLogger.warning(serverAlias +
@@ -339,8 +341,9 @@ enum X509Authentication implements SSLAuthentication {
                 }
 
                 // For ECC certs, check whether we support the EC domain
-                // parameters. If the client sent a SupportedEllipticCurves
+                // parameters.  If the client sent a supported_groups
-                // ClientHello extension, check against that too.
+                // ClientHello extension, check against that too for
+                // TLS 1.2 and prior versions.
                 ECParameterSpec params =
                         ((ECPublicKey)serverPublicKey).getParams();
                 NamedGroup namedGroup = NamedGroup.valueOf(params);

test/jdk/sun/security/ssl/SignatureScheme/Tls13NamedGroups.java

@@ -0,0 +1,188 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+//
+// SunJSSE does not support dynamic system properties, no way to re-use
+// system properties in samevm/agentvm mode.
+//
+
+/*
+ * @test
+ * @bug 8225766
+ * @summary Curve in certificate should not affect signature scheme
+ *          when using TLSv1.3
+ * @library /javax/net/ssl/templates
+ * @run main/othervm Tls13NamedGroups
+ */
+
+import java.net.*;
+import java.io.*;
+import javax.net.ssl.*;
+import java.security.*;
+import java.security.cert.*;
+import java.security.spec.*;
+import java.security.interfaces.*;
+import java.util.Base64;
+
+public class Tls13NamedGroups extends SSLSocketTemplate {
+
+    public static void main(String[] args) throws Exception {
+        // Limit the supported named group to secp521r1.
+        System.setProperty("jdk.tls.namedGroups", "secp521r1");
+
+        new Tls13NamedGroups().run();
+    }
+
+    @Override
+    protected SSLContext createServerSSLContext() throws Exception {
+        return generateSSLContext();
+    }
+
+    @Override
+    protected void configureServerSocket(SSLServerSocket socket) {
+        socket.setNeedClientAuth(true);
+    }
+
+    @Override
+    protected SSLContext createClientSSLContext() throws Exception {
+        return generateSSLContext();
+    }
+
+    /*
+     * =============================================================
+     * The remainder is just support stuff
+     */
+
+    // Certificates and key used in the test.
+    //
+    // Trusted Certificate.
+    static String trustedCertStr =
+        // SHA256withECDSA, curve prime256v1
+        // Validity
+        //     Not Before: May 22 07:18:16 2018 GMT
+        //     Not After : May 17 07:18:16 2038 GMT
+        // Subject Key Identifier:
+        //     60:CF:BD:73:FF:FA:1A:30:D2:A4:EC:D3:49:71:46:EF:1A:35:A0:86
+        "-----BEGIN CERTIFICATE-----\n" +
+        "MIIBvjCCAWOgAwIBAgIJAIvFG6GbTroCMAoGCCqGSM49BAMCMDsxCzAJBgNVBAYT\n" +
+        "AlVTMQ0wCwYDVQQKDARKYXZhMR0wGwYDVQQLDBRTdW5KU1NFIFRlc3QgU2VyaXZj\n" +
+        "ZTAeFw0xODA1MjIwNzE4MTZaFw0zODA1MTcwNzE4MTZaMDsxCzAJBgNVBAYTAlVT\n" +
+        "MQ0wCwYDVQQKDARKYXZhMR0wGwYDVQQLDBRTdW5KU1NFIFRlc3QgU2VyaXZjZTBZ\n" +
+        "MBMGByqGSM49AgEGCCqGSM49AwEHA0IABBz1WeVb6gM2mh85z3QlvaB/l11b5h0v\n" +
+        "LIzmkC3DKlVukZT+ltH2Eq1oEkpXuf7QmbM0ibrUgtjsWH3mULfmcWmjUDBOMB0G\n" +
+        "A1UdDgQWBBRgz71z//oaMNKk7NNJcUbvGjWghjAfBgNVHSMEGDAWgBRgz71z//oa\n" +
+        "MNKk7NNJcUbvGjWghjAMBgNVHRMEBTADAQH/MAoGCCqGSM49BAMCA0kAMEYCIQCG\n" +
+        "6wluh1r2/T6L31mZXRKf9JxeSf9pIzoLj+8xQeUChQIhAJ09wAi1kV8yePLh2FD9\n" +
+        "2YEHlSQUAbwwqCDEVB5KxaqP\n" +
+        "-----END CERTIFICATE-----";
+        // -----BEGIN PRIVATE KEY-----
+        // MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg/HcHdoLJCdq3haVd
+        // XZTSKP00YzM3xX97l98vGL/RI1KhRANCAAQc9VnlW+oDNpofOc90Jb2gf5ddW+Yd
+        // LyyM5pAtwypVbpGU/pbR9hKtaBJKV7n+0JmzNIm61ILY7Fh95lC35nFp
+        // -----END PRIVATE KEY-----
+
+    // End entity certificate.
+    static String targetCertStr =
+        // SHA256withECDSA, curve prime256v1
+        // Validity
+        //     Not Before: May 22 07:18:16 2018 GMT
+        //     Not After : May 17 07:18:16 2038 GMT
+        // Authority Key Identifier:
+        //     60:CF:BD:73:FF:FA:1A:30:D2:A4:EC:D3:49:71:46:EF:1A:35:A0:86
+        "-----BEGIN CERTIFICATE-----\n" +
+        "MIIBqjCCAVCgAwIBAgIJAPLY8qZjgNRAMAoGCCqGSM49BAMCMDsxCzAJBgNVBAYT\n" +
+        "AlVTMQ0wCwYDVQQKDARKYXZhMR0wGwYDVQQLDBRTdW5KU1NFIFRlc3QgU2VyaXZj\n" +
+        "ZTAeFw0xODA1MjIwNzE4MTZaFw0zODA1MTcwNzE4MTZaMFUxCzAJBgNVBAYTAlVT\n" +
+        "MQ0wCwYDVQQKDARKYXZhMR0wGwYDVQQLDBRTdW5KU1NFIFRlc3QgU2VyaXZjZTEY\n" +
+        "MBYGA1UEAwwPUmVncmVzc2lvbiBUZXN0MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcD\n" +
+        "QgAEb+9n05qfXnfHUb0xtQJNS4JeSi6IjOfW5NqchvKnfJey9VkJzR7QHLuOESdf\n" +
+        "xlR7q8YIWgih3iWLGfB+wxHiOqMjMCEwHwYDVR0jBBgwFoAUYM+9c//6GjDSpOzT\n" +
+        "SXFG7xo1oIYwCgYIKoZIzj0EAwIDSAAwRQIgWpRegWXMheiD3qFdd8kMdrkLxRbq\n" +
+        "1zj8nQMEwFTUjjQCIQDRIrAjZX+YXHN9b0SoWWLPUq0HmiFIi8RwMnO//wJIGQ==\n" +
+        "-----END CERTIFICATE-----";
+
+    // Private key in the format of PKCS#8.
+    static String targetPrivateKey =
+        //
+        // EC private key related to cert endEntityCertStrs[0].
+        //
+        "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgn5K03bpTLjEtFQRa\n" +
+        "JUtx22gtmGEvvSUSQdimhGthdtihRANCAARv72fTmp9ed8dRvTG1Ak1Lgl5KLoiM\n" +
+        "59bk2pyG8qd8l7L1WQnNHtAcu44RJ1/GVHurxghaCKHeJYsZ8H7DEeI6";
+
+    static char passphrase[] = "passphrase".toCharArray();
+
+    // Create the SSLContext instance.
+    private static SSLContext generateSSLContext() throws Exception {
+
+        // generate certificate from cert string
+        CertificateFactory cf = CertificateFactory.getInstance("X.509");
+
+        // create a key store
+        KeyStore ks = KeyStore.getInstance("JKS");
+        ks.load(null, null);
+
+        // import the trused cert
+        X509Certificate trusedCert = null;
+        ByteArrayInputStream is =
+                new ByteArrayInputStream(trustedCertStr.getBytes());
+        trusedCert = (X509Certificate)cf.generateCertificate(is);
+        is.close();
+
+        ks.setCertificateEntry("Trusted EC Signer", trusedCert);
+
+        // generate the private key.
+        PKCS8EncodedKeySpec priKeySpec = new PKCS8EncodedKeySpec(
+                            Base64.getMimeDecoder().decode(targetPrivateKey));
+        KeyFactory kf = KeyFactory.getInstance("EC");
+        ECPrivateKey priKey =
+                (ECPrivateKey)kf.generatePrivate(priKeySpec);
+
+        // generate certificate chain
+        is = new ByteArrayInputStream(targetCertStr.getBytes());
+        X509Certificate keyCert = (X509Certificate)cf.generateCertificate(is);
+        is.close();
+
+        X509Certificate[] chain = new X509Certificate[2];
+        chain[0] = keyCert;
+        chain[1] = trusedCert;
+
+        // import the key entry and the chain
+        ks.setKeyEntry("TheKey", priKey, passphrase, chain);
+
+        // create SSL context
+        TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
+        tmf.init(ks);
+
+        KeyManagerFactory kmf = KeyManagerFactory.getInstance("NewSunX509");
+        kmf.init(ks, passphrase);
+
+        SSLContext ctx = SSLContext.getInstance("TLSv1.3");
+        ctx.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);
+        ks = null;
+
+        return ctx;
+    }
+}