Bug 1292006 - Remove cipherType, keyBits, and secretKeyBits from ss->…

…sec, r=ekr

--HG--
extra : rebase_source : 22efc8e689af52fdc0fedbef95f43711d29849f2

external_tests/ssl_gtest/libssl_internals.c

@@ -1,4 +1,4 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
@@ -26,21 +26,28 @@ SSLInt_IncrementClientHandshakeVersion(PRFileDesc *fd)
     return SECSuccess;
 }
 
+// This function guesses what key exchange strength libssl will choose.
 PRUint32
-SSLInt_DetermineKEABits(PRUint16 serverKeyBits, SSLAuthType authAlgorithm) {
-    // For ECDSA authentication we expect a curve for key exchange with the
-    // same strength as the one used for the certificate's signature.
+SSLInt_DetermineKEABits(PRUint16 serverKeyBits,
+                        const SSLCipherSuiteInfo *info) {
+    PRUint32 authBits;
+    SSLAuthType authAlgorithm = info->authType;
     if (authAlgorithm == ssl_auth_ecdsa ||
         authAlgorithm == ssl_auth_ecdh_rsa ||
         authAlgorithm == ssl_auth_ecdh_ecdsa) {
-        return serverKeyBits;
+        authBits = serverKeyBits;
+    } else {
+        PORT_Assert(authAlgorithm == ssl_auth_rsa_decrypt ||
+                    authAlgorithm == ssl_auth_rsa_sign);
+        authBits = SSL_RSASTRENGTH_TO_ECSTRENGTH(serverKeyBits);
     }
 
-    PORT_Assert(authAlgorithm == ssl_auth_rsa_decrypt ||
-                authAlgorithm == ssl_auth_rsa_sign);
+    // We expect a curve for key exchange to be selected based on the symmetric
+    // key strength (times 2) or the server key size, whichever is smaller.
+    PRUint32 targetKeaBits = PR_MIN(info->symKeyBits * 2, authBits);
 
-    // P-256 is the smallest group we accept.
-    return PR_MAX(SSL_RSASTRENGTH_TO_ECSTRENGTH(serverKeyBits), 256U);
+    // P-256 is the preferred curve of minimum size.
+    return PR_MAX(256U, targetKeaBits);
 }
 
 /* Use this function to update the ClientRandom of a client's handshake state

external_tests/ssl_gtest/libssl_internals.h

@@ -16,7 +16,7 @@
 SECStatus SSLInt_IncrementClientHandshakeVersion(PRFileDesc *fd);
 
 PRUint32 SSLInt_DetermineKEABits(PRUint16 serverKeyBits,
-                                 SSLAuthType authAlgorithm);
+                                 const SSLCipherSuiteInfo *info);
 
 SECStatus SSLInt_UpdateSSLv2ClientRandom(PRFileDesc *fd,
                                          uint8_t *rnd, size_t rnd_len,

external_tests/ssl_gtest/ssl_auth_unittest.cc

@@ -47,7 +47,7 @@ TEST_P(TlsConnectGeneric, ClientAuthRequestedRejected) {
 
 
 TEST_P(TlsConnectGeneric, ClientAuthEcdsa) {
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   client_->SetupClientAuth();
   server_->RequestClientAuth(true);
   Connect();
@@ -70,11 +70,11 @@ static const SSLSignatureAndHashAlg SignatureRsaSha256[] = {
 // When signature algorithms match up, this should connect successfully; even
 // for TLS 1.1 and 1.0, where they should be ignored.
 TEST_P(TlsConnectGeneric, SignatureAlgorithmServerAuth) {
+  Reset(TlsAgent::kServerEcdsa384);
   client_->SetSignatureAlgorithms(SignatureEcdsaSha384,
                                   PR_ARRAY_SIZE(SignatureEcdsaSha384));
   server_->SetSignatureAlgorithms(SignatureEcdsaSha384,
                                   PR_ARRAY_SIZE(SignatureEcdsaSha384));
-  Reset(TlsAgent::kServerEcdsa);
   Connect();
 }
 
@@ -86,18 +86,18 @@ TEST_P(TlsConnectGeneric, SignatureAlgorithmClientOnly) {
     {ssl_hash_sha384, ssl_sign_rsa}, // supported but unusable
     {ssl_hash_md5, ssl_sign_ecdsa} // unsupported and ignored
   };
+  Reset(TlsAgent::kServerEcdsa384);
   client_->SetSignatureAlgorithms(clientAlgorithms,
                                   PR_ARRAY_SIZE(clientAlgorithms));
-  Reset(TlsAgent::kServerEcdsa);
   Connect();
 }
 
 // Here the server picks a single option, which should work in all versions.
 // Defaults on the client include the provided option.
 TEST_P(TlsConnectGeneric, SignatureAlgorithmServerOnly) {
+  Reset(TlsAgent::kServerEcdsa384);
   server_->SetSignatureAlgorithms(SignatureEcdsaSha384,
                                   PR_ARRAY_SIZE(SignatureEcdsaSha384));
-  Reset(TlsAgent::kServerEcdsa);
   Connect();
 }
 
@@ -117,17 +117,23 @@ TEST_P(TlsConnectGenericPre13, SignatureAlgorithmNoOverlapStaticRsa) {
 // TODO(ekr@rtfm.com): We need to enable this for 1.3 when we fix
 // bug 1287267.
 TEST_P(TlsConnectTls12, SignatureAlgorithmNoOverlapEcdsa) {
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   client_->SetSignatureAlgorithms(SignatureEcdsaSha384,
                                   PR_ARRAY_SIZE(SignatureEcdsaSha384));
   server_->SetSignatureAlgorithms(SignatureEcdsaSha256,
                                   PR_ARRAY_SIZE(SignatureEcdsaSha256));
   ConnectExpectFail();
+  server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_HASH_ALGORITHM);
+  if (mode_ == STREAM) {
+    client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_FAILURE_ALERT);
+  } else {
+    client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+  }
 }
 
 // Pre 1.2, a mismatch on signature algorithms shouldn't affect anything.
 TEST_P(TlsConnectPre12, SignatureAlgorithmNoOverlapEcdsa) {
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   client_->SetSignatureAlgorithms(SignatureEcdsaSha384,
                                   PR_ARRAY_SIZE(SignatureEcdsaSha384));
   server_->SetSignatureAlgorithms(SignatureEcdsaSha256,

external_tests/ssl_gtest/ssl_ciphersuite_unittest.cc

@@ -43,7 +43,7 @@ class TlsCipherSuiteTestBase : public TlsConnectTestBase {
         Reset(TlsAgent::kServerRsaDecrypt);
         break;
       case ssl_auth_ecdsa:
-        Reset(TlsAgent::kServerEcdsa);
+        Reset(TlsAgent::kServerEcdsa256);
         break;
       case ssl_auth_ecdh_ecdsa:
         Reset(TlsAgent::kServerEcdhEcdsa);
@@ -273,4 +273,92 @@ INSTANTIATE_CIPHER_TEST_P(CBCDatagram, Datagram, V11V12,
                           TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
                           TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA);
 
+// Fields are: version, cipher suite, bulk cipher name, secretKeySize
+struct SecStatusParams {
+  uint16_t version;
+  uint16_t cipher_suite;
+  std::string name;
+  int keySize;
+};
+
+inline std::ostream& operator<<(std::ostream& stream,
+                                const SecStatusParams& vals) {
+  SSLCipherSuiteInfo csinfo;
+  SECStatus rv = SSL_GetCipherSuiteInfo(vals.cipher_suite,
+                                        &csinfo, sizeof(csinfo));
+  if (rv != SECSuccess) {
+    return stream << "Error invoking SSL_GetCipherSuiteInfo()";
+  }
+
+  return stream << "TLS " << VersionString(vals.version)
+                << ", " << csinfo.cipherSuiteName
+                << ", name = \"" << vals.name
+                << "\", key size = " << vals.keySize;
+}
+
+class SecurityStatusTest
+    : public TlsCipherSuiteTestBase,
+      public ::testing::WithParamInterface<SecStatusParams> {
+
+ public:
+  SecurityStatusTest()
+      : TlsCipherSuiteTestBase("TLS", GetParam().version,
+                               GetParam().cipher_suite) {}
+};
+
+// SSL_SecurityStatus produces fairly useless output when compared to
+// SSL_GetCipherSuiteInfo and SSL_GetChannelInfo, but we can't break it, so we
+// need to check it.
+TEST_P(SecurityStatusTest, CheckSecurityStatus) {
+  SetupCertificate();
+  EnableSingleCipher();
+  ConnectAndCheckCipherSuite();
+
+  int on;
+  char *cipher;
+  int keySize;
+  int secretKeySize;
+  char *issuer;
+  char *subject;
+  EXPECT_EQ(SECSuccess,
+            SSL_SecurityStatus(client_->ssl_fd(), &on, &cipher, &keySize,
+                               &secretKeySize, &issuer, &subject));
+  if (std::string(cipher) == "NULL") {
+    EXPECT_EQ(0, on);
+  } else {
+    EXPECT_NE(0, on);
+  }
+  EXPECT_EQ(GetParam().name, std::string(cipher));
+  // All the ciphers we support have secret key size == key size.
+  EXPECT_EQ(GetParam().keySize, keySize);
+  EXPECT_EQ(GetParam().keySize, secretKeySize);
+  EXPECT_LT(0U, strlen(issuer));
+  EXPECT_LT(0U, strlen(subject));
+
+  PORT_Free(cipher);
+  PORT_Free(issuer);
+  PORT_Free(subject);
+}
+
+static const SecStatusParams kSecStatusTestValuesArr[] = {
+  { SSL_LIBRARY_VERSION_TLS_1_0,
+    TLS_ECDHE_RSA_WITH_NULL_SHA, "NULL", 0 },
+  { SSL_LIBRARY_VERSION_TLS_1_0,
+    TLS_RSA_WITH_RC4_128_SHA, "RC4", 128 },
+  { SSL_LIBRARY_VERSION_TLS_1_0,
+    TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, "3DES-EDE-CBC", 168 },
+  { SSL_LIBRARY_VERSION_TLS_1_0,
+    TLS_RSA_WITH_AES_128_CBC_SHA, "AES-128", 128 },
+  { SSL_LIBRARY_VERSION_TLS_1_2,
+    TLS_RSA_WITH_AES_256_CBC_SHA256, "AES-256", 256 },
+  { SSL_LIBRARY_VERSION_TLS_1_2,
+    TLS_RSA_WITH_AES_128_GCM_SHA256, "AES-128-GCM", 128 },
+  { SSL_LIBRARY_VERSION_TLS_1_2,
+    TLS_RSA_WITH_AES_256_GCM_SHA384, "AES-256-GCM", 256 },
+  { SSL_LIBRARY_VERSION_TLS_1_2,
+    TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, "ChaCha20-Poly1305", 256 }
+};
+INSTANTIATE_TEST_CASE_P(TestSecurityStatus, SecurityStatusTest,
+                        ::testing::ValuesIn(kSecStatusTestValuesArr));
+
 }  // namespace nspr_test

external_tests/ssl_gtest/ssl_ecdh_unittest.cc

@@ -48,6 +48,16 @@ TEST_P(TlsConnectGeneric, ConnectEcdhe) {
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign);
 }
 
+// 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.
+// TODO: Re-enable for 1.3 when Bug 1286140 lands.
+TEST_P(TlsConnectTls12, ConnectEcdheP384) {
+  Reset(TlsAgent::kServerEcdsa384);
+  ConnectWithCipherSuite(TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256);
+  CheckKeys(ssl_kea_ecdh, ssl_auth_ecdsa, 384);
+}
+
 TEST_P(TlsConnectGeneric, ConnectEcdheP384) {
   EnsureTlsSetup();
   client_->ConfigNamedGroup(ssl_grp_ec_secp256r1, false);

external_tests/ssl_gtest/ssl_loopback_unittest.cc

@@ -34,7 +34,7 @@ TEST_P(TlsConnectGeneric, Connect) {
 
 TEST_P(TlsConnectGeneric, ConnectEcdsa) {
   SetExpectedVersion(std::get<1>(GetParam()));
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   Connect();
   CheckKeys(ssl_kea_ecdh, ssl_auth_ecdsa);
 }

external_tests/ssl_gtest/ssl_resumption_unittest.cc

@@ -231,7 +231,7 @@ TEST_P(TlsConnectGeneric, ServerSNICertSwitch) {
 }
 
 TEST_P(TlsConnectGeneric, ServerSNICertTypeSwitch) {
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   Connect();
   ScopedCERTCertificate cert1(SSL_PeerCertificate(client_->ssl_fd()));
 

external_tests/ssl_gtest/ssl_skip_unittest.cc

@@ -120,7 +120,7 @@ TEST_P(TlsSkipTest, SkipCertificateEcdhe) {
 }
 
 TEST_P(TlsSkipTest, SkipCertificateEcdsa) {
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   ServerSkipTest(new TlsHandshakeSkipFilter(kTlsHandshakeCertificate));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_SERVER_KEY_EXCH);
 }
@@ -131,7 +131,7 @@ TEST_P(TlsSkipTest, SkipServerKeyExchange) {
 }
 
 TEST_P(TlsSkipTest, SkipServerKeyExchangeEcdsa) {
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   ServerSkipTest(new TlsHandshakeSkipFilter(kTlsHandshakeServerKeyExchange));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_HELLO_DONE);
 }
@@ -145,7 +145,7 @@ TEST_P(TlsSkipTest, SkipCertAndKeyExch) {
 }
 
 TEST_P(TlsSkipTest, SkipCertAndKeyExchEcdsa) {
-  Reset(TlsAgent::kServerEcdsa);
+  Reset(TlsAgent::kServerEcdsa256);
   auto chain = new ChainedPacketFilter();
   chain->Add(new TlsHandshakeSkipFilter(kTlsHandshakeCertificate));
   chain->Add(new TlsHandshakeSkipFilter(kTlsHandshakeServerKeyExchange));

external_tests/ssl_gtest/tls_agent.cc

@@ -33,7 +33,8 @@ const std::string TlsAgent::kServerRsa = "rsa"; // both sign and encrypt
 const std::string TlsAgent::kServerRsaSign = "rsa_sign";
 const std::string TlsAgent::kServerRsaPss = "rsa_pss";
 const std::string TlsAgent::kServerRsaDecrypt = "rsa_decrypt";
-const std::string TlsAgent::kServerEcdsa = "ecdsa";
+const std::string TlsAgent::kServerEcdsa256 = "ecdsa256";
+const std::string TlsAgent::kServerEcdsa384 = "ecdsa384";
 // TODO: const std::string TlsAgent::kServerEcdhRsa = "ecdh_rsa";
 const std::string TlsAgent::kServerEcdhEcdsa = "ecdh_ecdsa";
 const std::string TlsAgent::kServerDsa = "dsa";
@@ -411,7 +412,7 @@ void TlsAgent::CheckKEA(SSLKEAType type, size_t kea_size) const {
 
 void TlsAgent::CheckKEA(SSLKEAType type) const {
   PRUint32 ecKEAKeyBits = SSLInt_DetermineKEABits(server_key_bits_,
-                                                  csinfo_.authType);
+                                                  &csinfo_);
   switch (type) {
     case ssl_kea_ecdh:
       CheckKEA(type, ecKEAKeyBits);
@@ -433,16 +434,6 @@ void TlsAgent::CheckAuthType(SSLAuthType type) const {
   EXPECT_EQ(type, csinfo_.authType);
   EXPECT_EQ(server_key_bits_, info_.authKeyBits);
 
-  // Do some extra checks based on type.
-  switch (type) {
-      case ssl_auth_ecdsa:
-          // extra check for P-256
-          EXPECT_EQ(256U, info_.authKeyBits);
-          break;
-    default:
-      break;
-  }
-
   // Check authAlgorithm, which is the old value for authType.  This is a second switch
   // statement because default label is different.
   switch (type) {

external_tests/ssl_gtest/tls_agent.h

@@ -57,7 +57,8 @@ class TlsAgent : public PollTarget {
   static const std::string kServerRsaSign;
   static const std::string kServerRsaPss;
   static const std::string kServerRsaDecrypt;
-  static const std::string kServerEcdsa;
+  static const std::string kServerEcdsa256;
+  static const std::string kServerEcdsa384;
   static const std::string kServerEcdhEcdsa;
   // TODO: static const std::string kServerEcdhRsa;
   static const std::string kServerDsa;

external_tests/ssl_gtest/tls_connect.cc

@@ -85,7 +85,7 @@ static const uint16_t kTlsVAllArr[] = {
 ::testing::internal::ParamGenerator<uint16_t>
   TlsConnectTestBase::kTlsVAll = ::testing::ValuesIn(kTlsVAllArr);
 
-static std::string VersionString(uint16_t version) {
+std::string VersionString(uint16_t version) {
   switch(version) {
   case 0:
     return "(no version)";

external_tests/ssl_gtest/tls_connect.h

@@ -19,6 +19,8 @@
 
 namespace nss_test {
 
+extern std::string VersionString(uint16_t version);
+
 // A generic TLS connection test base.
 class TlsConnectTestBase : public ::testing::Test {
  public: