Bug 1334114 - Allow mismatched groups with signature scheme configuration, r=kaie NSS_3_29_BRANCH NSS_3_29_RTM
authorMartin Thomson <martin.thomson@gmail.com>
Mon, 30 Jan 2017 12:06:08 +1100
branchNSS_3_29_BRANCH
changeset 13083 7fd9984bde06ba7a9e58ee874701e8ed85c90228
parent 13080 1bbd8a1ba5a38334b8a9f9371319db139a81fe54
child 13093 a26fe6a1c6a94a0d50462a0d4137dbf4ff00f3e0
push id1979
push usermartin.thomson@gmail.com
push dateTue, 31 Jan 2017 05:13:01 +0000
reviewerskaie
bugs1334114
Bug 1334114 - Allow mismatched groups with signature scheme configuration, r=kaie
gtests/ssl_gtest/ssl_auth_unittest.cc
gtests/ssl_gtest/ssl_ecdh_unittest.cc
lib/ssl/ssl.h
lib/ssl/ssl3con.c
--- a/gtests/ssl_gtest/ssl_auth_unittest.cc
+++ b/gtests/ssl_gtest/ssl_auth_unittest.cc
@@ -193,26 +193,44 @@ TEST_P(TlsConnectGeneric, SignatureAlgor
   Reset(TlsAgent::kServerEcdsa384);
   server_->SetSignatureSchemes(SignatureSchemeEcdsaSha384,
                                PR_ARRAY_SIZE(SignatureSchemeEcdsaSha384));
   Connect();
   CheckKeys(ssl_kea_ecdh, NamedGroupForEcdsa384(version_), ssl_auth_ecdsa,
             ssl_sig_ecdsa_secp384r1_sha384);
 }
 
-TEST_P(TlsConnectTls12Plus, SignatureSchemeCurveMismatch) {
+// In TLS 1.2, curve and hash aren't bound together.
+TEST_P(TlsConnectTls12, SignatureSchemeCurveMismatch) {
+  Reset(TlsAgent::kServerEcdsa256);
+  client_->SetSignatureSchemes(SignatureSchemeEcdsaSha384,
+                               PR_ARRAY_SIZE(SignatureSchemeEcdsaSha384));
+  Connect();
+}
+
+// In TLS 1.3, curve and hash are coupled.
+TEST_P(TlsConnectTls13, SignatureSchemeCurveMismatch) {
   Reset(TlsAgent::kServerEcdsa256);
   client_->SetSignatureSchemes(SignatureSchemeEcdsaSha384,
                                PR_ARRAY_SIZE(SignatureSchemeEcdsaSha384));
   ConnectExpectFail();
   server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
   client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
 }
 
-TEST_P(TlsConnectTls12Plus, SignatureSchemeBadConfig) {
+// Configuring a P-256 cert with only SHA-384 signatures is OK in TLS 1.2.
+TEST_P(TlsConnectTls12, SignatureSchemeBadConfig) {
+  Reset(TlsAgent::kServerEcdsa256);  // P-256 cert can't be used.
+  server_->SetSignatureSchemes(SignatureSchemeEcdsaSha384,
+                               PR_ARRAY_SIZE(SignatureSchemeEcdsaSha384));
+  Connect();
+}
+
+// A P-256 certificate in TLS 1.3 needs a SHA-256 signature scheme.
+TEST_P(TlsConnectTls13, SignatureSchemeBadConfig) {
   Reset(TlsAgent::kServerEcdsa256);  // P-256 cert can't be used.
   server_->SetSignatureSchemes(SignatureSchemeEcdsaSha384,
                                PR_ARRAY_SIZE(SignatureSchemeEcdsaSha384));
   ConnectExpectFail();
   server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
   client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
 }
 
--- a/gtests/ssl_gtest/ssl_ecdh_unittest.cc
+++ b/gtests/ssl_gtest/ssl_ecdh_unittest.cc
@@ -53,17 +53,17 @@ TEST_P(TlsConnectGeneric, ConnectEcdhe) 
 // If we pick a 256-bit cipher suite and use a P-384 certificate, the server
 // should choose P-384 for key exchange too.  Only valid for TLS == 1.2 because
 // we don't have 256-bit ciphers before then and 1.3 doesn't try to couple
 // DHE size to symmetric size.
 TEST_P(TlsConnectTls12, ConnectEcdheP384) {
   Reset(TlsAgent::kServerEcdsa384);
   ConnectWithCipherSuite(TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256);
   CheckKeys(ssl_kea_ecdh, ssl_grp_ec_secp384r1, ssl_auth_ecdsa,
-            ssl_sig_ecdsa_secp384r1_sha384);
+            ssl_sig_ecdsa_secp256r1_sha256);
 }
 
 TEST_P(TlsConnectGeneric, ConnectEcdheP384Client) {
   EnsureTlsSetup();
   const std::vector<SSLNamedGroup> groups = {ssl_grp_ec_secp384r1,
                                              ssl_grp_ffdhe_2048};
   client_->ConfigNamedGroups(groups);
   server_->ConfigNamedGroups(groups);
--- a/lib/ssl/ssl.h
+++ b/lib/ssl/ssl.h
@@ -351,20 +351,21 @@ SSL_IMPORT SECStatus SSL_CipherPolicyGet
 ** This also governs what the server sends in the supported_signature_algorithms
 ** field of a CertificateRequest.
 **
 ** This changes what the server uses to sign ServerKeyExchange and
 ** CertificateVerify messages.  An endpoint uses the first entry from this list
 ** that is compatible with both its certificate and its peer's supported
 ** values.
 **
-** NSS uses the strict signature schemes from TLS 1.3 in TLS 1.2.  That means
-** that if a peer indicates support for SHA-384 and ECDSA, NSS will not
-** generate a signature if it has a P-256 key, even though that is permitted in
-** TLS 1.2.
+** This configuration affects TLS 1.2, but the combination of EC group and hash
+** algorithm is interpreted loosely to be compatible with other implementations.
+** For TLS 1.2, NSS will ignore the curve group when generating or verifying
+** ECDSA signatures.  For example, a P-384 ECDSA certificate is used with
+** SHA-256 if ssl_sig_ecdsa_secp256r1_sha256 is enabled.
 **
 ** Omitting SHA-256 schemes from this list might be foolish.  Support is
 ** mandatory in TLS 1.2 and 1.3 and there might be interoperability issues.
 */
 SSL_IMPORT SECStatus SSL_SignatureSchemePrefSet(
     PRFileDesc *fd, const SSLSignatureScheme *schemes, unsigned int count);
 
 /* Deprecated, use SSL_SignatureSchemePrefSet() instead. */
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -6376,17 +6376,17 @@ ssl_PickSignatureScheme(sslSocket *ss,
      * indicated support for in their signature_algorithms extension. */
     for (i = 0; i < ss->ssl3.signatureSchemeCount; ++i) {
         SSLHashType hashType;
         SECOidTag hashOID;
         SSLSignatureScheme preferred = ss->ssl3.signatureSchemes[i];
         PRUint32 policy;
 
         if (!ssl_SignatureSchemeValidForKey(!isTLS13 /* allowSha1 */,
-                                            PR_TRUE /* matchGroup */,
+                                            isTLS13 /* matchGroup */,
                                             keyType, group, preferred)) {
             continue;
         }
 
         /* Skip RSA-PSS schemes when the certificate's private key slot does
          * not support this signature mechanism. */
         if (ssl_IsRsaPssSignatureScheme(preferred) && !slotDoesPss) {
             continue;