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>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 0aaabfa..a3b530e 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4039,10 +4039,16 @@
// |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
diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index e3910f0..533142c 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -275,6 +275,7 @@
// 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
diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc
index 7326df9..9e9adfe 100644
--- a/ssl/encrypted_client_hello.cc
+++ b/ssl/encrypted_client_hello.cc
@@ -319,8 +319,14 @@
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 @@
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;
}
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 17b41e0..e630121 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -331,7 +331,7 @@
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)) {
diff --git a/ssl/internal.h b/ssl/internal.h
index 67cf5b0..8f68fc5 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -3513,7 +3513,7 @@
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;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index acffde9..370fa94 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -16869,6 +16869,45 @@
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 @@
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},
+ })
}
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 9a0f63d..a6409d6 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -602,6 +602,7 @@
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 @@
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;
}
