Bug 1617968 - Update Delegated Credentials implementation to draft-07…

… r=mt

Remove support for RSAE in delegated credentials (both in DC signatures and SPKIs), add SignatureScheme list functionality to initial DC extension.

Differential Revision: https://phabricator.services.mozilla.com/D65252

--HG--
extra : moz-landing-system : lando

automation/abi-check/expected-report-libssl3.so.txt

@@ -0,0 +1,11 @@
+
+1 function with some indirect sub-type change:
+
+  [C]'function SECStatus SSL_HandshakeNegotiatedExtension(PRFileDesc*, SSLExtensionType, PRBool*)' at sslreveal.c:72:1 has some indirect sub-type changes:
+    parameter 2 of type 'typedef SSLExtensionType' has sub-type changes:
+      underlying type 'enum __anonymous_enum__' at sslt.h:497:1 changed:
+        type size hasn't changed
+        1 enumerator change:
+          '__anonymous_enum__::ssl_delegated_credentials_xtn' from value '65282' to '34' at sslt.h:497:1
+
+

gtests/ssl_gtest/libssl_internals.c

@@ -12,6 +12,33 @@
 #include "seccomon.h"
 #include "selfencrypt.h"
 
+SECStatus SSLInt_SetDCAdvertisedSigSchemes(PRFileDesc *fd,
+                                           const SSLSignatureScheme *schemes,
+                                           uint32_t num_sig_schemes) {
+  if (!fd) {
+    return SECFailure;
+  }
+  sslSocket *ss = ssl_FindSocket(fd);
+  if (!ss) {
+    return SECFailure;
+  }
+
+  // Alloc and copy, libssl will free.
+  SSLSignatureScheme *dc_schemes =
+      PORT_ZNewArray(SSLSignatureScheme, num_sig_schemes);
+  if (!dc_schemes) {
+    return SECFailure;
+  }
+  memcpy(dc_schemes, schemes, sizeof(SSLSignatureScheme) * num_sig_schemes);
+
+  if (ss->xtnData.delegCredSigSchemesAdvertised) {
+    PORT_Free(ss->xtnData.delegCredSigSchemesAdvertised);
+  }
+  ss->xtnData.delegCredSigSchemesAdvertised = dc_schemes;
+  ss->xtnData.numDelegCredSigSchemesAdvertised = num_sig_schemes;
+  return SECSuccess;
+}
+
 SECStatus SSLInt_TweakChannelInfoForDC(PRFileDesc *fd, PRBool changeAuthKeyBits,
                                        PRBool changeScheme) {
   if (!fd) {

gtests/ssl_gtest/libssl_internals.h

@@ -44,5 +44,8 @@ SECStatus SSLInt_HasPendingHandshakeData(PRFileDesc *fd, PRBool *pending);
 SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size);
 SECStatus SSLInt_TweakChannelInfoForDC(PRFileDesc *fd, PRBool changeAuthKeyBits,
                                        PRBool changeScheme);
+SECStatus SSLInt_SetDCAdvertisedSigSchemes(PRFileDesc *fd,
+                                           const SSLSignatureScheme *schemes,
+                                           uint32_t num_sig_schemes);
 
 #endif  // ndef libssl_internals_h_

gtests/ssl_gtest/tls_agent.cc

@@ -49,6 +49,7 @@ const std::string TlsAgent::kServerEcdhEcdsa = "ecdh_ecdsa";
 const std::string TlsAgent::kServerDsa = "dsa";
 const std::string TlsAgent::kDelegatorEcdsa256 = "delegator_ecdsa256";
 const std::string TlsAgent::kDelegatorRsae2048 = "delegator_rsae2048";
+const std::string TlsAgent::kDelegatorRsaPss2048 = "delegator_rsa_pss2048";
 
 static const uint8_t kCannedTls13ServerHello[] = {
     0x03, 0x03, 0x9c, 0xbc, 0x14, 0x9b, 0x0e, 0x2e, 0xfa, 0x0d, 0xf3,

gtests/ssl_gtest/tls_agent.h

@@ -76,8 +76,9 @@ class TlsAgent : public PollTarget {
   static const std::string kServerEcdhEcdsa;
   static const std::string kServerEcdhRsa;
   static const std::string kServerDsa;
-  static const std::string kDelegatorEcdsa256;  // draft-ietf-tls-subcerts
-  static const std::string kDelegatorRsae2048;  // draft-ietf-tls-subcerts
+  static const std::string kDelegatorEcdsa256;    // draft-ietf-tls-subcerts
+  static const std::string kDelegatorRsae2048;    // draft-ietf-tls-subcerts
+  static const std::string kDelegatorRsaPss2048;  // draft-ietf-tls-subcerts
 
   TlsAgent(const std::string& name, Role role, SSLProtocolVariant variant);
   virtual ~TlsAgent();

gtests/ssl_gtest/tls_subcerts_unittest.cc

@@ -18,9 +18,10 @@ namespace nss_test {
 
 const std::string kEcdsaDelegatorId = TlsAgent::kDelegatorEcdsa256;
 const std::string kRsaeDelegatorId = TlsAgent::kDelegatorRsae2048;
+const std::string kPssDelegatorId = TlsAgent::kDelegatorRsaPss2048;
 const std::string kDCId = TlsAgent::kServerEcdsa256;
 const SSLSignatureScheme kDCScheme = ssl_sig_ecdsa_secp256r1_sha256;
-const PRUint32 kDCValidFor = 60 * 60 * 24 * 7 /* 1 week (seconds */;
+const PRUint32 kDCValidFor = 60 * 60 * 24 * 7 /* 1 week (seconds) */;
 
 static void CheckPreliminaryPeerDelegCred(
     const std::shared_ptr<TlsAgent>& client, bool expected,
@@ -122,6 +123,23 @@ TEST_P(TlsConnectTls13, DCConnectEcdsaP256) {
   EXPECT_EQ(ssl_sig_ecdsa_secp256r1_sha256, client_->info().signatureScheme);
 }
 
+// Connected with ECDSA-P384.
+TEST_P(TlsConnectTls13, DCConnectEcdsaP483) {
+  Reset(kEcdsaDelegatorId);
+  client_->EnableDelegatedCredentials();
+  server_->AddDelegatedCredential(TlsAgent::kServerEcdsa384,
+                                  ssl_sig_ecdsa_secp384r1_sha384, kDCValidFor,
+                                  now());
+
+  auto cfilter = MakeTlsFilter<TlsExtensionCapture>(
+      client_, ssl_delegated_credentials_xtn);
+  Connect();
+
+  EXPECT_TRUE(cfilter->captured());
+  CheckPeerDelegCred(client_, true, 384);
+  EXPECT_EQ(ssl_sig_ecdsa_secp384r1_sha384, client_->info().signatureScheme);
+}
+
 // Connected with ECDSA-P521.
 TEST_P(TlsConnectTls13, DCConnectEcdsaP521) {
   Reset(kEcdsaDelegatorId);
@@ -140,27 +158,35 @@ TEST_P(TlsConnectTls13, DCConnectEcdsaP521) {
   EXPECT_EQ(ssl_sig_ecdsa_secp521r1_sha512, client_->info().signatureScheme);
 }
 
-// Connected with RSA-PSS, using an RSAE DC SPKI.
-TEST_P(TlsConnectTls13, DCConnectRsaPssRsae) {
+// Connected with RSA-PSS, using a PSS SPKI and ECDSA delegation cert.
+TEST_P(TlsConnectTls13, DCConnectRsaPssEcdsa) {
   Reset(kEcdsaDelegatorId);
+
+  // Need to enable PSS-PSS, which is not on by default.
+  static const SSLSignatureScheme kSchemes[] = {ssl_sig_ecdsa_secp256r1_sha256,
+                                                ssl_sig_rsa_pss_pss_sha256};
+  client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+  server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+
   client_->EnableDelegatedCredentials();
   server_->AddDelegatedCredential(
-      TlsAgent::kServerRsaPss, ssl_sig_rsa_pss_rsae_sha256, kDCValidFor, now());
+      TlsAgent::kServerRsaPss, ssl_sig_rsa_pss_pss_sha256, kDCValidFor, now());
 
   auto cfilter = MakeTlsFilter<TlsExtensionCapture>(
       client_, ssl_delegated_credentials_xtn);
   Connect();
 
   EXPECT_TRUE(cfilter->captured());
   CheckPeerDelegCred(client_, true, 1024);
-  EXPECT_EQ(ssl_sig_rsa_pss_rsae_sha256, client_->info().signatureScheme);
+  EXPECT_EQ(ssl_sig_rsa_pss_pss_sha256, client_->info().signatureScheme);
 }
 
-// Connected with RSA-PSS, using a RSAE Delegator SPKI.
-TEST_P(TlsConnectTls13, DCConnectRsaeDelegator) {
-  Reset(kRsaeDelegatorId);
+// Connected with RSA-PSS, using a PSS SPKI and PSS delegation cert.
+TEST_P(TlsConnectTls13, DCConnectRsaPssRsaPss) {
+  Reset(kPssDelegatorId);
 
-  static const SSLSignatureScheme kSchemes[] = {ssl_sig_rsa_pss_rsae_sha256,
+  // Need to enable PSS-PSS, which is not on by default.
+  static const SSLSignatureScheme kSchemes[] = {ssl_sig_ecdsa_secp256r1_sha256,
                                                 ssl_sig_rsa_pss_pss_sha256};
   client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
   server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
@@ -178,27 +204,141 @@ TEST_P(TlsConnectTls13, DCConnectRsaeDelegator) {
   EXPECT_EQ(ssl_sig_rsa_pss_pss_sha256, client_->info().signatureScheme);
 }
 
-// Connected with RSA-PSS, using a PSS SPKI.
-TEST_P(TlsConnectTls13, DCConnectRsaPssPss) {
-  Reset(kEcdsaDelegatorId);
+// Connected with ECDSA-P256 using a PSS delegation cert.
+TEST_P(TlsConnectTls13, DCConnectEcdsaP256RsaPss) {
+  Reset(kPssDelegatorId);
 
   // Need to enable PSS-PSS, which is not on by default.
   static const SSLSignatureScheme kSchemes[] = {ssl_sig_ecdsa_secp256r1_sha256,
                                                 ssl_sig_rsa_pss_pss_sha256};
   client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
   server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
 
+  client_->EnableDelegatedCredentials();
+  server_->AddDelegatedCredential(TlsAgent::kServerEcdsa256,
+                                  ssl_sig_ecdsa_secp256r1_sha256, kDCValidFor,
+                                  now());
+
+  auto cfilter = MakeTlsFilter<TlsExtensionCapture>(
+      client_, ssl_delegated_credentials_xtn);
+  Connect();
+
+  EXPECT_TRUE(cfilter->captured());
+  CheckPeerDelegCred(client_, true, 256);
+  EXPECT_EQ(ssl_sig_ecdsa_secp256r1_sha256, client_->info().signatureScheme);
+}
+
+// Simulate the client receiving a DC containing algorithms not advertised.
+// Do this by tweaking the client's supported sigSchemes after the CH.
+TEST_P(TlsConnectTls13, DCReceiveUnadvertisedScheme) {
+  Reset(kEcdsaDelegatorId);
+  static const SSLSignatureScheme kClientSchemes[] = {
+      ssl_sig_ecdsa_secp256r1_sha256, ssl_sig_ecdsa_secp384r1_sha384};
+  static const SSLSignatureScheme kServerSchemes[] = {
+      ssl_sig_ecdsa_secp384r1_sha384, ssl_sig_ecdsa_secp256r1_sha256};
+  static const SSLSignatureScheme kEcdsaP256Only[] = {
+      ssl_sig_ecdsa_secp256r1_sha256};
+  client_->SetSignatureSchemes(kClientSchemes, PR_ARRAY_SIZE(kClientSchemes));
+  server_->SetSignatureSchemes(kServerSchemes, PR_ARRAY_SIZE(kServerSchemes));
+  client_->EnableDelegatedCredentials();
+  server_->AddDelegatedCredential(TlsAgent::kServerEcdsa384,
+                                  ssl_sig_ecdsa_secp384r1_sha384, kDCValidFor,
+                                  now());
+  StartConnect();
+  client_->Handshake();  // CH with P256/P384.
+  server_->Handshake();  // Respond with P384 DC.
+                         // Tell the client it only advertised P256.
+  SECStatus rv = SSLInt_SetDCAdvertisedSigSchemes(
+      client_->ssl_fd(), kEcdsaP256Only, PR_ARRAY_SIZE(kEcdsaP256Only));
+  EXPECT_EQ(SECSuccess, rv);
+  ExpectAlert(client_, kTlsAlertIllegalParameter);
+  Handshake();
+  client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+}
+
+// Server schemes includes only RSAE schemes. Connection should succeed
+// without delegation.
+TEST_P(TlsConnectTls13, DCConnectServerRsaeOnly) {
+  Reset(kRsaeDelegatorId);
+  static const SSLSignatureScheme kClientSchemes[] = {
+      ssl_sig_rsa_pss_rsae_sha256, ssl_sig_rsa_pss_pss_sha256};
+  static const SSLSignatureScheme kServerSchemes[] = {
+      ssl_sig_rsa_pss_rsae_sha256};
+  client_->SetSignatureSchemes(kClientSchemes, PR_ARRAY_SIZE(kClientSchemes));
+  server_->SetSignatureSchemes(kServerSchemes, PR_ARRAY_SIZE(kServerSchemes));
+  client_->EnableDelegatedCredentials();
+  Connect();
+
+  CheckPeerDelegCred(client_, false);
+}
+
+// Connect with an RSA-PSS DC SPKI, and an RSAE Delegator SPKI.
+TEST_P(TlsConnectTls13, DCConnectRsaeDelegator) {
+  Reset(kRsaeDelegatorId);
+
+  static const SSLSignatureScheme kSchemes[] = {ssl_sig_rsa_pss_rsae_sha256,
+                                                ssl_sig_rsa_pss_pss_sha256};
+  client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+  server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+
   client_->EnableDelegatedCredentials();
   server_->AddDelegatedCredential(
       TlsAgent::kServerRsaPss, ssl_sig_rsa_pss_pss_sha256, kDCValidFor, now());
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+  client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
+}
 
+// Client schemes includes only RSAE schemes. Connection should succeed
+// without delegation, and no DC extension should be present in the CH.
+TEST_P(TlsConnectTls13, DCConnectClientRsaeOnly) {
+  Reset(kRsaeDelegatorId);
+  static const SSLSignatureScheme kClientSchemes[] = {
+      ssl_sig_rsa_pss_rsae_sha256};
+  static const SSLSignatureScheme kServerSchemes[] = {
+      ssl_sig_rsa_pss_rsae_sha256, ssl_sig_rsa_pss_pss_sha256};
+  client_->SetSignatureSchemes(kClientSchemes, PR_ARRAY_SIZE(kClientSchemes));
+  server_->SetSignatureSchemes(kServerSchemes, PR_ARRAY_SIZE(kServerSchemes));
+  client_->EnableDelegatedCredentials();
   auto cfilter = MakeTlsFilter<TlsExtensionCapture>(
       client_, ssl_delegated_credentials_xtn);
   Connect();
+  EXPECT_FALSE(cfilter->captured());
+  CheckPeerDelegCred(client_, false);
+}
 
-  EXPECT_TRUE(cfilter->captured());
-  CheckPeerDelegCred(client_, true, 1024);
-  EXPECT_EQ(ssl_sig_rsa_pss_pss_sha256, client_->info().signatureScheme);
+// Test fallback. DC extension will not advertise RSAE schemes.
+// The server will attempt to set one, but decline to after seeing
+// the client-advertised schemes does not include it. Expect non-
+// delegated success.
+TEST_P(TlsConnectTls13, DCConnectRsaeDcSpki) {
+  Reset(kRsaeDelegatorId);
+
+  static const SSLSignatureScheme kSchemes[] = {ssl_sig_rsa_pss_rsae_sha256,
+                                                ssl_sig_rsa_pss_pss_sha256};
+  client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+  server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+  client_->EnableDelegatedCredentials();
+
+  EnsureTlsSetup();
+  ScopedSECKEYPublicKey pub;
+  ScopedSECKEYPrivateKey priv;
+  EXPECT_TRUE(
+      TlsAgent::LoadKeyPairFromCert(TlsAgent::kDelegatorRsae2048, &pub, &priv));
+
+  StackSECItem dc;
+  server_->DelegateCredential(server_->name(), pub, ssl_sig_rsa_pss_rsae_sha256,
+                              kDCValidFor, now(), &dc);
+
+  SSLExtraServerCertData extra_data = {ssl_auth_null, nullptr, nullptr,
+                                       nullptr,       &dc,     priv.get()};
+  EXPECT_TRUE(server_->ConfigServerCert(server_->name(), true, &extra_data));
+  auto sfilter = MakeTlsFilter<TlsExtensionCapture>(
+      server_, ssl_delegated_credentials_xtn);
+  Connect();
+  EXPECT_FALSE(sfilter->captured());
+  CheckPeerDelegCred(client_, false);
 }
 
 // Generate a weak key.  We can't do this in the fixture because certutil
@@ -244,8 +384,12 @@ static void GenerateWeakRsaKey(ScopedSECKEYPrivateKey& priv,
 
 // Fail to connect with a weak RSA key.
 TEST_P(TlsConnectTls13, DCWeakKey) {
-  Reset(kEcdsaDelegatorId);
+  Reset(kPssDelegatorId);
   EnsureTlsSetup();
+  static const SSLSignatureScheme kSchemes[] = {ssl_sig_rsa_pss_rsae_sha256,
+                                                ssl_sig_rsa_pss_pss_sha256};
+  client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+  server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
 
   ScopedSECKEYPrivateKey dc_priv;
   ScopedSECKEYPublicKey dc_pub;
@@ -254,14 +398,14 @@ TEST_P(TlsConnectTls13, DCWeakKey) {
 
   // Construct a DC.
   StackSECItem dc;
-  TlsAgent::DelegateCredential(kEcdsaDelegatorId, dc_pub,
-                               ssl_sig_rsa_pss_rsae_sha256, kDCValidFor, now(),
+  TlsAgent::DelegateCredential(kPssDelegatorId, dc_pub,
+                               ssl_sig_rsa_pss_pss_sha256, kDCValidFor, now(),
                                &dc);
 
   // Configure the DC on the server.
   SSLExtraServerCertData extra_data = {ssl_auth_null, nullptr, nullptr,
                                        nullptr,       &dc,     dc_priv.get()};
-  EXPECT_TRUE(server_->ConfigServerCert(kEcdsaDelegatorId, true, &extra_data));
+  EXPECT_TRUE(server_->ConfigServerCert(kPssDelegatorId, true, &extra_data));
 
   client_->EnableDelegatedCredentials();
 
@@ -314,8 +458,8 @@ TEST_P(TlsConnectTls13, DCAbortBadSignature) {
                                now(), &dc);
   ASSERT_TRUE(dc.data != nullptr);
 
-  // Flip the first bit of the DC so that the signature is invalid.
-  dc.data[0] ^= 0x01;
+  // Flip the last bit of the DC so that the signature is invalid.
+  dc.data[dc.len - 1] ^= 0x01;
 
   SSLExtraServerCertData extra_data = {ssl_auth_null, nullptr, nullptr,
                                        nullptr,       &dc,     priv.get()};
@@ -339,6 +483,17 @@ TEST_P(TlsConnectTls13, DCAbortExpired) {
   server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
+// Aborted due to remaining TTL > max validity period.
+TEST_P(TlsConnectTls13, DCAbortExcessiveTTL) {
+  Reset(kEcdsaDelegatorId);
+  server_->AddDelegatedCredential(kDCId, kDCScheme,
+                                  kDCValidFor + 1 /* seconds */, now());
+  client_->EnableDelegatedCredentials();
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  client_->CheckErrorCode(SSL_ERROR_DC_INAPPROPRIATE_VALIDITY_PERIOD);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+}
+
 // Aborted because of invalid key usage.
 TEST_P(TlsConnectTls13, DCAbortBadKeyUsage) {
   // The sever does not have the delegationUsage extension.
@@ -529,20 +684,19 @@ TEST_F(DCDelegation, DCDelegations) {
   EXPECT_EQ(SSL_ERROR_INCORRECT_SIGNATURE_ALGORITHM, PORT_GetError());
 
   // Using different PSS hashes should be OK.
-  EXPECT_EQ(SECSuccess,
-            SSL_DelegateCredential(cert.get(), priv.get(), pub_rsa.get(),
-                                   ssl_sig_rsa_pss_rsae_sha256, kDCValidFor,
-                                   now, &dc));
-  // Make sure to reset |dc| after each success.
-  dc.Reset();
   EXPECT_EQ(SECSuccess, SSL_DelegateCredential(
                             cert.get(), priv.get(), pub_rsa.get(),
                             ssl_sig_rsa_pss_pss_sha256, kDCValidFor, now, &dc));
+  // Make sure to reset |dc| after each success.
   dc.Reset();
   EXPECT_EQ(SECSuccess, SSL_DelegateCredential(
                             cert.get(), priv.get(), pub_rsa.get(),
                             ssl_sig_rsa_pss_pss_sha384, kDCValidFor, now, &dc));
   dc.Reset();
+  EXPECT_EQ(SECSuccess, SSL_DelegateCredential(
+                            cert.get(), priv.get(), pub_rsa.get(),
+                            ssl_sig_rsa_pss_pss_sha512, kDCValidFor, now, &dc));
+  dc.Reset();
 
   ScopedSECKEYPublicKey pub_ecdsa;
   ScopedSECKEYPrivateKey priv_ecdsa;

lib/ssl/SSLerrs.h

@@ -582,3 +582,6 @@ ER3(SSL_ERROR_DC_INVALID_KEY_USAGE, (SSL_ERROR_BASE + 184),
 
 ER3(SSL_ERROR_DC_EXPIRED, (SSL_ERROR_BASE + 185),
     "SSL received a delegated credential that expired.")
+
+ER3(SSL_ERROR_DC_INAPPROPRIATE_VALIDITY_PERIOD, (SSL_ERROR_BASE + 186),
+    "SSL received a delegated credential with excessive TTL.")