Delay delivering 1-RTT read key to QUIC This change simulates the behavior pending in https://boringssl-review.googlesource.com/c/boringssl/+/40127, so that when BoringSSL is updated it will be a no-op. gfe-relnote: Protected by reloadable flag quic_enable_version_draft_25_v2 PiperOrigin-RevId: 297650164 Change-Id: I6822ebbd3cb95abb5ef816a2629e5e4b6b61b630
diff --git a/quic/core/quic_version_manager.cc b/quic/core/quic_version_manager.cc index b6a3c53..c5ccf62 100644 --- a/quic/core/quic_version_manager.cc +++ b/quic/core/quic_version_manager.cc
@@ -17,7 +17,7 @@ ParsedQuicVersionVector supported_versions) : enable_version_t099_(GetQuicReloadableFlag(quic_enable_version_t099)), enable_version_draft_25_( - GetQuicReloadableFlag(quic_enable_version_draft_25)), + GetQuicReloadableFlag(quic_enable_version_draft_25_v2)), disable_version_q050_(GetQuicReloadableFlag(quic_disable_version_q050)), enable_version_t050_(GetQuicReloadableFlag(quic_enable_version_t050)), disable_version_q049_(GetQuicReloadableFlag(quic_disable_version_q049)), @@ -48,7 +48,7 @@ "Supported versions out of sync"); if (enable_version_t099_ != GetQuicReloadableFlag(quic_enable_version_t099) || enable_version_draft_25_ != - GetQuicReloadableFlag(quic_enable_version_draft_25) || + GetQuicReloadableFlag(quic_enable_version_draft_25_v2) || disable_version_q050_ != GetQuicReloadableFlag(quic_disable_version_q050) || enable_version_t050_ != GetQuicReloadableFlag(quic_enable_version_t050) || @@ -62,7 +62,7 @@ GetQuicReloadableFlag(quic_disable_version_q043)) { enable_version_t099_ = GetQuicReloadableFlag(quic_enable_version_t099); enable_version_draft_25_ = - GetQuicReloadableFlag(quic_enable_version_draft_25); + GetQuicReloadableFlag(quic_enable_version_draft_25_v2); disable_version_q050_ = GetQuicReloadableFlag(quic_disable_version_q050); enable_version_t050_ = GetQuicReloadableFlag(quic_enable_version_t050); disable_version_q049_ = GetQuicReloadableFlag(quic_disable_version_q049);
diff --git a/quic/core/quic_version_manager.h b/quic/core/quic_version_manager.h index 19b4928..c434dae 100644 --- a/quic/core/quic_version_manager.h +++ b/quic/core/quic_version_manager.h
@@ -43,7 +43,7 @@ // Cached value of reloadable flags. // quic_enable_version_t099 flag bool enable_version_t099_; - // quic_enable_version_draft_25 flag + // quic_enable_version_draft_25_v2 flag bool enable_version_draft_25_; // quic_disable_version_q050 flag bool disable_version_q050_;
diff --git a/quic/core/quic_version_manager_test.cc b/quic/core/quic_version_manager_test.cc index dd3c064..e64bc3a 100644 --- a/quic/core/quic_version_manager_test.cc +++ b/quic/core/quic_version_manager_test.cc
@@ -19,7 +19,7 @@ static_assert(SupportedVersions().size() == 8u, "Supported versions out of sync"); SetQuicReloadableFlag(quic_enable_version_t099, false); - SetQuicReloadableFlag(quic_enable_version_draft_25, false); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false); SetQuicReloadableFlag(quic_enable_version_t050, false); SetQuicReloadableFlag(quic_disable_version_q050, false); SetQuicReloadableFlag(quic_disable_version_q049, false); @@ -52,7 +52,7 @@ EXPECT_EQ(FilterSupportedVersions(AllSupportedVersions()), manager.GetSupportedVersions()); - SetQuicReloadableFlag(quic_enable_version_draft_25, true); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true); expected_parsed_versions.push_back( ParsedQuicVersion(PROTOCOL_TLS1_3, QUIC_VERSION_IETF_DRAFT_25)); EXPECT_EQ(expected_parsed_versions, manager.GetSupportedVersions());
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc index cbfdcaa..e6efcc5 100644 --- a/quic/core/quic_versions.cc +++ b/quic/core/quic_versions.cc
@@ -293,7 +293,7 @@ } } else if (version.transport_version == QUIC_VERSION_IETF_DRAFT_25) { QUIC_BUG_IF(version.handshake_protocol != PROTOCOL_TLS1_3); - if (GetQuicReloadableFlag(quic_enable_version_draft_25)) { + if (GetQuicReloadableFlag(quic_enable_version_draft_25_v2)) { filtered_versions.push_back(version); } } else if (version.transport_version == QUIC_VERSION_50) { @@ -548,7 +548,7 @@ SetQuicReloadableFlag(quic_enable_version_t099, true); } else if (parsed_version.transport_version == QUIC_VERSION_IETF_DRAFT_25) { QUIC_BUG_IF(parsed_version.handshake_protocol != PROTOCOL_TLS1_3); - SetQuicReloadableFlag(quic_enable_version_draft_25, true); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true); } else if (parsed_version.transport_version == QUIC_VERSION_50) { if (parsed_version.handshake_protocol == PROTOCOL_QUIC_CRYPTO) { SetQuicReloadableFlag(quic_disable_version_q050, false);
diff --git a/quic/core/quic_versions_test.cc b/quic/core/quic_versions_test.cc index 55c38bd..78f3f3b 100644 --- a/quic/core/quic_versions_test.cc +++ b/quic/core/quic_versions_test.cc
@@ -270,7 +270,7 @@ static_assert(SupportedVersions().size() == 8u, "Supported versions out of sync"); SetQuicReloadableFlag(quic_enable_version_t099, true); - SetQuicReloadableFlag(quic_enable_version_draft_25, true); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true); SetQuicReloadableFlag(quic_enable_version_t050, true); SetQuicReloadableFlag(quic_disable_version_q050, false); SetQuicReloadableFlag(quic_disable_version_q049, false); @@ -305,7 +305,7 @@ static_assert(SupportedVersions().size() == 8u, "Supported versions out of sync"); SetQuicReloadableFlag(quic_enable_version_t099, false); - SetQuicReloadableFlag(quic_enable_version_draft_25, true); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true); SetQuicReloadableFlag(quic_enable_version_t050, true); SetQuicReloadableFlag(quic_disable_version_q050, false); SetQuicReloadableFlag(quic_disable_version_q049, false); @@ -337,7 +337,7 @@ static_assert(SupportedVersions().size() == 8u, "Supported versions out of sync"); SetQuicReloadableFlag(quic_enable_version_t099, false); - SetQuicReloadableFlag(quic_enable_version_draft_25, false); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false); SetQuicReloadableFlag(quic_enable_version_t050, false); SetQuicReloadableFlag(quic_disable_version_q050, false); SetQuicReloadableFlag(quic_disable_version_q049, false); @@ -458,9 +458,9 @@ { QuicFlagSaver flag_saver; - SetQuicReloadableFlag(quic_enable_version_draft_25, false); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false); QuicEnableVersion(parsed_version_draft_25); - EXPECT_TRUE(GetQuicReloadableFlag(quic_enable_version_draft_25)); + EXPECT_TRUE(GetQuicReloadableFlag(quic_enable_version_draft_25_v2)); } {
diff --git a/quic/core/tls_handshaker.cc b/quic/core/tls_handshaker.cc index d8cf338..ac8bcee 100644 --- a/quic/core/tls_handshaker.cc +++ b/quic/core/tls_handshaker.cc
@@ -56,9 +56,8 @@ ssl(), TlsConnection::BoringEncryptionLevel(level)); } -const EVP_MD* TlsHandshaker::Prf() { - return EVP_get_digestbynid( - SSL_CIPHER_get_prf_nid(SSL_get_pending_cipher(ssl()))); +const EVP_MD* TlsHandshaker::Prf(const SSL_CIPHER* cipher) { + return EVP_get_digestbynid(SSL_CIPHER_get_prf_nid(cipher)); } void TlsHandshaker::SetWriteSecret(EncryptionLevel level, @@ -66,7 +65,7 @@ const std::vector<uint8_t>& write_secret) { std::unique_ptr<QuicEncrypter> encrypter = QuicEncrypter::CreateFromCipherSuite(SSL_CIPHER_get_id(cipher)); - CryptoUtils::SetKeyAndIV(Prf(), write_secret, encrypter.get()); + CryptoUtils::SetKeyAndIV(Prf(cipher), write_secret, encrypter.get()); handshaker_delegate_->OnNewEncryptionKeyAvailable(level, std::move(encrypter)); } @@ -76,7 +75,7 @@ const std::vector<uint8_t>& read_secret) { std::unique_ptr<QuicDecrypter> decrypter = QuicDecrypter::CreateFromCipherSuite(SSL_CIPHER_get_id(cipher)); - CryptoUtils::SetKeyAndIV(Prf(), read_secret, decrypter.get()); + CryptoUtils::SetKeyAndIV(Prf(cipher), read_secret, decrypter.get()); return handshaker_delegate_->OnNewDecryptionKeyAvailable( level, std::move(decrypter), /*set_alternative_decrypter=*/false,
diff --git a/quic/core/tls_handshaker.h b/quic/core/tls_handshaker.h index ab84e3b..3d5f1e2 100644 --- a/quic/core/tls_handshaker.h +++ b/quic/core/tls_handshaker.h
@@ -60,7 +60,7 @@ const std::string& reason_phrase) = 0; // Returns the PRF used by the cipher suite negotiated in the TLS handshake. - const EVP_MD* Prf(); + const EVP_MD* Prf(const SSL_CIPHER* cipher); virtual const TlsConnection* tls_connection() const = 0;
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc index e5587cd..bb8a821 100644 --- a/quic/core/tls_server_handshaker.cc +++ b/quic/core/tls_server_handshaker.cc
@@ -160,6 +160,19 @@ return TlsHandshaker::BufferSizeLimitForLevel(level); } +bool TlsServerHandshaker::SetReadSecret( + EncryptionLevel level, + const SSL_CIPHER* cipher, + const std::vector<uint8_t>& read_secret) { + if (level != ENCRYPTION_FORWARD_SECURE || one_rtt_keys_available_) { + return TlsHandshaker::SetReadSecret(level, cipher, read_secret); + } + // Delay setting read secret for ENCRYPTION_FORWARD_SECURE until handshake + // completes. + app_data_read_secret_ = read_secret; + return true; +} + void TlsServerHandshaker::AdvanceHandshake() { if (state_ == STATE_CONNECTION_CLOSED) { QUIC_LOG(INFO) << "TlsServerHandshaker received handshake message after " @@ -287,6 +300,16 @@ } crypto_negotiated_params_->key_exchange_group = SSL_get_curve_id(ssl()); + if (!app_data_read_secret_.empty()) { + if (!SetReadSecret(ENCRYPTION_FORWARD_SECURE, cipher, + app_data_read_secret_)) { + QUIC_BUG << "Failed to set forward secure read key."; + CloseConnection(QUIC_HANDSHAKE_FAILED, "Failed to set app data read key"); + return; + } + app_data_read_secret_.clear(); + } + handshaker_delegate()->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); handshaker_delegate()->DiscardOldEncryptionKey(ENCRYPTION_HANDSHAKE); handshaker_delegate()->DiscardOldDecryptionKey(ENCRYPTION_HANDSHAKE);
diff --git a/quic/core/tls_server_handshaker.h b/quic/core/tls_server_handshaker.h index d377f4d..ff0d6b9 100644 --- a/quic/core/tls_server_handshaker.h +++ b/quic/core/tls_server_handshaker.h
@@ -65,6 +65,13 @@ return &tls_connection_; } + // Override of TlsHandshaker::SetReadSecret so that setting the read secret + // for ENCRYPTION_FORWARD_SECURE can be delayed until the handshake is + // complete. + bool SetReadSecret(EncryptionLevel level, + const SSL_CIPHER* cipher, + const std::vector<uint8_t>& read_secret) override; + // Called when a new message is received on the crypto stream and is available // for the TLS stack to read. void AdvanceHandshake() override; @@ -127,6 +134,11 @@ std::string hostname_; std::string cert_verify_sig_; + // Used to hold the ENCRYPTION_FORWARD_SECURE read secret until the handshake + // is complete. This is temporary until + // https://bugs.chromium.org/p/boringssl/issues/detail?id=303 is resolved. + std::vector<uint8_t> app_data_read_secret_; + bool encryption_established_ = false; bool one_rtt_keys_available_ = false; bool valid_alpn_received_ = false;
diff --git a/quic/quartc/quartc_multiplexer_test.cc b/quic/quartc/quartc_multiplexer_test.cc index dcdef66..df05f7a 100644 --- a/quic/quartc/quartc_multiplexer_test.cc +++ b/quic/quartc/quartc_multiplexer_test.cc
@@ -203,7 +203,7 @@ // TODO(b/150224094): Re-enable TLS handshake. // TODO(b/150236522): Parametrize by QUIC version. SetQuicReloadableFlag(quic_enable_version_t099, false); - SetQuicReloadableFlag(quic_enable_version_draft_25, false); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false); SetQuicReloadableFlag(quic_enable_version_t050, false); }
diff --git a/quic/quartc/quartc_session_test.cc b/quic/quartc/quartc_session_test.cc index 0a01f81..10d3128 100644 --- a/quic/quartc/quartc_session_test.cc +++ b/quic/quartc/quartc_session_test.cc
@@ -57,7 +57,7 @@ // TODO(b/150224094): Re-enable TLS handshake. // TODO(b/150236522): Parametrize by QUIC version. SetQuicReloadableFlag(quic_enable_version_t099, false); - SetQuicReloadableFlag(quic_enable_version_draft_25, false); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false); SetQuicReloadableFlag(quic_enable_version_t050, false); client_transport_ =
diff --git a/quic/quartc/test/quartc_bidi_test.cc b/quic/quartc/test/quartc_bidi_test.cc index 836c276..1d11d99 100644 --- a/quic/quartc/test/quartc_bidi_test.cc +++ b/quic/quartc/test/quartc_bidi_test.cc
@@ -29,7 +29,7 @@ // TODO(b/150224094): Re-enable TLS handshake. // TODO(b/150236522): Parametrize by QUIC version. SetQuicReloadableFlag(quic_enable_version_t099, false); - SetQuicReloadableFlag(quic_enable_version_draft_25, false); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false); SetQuicReloadableFlag(quic_enable_version_t050, false); uint64_t seed = QuicRandom::GetInstance()->RandUint64();
diff --git a/quic/quartc/test/quartc_peer_test.cc b/quic/quartc/test/quartc_peer_test.cc index 563eb02..0f0d0a6 100644 --- a/quic/quartc/test/quartc_peer_test.cc +++ b/quic/quartc/test/quartc_peer_test.cc
@@ -38,7 +38,7 @@ // TODO(b/150224094): Re-enable TLS handshake. // TODO(b/150236522): Parametrize by QUIC version. SetQuicReloadableFlag(quic_enable_version_t099, false); - SetQuicReloadableFlag(quic_enable_version_draft_25, false); + SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false); SetQuicReloadableFlag(quic_enable_version_t050, false); SetQuicReloadableFlag(quic_default_to_bbr, true);