Bug 1588244 - Store TLS 1.3 peerDelegCred, authKeyBits, and scheme in…

… SSLPreliminaryChannelInfo. r=mt

This patch adjusts where we set `authKeyBits` (Et al.) for TLS 1.3, such that `CertVerifier` can check the strength of a delegated credential keypair.


The corresponding PSM changeset is in D47181.

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

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

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

@@ -0,0 +1,13 @@
+
+1 function with some indirect sub-type change:
+
+  [C]'function SECStatus SSL_GetPreliminaryChannelInfo(PRFileDesc*, SSLPreliminaryChannelInfo*, PRUintn)' at sslinfo.c:113:1 has some indirect sub-type changes:
+    parameter 2 of type 'SSLPreliminaryChannelInfo*' has sub-type changes:
+      in pointed to type 'typedef SSLPreliminaryChannelInfo' at sslt.h:424:1:
+        underlying type 'struct SSLPreliminaryChannelInfoStr' at sslt.h:373:1 changed:
+          type size changed from 192 to 288 (in bits)
+          3 data member insertions:
+            'PRBool SSLPreliminaryChannelInfoStr::peerDelegCred', at offset 192 (in bits) at sslt.h:418:1
+            'PRUint32 SSLPreliminaryChannelInfoStr::authKeyBits', at offset 224 (in bits) at sslt.h:419:1
+            'SSLSignatureScheme SSLPreliminaryChannelInfoStr::signatureScheme', at offset 256 (in bits) at sslt.h:420:1
+

gtests/ssl_gtest/libssl_internals.c

@@ -12,6 +12,29 @@
 #include "seccomon.h"
 #include "selfencrypt.h"
 
+SECStatus SSLInt_TweakChannelInfoForDC(PRFileDesc *fd, PRBool changeAuthKeyBits,
+                                       PRBool changeScheme) {
+  if (!fd) {
+    return SECFailure;
+  }
+  sslSocket *ss = ssl_FindSocket(fd);
+  if (!ss) {
+    return SECFailure;
+  }
+
+  // Just toggle so we'll always have a valid value.
+  if (changeScheme) {
+    ss->sec.signatureScheme = (ss->sec.signatureScheme == ssl_sig_ed25519)
+                                  ? ssl_sig_ecdsa_secp256r1_sha256
+                                  : ssl_sig_ed25519;
+  }
+  if (changeAuthKeyBits) {
+    ss->sec.authKeyBits = ss->sec.authKeyBits ? ss->sec.authKeyBits * 2 : 384;
+  }
+
+  return SECSuccess;
+}
+
 SECStatus SSLInt_GetHandshakeRandoms(PRFileDesc *fd, SSL3Random client_random,
                                      SSL3Random server_random) {
   if (!fd) {

gtests/ssl_gtest/libssl_internals.h

@@ -42,5 +42,7 @@ SECStatus SSLInt_AdvanceWriteSeqByAWindow(PRFileDesc *fd, PRInt32 extra);
 SSLKEAType SSLInt_GetKEAType(SSLNamedGroup group);
 SECStatus SSLInt_HasPendingHandshakeData(PRFileDesc *fd, PRBool *pending);
 SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size);
+SECStatus SSLInt_TweakChannelInfoForDC(PRFileDesc *fd, PRBool changeAuthKeyBits,
+                                       PRBool changeScheme);
 
 #endif  // ndef libssl_internals_h_

gtests/ssl_gtest/tls_agent.cc

@@ -842,6 +842,13 @@ void TlsAgent::ResetPreliminaryInfo() {
   expected_cipher_suite_ = 0;
 }
 
+void TlsAgent::UpdatePreliminaryChannelInfo() {
+  SECStatus rv = SSL_GetPreliminaryChannelInfo(ssl_fd_.get(), &pre_info_,
+                                               sizeof(pre_info_));
+  EXPECT_EQ(SECSuccess, rv);
+  EXPECT_EQ(sizeof(pre_info_), pre_info_.length);
+}
+
 void TlsAgent::ValidateCipherSpecs() {
   PRInt32 cipherSpecs = SSLInt_CountCipherSpecs(ssl_fd());
   // We use one ciphersuite in each direction.
@@ -904,6 +911,7 @@ void TlsAgent::Connected() {
   // Preliminary values are exposed through callbacks during the handshake.
   // If either expected values were set or the callbacks were called, check
   // that the final values are correct.
+  UpdatePreliminaryChannelInfo();
   EXPECT_EQ(expected_version_, info_.protocolVersion);
   EXPECT_EQ(expected_cipher_suite_, info_.cipherSuite);
 

gtests/ssl_gtest/tls_agent.h

@@ -134,6 +134,7 @@ class TlsAgent : public PollTarget {
   void AddDelegatedCredential(const std::string& dc_name,
                               SSLSignatureScheme dcCertVerifyAlg,
                               PRUint32 dcValidFor, PRTime now);
+  void UpdatePreliminaryChannelInfo();
 
   bool ConfigServerCert(const std::string& name, bool updateKeyBits = false,
                         const SSLExtraServerCertData* serverCertData = nullptr);
@@ -228,6 +229,9 @@ class TlsAgent : public PollTarget {
     EXPECT_EQ(STATE_CONNECTED, state_);
     return info_;
   }
+
+  const SSLPreliminaryChannelInfo& pre_info() const { return pre_info_; }
+
   bool is_compressed() const {
     return info().compressionMethod != ssl_compression_null;
   }
@@ -425,6 +429,7 @@ class TlsAgent : public PollTarget {
   bool handshake_callback_called_;
   bool resumption_callback_called_;
   SSLChannelInfo info_;
+  SSLPreliminaryChannelInfo pre_info_;
   SSLCipherSuiteInfo csinfo_;
   SSLVersionRange vrange_;
   PRErrorCode error_code_;

gtests/ssl_gtest/tls_filter.h

@@ -441,6 +441,19 @@ class TlsExtensionDropper : public TlsExtensionFilter {
   uint16_t extension_;
 };
 
+class TlsHandshakeDropper : public TlsHandshakeFilter {
+ public:
+  TlsHandshakeDropper(const std::shared_ptr<TlsAgent>& a)
+      : TlsHandshakeFilter(a) {}
+
+ protected:
+  PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
+                                       const DataBuffer& input,
+                                       DataBuffer* output) override {
+    return DROP;
+  }
+};
+
 class TlsExtensionInjector : public TlsHandshakeFilter {
  public:
   TlsExtensionInjector(const std::shared_ptr<TlsAgent>& a, uint16_t ext,

gtests/ssl_gtest/tls_subcerts_unittest.cc

@@ -22,14 +22,63 @@ const std::string kDCId = TlsAgent::kServerEcdsa256;
 const SSLSignatureScheme kDCScheme = ssl_sig_ecdsa_secp256r1_sha256;
 const PRUint32 kDCValidFor = 60 * 60 * 24 * 7 /* 1 week (seconds */;
 
+static void CheckPreliminaryPeerDelegCred(
+    const std::shared_ptr<TlsAgent>& client, bool expected,
+    PRUint32 key_bits = 0, SSLSignatureScheme sig_scheme = ssl_sig_none) {
+  EXPECT_NE(0U, (client->pre_info().valuesSet & ssl_preinfo_peer_auth));
+  EXPECT_EQ(expected, client->pre_info().peerDelegCred);
+  if (expected) {
+    EXPECT_EQ(key_bits, client->pre_info().authKeyBits);
+    EXPECT_EQ(sig_scheme, client->pre_info().signatureScheme);
+  }
+}
+
 static void CheckPeerDelegCred(const std::shared_ptr<TlsAgent>& client,
                                bool expected, PRUint32 key_bits = 0) {
   EXPECT_EQ(expected, client->info().peerDelegCred);
+  EXPECT_EQ(expected, client->pre_info().peerDelegCred);
   if (expected) {
     EXPECT_EQ(key_bits, client->info().authKeyBits);
+    EXPECT_EQ(key_bits, client->pre_info().authKeyBits);
+    EXPECT_EQ(client->info().signatureScheme,
+              client->pre_info().signatureScheme);
   }
 }
 
+// AuthCertificate callbacks to simulate DC validation
+static SECStatus CheckPreliminaryDC(TlsAgent* agent, bool checksig,
+                                    bool isServer) {
+  agent->UpdatePreliminaryChannelInfo();
+  EXPECT_EQ(PR_TRUE, agent->pre_info().peerDelegCred);
+  EXPECT_EQ(256U, agent->pre_info().authKeyBits);
+  EXPECT_EQ(ssl_sig_ecdsa_secp256r1_sha256, agent->pre_info().signatureScheme);
+  return SECSuccess;
+}
+
+static SECStatus CheckPreliminaryNoDC(TlsAgent* agent, bool checksig,
+                                      bool isServer) {
+  agent->UpdatePreliminaryChannelInfo();
+  EXPECT_EQ(PR_FALSE, agent->pre_info().peerDelegCred);
+  return SECSuccess;
+}
+
+// AuthCertificate callbacks for modifying DC attributes.
+// This allows testing tls13_CertificateVerify for rejection
+// of DC attributes that have changed since AuthCertificateHook
+// may have handled them.
+static SECStatus ModifyDCAuthKeyBits(TlsAgent* agent, bool checksig,
+                                     bool isServer) {
+  return SSLInt_TweakChannelInfoForDC(agent->ssl_fd(),
+                                      PR_TRUE,    // Change authKeyBits
+                                      PR_FALSE);  // Change scheme
+}
+
+static SECStatus ModifyDCScheme(TlsAgent* agent, bool checksig, bool isServer) {
+  return SSLInt_TweakChannelInfoForDC(agent->ssl_fd(),
+                                      PR_FALSE,  // Change authKeyBits
+                                      PR_TRUE);  // Change scheme
+}
+
 // Attempt to configure a DC when either the DC or DC private key is missing.
 TEST_P(TlsConnectTls13, DCNotConfigured) {
   // Load and delegate the credential.
@@ -388,6 +437,77 @@ TEST_P(TlsConnectTls13, DCConnectExpectedCertVerifyAlgNotSupported) {
   CheckPeerDelegCred(client_, false);
 }
 
+// Check that preliminary channel info properly reflects the DC.
+TEST_P(TlsConnectTls13, DCCheckPreliminaryInfo) {
+  Reset(kEcdsaDelegatorId);
+  EnsureTlsSetup();
+  client_->EnableDelegatedCredentials();
+  server_->AddDelegatedCredential(TlsAgent::kServerEcdsa256,
+                                  ssl_sig_ecdsa_secp256r1_sha256, kDCValidFor,
+                                  now());
+
+  auto filter = MakeTlsFilter<TlsHandshakeDropper>(server_);
+  filter->SetHandshakeTypes(
+      {kTlsHandshakeCertificateVerify, kTlsHandshakeFinished});
+  filter->EnableDecryption();
+  StartConnect();
+  client_->Handshake();  // Send ClientHello
+  server_->Handshake();  // Send ServerHello
+
+  client_->SetAuthCertificateCallback(CheckPreliminaryDC);
+  client_->Handshake();  // Process response
+
+  client_->UpdatePreliminaryChannelInfo();
+  CheckPreliminaryPeerDelegCred(client_, true, 256,
+                                ssl_sig_ecdsa_secp256r1_sha256);
+}
+
+// Check that preliminary channel info properly reflects a lack of DC.
+TEST_P(TlsConnectTls13, DCCheckPreliminaryInfoNoDC) {
+  Reset(kEcdsaDelegatorId);
+  EnsureTlsSetup();
+  client_->EnableDelegatedCredentials();
+  auto filter = MakeTlsFilter<TlsHandshakeDropper>(server_);
+  filter->SetHandshakeTypes(
+      {kTlsHandshakeCertificateVerify, kTlsHandshakeFinished});
+  filter->EnableDecryption();
+  StartConnect();
+  client_->Handshake();  // Send ClientHello
+  server_->Handshake();  // Send ServerHello
+
+  client_->SetAuthCertificateCallback(CheckPreliminaryNoDC);
+  client_->Handshake();  // Process response
+
+  client_->UpdatePreliminaryChannelInfo();
+  CheckPreliminaryPeerDelegCred(client_, false);
+}
+
+// Tweak the scheme in between |Cert| and |CertVerify|.
+TEST_P(TlsConnectTls13, DCRejectModifiedDCScheme) {
+  Reset(kEcdsaDelegatorId);
+  client_->EnableDelegatedCredentials();
+  client_->SetAuthCertificateCallback(ModifyDCScheme);
+  server_->AddDelegatedCredential(TlsAgent::kServerEcdsa521,
+                                  ssl_sig_ecdsa_secp521r1_sha512, kDCValidFor,
+                                  now());
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+  client_->CheckErrorCode(SSL_ERROR_DC_CERT_VERIFY_ALG_MISMATCH);
+}
+
+// Tweak the authKeyBits in between |Cert| and |CertVerify|.
+TEST_P(TlsConnectTls13, DCRejectModifiedDCAuthKeyBits) {
+  Reset(kEcdsaDelegatorId);
+  client_->EnableDelegatedCredentials();
+  client_->SetAuthCertificateCallback(ModifyDCAuthKeyBits);
+  server_->AddDelegatedCredential(TlsAgent::kServerEcdsa521,
+                                  ssl_sig_ecdsa_secp521r1_sha512, kDCValidFor,
+                                  now());
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+  client_->CheckErrorCode(SSL_ERROR_DC_CERT_VERIFY_ALG_MISMATCH);
+}
+
 class DCDelegation : public ::testing::Test {};
 
 TEST_F(DCDelegation, DCDelegations) {

lib/ssl/ssl3con.c

@@ -21,6 +21,7 @@
 #include "sslerr.h"
 #include "ssl3ext.h"
 #include "ssl3exthandle.h"
+#include "tls13subcerts.h"
 #include "prtime.h"
 #include "prinrval.h"
 #include "prerror.h"
@@ -11047,6 +11048,47 @@ ssl_SetAuthKeyBits(sslSocket *ss, const SECKEYPublicKey *pubKey)
                         : illegal_parameter);
         return SECFailure;
     }
+
+    /* PreliminaryChannelInfo.authKeyBits, scheme, and peerDelegCred are now valid. */
+    ss->ssl3.hs.preliminaryInfo |= ssl_preinfo_peer_auth;
+
+    return SECSuccess;
+}
+
+SECStatus
+ssl3_HandleServerSpki(sslSocket *ss)
+{
+    PORT_Assert(!ss->sec.isServer);
+    SECKEYPublicKey *pubKey;
+
+    if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3 &&
+        tls13_IsVerifyingWithDelegatedCredential(ss)) {
+        sslDelegatedCredential *dc = ss->xtnData.peerDelegCred;
+        pubKey = SECKEY_ExtractPublicKey(dc->spki);
+        if (!pubKey) {
+            PORT_SetError(SSL_ERROR_EXTRACT_PUBLIC_KEY_FAILURE);
+            return SECFailure;
+        }
+
+        /* Because we have only a single authType (ssl_auth_tls13_any)
+         * for TLS 1.3 at this point, set the scheme so that the
+         * callback can interpret |authKeyBits| correctly.
+         */
+        ss->sec.signatureScheme = dc->expectedCertVerifyAlg;
+    } else {
+        pubKey = CERT_ExtractPublicKey(ss->sec.peerCert);
+        if (!pubKey) {
+            PORT_SetError(SSL_ERROR_EXTRACT_PUBLIC_KEY_FAILURE);
+            return SECFailure;
+        }
+    }
+
+    SECStatus rv = ssl_SetAuthKeyBits(ss, pubKey);
+    SECKEY_DestroyPublicKey(pubKey);
+    if (rv != SECSuccess) {
+        return rv; /* Alert sent and code set. */
+    }
+
     return SECSuccess;
 }
 
@@ -11061,6 +11103,26 @@ ssl3_AuthCertificate(sslSocket *ss)
 
     PORT_Assert((ss->ssl3.hs.preliminaryInfo & ssl_preinfo_all) ==
                 ssl_preinfo_all);
+
+    if (!ss->sec.isServer) {
+        /* Set the |spki| used to verify the handshake. When verifying with a
+         * delegated credential (DC), this corresponds to the DC public key;
+         * otherwise it correspond to the public key of the peer's end-entity
+         * certificate. */
+        rv = ssl3_HandleServerSpki(ss);
+        if (rv != SECSuccess) {
+            /* Alert sent and code set (if not SSL_ERROR_EXTRACT_PUBLIC_KEY_FAILURE).
+             * In either case, we're done here. */
+            errCode = PORT_GetError();
+            goto loser;
+        }
+
+        if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
+            ss->sec.authType = ss->ssl3.hs.kea_def->authKeyType;
+            ss->sec.keaType = ss->ssl3.hs.kea_def->exchKeyType;
+        }
+    }
+
     /*
      * Ask caller-supplied callback function to validate cert chain.
      */
@@ -11099,21 +11161,6 @@ ssl3_AuthCertificate(sslSocket *ss)
     ss->sec.ci.sid->peerCert = CERT_DupCertificate(ss->sec.peerCert);
 
     if (!ss->sec.isServer) {
-        if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
-            /* These are filled in in tls13_HandleCertificateVerify and
-             * tls13_HandleServerKeyShare (keaType). */
-            SECKEYPublicKey *pubKey = CERT_ExtractPublicKey(ss->sec.peerCert);
-            if (pubKey) {
-                rv = ssl_SetAuthKeyBits(ss, pubKey);
-                SECKEY_DestroyPublicKey(pubKey);
-                if (rv != SECSuccess) {
-                    return SECFailure; /* Alert sent and code set. */
-                }
-            }
-            ss->sec.authType = ss->ssl3.hs.kea_def->authKeyType;
-            ss->sec.keaType = ss->ssl3.hs.kea_def->exchKeyType;
-        }
-
         if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
             TLS13_SET_HS_STATE(ss, wait_cert_verify);
         } else {

lib/ssl/sslimpl.h

@@ -1678,6 +1678,7 @@ SECStatus ssl3_SendEmptyCertificate(sslSocket *ss);
 void ssl3_CleanupPeerCerts(sslSocket *ss);
 SECStatus ssl3_SendCertificateStatus(sslSocket *ss);
 SECStatus ssl_SetAuthKeyBits(sslSocket *ss, const SECKEYPublicKey *pubKey);
+SECStatus ssl3_HandleServerSpki(sslSocket *ss);
 SECStatus ssl3_AuthCertificate(sslSocket *ss);
 SECStatus ssl_ReadCertificateStatus(sslSocket *ss, PRUint8 *b,
                                     PRUint32 length);

lib/ssl/sslinfo.c

@@ -154,6 +154,10 @@ SSL_GetPreliminaryChannelInfo(PRFileDesc *fd,
     }
     inf.zeroRttCipherSuite = ss->ssl3.hs.zeroRttSuite;
 
+    inf.peerDelegCred = tls13_IsVerifyingWithDelegatedCredential(ss);
+    inf.authKeyBits = ss->sec.authKeyBits;
+    inf.signatureScheme = ss->sec.signatureScheme;
+
     memcpy(info, &inf, inf.length);
     return SECSuccess;
 }