Bug 1304407 - Ignore MD5 signature algorithms in certificate requests r=franziskus NSS_3_26_BRANCH
authorTim Taubert <ttaubert@mozilla.com>
Mon, 10 Oct 2016 16:13:01 +0200
branchNSS_3_26_BRANCH
changeset 12696 72700adfca79c93c57a9c1eb1a6e9af446cc0bd0
parent 12666 d8200544657fca2f2d458f8782d18cc57f562867
child 12699 5bb734f18d10e207cdfb222dbdb8be56dfdd0f64
push id1650
push userttaubert@mozilla.com
push dateMon, 10 Oct 2016 14:40:35 +0000
reviewersfranziskus
bugs1304407
Bug 1304407 - Ignore MD5 signature algorithms in certificate requests r=franziskus Differential Revision: https://nss-dev.phacility.com/D64
external_tests/ssl_gtest/ssl_auth_unittest.cc
external_tests/ssl_gtest/tls_parser.h
lib/ssl/ssl3con.c
--- a/external_tests/ssl_gtest/ssl_auth_unittest.cc
+++ b/external_tests/ssl_gtest/ssl_auth_unittest.cc
@@ -17,16 +17,48 @@ extern "C" {
 #include "scoped_ptrs.h"
 #include "tls_parser.h"
 #include "tls_filter.h"
 #include "tls_connect.h"
 #include "gtest_utils.h"
 
 namespace nss_test {
 
+class TlsInspectorCertificateRequestSigAlgSetter : public TlsHandshakeFilter {
+ public:
+  TlsInspectorCertificateRequestSigAlgSetter(SSLSignatureAndHashAlg sig_alg)
+    : sig_alg_(sig_alg) {}
+
+  virtual PacketFilter::Action FilterHandshake(
+      const HandshakeHeader& header,
+      const DataBuffer& input, DataBuffer* output) {
+    if (header.handshake_type() != kTlsHandshakeCertificateRequest) {
+      return KEEP;
+    }
+
+    TlsParser parser(input);
+    *output = input;
+
+    // Skip certificate types.
+    parser.SkipVariable(1);
+
+    // Skip sig algs length.
+    parser.Skip(2);
+
+    // Write signature algorithm.
+    output->Write(parser.consumed(), sig_alg_.hashAlg, 1);
+    output->Write(parser.consumed() + 1, sig_alg_.sigAlg, 1);
+
+    return CHANGE;
+  }
+
+ private:
+  SSLSignatureAndHashAlg sig_alg_;
+};
+
 TEST_P(TlsConnectGeneric, ClientAuth) {
   client_->SetupClientAuth();
   server_->RequestClientAuth(true);
   Connect();
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign);
 }
 
 // In TLS 1.3, the client sends its cert rejection on the
@@ -346,9 +378,32 @@ TEST_P(TlsConnectTls12, ClientAuthNoMatc
   server_->SetSignatureAlgorithms(SignatureEcdsaSha256,
                                   PR_ARRAY_SIZE(SignatureEcdsaSha256));
 
   Connect();
   CheckKeys(ssl_kea_ecdh, ssl_auth_ecdsa);
   EXPECT_TRUE(!SSL_PeerCertificate(server_->ssl_fd()));
 }
 
+TEST_P(TlsConnectTls12, CertificateRequestMd5) {
+  const SSLSignatureAndHashAlg md5_sig_alg = {ssl_hash_md5, ssl_sign_rsa};
+
+  const SSLSignatureAndHashAlg serverAlgorithms[] = {
+    {ssl_hash_sha1, ssl_sign_rsa},
+    {ssl_hash_sha256, ssl_sign_rsa}
+  };
+
+  client_->SetupClientAuth();
+  server_->RequestClientAuth(true);
+  server_->SetPacketFilter(new TlsInspectorCertificateRequestSigAlgSetter
+                           (md5_sig_alg));
+  server_->SetSignatureAlgorithms(serverAlgorithms,
+                                  PR_ARRAY_SIZE(serverAlgorithms));
+
+  client_->EnableSingleCipher(TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384);
+  server_->EnableSingleCipher(TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384);
+
+  ConnectExpectFail();
+  ASSERT_EQ(SEC_ERROR_BAD_SIGNATURE, server_->error_code());
+  ASSERT_EQ(SSL_ERROR_DECRYPT_ERROR_ALERT, client_->error_code());
 }
+
+}
--- a/external_tests/ssl_gtest/tls_parser.h
+++ b/external_tests/ssl_gtest/tls_parser.h
@@ -24,16 +24,17 @@ const uint8_t kTlsAlertType = 21;
 const uint8_t kTlsHandshakeType = 22;
 const uint8_t kTlsApplicationDataType = 23;
 
 const uint8_t kTlsHandshakeClientHello = 1;
 const uint8_t kTlsHandshakeServerHello = 2;
 const uint8_t kTlsHandshakeEncryptedExtensions = 8;
 const uint8_t kTlsHandshakeCertificate = 11;
 const uint8_t kTlsHandshakeServerKeyExchange = 12;
+const uint8_t kTlsHandshakeCertificateRequest = 13;
 const uint8_t kTlsHandshakeCertificateVerify = 15;
 const uint8_t kTlsHandshakeClientKeyExchange = 16;
 const uint8_t kTlsHandshakeFinished = 20;
 
 const uint8_t kTlsAlertWarning = 1;
 const uint8_t kTlsAlertFatal = 2;
 
 const uint8_t kTlsAlertUnexpectedMessage = 10;
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -10208,16 +10208,19 @@ ssl3_DecideTls12CertVerifyHash(sslSocket
     }
 
     /* Determine the server's hash support for that signature algorithm. */
     for (i = 0; i < algorithms->len; i += 2) {
         if (algorithms->data[i + 1] == sigAlg) {
             SSLHashType hashAlg = algorithms->data[i];
             SECOidTag hashOID;
             PRUint32 policy;
+            if (hashAlg == ssl_hash_md5) {
+                continue; /* No MD5 signature support. */
+            }
             if (hashAlg == ssl_hash_sha1 &&
                 ss->ssl3.pwSpec->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
                 /* TLS 1.3 explicitly forbids using SHA-1 with certificate_verify. */
                 continue;
             }
             hashOID = ssl3_TLSHashAlgorithmToOID(hashAlg);
             if ((NSS_GetAlgorithmPolicy(hashOID, &policy) == SECSuccess) &&
                 !(policy & NSS_USE_ALG_IN_SSL_KX)) {