Bug 1771100 - Added ECH server support to BoGo shim. Fixed NSS ECH server accept_confirmation bugs. r=djackson

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

8 files changed, 86 insertions, 33 deletions

diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c
index ad7480586..99dcc32fa 100644
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -3835,7 +3835,7 @@ ssl3_InitHandshakeHashes(sslSocket *ss)
                 return SECFailure;
             }
 
-            /* Alternate transcript hash used in Encrypted Client Hello. */
+            /* Transcript hash used on ECH client. */
             if (!ss->sec.isServer && ss->ssl3.hs.echHpkeCtx) {
                 ss->ssl3.hs.shaEchInner = PK11_CreateDigestContext(hash_oid->offset);
                 if (ss->ssl3.hs.shaEchInner == NULL) {
@@ -3877,16 +3877,21 @@ ssl3_InitHandshakeHashes(sslSocket *ss)
 
     if (ss->ssl3.hs.hashType != handshake_hash_record &&
         ss->ssl3.hs.messages.len > 0) {
-        /* 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). */
+        /* When doing ECH, ssl3_UpdateHandshakeHashes will store outer messages
+         * into the both the outer and inner transcripts.
+         * ssl3_UpdateDefaultHandshakeHashes uses the default context which is
+         * the outer when doing client ECH. For ECH shared-mode or backend
+         * servers only the hs.messages buffer is used. */
         if (ssl3_UpdateDefaultHandshakeHashes(ss, ss->ssl3.hs.messages.buf,
                                               ss->ssl3.hs.messages.len) != SECSuccess) {
             return SECFailure;
         }
-        /* 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) {
+        /* When doing ECH, deriving the accept_confirmation value requires all
+         * messages up to and including the ServerHello
+         * (see draft-ietf-tls-esni-14, Section 7.2).
+         *
+         * Don't free the transcript buffer until confirmation calculation. */
+        if (!ss->ssl3.hs.echHpkeCtx && !ss->opt.enableTls13BackendEch) {
             sslBuffer_Clear(&ss->ssl3.hs.messages);
         }
     }
diff --git a/lib/ssl/sslsecur.c b/lib/ssl/sslsecur.c
index 0422b39c7..4d2b2070c 100644
--- a/lib/ssl/sslsecur.c
+++ b/lib/ssl/sslsecur.c
@@ -183,6 +183,11 @@ SSL_ResetHandshake(PRFileDesc *s, PRBool asServer)
         PORT_Assert(ss->ssl3.hs.echPublicName);
         PORT_Free((void *)ss->ssl3.hs.echPublicName); /* CONST */
         ss->ssl3.hs.echPublicName = NULL;
+    }
+    /* Make sure greaseEchBuf is freed in ECH setups without echHpkeCtx. */
+    if (ss->ssl3.hs.echHpkeCtx ||
+        ss->opt.enableTls13BackendEch ||
+        ss->opt.enableTls13GreaseEch) {
         sslBuffer_Clear(&ss->ssl3.hs.greaseEchBuf);
     }
 
diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c
index 51dfebff3..fac00cea3 100644
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -2169,6 +2169,7 @@ tls13_ConstructHelloRetryRequest(sslSocket *ss,
                                  ssl3CipherSuite cipherSuite,
                                  const sslNamedGroupDef *selectedGroup,
                                  PRUint8 *cookie, unsigned int cookieLen,
+                                 const PRUint8 *cookieGreaseEchSignal,
                                  sslBuffer *buffer)
 {
     SECStatus rv;
@@ -2180,8 +2181,24 @@ tls13_ConstructHelloRetryRequest(sslSocket *ss,
     ss->xtnData.selectedGroup = selectedGroup;
     ss->xtnData.cookie.data = cookie;
     ss->xtnData.cookie.len = cookieLen;
+
+    /* Set restored ss->ssl3.hs.greaseEchBuf value for ECH HRR extension
+     * reconstruction. */
+    if (cookieGreaseEchSignal) {
+        PORT_Assert(!ss->ssl3.hs.greaseEchBuf.len);
+        rv = sslBuffer_Append(&ss->ssl3.hs.greaseEchBuf,
+                              cookieGreaseEchSignal,
+                              TLS13_ECH_SIGNAL_LEN);
+        if (rv != SECSuccess) {
+            goto loser;
+        }
+    }
     rv = ssl_ConstructExtensions(ss, &extensionsBuf,
                                  ssl_hs_hello_retry_request);
+    /* Reset ss->ssl3.hs.greaseEchBuf if it was changed. */
+    if (cookieGreaseEchSignal) {
+        sslBuffer_Clear(&ss->ssl3.hs.greaseEchBuf);
+    }
     if (rv != SECSuccess) {
         goto loser;
     }
@@ -2220,21 +2237,35 @@ tls13_SendHelloRetryRequest(sslSocket *ss,
 
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
 
+    /* If an ECH backend or shared-mode server accepted ECH when offered,
+     * the HRR extension's payload must be set to 8 zero bytes, these are
+     * overwritten with the accept_confirmation value after the handshake
+     * transcript calculation.
+     * If a client-facing or shared-mode server did not accept ECH when offered
+     * OR if ECH GREASE is enabled on the server and a ECH extension was
+     * received, a 8 byte random value is set as the extension's payload
+     * [draft-ietf-tls-esni-14, Section 7].
+     *
+     * The (temporary) payload is written to the extension in tls13exthandle.c/
+     * tls13_ServerSendHrrEchXtn(). */
     if (ss->xtnData.ech) {
         PRUint8 echGreaseRaw[TLS13_ECH_SIGNAL_LEN] = { 0 };
-        if (!ss->ssl3.hs.echAccepted) {
+        if (!(ss->ssl3.hs.echAccepted ||
+              (ss->opt.enableTls13BackendEch &&
+               ss->xtnData.ech &&
+               ss->xtnData.ech->receivedInnerXtn))) {
             rv = PK11_GenerateRandom(echGreaseRaw, TLS13_ECH_SIGNAL_LEN);
             if (rv != SECSuccess) {
                 return SECFailure;
             }
+            SSL_TRC(100, ("Generated random value for ECH HRR GREASE."));
         }
         sslBuffer echGreaseBuffer = SSL_BUFFER_EMPTY;
         rv = sslBuffer_Append(&echGreaseBuffer, echGreaseRaw, sizeof(echGreaseRaw));
         if (rv != SECSuccess) {
             return SECFailure;
         }
-        /* Store the GREASE signal so we can later include it in the cookie and HRR extension */
-        SSL_TRC(100, ("Generating and storing a random value for ECH HRR Grease value."));
+        /* HRR GREASE/accept_confirmation zero bytes placeholder buffer. */
         ss->ssl3.hs.greaseEchBuf = echGreaseBuffer;
     }
 
@@ -2250,7 +2281,8 @@ tls13_SendHelloRetryRequest(sslSocket *ss,
     /* Now build the body of the message. */
     rv = tls13_ConstructHelloRetryRequest(ss, ss->ssl3.hs.cipher_suite,
                                           requestedGroup,
-                                          cookie, cookieLen, &messageBuf);
+                                          cookie, cookieLen,
+                                          NULL, &messageBuf);
     if (rv != SECSuccess) {
         FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
         return SECFailure;
@@ -5048,8 +5080,11 @@ tls13_FinishHandshake(sslSocket *ss)
 
     TLS13_SET_HS_STATE(ss, idle_handshake);
 
-    PORT_Assert(ss->ssl3.hs.echAccepted ==
-                ssl3_ExtensionNegotiated(ss, ssl_tls13_encrypted_client_hello_xtn));
+    PORT_Assert(ss->ssl3.hs.echAccepted ||
+                (ss->opt.enableTls13BackendEch &&
+                 ss->xtnData.ech &&
+                 ss->xtnData.ech->receivedInnerXtn) ==
+                    ssl3_ExtensionNegotiated(ss, ssl_tls13_encrypted_client_hello_xtn));
     if (offeredEch && !ss->ssl3.hs.echAccepted) {
         SSL3_SendAlert(ss, alert_fatal, ech_required);
 
diff --git a/lib/ssl/tls13con.h b/lib/ssl/tls13con.h
index 8c648ac30..a635dbd86 100644
--- a/lib/ssl/tls13con.h
+++ b/lib/ssl/tls13con.h
@@ -84,6 +84,7 @@ SECStatus tls13_ConstructHelloRetryRequest(sslSocket *ss,
                                            const sslNamedGroupDef *selectedGroup,
                                            PRUint8 *cookie,
                                            unsigned int cookieLen,
+                                           const PRUint8 *cookieGreaseEchSignal,
                                            sslBuffer *buffer);
 SECStatus tls13_HandleHelloRetryRequest(sslSocket *ss, const PRUint8 *b,
                                         PRUint32 length);
diff --git a/lib/ssl/tls13ech.c b/lib/ssl/tls13ech.c
index 7adeabf76..d752d6ebf 100644
--- a/lib/ssl/tls13ech.c
+++ b/lib/ssl/tls13ech.c
@@ -1990,10 +1990,10 @@ loser:
     return SECFailure;
 }
 
-/* Compute the ECH signal using the transcript (up to, excluding) Server Hello.
- * We'll append an artificial SH (ServerHelloECHConf). The server sources
- * this transcript prefix from ss->ssl3.hs.messages, as it never uses
- * ss->ssl3.hs.echInnerMessages. The client uses the inner transcript, echInnerMessages. */
+/* Compute the ECH signal using the transcript (up to, including)
+ * ServerHello. The server sources this transcript prefix from
+ * ss->ssl3.hs.messages, as it never uses ss->ssl3.hs.echInnerMessages.
+ * The client uses the inner transcript, echInnerMessages. */
 SECStatus
 tls13_ComputeEchSignal(sslSocket *ss, PRBool isHrr, const PRUint8 *sh, unsigned int shLen, PRUint8 *out)
 {
@@ -2655,11 +2655,10 @@ tls13_MaybeAcceptEch(sslSocket *ss, const SECItem *sidBytes, const PRUint8 *chOu
         }
 
         ss->ssl3.hs.echHpkeCtx = echData.hpkeCtx;
-        ss->ssl3.hs.greaseEchBuf = echData.signal;
 
         const PRUint8 greaseConstant[TLS13_ECH_SIGNAL_LEN] = { 0 };
         ss->ssl3.hs.echAccepted = previouslyOfferedEch &&
-                                  !NSS_SecureMemcmp(greaseConstant, ss->ssl3.hs.greaseEchBuf.buf, TLS13_ECH_SIGNAL_LEN);
+                                  !NSS_SecureMemcmp(greaseConstant, echData.signal, TLS13_ECH_SIGNAL_LEN);
 
         if (echData.configId != ss->xtnData.ech->configId ||
             echData.kdfId != ss->xtnData.ech->kdfId ||
@@ -2787,7 +2786,8 @@ tls13_WriteServerEchHrrSignal(sslSocket *ss, PRUint8 *sh, unsigned int shLen)
     if (rv != SECSuccess) {
         return SECFailure;
     }
-    /* We extract this value from the HRR Cookie later when we need it. */
+    /* Free HRR GREASE/accept_confirmation value, it MUST be restored from
+     * cookie when handling CH2 after HRR. */
     sslBuffer_Clear(&ss->ssl3.hs.greaseEchBuf);
     return SECSuccess;
 }
\ No newline at end of file
diff --git a/lib/ssl/tls13ech.h b/lib/ssl/tls13ech.h
index c60375660..0ad98592d 100644
--- a/lib/ssl/tls13ech.h
+++ b/lib/ssl/tls13ech.h
@@ -57,7 +57,7 @@ struct sslEchCookieDataStr {
     HpkeKdfId kdfId;
     HpkeAeadId aeadId;
     HpkeContext *hpkeCtx;
-    sslBuffer signal;
+    PRUint8 signal[TLS13_ECH_SIGNAL_LEN];
 };
 
 struct sslEchConfigStr {
diff --git a/lib/ssl/tls13exthandle.c b/lib/ssl/tls13exthandle.c
index 6acdf2473..c797feb26 100644
--- a/lib/ssl/tls13exthandle.c
+++ b/lib/ssl/tls13exthandle.c
@@ -1482,13 +1482,27 @@ tls13_ServerSendEchXtn(const sslSocket *ss,
     return SECSuccess;
 }
 
-/* If ECH is accepted, this value will be a placeholder and overwritten later. */
+/* If an ECH server sends the HRR ECH extension after it accepted ECH, the
+ * extension's payload must be set to 8 zero bytes, these are overwritten with
+ * the accept_confirmation value after the required transcript calculation.
+ * If a client-facing/shared-mode server did not accept ECH when offered in CH
+ * or if ECH GREASE is enabled on the server and a ECH extension was received,
+ * a 8 byte random value is set as the extension's payload
+ * [draft-ietf-tls-esni-14, Section 7].
+ *
+ * Depending on the acceptance of ECH, zero or random bytes are written to
+ * ss->ssl3.hs.greaseEchBuf.buf in tls13con.c/tls13_SendHelloRetryRequest(). */
 SECStatus
 tls13_ServerSendHrrEchXtn(const sslSocket *ss, TLSExtensionData *xtnData,
                           sslBuffer *buf, PRBool *added)
 {
     SECStatus rv;
-    if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3 || !xtnData->ech || (!ss->echPubKey && !ss->opt.enableTls13GreaseEch)) {
+    /* Do not send HRR ECH extension if TLS < 1.3 was negotiated OR no ECH
+     * extension was received OR the server is NOT in any ECH server mode AND
+     * ECH GREASE is NOT enabled. */
+    if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3 ||
+        !xtnData->ech ||
+        (!ss->echPubKey && !ss->opt.enableTls13BackendEch && !ss->opt.enableTls13GreaseEch)) {
         SSL_TRC(100, ("%d: TLS13[%d]: server not sending HRR ECH Xtn",
                       SSL_GETPID(), ss->fd));
         return SECSuccess;
diff --git a/lib/ssl/tls13hashstate.c b/lib/ssl/tls13hashstate.c
index dbfb7d42c..d14e32558 100644
--- a/lib/ssl/tls13hashstate.c
+++ b/lib/ssl/tls13hashstate.c
@@ -171,7 +171,6 @@ tls13_HandleHrrCookie(sslSocket *ss,
     PRUint64 tmp64;
     const sslNamedGroupDef *selectedGroup;
     PRUint64 appTokenLen;
-    sslBuffer greaseBuf = SSL_BUFFER_EMPTY;
 
     rv = ssl_SelfEncryptUnprotect(ss, cookie, cookieLen,
                                   plaintext, &plaintextLen, sizeof(plaintext));
@@ -235,20 +234,13 @@ tls13_HandleHrrCookie(sslSocket *ss,
         }
         parsedEchData.aeadId = (HpkeAeadId)tmp64;
 
+        /* ECH accept_confirmation signal. */
         rv = sslRead_Read(&reader, TLS13_ECH_SIGNAL_LEN, &greaseReadBuf);
         if (rv != SECSuccess) {
             FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter);
             return SECFailure;
         }
-
-        if (echData) {
-            rv = sslBuffer_Append(&greaseBuf, greaseReadBuf.buf, greaseReadBuf.len);
-            if (rv != SECSuccess) {
-                FATAL_ERROR(ss, SSL_ERROR_INTERNAL_ERROR_ALERT, internal_error);
-                return SECFailure;
-            }
-            parsedEchData.signal = greaseBuf;
-        }
+        PORT_Memcpy(parsedEchData.signal, greaseReadBuf.buf, TLS13_ECH_SIGNAL_LEN);
 
         /* ECH HPKE context may be empty. */
         rv = sslRead_ReadVariable(&reader, 2, &echHpkeBuf);
@@ -311,6 +303,7 @@ tls13_HandleHrrCookie(sslSocket *ss,
         rv = tls13_ConstructHelloRetryRequest(ss, cipherSuite,
                                               selectedGroup,
                                               cookie, cookieLen,
+                                              parsedEchData.signal,
                                               &messageBuf);
         if (rv != SECSuccess) {
             return SECFailure;