Record ClientHelloInner values in msg_callback.

In testing out the ECH bits on the Chromium side, it is much harder to
tell what's going on without some indication that we sent a
ClientHelloInner. This CL routes it into the callback. A corresponding
CL in Chromium will add it to NetLog.

Bug: 275
Change-Id: I945ab2679614583e875a0ba90d6cf1481ed315d9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51205
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

include/openssl/ssl.h

@@ -4039,10 +4039,16 @@ OPENSSL_EXPORT int SSL_set_handshake_hints(SSL *ssl, const uint8_t *hints,
 // |len| bytes from |buf| contain the handshake message, one-byte
 // ChangeCipherSpec body, and two-byte alert, respectively.
 //
+// In connections that enable ECH, |cb| is additionally called with
+// |content_type| = |SSL3_RT_CLIENT_HELLO_INNER| for each ClientHelloInner that
+// is encrypted or decrypted. The |len| bytes from |buf| contain the
+// ClientHelloInner, including the reconstructed outer extensions and handshake
+// header.
+//
 // For a V2ClientHello, |version| is |SSL2_VERSION|, |content_type| is zero, and
 // the |len| bytes from |buf| contain the V2ClientHello structure.
 OPENSSL_EXPORT void SSL_CTX_set_msg_callback(
-    SSL_CTX *ctx, void (*cb)(int write_p, int version, int content_type,
+    SSL_CTX *ctx, void (*cb)(int is_write, int version, int content_type,
                              const void *buf, size_t len, SSL *ssl, void *arg));
 
 // SSL_CTX_set_msg_callback_arg sets the |arg| parameter of the message

include/openssl/ssl3.h

@@ -275,6 +275,7 @@ OPENSSL_STATIC_ASSERT(SSL3_RT_MAX_ENCRYPTED_OVERHEAD >=
 
 // Pseudo content type for SSL/TLS header info
 #define SSL3_RT_HEADER 0x100
+#define SSL3_RT_CLIENT_HELLO_INNER 0x101
 
 #define SSL3_AL_WARNING 1
 #define SSL3_AL_FATAL 2

ssl/encrypted_client_hello.cc

@@ -319,8 +319,14 @@ bool ssl_client_hello_decrypt(SSL_HANDSHAKE *hs, uint8_t *out_alert,
   encoded.Shrink(len);
 #endif
 
-  return ssl_decode_client_hello_inner(hs->ssl, out_alert, out, encoded,
-                                       client_hello_outer);
+  if (!ssl_decode_client_hello_inner(hs->ssl, out_alert, out, encoded,
+                                     client_hello_outer)) {
+    return false;
+  }
+
+  ssl_do_msg_callback(hs->ssl, /*is_write=*/0, SSL3_RT_CLIENT_HELLO_INNER,
+                      *out);
+  return true;
 }
 
 static bool is_hex_component(Span<const uint8_t> in) {
@@ -802,6 +808,8 @@ bool ssl_encrypt_client_hello(SSL_HANDSHAKE *hs, Span<const uint8_t> enc) {
                    binder_len);
   }
 
+  ssl_do_msg_callback(ssl, /*is_write=*/1, SSL3_RT_CLIENT_HELLO_INNER,
+                      hello_inner);
   if (!hs->inner_transcript.Update(hello_inner)) {
     return false;
   }

ssl/handshake_client.cc

@@ -331,7 +331,7 @@ bool ssl_add_client_hello(SSL_HANDSHAKE *hs) {
   Array<uint8_t> msg;
   if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CLIENT_HELLO) ||
       !ssl_write_client_hello_without_extensions(hs, &body, type,
-                                                 /*empty_session_id*/ false) ||
+                                                 /*empty_session_id=*/false) ||
       !ssl_add_clienthello_tlsext(hs, &body, /*out_encoded=*/nullptr,
                                   &needs_psk_binder, type, CBB_len(&body)) ||
       !ssl->method->finish_message(ssl, cbb.get(), &msg)) {

ssl/internal.h

@@ -3513,7 +3513,7 @@ struct ssl_ctx_st {
   bssl::UniquePtr<bssl::CERT> cert;
 
   // callback that allows applications to peek at protocol messages
-  void (*msg_callback)(int write_p, int version, int content_type,
+  void (*msg_callback)(int is_write, int version, int content_type,
                        const void *buf, size_t len, SSL *ssl,
                        void *arg) = nullptr;
   void *msg_callback_arg = nullptr;

ssl/test/runner/runner.go

@@ -16869,6 +16869,45 @@ func addEncryptedClientHelloTests() {
 				expectedLocalError: "remote error: illegal parameter",
 				expectedError:      ":INVALID_OUTER_EXTENSION:",
 			})
+
+			// Test the message callback is correctly reported with ECH.
+			clientAndServerHello := "read hs 1\nread clienthelloinner\nwrite hs 2\n"
+			expectMsgCallback := clientAndServerHello + "write ccs\n"
+			if hrr {
+				expectMsgCallback += clientAndServerHello
+			}
+			// EncryptedExtensions onwards.
+			expectMsgCallback += `write hs 8
+write hs 11
+write hs 15
+write hs 20
+read hs 20
+write hs 4
+write hs 4
+`
+			testCases = append(testCases, testCase{
+				testType: serverTest,
+				protocol: protocol,
+				name:     prefix + "ECH-Server-MessageCallback" + suffix,
+				config: Config{
+					ServerName:      "secret.example",
+					ClientECHConfig: echConfig.ECHConfig,
+					DefaultCurves:   defaultCurves,
+					Bugs: ProtocolBugs{
+						NoCloseNotify: true, // Align QUIC and TCP traces.
+					},
+				},
+				flags: []string{
+					"-ech-server-config", base64FlagValue(echConfig.ECHConfig.Raw),
+					"-ech-server-key", base64FlagValue(echConfig.Key),
+					"-ech-is-retry-config", "1",
+					"-expect-ech-accept",
+					"-expect-msg-callback", expectMsgCallback,
+				},
+				expectations: connectionExpectations{
+					echAccepted: true,
+				},
+			})
 		}
 
 		// Test that ECH, which runs before an async early callback, interacts
@@ -18618,6 +18657,60 @@ func addEncryptedClientHelloTests() {
 			shouldFail:    true,
 			expectedError: ":INCONSISTENT_ECH_NEGOTIATION:",
 		})
+
+		// Test the message callback is correctly reported, with and without
+		// HelloRetryRequest.
+		clientAndServerHello := "write clienthelloinner\nwrite hs 1\nread hs 2\n"
+		// EncryptedExtensions onwards.
+		finishHandshake := `read hs 8
+read hs 11
+read hs 15
+read hs 20
+write hs 20
+read hs 4
+read hs 4
+`
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-MessageCallback",
+			config: Config{
+				MinVersion:       VersionTLS13,
+				MaxVersion:       VersionTLS13,
+				ServerECHConfigs: []ServerECHConfig{echConfig},
+				Bugs: ProtocolBugs{
+					NoCloseNotify: true, // Align QUIC and TCP traces.
+				},
+			},
+			flags: []string{
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-ech-accept",
+				"-expect-msg-callback", clientAndServerHello + "write ccs\n" + finishHandshake,
+			},
+			expectations: connectionExpectations{echAccepted: true},
+		})
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-MessageCallback-HelloRetryRequest",
+			config: Config{
+				MinVersion:       VersionTLS13,
+				MaxVersion:       VersionTLS13,
+				CurvePreferences: []CurveID{CurveP384},
+				ServerECHConfigs: []ServerECHConfig{echConfig},
+				Bugs: ProtocolBugs{
+					ExpectMissingKeyShare: true, // Check we triggered HRR.
+					NoCloseNotify:         true, // Align QUIC and TCP traces.
+				},
+			},
+			flags: []string{
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-ech-accept",
+				"-expect-hrr", // Check we triggered HRR.
+				"-expect-msg-callback", clientAndServerHello + "write ccs\n" + clientAndServerHello + finishHandshake,
+			},
+			expectations: connectionExpectations{echAccepted: true},
+		})
 	}
 }
 

ssl/test/test_config.cc

@@ -602,6 +602,7 @@ static void MessageCallback(int is_write, int version, int content_type,
       state->msg_callback_text += "v2clienthello\n";
       return;
 
+    case SSL3_RT_CLIENT_HELLO_INNER:
     case SSL3_RT_HANDSHAKE: {
       CBS cbs;
       CBS_init(&cbs, buf_u8, len);
@@ -619,10 +620,19 @@ static void MessageCallback(int is_write, int version, int content_type,
         return;
       }
       char text[16];
-      snprintf(text, sizeof(text), "hs %d\n", type);
-      state->msg_callback_text += text;
-      if (!is_write) {
-        state->last_message_received = type;
+      if (content_type == SSL3_RT_CLIENT_HELLO_INNER) {
+        if (type != SSL3_MT_CLIENT_HELLO) {
+          fprintf(stderr, "Invalid header for ClientHelloInner.\n");
+          state->msg_callback_ok = false;
+          return;
+        }
+        state->msg_callback_text += "clienthelloinner\n";
+      } else {
+        snprintf(text, sizeof(text), "hs %d\n", type);
+        state->msg_callback_text += text;
+        if (!is_write) {
+          state->last_message_received = type;
+        }
       }
       return;
     }