commit e21e50eb2c38ad5de6493147475b54e5e02ca596
author David Benjamin <davidben@google.com> Wed Nov 27 18:03:28 2024 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Dec 05 23:48:18 2024 +0000
tree 5986d374c7b689183b19ea14530d0a1a41183857
parent c971a961b6f413adf4374a8b2b2fbb0ef3d65e17

Don't report ChangeCipherSpec through the message callback in QUIC

Reporting it doesn't make much sense when QUIC doesn't send
ChangeCipherSpec in the first place.

Change-Id: I34af531eb14a37a0aa90da447146d5290db24494
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73727
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>

ssl/s3_both.cc

diff --git a/ssl/s3_both.cc b/ssl/s3_both.cc
index 82671c0..6d6b357 100644
--- a/ssl/s3_both.cc
+++ b/ssl/s3_both.cc
@@ -263,13 +263,12 @@
 }
 
 bool tls_add_change_cipher_spec(SSL *ssl) {
-  static const uint8_t kChangeCipherSpec[1] = {SSL3_MT_CCS};
-
-  if (!tls_flush_pending_hs_data(ssl)) {
-    return false;
+  if (SSL_is_quic(ssl)) {
+    return true;
   }
 
-  if (!SSL_is_quic(ssl) &&
+  static const uint8_t kChangeCipherSpec[1] = {SSL3_MT_CCS};
+  if (!tls_flush_pending_hs_data(ssl) ||
       !add_record_to_flight(ssl, SSL3_RT_CHANGE_CIPHER_SPEC,
                             kChangeCipherSpec)) {
     return false;

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 7173f92..f6b545a 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -18875,7 +18875,10 @@
 
 			// Test the message callback is correctly reported with ECH.
 			clientAndServerHello := "read hs 1\nread clienthelloinner\nwrite hs 2\n"
-			expectMsgCallback := clientAndServerHello + "write ccs\n"
+			expectMsgCallback := clientAndServerHello
+			if protocol == tls {
+				expectMsgCallback += "write ccs\n"
+			}
 			if hrr {
 				expectMsgCallback += clientAndServerHello
 			}
@@ -20744,6 +20747,10 @@
 		// Test the message callback is correctly reported, with and without
 		// HelloRetryRequest.
 		clientAndServerHello := "write clienthelloinner\nwrite hs 1\nread hs 2\n"
+		clientAndServerHelloInitial := clientAndServerHello
+		if protocol == tls {
+			clientAndServerHelloInitial += "write ccs\n"
+		}
 		// EncryptedExtensions onwards.
 		finishHandshake := `read hs 8
 read hs 11
@@ -20768,7 +20775,7 @@
 			flags: []string{
 				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
 				"-expect-ech-accept",
-				"-expect-msg-callback", clientAndServerHello + "write ccs\n" + finishHandshake,
+				"-expect-msg-callback", clientAndServerHelloInitial + finishHandshake,
 			},
 			expectations: connectionExpectations{echAccepted: true},
 		})
@@ -20790,7 +20797,7 @@
 				"-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,
+				"-expect-msg-callback", clientAndServerHelloInitial + clientAndServerHello + finishHandshake,
 			},
 			expectations: connectionExpectations{echAccepted: true},
 		})