Bug 1681585 - Update ECH to Draft-09. r=mt

This patch updates ECH implementation to draft-09. Changes of note are:

- Acceptance signal derivation is now based on the handshake secret.
- `config_id` hint changes from 32B to 8B, trial decryption added on the server.
- Duplicate code in HRR cookie handling has been consolidated into `tls13_HandleHrrCookie`.
- `ech_is_inner` extension is added, which causes a server to indicate ECH acceptance.
- Per the above, support signaling ECH acceptance when acting as a backend server in split-mode
  (i.e. when there is no other local Encrypted Client Hello state).

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

--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:519:1 changed:
+        type size hasn't changed
+        1 enumerator insertion:
+          '__anonymous_enum__::ssl_tls13_ech_is_inner_xtn' value '55817'
+        1 enumerator change:
+          '__anonymous_enum__::ssl_tls13_encrypted_client_hello_xtn' from value '65032' to '65033' at sslt.h:519:1

gtests/ssl_gtest/libssl_internals.c

@@ -494,6 +494,27 @@ SECStatus SSLInt_SetRawEchConfigForRetry(PRFileDesc *fd, const uint8_t *buf,
   sslEchConfig *cfg = (sslEchConfig *)PR_LIST_HEAD(&ss->echConfigs);
   SECITEM_FreeItem(&cfg->raw, PR_FALSE);
   SECITEM_AllocItem(NULL, &cfg->raw, len);
-  memcpy(cfg->raw.data, buf, len);
+  PORT_Memcpy(cfg->raw.data, buf, len);
+  return SECSuccess;
+}
+
+// Zero the echConfig.config_id for all configured echConfigs.
+// This mimics a collision on the 8B config ID so that we can
+// test trial decryption.
+SECStatus SSLInt_ZeroEchConfigIds(PRFileDesc *fd) {
+  if (!fd) {
+    return SECFailure;
+  }
+  sslSocket *ss = ssl_FindSocket(fd);
+  if (!ss) {
+    return SECFailure;
+  }
+
+  for (PRCList *cur_p = PR_LIST_HEAD(&ss->echConfigs); cur_p != &ss->echConfigs;
+       cur_p = PR_NEXT_LINK(cur_p)) {
+    PORT_Memset(((sslEchConfig *)cur_p)->configId, 0,
+                sizeof(((sslEchConfig *)cur_p)->configId));
+  }
+
   return SECSuccess;
 }

gtests/ssl_gtest/libssl_internals.h

@@ -51,5 +51,5 @@ SECStatus SSLInt_SetDCAdvertisedSigSchemes(PRFileDesc *fd,
 SECStatus SSLInt_RemoveServerCertificates(PRFileDesc *fd);
 SECStatus SSLInt_SetRawEchConfigForRetry(PRFileDesc *fd, const uint8_t *buf,
                                          size_t len);
-
-#endif  // ndef libssl_internals_h_
+SECStatus SSLInt_ZeroEchConfigIds(PRFileDesc *fd);
+#endif  // ifndef libssl_internals_h_

gtests/ssl_gtest/ssl_extension_unittest.cc

@@ -1098,22 +1098,6 @@ TEST_P(TlsExtensionTest13, HrrThenRemoveSupportedGroups) {
                               SSL_ERROR_MISSING_SUPPORTED_GROUPS_EXTENSION);
 }
 
-#ifdef NSS_ENABLE_DRAFT_HPKE
-TEST_P(TlsExtensionTest13, HrrThenRemoveEch) {
-  if (variant_ == ssl_variant_datagram) {
-    // ECH not supported in DTLS.
-    GTEST_SKIP();
-  }
-
-  EnsureTlsSetup();
-  SetupEch(client_, server_);
-  ExpectAlert(server_, kTlsAlertIllegalParameter);
-  HrrThenRemoveExtensionsTest(ssl_tls13_encrypted_client_hello_xtn,
-                              SSL_ERROR_ILLEGAL_PARAMETER_ALERT,
-                              SSL_ERROR_BAD_2ND_CLIENT_HELLO);
-}
-#endif
-
 TEST_P(TlsExtensionTest13, EmptyVersionList) {
   static const uint8_t ext[] = {0x00, 0x00};
   ConnectWithBogusVersionList(ext, sizeof(ext));

lib/ssl/ssl3con.c

@@ -70,6 +70,9 @@ PRBool ssl_IsRsaPssSignatureScheme(SSLSignatureScheme scheme);
 PRBool ssl_IsRsaeSignatureScheme(SSLSignatureScheme scheme);
 PRBool ssl_IsRsaPkcs1SignatureScheme(SSLSignatureScheme scheme);
 PRBool ssl_IsDsaSignatureScheme(SSLSignatureScheme scheme);
+static SECStatus ssl3_UpdateDefaultHandshakeHashes(sslSocket *ss,
+                                                   const unsigned char *b,
+                                                   unsigned int l);
 
 const PRUint8 ssl_hello_retry_random[] = {
     0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11,
@@ -3848,11 +3851,18 @@ ssl3_InitHandshakeHashes(sslSocket *ss)
 
     if (ss->ssl3.hs.hashType != handshake_hash_record &&
         ss->ssl3.hs.messages.len > 0) {
-        if (ssl3_UpdateHandshakeHashes(ss, ss->ssl3.hs.messages.buf,
-                                       ss->ssl3.hs.messages.len) != SECSuccess) {
+        /* When doing ECH, ssl3_UpdateHandshakeHashes will store outer messages into
+         * the both the outer and inner transcripts. ssl3_UpdateDefaultHandshakeHashes
+         * uses only the default context (which is the outer when doing ECH). */
+        if (ssl3_UpdateDefaultHandshakeHashes(ss, ss->ssl3.hs.messages.buf,
+                                              ss->ssl3.hs.messages.len) != SECSuccess) {
             return SECFailure;
         }
-        sslBuffer_Clear(&ss->ssl3.hs.messages);
+        /* When doing ECH, deriving accept_confirmation requires all messages
+         * up to SH, then a synthetic SH. Don't free the buffers just yet. */
+        if (!ss->ssl3.hs.echHpkeCtx) {
+            sslBuffer_Clear(&ss->ssl3.hs.messages);
+        }
     }
     if (ss->ssl3.hs.shaEchInner &&
         ss->ssl3.hs.echInnerMessages.len > 0) {
@@ -3861,7 +3871,9 @@ ssl3_InitHandshakeHashes(sslSocket *ss)
             ssl_MapLowLevelError(SSL_ERROR_DIGEST_FAILURE);
             return SECFailure;
         }
-        sslBuffer_Clear(&ss->ssl3.hs.echInnerMessages);
+        if (!ss->ssl3.hs.echHpkeCtx) {
+            sslBuffer_Clear(&ss->ssl3.hs.echInnerMessages);
+        }
     }
 
     return SECSuccess;
@@ -3893,52 +3905,28 @@ ssl3_RestartHandshakeHashes(sslSocket *ss)
     }
 }
 
-/* For TLS 1.3 EncryptedClientHello, add the provided buffer to the
- * given hash context. This is only needed for the initial CH,
- * after which ssl3_UpdateHandshakeHashes will update both contexts
- * until ssl3_CoalesceEchHandshakeHashes. */
+/* Add the provided bytes to the handshake hash context. When doing
+ * TLS 1.3 ECH, |target| may be provided to specify only the inner/outer
+ * transcript, else the input is added to both contexts. This happens
+ * only on the client. On the server, only the default context is used. */
 SECStatus
-ssl3_UpdateExplicitHandshakeTranscript(sslSocket *ss, const unsigned char *b,
-                                       unsigned int l, sslBuffer *target)
-{
-    PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
-    PORT_Assert(ss->vrange.max >= SSL_LIBRARY_VERSION_TLS_1_3);
-    if (ss->sec.isServer) {
-        /* Only the client maintains two states at the outset. */
-        PORT_Assert(target != &ss->ssl3.hs.echInnerMessages);
-    }
-    return sslBuffer_Append(target, b, l);
-}
-static SECStatus
-ssl3_UpdateOuterHandshakeHashes(sslSocket *ss, const unsigned char *b,
-                                unsigned int l)
+ssl3_UpdateHandshakeHashesInt(sslSocket *ss, const unsigned char *b,
+                              unsigned int l, sslBuffer *target)
 {
-    return ssl3_UpdateExplicitHandshakeTranscript(ss, b, l,
-                                                  &ss->ssl3.hs.messages);
-}
-static SECStatus
-ssl3_UpdateInnerHandshakeHashes(sslSocket *ss, const unsigned char *b,
-                                unsigned int l)
-{
-    return ssl3_UpdateExplicitHandshakeTranscript(ss, b, l,
-                                                  &ss->ssl3.hs.echInnerMessages);
-}
-/*
- * Handshake messages
- */
-/* Called from  ssl3_InitHandshakeHashes()
-**      ssl3_AppendHandshake()
-**      ssl3_HandleV2ClientHello()
-**      ssl3_HandleHandshakeMessage()
-** Caller must hold the ssl3Handshake lock.
-*/
-SECStatus
-ssl3_UpdateHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l)
-{
-    SECStatus rv = SECSuccess;
 
+    SECStatus rv = SECSuccess;
+    PRBool explicit = (target != NULL);
+    PRBool appendToEchInner = !ss->sec.isServer &&
+                              ss->ssl3.hs.echHpkeCtx &&
+                              !explicit;
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
+    PORT_Assert(target != &ss->ssl3.hs.echInnerMessages ||
+                !ss->sec.isServer);
 
+    if (target == NULL) {
+        /* Default context. */
+        target = &ss->ssl3.hs.messages;
+    }
     /* With TLS 1.3, and versions TLS.1.1 and older, we keep the hash(es)
      * always up to date. However, we must initially buffer the handshake
      * messages, until we know what to do.
@@ -3950,15 +3938,14 @@ ssl3_UpdateHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l
      * and never update the hash, because the hash function we must use for
      * certificate_verify might be different from the hash function we use
      * when signing other handshake hashes. */
-
     if (ss->ssl3.hs.hashType == handshake_hash_unknown ||
         ss->ssl3.hs.hashType == handshake_hash_record) {
-        rv = sslBuffer_Append(&ss->ssl3.hs.messages, b, l);
+        rv = sslBuffer_Append(target, b, l);
         if (rv != SECSuccess) {
             return SECFailure;
         }
-        if (!ss->sec.isServer && ss->ssl3.hs.echHpkeCtx) {
-            return ssl3_UpdateInnerHandshakeHashes(ss, b, l);
+        if (appendToEchInner) {
+            return sslBuffer_Append(&ss->ssl3.hs.echInnerMessages, b, l);
         }
         return SECSuccess;
     }
@@ -3967,10 +3954,20 @@ ssl3_UpdateHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l
 
     if (ss->ssl3.hs.hashType == handshake_hash_single) {
         PORT_Assert(ss->version >= SSL_LIBRARY_VERSION_TLS_1_3);
-        rv = PK11_DigestOp(ss->ssl3.hs.sha, b, l);
-        if (rv != SECSuccess) {
-            ssl_MapLowLevelError(SSL_ERROR_DIGEST_FAILURE);
-            return rv;
+        if (target == &ss->ssl3.hs.messages) {
+            rv = PK11_DigestOp(ss->ssl3.hs.sha, b, l);
+            if (rv != SECSuccess) {
+                ssl_MapLowLevelError(SSL_ERROR_DIGEST_FAILURE);
+                return rv;
+            }
+        }
+        if (ss->ssl3.hs.shaEchInner &&
+            (target == &ss->ssl3.hs.echInnerMessages || !explicit)) {
+            rv = PK11_DigestOp(ss->ssl3.hs.shaEchInner, b, l);
+            if (rv != SECSuccess) {
+                ssl_MapLowLevelError(SSL_ERROR_DIGEST_FAILURE);
+                return rv;
+            }
         }
     } else if (ss->ssl3.hs.hashType == handshake_hash_combo) {
         rv = PK11_DigestOp(ss->ssl3.hs.md5, b, l);
@@ -3987,6 +3984,37 @@ ssl3_UpdateHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l
     return rv;
 }
 
+static SECStatus
+ssl3_UpdateDefaultHandshakeHashes(sslSocket *ss, const unsigned char *b,
+                                  unsigned int l)
+{
+    return ssl3_UpdateHandshakeHashesInt(ss, b, l,
+                                         &ss->ssl3.hs.messages);
+}
+
+static SECStatus
+ssl3_UpdateInnerHandshakeHashes(sslSocket *ss, const unsigned char *b,
+                                unsigned int l)
+{
+    return ssl3_UpdateHandshakeHashesInt(ss, b, l,
+                                         &ss->ssl3.hs.echInnerMessages);
+}
+
+/*
+ * Handshake messages
+ */
+/* Called from  ssl3_InitHandshakeHashes()
+**      ssl3_AppendHandshake()
+**      ssl3_HandleV2ClientHello()
+**      ssl3_HandleHandshakeMessage()
+** Caller must hold the ssl3Handshake lock.
+*/
+SECStatus
+ssl3_UpdateHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l)
+{
+    return ssl3_UpdateHandshakeHashesInt(ss, b, l, NULL);
+}
+
 SECStatus
 ssl3_UpdatePostHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l)
 {
@@ -5513,7 +5541,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
         if (rv != SECSuccess) {
             goto loser; /* code set */
         }
-        rv = ssl3_UpdateOuterHandshakeHashes(ss, chBuf.buf, chBuf.len);
+        rv = ssl3_UpdateDefaultHandshakeHashes(ss, chBuf.buf, chBuf.len);
         if (rv != SECSuccess) {
             goto loser; /* code set */
         }
@@ -7064,11 +7092,6 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
         return SECSuccess;
     }
 
-    rv = tls13_MaybeHandleEchSignal(ss);
-    if (rv != SECSuccess) {
-        goto alert_loser;
-    }
-
     rv = ssl3_HandleParsedExtensions(ss, ssl_hs_server_hello);
     ssl3_DestroyRemoteExtensions(&ss->ssl3.hs.remoteExtensions);
     if (rv != SECSuccess) {
@@ -7082,7 +7105,7 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
     }
 
     if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
-        rv = tls13_HandleServerHelloPart2(ss);
+        rv = tls13_HandleServerHelloPart2(ss, savedMsg, savedLength);
         if (rv != SECSuccess) {
             errCode = PORT_GetError();
             goto loser;
@@ -7093,6 +7116,7 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
             goto loser;
     }
 
+    ss->ssl3.hs.preliminaryInfo |= ssl_preinfo_ech;
     return SECSuccess;
 
 alert_loser:
@@ -8628,13 +8652,6 @@ ssl_GenerateServerRandom(sslSocket *ss)
         return SECFailure;
     }
 
-    if (ss->ssl3.hs.echAccepted) {
-        rv = tls13_WriteServerEchSignal(ss);
-        if (rv != SECSuccess) {
-            return SECFailure;
-        }
-    }
-
     if (ss->version == ss->vrange.max) {
         return SECSuccess;
     }
@@ -9775,6 +9792,15 @@ ssl_ConstructServerHello(sslSocket *ss, PRBool helloRetry,
         }
     }
 
+    if (!helloRetry && ssl3_ExtensionNegotiated(ss, ssl_tls13_ech_is_inner_xtn)) {
+        /* Signal ECH acceptance if we handled handled both CHOuter/CHInner (i.e.
+         * in shared mode), or if we received a CHInner in split/backend mode. */
+        if (ss->ssl3.hs.echAccepted || ss->opt.enableTls13BackendEch) {
+            return tls13_WriteServerEchSignal(ss, SSL_BUFFER_BASE(messageBuf),
+                                              SSL_BUFFER_LEN(messageBuf));
+        }
+    }
+
     return SECSuccess;
 }
 
@@ -12284,7 +12310,7 @@ ssl_HashHandshakeMessageDefault(sslSocket *ss, SSLHandshakeType ct,
                                 const PRUint8 *b, PRUint32 length)
 {
     return ssl_HashHandshakeMessageInt(ss, ct, ss->ssl3.hs.recvMessageSeq,
-                                       b, length, ssl3_UpdateOuterHandshakeHashes);
+                                       b, length, ssl3_UpdateDefaultHandshakeHashes);
 }
 SECStatus
 ssl_HashHandshakeMessageEchInner(sslSocket *ss, SSLHandshakeType ct,
@@ -13847,6 +13873,7 @@ ssl3_DestroySSL3Info(sslSocket *ss)
     /* TLS 1.3 ECH state. */
     PK11_HPKE_DestroyContext(ss->ssl3.hs.echHpkeCtx, PR_TRUE);
     PORT_Free((void *)ss->ssl3.hs.echPublicName); /* CONST */
+    sslBuffer_Clear(&ss->ssl3.hs.greaseEchBuf);
 }
 
 /*

lib/ssl/ssl3ext.c

@@ -15,6 +15,7 @@
 #include "sslimpl.h"
 #include "sslproto.h"
 #include "ssl3exthandle.h"
+#include "tls13ech.h"
 #include "tls13err.h"
 #include "tls13exthandle.h"
 #include "tls13subcerts.h"
@@ -54,6 +55,7 @@ static const ssl3ExtensionHandler clientHelloHandlers[] = {
     { ssl_tls13_psk_key_exchange_modes_xtn, &tls13_ServerHandlePskModesXtn },
     { ssl_tls13_cookie_xtn, &tls13_ServerHandleCookieXtn },
     { ssl_tls13_post_handshake_auth_xtn, &tls13_ServerHandlePostHandshakeAuthXtn },
+    { ssl_tls13_ech_is_inner_xtn, &tls13_ServerHandleEchIsInnerXtn },
     { ssl_record_size_limit_xtn, &ssl_HandleRecordSizeLimitXtn },
     { 0, NULL }
 };
@@ -1020,12 +1022,8 @@ ssl3_DestroyExtensionData(TLSExtensionData *xtnData)
     PORT_Free(xtnData->advertised);
     tls13_DestroyDelegatedCredential(xtnData->peerDelegCred);
 
-    /* ECH State */
-    SECITEM_FreeItem(&xtnData->innerCh, PR_FALSE);
-    SECITEM_FreeItem(&xtnData->echSenderPubKey, PR_FALSE);
-    SECITEM_FreeItem(&xtnData->echConfigId, PR_FALSE);
-    SECITEM_FreeItem(&xtnData->echRetryConfigs, PR_FALSE);
-    xtnData->echRetryConfigsValid = PR_FALSE;
+    tls13_DestroyEchXtnState(xtnData->ech);
+    xtnData->ech = NULL;
 }
 
 /* Free everything that has been allocated and then reset back to

lib/ssl/ssl3ext.h

@@ -131,13 +131,9 @@ struct TLSExtensionDataStr {
      * rather through tls13_DestoryPskList(). */
     sslPsk *selectedPsk;
 
-    /* ECH working state. */
-    SECItem innerCh;             /* Server: "payload value of ClientECH. */
-    SECItem echSenderPubKey;     /* Server: "enc value of ClientECH, required for CHInner decryption. */
-    SECItem echConfigId;         /* Server: "config_id" value of ClientECH.  */
-    PRUint32 echCipherSuite;     /* Server: "cipher_suite" value of ClientECH. */
-    SECItem echRetryConfigs;     /* Client: Retry_configs from ServerEncryptedCH. */
-    PRBool echRetryConfigsValid; /* Client: Permits retry_configs to be extracted. */
+    /* ECH working state. Non-null when a valid Encrypted Client Hello extension
+     * was received. */
+    sslEchXtnState *ech;
 };
 
 typedef struct TLSExtensionStr {