gfe-relnote: Call SetDefaultEncryptionLevel after setting crypto_negotiated_params in TlsServerHandshaker and TlsClientHandshaker. Protected by disabled QUIC versions. Also add a QUIC_BUG in QuicSession::SetDefaultEncryptionLevel to check this condition. This fixes a DCHECK failure in chrome in which the handshake completes, but GetSSLInfo fails because the cipher suite is not yet set. https://bugs.chromium.org/p/chromium/issues/detail?id=1032263 PiperOrigin-RevId: 286011546 Change-Id: Ie9e03fa5cf6e3c346181435d45f362de10dc7083
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 2867f5e..ac5f707 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -74,7 +74,10 @@ QuicCryptoHandshaker(this, session), encryption_established_(false), handshake_confirmed_(false), - params_(new QuicCryptoNegotiatedParameters) {} + params_(new QuicCryptoNegotiatedParameters) { + // Simulate a negotiated cipher_suite with a fake value. + params_->cipher_suite = 1; + } void OnHandshakeMessage(const CryptoHandshakeMessage& /*message*/) override { encryption_established_ = true;
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 9a0bae6..74dc15d 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -1357,6 +1357,11 @@ case ENCRYPTION_HANDSHAKE: break; case ENCRYPTION_FORWARD_SECURE: + if (connection_->version().handshake_protocol == PROTOCOL_TLS1_3) { + QUIC_BUG_IF(!GetCryptoStream()->crypto_negotiated_params().cipher_suite) + << ENDPOINT + << "Handshake confirmed without cipher suite negotiation."; + } QUIC_BUG_IF(!config_.negotiated()) << ENDPOINT << "Handshake confirmed without parameter negotiation."; break;
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 4c7e112..935ef0b 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -61,7 +61,10 @@ QuicCryptoHandshaker(this, session), encryption_established_(false), handshake_confirmed_(false), - params_(new QuicCryptoNegotiatedParameters) {} + params_(new QuicCryptoNegotiatedParameters) { + // Simulate a negotiated cipher_suite with a fake value. + params_->cipher_suite = 1; + } void OnHandshakeMessage(const CryptoHandshakeMessage& /*message*/) override { encryption_established_ = true;
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc index b027004..5d9ffd4 100644 --- a/quic/core/tls_client_handshaker.cc +++ b/quic/core/tls_client_handshaker.cc
@@ -337,7 +337,6 @@ encryption_established_ = true; handshake_confirmed_ = true; - delegate()->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); // Fill crypto_negotiated_params_: const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl()); @@ -347,6 +346,8 @@ crypto_negotiated_params_->key_exchange_group = SSL_get_curve_id(ssl()); crypto_negotiated_params_->peer_signature_algorithm = SSL_get_peer_signature_algorithm(ssl()); + + delegate()->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); // TODO(fayang): Replace this with DiscardOldKeys(ENCRYPTION_HANDSHAKE) when // handshake key discarding settles down. delegate()->NeuterHandshakeData();
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc index 2d8cd79..87c146f 100644 --- a/quic/core/tls_server_handshaker.cc +++ b/quic/core/tls_server_handshaker.cc
@@ -265,7 +265,6 @@ encryption_established_ = true; handshake_confirmed_ = true; - delegate()->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); // Fill crypto_negotiated_params_: const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl()); @@ -273,6 +272,8 @@ crypto_negotiated_params_->cipher_suite = SSL_CIPHER_get_value(cipher); } crypto_negotiated_params_->key_exchange_group = SSL_get_curve_id(ssl()); + + delegate()->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); // TODO(fayang): Replace this with DiscardOldKeys(ENCRYPTION_HANDSHAKE) when // handshake key discarding settles down. delegate()->NeuterHandshakeData();