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 <[email protected]> Reviewed-by: Nick Harper <[email protected]>
google · Dec 5, 2024 · e21e50e · e21e50e
1 parent c971a96
commit e21e50e
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 8 deletions.
diff --git a/ssl/s3_both.cc b/ssl/s3_both.cc
@@ -263,13 +263,12 @@ bool tls_flush_pending_hs_data(SSL *ssl) {
 }
 
 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;

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
@@ -18875,7 +18875,10 @@ func addEncryptedClientHelloTests() {
 
 			// 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 @@ write hs 4
 		// 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 @@ read hs 4
 			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 @@ read hs 4
 				"-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},
 		})