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);
