Bug 1287279 - Configure RSA server certs with PKCS#1 v1.5 signatures to be available for RSA-PSS signatures r=mt
authorTim Taubert <ttaubert@mozilla.com>
Fri, 12 Aug 2016 07:53:51 +0200
changeset 12465 71987d6f797f9acb8681729cb6368778421b3c6e
parent 12464 1186251049f30bc811e79c4e9617ce736154aa5d
child 12466 ab21f845dbf9289f30eb97b549252257298255b3
push id1468
push userttaubert@mozilla.com
push dateFri, 12 Aug 2016 06:52:23 +0000
reviewersmt
bugs1287279
Bug 1287279 - Configure RSA server certs with PKCS#1 v1.5 signatures to be available for RSA-PSS signatures r=mt
external_tests/ssl_gtest/libssl_internals.c
external_tests/ssl_gtest/libssl_internals.h
external_tests/ssl_gtest/ssl_auth_unittest.cc
external_tests/ssl_gtest/tls_agent.cc
external_tests/ssl_gtest/tls_agent.h
lib/ssl/sslcert.c
--- a/external_tests/ssl_gtest/libssl_internals.c
+++ b/external_tests/ssl_gtest/libssl_internals.c
@@ -221,8 +221,19 @@ SSLInt_Set0RttAlpn(PRFileDesc *fd, PRUin
     SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE);
   }
   if (!SECITEM_AllocItem(NULL, &ss->ssl3.nextProto, len))
     return SECFailure;
   PORT_Memcpy(ss->ssl3.nextProto.data, data, len);
 
   return SECSuccess;
 }
+
+PRBool
+SSLInt_HasCertWithAuthType(PRFileDesc *fd, SSLAuthType authType)
+{
+  sslSocket *ss = ssl_FindSocket(fd);
+  if (!ss) {
+    return PR_FALSE;
+  }
+
+  return (PRBool)(!!ssl_FindServerCertByAuthType(ss, authType));
+}
--- a/external_tests/ssl_gtest/libssl_internals.h
+++ b/external_tests/ssl_gtest/libssl_internals.h
@@ -26,10 +26,11 @@ PRBool SSLInt_ExtensionNegotiated(PRFile
 void SSLInt_ClearSessionTicketKey();
 PRInt32 SSLInt_CountTls13CipherSpecs(PRFileDesc *fd);
 void SSLInt_ForceTimerExpiry(PRFileDesc *fd);
 SECStatus SSLInt_SetMTU(PRFileDesc *fd, PRUint16 mtu);
 PRBool SSLInt_CheckSecretsDestroyed(PRFileDesc *fd);
 PRBool SSLInt_DamageHsTrafficSecret(PRFileDesc *fd);
 PRBool SSLInt_DamageEarlyTrafficSecret(PRFileDesc *fd);
 SECStatus SSLInt_Set0RttAlpn(PRFileDesc *fd, PRUint8 *data, unsigned int len);
+PRBool SSLInt_HasCertWithAuthType(PRFileDesc *fd, SSLAuthType authType);
 
 #endif // ndef libssl_internals_h_
--- a/external_tests/ssl_gtest/ssl_auth_unittest.cc
+++ b/external_tests/ssl_gtest/ssl_auth_unittest.cc
@@ -330,9 +330,77 @@ TEST_P(TlsConnectGenericPre13, AuthCompl
         client_->SendData(10);
       }));
 
   Connect();
   server_->SendData(10);
   Receive(10);
 }
 
+static const SSLExtraServerCertData ServerCertDataRsaPkcs1Decrypt = {
+  ssl_auth_rsa_decrypt, nullptr, nullptr, nullptr
+};
+static const SSLExtraServerCertData ServerCertDataRsaPkcs1Sign = {
+  ssl_auth_rsa_sign, nullptr, nullptr, nullptr
+};
+static const SSLExtraServerCertData ServerCertDataRsaPss = {
+  ssl_auth_rsa_pss, nullptr, nullptr, nullptr
+};
+
+// Test RSA cert with usage=[signature, encipherment].
+TEST_F(TlsAgentStreamTestServer, ConfigureCertRsaPkcs1SignAndKEX) {
+  Reset(TlsAgent::kServerRsa);
+
+  PRFileDesc *ssl_fd = agent_->ssl_fd();
+  EXPECT_TRUE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_decrypt));
+  EXPECT_TRUE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_sign));
+  EXPECT_TRUE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_pss));
+
+  // Configuring for only rsa_sign, rsa_pss, or rsa_decrypt should work.
+  EXPECT_TRUE(agent_->ConfigServerCert(TlsAgent::kServerRsa, false,
+                                       &ServerCertDataRsaPkcs1Decrypt));
+  EXPECT_TRUE(agent_->ConfigServerCert(TlsAgent::kServerRsa, false,
+                                       &ServerCertDataRsaPkcs1Sign));
+  EXPECT_TRUE(agent_->ConfigServerCert(TlsAgent::kServerRsa, false,
+                                       &ServerCertDataRsaPss));
 }
+
+// Test RSA cert with usage=[signature].
+TEST_F(TlsAgentStreamTestServer, ConfigureCertRsaPkcs1Sign) {
+  Reset(TlsAgent::kServerRsaSign);
+
+  PRFileDesc *ssl_fd = agent_->ssl_fd();
+  EXPECT_FALSE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_decrypt));
+  EXPECT_TRUE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_sign));
+  EXPECT_TRUE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_pss));
+
+  // Configuring for only rsa_decrypt should fail.
+  EXPECT_FALSE(agent_->ConfigServerCert(TlsAgent::kServerRsaSign, false,
+                                        &ServerCertDataRsaPkcs1Decrypt));
+
+  // Configuring for only rsa_sign or rsa_pss should work.
+  EXPECT_TRUE(agent_->ConfigServerCert(TlsAgent::kServerRsaSign, false,
+                                       &ServerCertDataRsaPkcs1Sign));
+  EXPECT_TRUE(agent_->ConfigServerCert(TlsAgent::kServerRsaSign, false,
+                                       &ServerCertDataRsaPss));
+}
+
+// Test RSA cert with usage=[encipherment].
+TEST_F(TlsAgentStreamTestServer, ConfigureCertRsaPkcs1KEX) {
+  Reset(TlsAgent::kServerRsaDecrypt);
+
+  PRFileDesc *ssl_fd = agent_->ssl_fd();
+  EXPECT_TRUE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_decrypt));
+  EXPECT_FALSE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_sign));
+  EXPECT_FALSE(SSLInt_HasCertWithAuthType(ssl_fd, ssl_auth_rsa_pss));
+
+  // Configuring for only rsa_sign or rsa_pss should fail.
+  EXPECT_FALSE(agent_->ConfigServerCert(TlsAgent::kServerRsaDecrypt, false,
+                                        &ServerCertDataRsaPkcs1Sign));
+  EXPECT_FALSE(agent_->ConfigServerCert(TlsAgent::kServerRsaDecrypt, false,
+                                        &ServerCertDataRsaPss));
+
+  // Configuring for only rsa_decrypt should work.
+  EXPECT_TRUE(agent_->ConfigServerCert(TlsAgent::kServerRsaDecrypt, false,
+                                       &ServerCertDataRsaPkcs1Decrypt));
+}
+
+}
--- a/external_tests/ssl_gtest/tls_agent.cc
+++ b/external_tests/ssl_gtest/tls_agent.cc
@@ -85,17 +85,18 @@ TlsAgent::~TlsAgent() {
     PR_Close(pr_fd_);
   }
 
   if (ssl_fd_) {
     PR_Close(ssl_fd_);
   }
 }
 
-bool TlsAgent::ConfigServerCert(const std::string& name, bool updateKeyBits) {
+bool TlsAgent::ConfigServerCert(const std::string& name, bool updateKeyBits,
+                                const SSLExtraServerCertData *serverCertData) {
   ScopedCERTCertificate cert(PK11_FindCertFromNickname(name.c_str(), nullptr));
   EXPECT_NE(nullptr, cert.get());
   if (!cert.get()) return false;
 
   ScopedSECKEYPublicKey pub(CERT_ExtractPublicKey(cert.get()));
   EXPECT_NE(nullptr, pub.get());
   if (!pub.get()) return false;
   if (updateKeyBits) {
@@ -103,19 +104,18 @@ bool TlsAgent::ConfigServerCert(const st
   }
 
   ScopedSECKEYPrivateKey priv(PK11_FindKeyByAnyCert(cert.get(), nullptr));
   EXPECT_NE(nullptr, priv.get());
   if (!priv.get()) return false;
 
   SECStatus rv = SSL_ConfigSecureServer(ssl_fd_, nullptr, nullptr, ssl_kea_null);
   EXPECT_EQ(SECFailure, rv);
-  rv = SSL_ConfigServerCert(ssl_fd_, cert.get(), priv.get(), nullptr, 0);
-  EXPECT_EQ(SECSuccess, rv);
-
+  rv = SSL_ConfigServerCert(ssl_fd_, cert.get(), priv.get(), serverCertData,
+                            serverCertData ? sizeof(*serverCertData) : 0);
   return rv == SECSuccess;
 }
 
 // The tests expect that only curves secp256r1, secp384r1, and secp521r1
 // (NIST P-256, P-384, and P-521) are enabled. Disable all other curves.
 void TlsAgent::DisableLameGroups() {
 #ifdef NSS_ECC_MORE_THAN_SUITE_B
   static const SSLNamedGroup lame_groups[] = {
@@ -793,19 +793,24 @@ void TlsAgentTestBase::SetUp() {
 }
 
 void TlsAgentTestBase::TearDown() {
   delete agent_;
   SSL_ClearSessionCache();
   SSL_ShutdownServerSessionIDCache();
 }
 
-void TlsAgentTestBase::Init() {
+void TlsAgentTestBase::Reset(const std::string& server_name) {
+  delete agent_;
+  Init(server_name);
+}
+
+void TlsAgentTestBase::Init(const std::string& server_name) {
   agent_ = new TlsAgent(
-      role_ == TlsAgent::CLIENT ? TlsAgent::kClient : TlsAgent::kServerRsa,
+      role_ == TlsAgent::CLIENT ? TlsAgent::kClient : server_name,
       role_, mode_);
   agent_->Init();
   fd_ = DummyPrSocket::CreateFD(agent_->role_str(), mode_);
   agent_->adapter()->SetPeer(DummyPrSocket::GetAdapter(fd_));
   agent_->StartConnect();
 }
 
 void TlsAgentTestBase::EnsureInit() {
--- a/external_tests/ssl_gtest/tls_agent.h
+++ b/external_tests/ssl_gtest/tls_agent.h
@@ -91,17 +91,18 @@ class TlsAgent : public PollTarget {
   void EnableCiphersByKeyExchange(SSLKEAType kea);
   void EnableSingleCipher(uint16_t cipher);
 
   void Handshake();
   // Marks the internal state as CONNECTING in anticipation of renegotiation.
   void PrepareForRenegotiate();
   // Prepares for renegotiation, then actually triggers it.
   void StartRenegotiate();
-  bool ConfigServerCert(const std::string& name, bool updateKeyBits = false);
+  bool ConfigServerCert(const std::string& name, bool updateKeyBits = false,
+                        const SSLExtraServerCertData *serverCertData = nullptr);
   bool EnsureTlsSetup(PRFileDesc *modelSocket = nullptr);
 
   void SetupClientAuth();
   void RequestClientAuth(bool requireAuth);
   bool GetClientAuthCredentials(CERTCertificate** cert,
                                 SECKEYPrivateKey** priv) const;
 
   void ConfigureSessionCache(SessionResumptionMode mode);
@@ -372,17 +373,18 @@ class TlsAgentTestBase : public ::testin
   static inline TlsAgent::Role ToRole(const std::string& str) {
     return str == "CLIENT" ? TlsAgent::CLIENT : TlsAgent::SERVER;
   }
 
   static inline Mode ToMode(const std::string& str) {
     return str == "TLS" ? STREAM : DGRAM;
   }
 
-  void Init();
+  void Init(const std::string& server_name = TlsAgent::kServerRsa);
+  void Reset(const std::string& server_name = TlsAgent::kServerRsa);
 
  protected:
   void EnsureInit();
   void ProcessMessage(const DataBuffer& buffer,
                       TlsAgent::State expected_state,
                       int32_t error_code = 0);
 
 
--- a/lib/ssl/sslcert.c
+++ b/lib/ssl/sslcert.c
@@ -385,59 +385,96 @@ ssl_GetEcdhAuthType(CERTCertificate *cer
         case SEC_OID_ANSIX962_ECDSA_SIGNATURE_RECOMMENDED_DIGEST:
         case SEC_OID_ANSIX962_ECDSA_SIGNATURE_SPECIFIED_DIGEST:
             return ssl_auth_ecdh_ecdsa;
         default:
             return ssl_auth_null;
     }
 }
 
+/* This function examines the key usages of the given RSA-PKCS1 certificate
+ * and configures one or multiple server certificates based on that data.
+ *
+ * If the data argument contains an authType value other than ssl_auth_null,
+ * then only that slot will be used.  If that choice is invalid,
+ * then this will fail. */
+static SECStatus
+ssl_ConfigRsaPkcs1CertByUsage(sslSocket *ss, CERTCertificate *cert,
+                              sslKeyPair *keyPair,
+                              SSLExtraServerCertData *data)
+{
+    SECStatus rv = SECFailure;
+
+    PRBool ku_sig = (PRBool)(cert->keyUsage & KU_DIGITAL_SIGNATURE);
+    PRBool ku_enc = (PRBool)(cert->keyUsage & KU_KEY_ENCIPHERMENT);
+
+    if ((data->authType == ssl_auth_rsa_sign && ku_sig) ||
+        (data->authType == ssl_auth_rsa_pss && ku_sig) ||
+        (data->authType == ssl_auth_rsa_decrypt && ku_enc)) {
+        return ssl_ConfigCert(ss, cert, keyPair, data);
+    }
+
+    if (data->authType != ssl_auth_null || !(ku_sig || ku_enc)) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
+
+    if (ku_sig) {
+        data->authType = ssl_auth_rsa_sign;
+        rv = ssl_ConfigCert(ss, cert, keyPair, data);
+        if (rv != SECSuccess) {
+            return rv;
+        }
+
+        /* This certificate is RSA, assume that it's also PSS. */
+        data->authType = ssl_auth_rsa_pss;
+        rv = ssl_ConfigCert(ss, cert, keyPair, data);
+        if (rv != SECSuccess) {
+            return rv;
+        }
+    }
+
+    if (ku_enc) {
+        /* If ku_sig=true we configure signature and encryption slots with the
+         * same cert. This is bad form, but there are enough dual-usage RSA
+         * certs that we can't really break by limiting this to one type. */
+        data->authType = ssl_auth_rsa_decrypt;
+        rv = ssl_ConfigCert(ss, cert, keyPair, data);
+        if (rv != SECSuccess) {
+            return rv;
+        }
+    }
+
+    return rv;
+}
+
 /* This function examines the type of certificate and its key usage and
- * configures a certificate based on that information.  For RSA certificates
- * only, this can mean that two certificates are configured.
+ * configures a certificate based on that information.  For some certificates
+ * this can mean that multiple server certificates are configured.
  *
- * If the optional data argument contains an authType value other than
- * ssl_auth_null, then only that slot will be used.  If that choice is invalid,
+ * If the data argument contains an authType value other than ssl_auth_null,
+ * then only that slot will be used.  If that choice is invalid,
  * then this will fail. */
 static SECStatus
 ssl_ConfigCertByUsage(sslSocket *ss, CERTCertificate *cert,
                       sslKeyPair *keyPair, const SSLExtraServerCertData *data)
 {
     SECStatus rv = SECFailure;
-    SSLExtraServerCertData arg = {
-        ssl_auth_null, NULL, NULL, NULL
-    };
+    SSLExtraServerCertData arg;
     SECOidTag tag;
 
-    if (data) {
-        /* Take a (shallow) copy so that we can play with it */
-        memcpy(&arg, data, sizeof(arg));
-    }
+    PORT_Assert(data);
+    /* Take a (shallow) copy so that we can play with it */
+    memcpy(&arg, data, sizeof(arg));
+
     tag = SECOID_GetAlgorithmTag(&cert->subjectPublicKeyInfo.algorithm);
     switch (tag) {
         case SEC_OID_X500_RSA_ENCRYPTION:
         case SEC_OID_PKCS1_RSA_ENCRYPTION:
-            if (cert->keyUsage & KU_DIGITAL_SIGNATURE) {
-                if ((cert->keyUsage & KU_KEY_ENCIPHERMENT) &&
-                    arg.authType == ssl_auth_null) {
-                    /* Two usages is bad form, but there are enough dual-usage RSA
-                     * certs that we can't really break by limiting this to one.
-                     * Configure both slots only if no explicit slot was set. */
-                    arg.authType = ssl_auth_rsa_decrypt;
-                    rv = ssl_ConfigCert(ss, cert, keyPair, &arg);
-                    if (rv != SECSuccess) {
-                        return rv;
-                    }
-                }
-
-                arg.authType = ssl_auth_rsa_sign;
-            } else if (cert->keyUsage & KU_KEY_ENCIPHERMENT) {
-                arg.authType = ssl_auth_rsa_decrypt;
-            }
-            break;
+            return ssl_ConfigRsaPkcs1CertByUsage(ss, cert, keyPair, &arg);
 
         case SEC_OID_PKCS1_RSA_PSS_SIGNATURE:
             if (cert->keyUsage & KU_DIGITAL_SIGNATURE) {
                 arg.authType = ssl_auth_rsa_pss;
             }
             break;
 
         case SEC_OID_ANSIX9_DSA_SIGNATURE: