Modify how QUIC 0-RTT go/no-go decision is made.
The previous implementation was too strict in its byte-for-byte equality
check including Transport Parameters, because the Transport Parameters
contain a field that QUIC requires be different on each connection. This
change still has BoringSSL do a byte-for-byte check, but now it is only
done over the quic_early_data_context. An additional requirement is
imposed that the quic_early_data_context must be set for early data
capable tickets to be issued.
Bug: 295
Change-Id: I5145c10752b41908b6807c3a3c967653b0c13f37
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41427
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index a61488e..5a5cbdc 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3153,12 +3153,15 @@
// QUIC may impose similar restrictions, for example HTTP/3's restrictions on
// SETTINGS frames.
//
-// BoringSSL imposes a stricter check on the server to enforce these
-// restrictions. BoringSSL requires that the transport parameters and
-// application protocol state be a byte-for-byte match between the connection
-// where the ticket was issued and the connection where it is used for 0-RTT. If
-// there is a mismatch, BoringSSL will reject early data (but not reject the
-// resumption attempt).
+// BoringSSL implements this check by doing a byte-for-byte comparison of an
+// opaque context passed in by the server. This context must be the same on the
+// connection where the ticket was issued and the connection where that ticket
+// is used for 0-RTT. If there is a mismatch, or the context was not set,
+// BoringSSL will reject early data (but not reject the resumption attempt).
+// This context is set via |SSL_set_quic_early_data_context| and should cover
+// both transport parameters and any application state.
+// |SSL_set_quic_early_data_context| must be called on the server with a
+// non-empty context if the server is to support 0-RTT in QUIC.
//
// BoringSSL does not perform any client-side checks on the transport
// parameters received from a server that also accepted early data. It is up to
@@ -3166,12 +3169,6 @@
// limits, and to close the QUIC connection if that is not the case. The same
// holds for any application protocol state remembered for 0-RTT, e.g. HTTP/3
// SETTINGS.
-//
-// The transport parameter check happens automatically with
-// |SSL_set_quic_transport_params|. QUIC servers must set application state via
-// |SSL_set_quic_early_data_context| to configure the application protocol
-// check. No other mechanisms are provided to have BoringSSL reject early data
-// because of QUIC transport or application protocol restrictions.
// ssl_encryption_level_t represents a specific QUIC encryption level used to
// transmit handshake messages.
@@ -3321,8 +3318,12 @@
// SSL_set_quic_early_data_context configures a context string in QUIC servers
// for accepting early data. If a resumption connection offers early data, the
// server will check if the value matches that of the connection which minted
-// the ticket. If not, resumption still succeeds but early data is rejected. For
-// HTTP/3, this should be the serialized server SETTINGS frame.
+// the ticket. If not, resumption still succeeds but early data is rejected.
+// This should include all QUIC Transport Parameters except ones specified that
+// the client MUST NOT remember. This should also include any application
+// protocol-specific state. For HTTP/3, this should be the serialized server
+// SETTINGS frame and the QUIC Transport Parameters (except the stateless reset
+// token).
//
// This function may be called before |SSL_do_handshake| or during server
// certificate selection. It returns 1 on success and 0 on failure.
diff --git a/ssl/internal.h b/ssl/internal.h
index e1b0925..b1c8bd1 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2740,11 +2740,6 @@
bool jdk11_workaround : 1;
};
-// Computes a SHA-256 hash of the transport parameters and early data context
-// for QUIC, putting the hash in |SHA256_DIGEST_LENGTH| bytes at |hash_out|.
-bool compute_quic_early_data_hash(const SSL_CONFIG *config,
- uint8_t hash_out[SHA256_DIGEST_LENGTH]);
-
// From RFC 8446, used in determining PSK modes.
#define SSL_PSK_DHE_KE 0x1
@@ -3559,9 +3554,9 @@
// is_quic indicates whether this session was created using QUIC.
bool is_quic : 1;
- // quic_early_data_hash is used to determine whether early data must be
+ // quic_early_data_context is used to determine whether early data must be
// rejected when performing a QUIC handshake.
- bssl::Array<uint8_t> quic_early_data_hash;
+ bssl::Array<uint8_t> quic_early_data_context;
private:
~ssl_session_st();
diff --git a/ssl/ssl_asn1.cc b/ssl/ssl_asn1.cc
index 7401d09..e6274f1 100644
--- a/ssl/ssl_asn1.cc
+++ b/ssl/ssl_asn1.cc
@@ -192,7 +192,7 @@
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 26;
static const unsigned kIsQuicTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 27;
-static const unsigned kQuicEarlyDataHashTag =
+static const unsigned kQuicEarlyDataContextTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 28;
static int SSL_SESSION_to_bytes_full(const SSL_SESSION *in, CBB *cbb,
@@ -402,10 +402,10 @@
}
}
- if (!in->quic_early_data_hash.empty()) {
- if (!CBB_add_asn1(&session, &child, kQuicEarlyDataHashTag) ||
- !CBB_add_asn1_octet_string(&child, in->quic_early_data_hash.data(),
- in->quic_early_data_hash.size())) {
+ if (!in->quic_early_data_context.empty()) {
+ if (!CBB_add_asn1(&session, &child, kQuicEarlyDataContextTag) ||
+ !CBB_add_asn1_octet_string(&child, in->quic_early_data_context.data(),
+ in->quic_early_data_context.size())) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
return 0;
}
@@ -752,8 +752,8 @@
kEarlyALPNTag) ||
!CBS_get_optional_asn1_bool(&session, &is_quic, kIsQuicTag,
/*default_value=*/false) ||
- !SSL_SESSION_parse_octet_string(&session, &ret->quic_early_data_hash,
- kQuicEarlyDataHashTag) ||
+ !SSL_SESSION_parse_octet_string(&session, &ret->quic_early_data_context,
+ kQuicEarlyDataContextTag) ||
CBS_len(&session) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
return nullptr;
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index fa994e8..4c6d045 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -269,8 +269,8 @@
return nullptr;
}
- if (!new_session->quic_early_data_hash.CopyFrom(
- session->quic_early_data_hash)) {
+ if (!new_session->quic_early_data_context.CopyFrom(
+ session->quic_early_data_context)) {
return nullptr;
}
}
@@ -349,25 +349,6 @@
session->cipher);
}
-bool compute_quic_early_data_hash(const SSL_CONFIG *config,
- uint8_t hash_out[SHA256_DIGEST_LENGTH]) {
- ScopedEVP_MD_CTX hash_ctx;
- uint32_t transport_param_len = config->quic_transport_params.size();
- uint32_t context_len = config->quic_early_data_context.size();
- if (!EVP_DigestInit(hash_ctx.get(), EVP_sha256()) ||
- !EVP_DigestUpdate(hash_ctx.get(), &transport_param_len,
- sizeof(transport_param_len)) ||
- !EVP_DigestUpdate(hash_ctx.get(), config->quic_transport_params.data(),
- config->quic_transport_params.size()) ||
- !EVP_DigestUpdate(hash_ctx.get(), &context_len, sizeof(context_len)) ||
- !EVP_DigestUpdate(hash_ctx.get(), config->quic_early_data_context.data(),
- config->quic_early_data_context.size()) ||
- !EVP_DigestFinal(hash_ctx.get(), hash_out, nullptr)) {
- return false;
- }
- return true;
-}
-
int ssl_get_new_session(SSL_HANDSHAKE *hs, int is_server) {
SSL *const ssl = hs->ssl;
if (ssl->mode & SSL_MODE_NO_SESSION_CREATION) {
@@ -384,9 +365,8 @@
session->ssl_version = ssl->version;
session->is_quic = ssl->quic_method != nullptr;
if (is_server && ssl->enable_early_data && session->is_quic) {
- if (!session->quic_early_data_hash.Init(SHA256_DIGEST_LENGTH) ||
- !compute_quic_early_data_hash(hs->config,
- session->quic_early_data_hash.data())) {
+ if (!session->quic_early_data_context.CopyFrom(
+ hs->config->quic_early_data_context)) {
return 0;
}
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index fc7976e..73f0b9d 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -5433,73 +5433,68 @@
bssl::UniquePtr<SSL_SESSION> session = CreateClientSessionForQUIC();
ASSERT_TRUE(session);
- for (bool change_transport_params : {false, true}) {
- SCOPED_TRACE(change_transport_params);
- for (bool change_context : {false, true}) {
- if (!change_transport_params && !change_context) {
- continue;
- }
- SCOPED_TRACE(change_context);
+ ASSERT_TRUE(CreateClientAndServer());
+ static const uint8_t new_context[] = {4};
+ ASSERT_TRUE(SSL_set_quic_early_data_context(server_.get(), new_context,
+ sizeof(new_context)));
+ SSL_set_session(client_.get(), session.get());
- ASSERT_TRUE(CreateClientAndServer());
- static const uint8_t new_transport_params[] = {3};
- static const uint8_t new_context[] = {4};
- if (change_transport_params) {
- ASSERT_TRUE(SSL_set_quic_transport_params(
- server_.get(), new_transport_params, sizeof(new_transport_params)));
- }
- if (change_context) {
- ASSERT_TRUE(SSL_set_quic_early_data_context(server_.get(), new_context,
- sizeof(new_context)));
- }
- SSL_set_session(client_.get(), session.get());
+ // The client handshake should return immediately into the early data
+ // state.
+ ASSERT_EQ(SSL_do_handshake(client_.get()), 1);
+ EXPECT_TRUE(SSL_in_early_data(client_.get()));
+ // The transport should have keys for sending 0-RTT data.
+ EXPECT_TRUE(transport_->client()->HasWriteSecret(ssl_encryption_early_data));
- // The client handshake should return immediately into the early data
- // state.
- ASSERT_EQ(SSL_do_handshake(client_.get()), 1);
- EXPECT_TRUE(SSL_in_early_data(client_.get()));
- // The transport should have keys for sending 0-RTT data.
- EXPECT_TRUE(
- transport_->client()->HasWriteSecret(ssl_encryption_early_data));
+ // The server will consume the ClientHello, but it will not accept 0-RTT.
+ ASSERT_TRUE(ProvideHandshakeData(server_.get()));
+ ASSERT_EQ(SSL_do_handshake(server_.get()), -1);
+ EXPECT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(server_.get(), -1));
+ EXPECT_FALSE(SSL_in_early_data(server_.get()));
+ EXPECT_FALSE(transport_->server()->HasReadSecret(ssl_encryption_early_data));
- // The server will consume the ClientHello, but it will not accept 0-RTT.
- ASSERT_TRUE(ProvideHandshakeData(server_.get()));
- ASSERT_EQ(SSL_do_handshake(server_.get()), -1);
- EXPECT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(server_.get(), -1));
- EXPECT_FALSE(SSL_in_early_data(server_.get()));
- EXPECT_FALSE(
- transport_->server()->HasReadSecret(ssl_encryption_early_data));
-
- // The client consumes the server response and signals 0-RTT rejection.
- for (;;) {
- ASSERT_TRUE(ProvideHandshakeData(client_.get()));
- ASSERT_EQ(-1, SSL_do_handshake(client_.get()));
- int err = SSL_get_error(client_.get(), -1);
- if (err == SSL_ERROR_EARLY_DATA_REJECTED) {
- break;
- }
- ASSERT_EQ(SSL_ERROR_WANT_READ, err);
- }
-
- // As in TLS over TCP, 0-RTT rejection is sticky.
- ASSERT_EQ(-1, SSL_do_handshake(client_.get()));
- ASSERT_EQ(SSL_ERROR_EARLY_DATA_REJECTED,
- SSL_get_error(client_.get(), -1));
-
- // Finish up the client and server handshakes.
- SSL_reset_early_data_reject(client_.get());
- ASSERT_TRUE(CompleteHandshakesForQUIC());
-
- // Both sides can now exchange 1-RTT data.
- ExpectHandshakeSuccess();
- EXPECT_TRUE(SSL_session_reused(client_.get()));
- EXPECT_TRUE(SSL_session_reused(server_.get()));
- EXPECT_FALSE(SSL_in_early_data(client_.get()));
- EXPECT_FALSE(SSL_in_early_data(server_.get()));
- EXPECT_FALSE(SSL_early_data_accepted(client_.get()));
- EXPECT_FALSE(SSL_early_data_accepted(server_.get()));
+ // The client consumes the server response and signals 0-RTT rejection.
+ for (;;) {
+ ASSERT_TRUE(ProvideHandshakeData(client_.get()));
+ ASSERT_EQ(-1, SSL_do_handshake(client_.get()));
+ int err = SSL_get_error(client_.get(), -1);
+ if (err == SSL_ERROR_EARLY_DATA_REJECTED) {
+ break;
}
+ ASSERT_EQ(SSL_ERROR_WANT_READ, err);
}
+
+ // As in TLS over TCP, 0-RTT rejection is sticky.
+ ASSERT_EQ(-1, SSL_do_handshake(client_.get()));
+ ASSERT_EQ(SSL_ERROR_EARLY_DATA_REJECTED, SSL_get_error(client_.get(), -1));
+
+ // Finish up the client and server handshakes.
+ SSL_reset_early_data_reject(client_.get());
+ ASSERT_TRUE(CompleteHandshakesForQUIC());
+
+ // Both sides can now exchange 1-RTT data.
+ ExpectHandshakeSuccess();
+ EXPECT_TRUE(SSL_session_reused(client_.get()));
+ EXPECT_TRUE(SSL_session_reused(server_.get()));
+ EXPECT_FALSE(SSL_in_early_data(client_.get()));
+ EXPECT_FALSE(SSL_in_early_data(server_.get()));
+ EXPECT_FALSE(SSL_early_data_accepted(client_.get()));
+ EXPECT_FALSE(SSL_early_data_accepted(server_.get()));
+}
+
+TEST_F(QUICMethodTest, NoZeroRTTTicketWithoutEarlyDataContext) {
+ server_quic_early_data_context_ = {};
+ const SSL_QUIC_METHOD quic_method = DefaultQUICMethod();
+
+ SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH);
+ SSL_CTX_set_early_data_enabled(client_ctx_.get(), 1);
+ SSL_CTX_set_early_data_enabled(server_ctx_.get(), 1);
+ ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method));
+ ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method));
+
+ bssl::UniquePtr<SSL_SESSION> session = CreateClientSessionForQUIC();
+ ASSERT_TRUE(session);
+ EXPECT_FALSE(SSL_SESSION_early_data_capable(session.get()));
}
TEST_F(QUICMethodTest, ZeroRTTReject) {
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 683a2ca..33f821e 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -127,7 +127,10 @@
return false;
}
session->ticket_age_add_valid = true;
- if (ssl->enable_early_data) {
+ bool enable_early_data =
+ ssl->enable_early_data &&
+ (!ssl->quic_method || !ssl->config->quic_early_data_context.empty());
+ if (enable_early_data) {
// QUIC does not use the max_early_data_size parameter and always sets it
// to a fixed value. See draft-ietf-quic-tls-22, section 4.5.
session->ticket_max_early_data =
@@ -152,7 +155,7 @@
return false;
}
- if (ssl->enable_early_data) {
+ if (enable_early_data) {
CBB early_data;
if (!CBB_add_u16(&extensions, TLSEXT_TYPE_early_data) ||
!CBB_add_u16_length_prefixed(&extensions, &early_data) ||
@@ -314,13 +317,13 @@
if (!session->is_quic) {
return true;
}
- if (session->quic_early_data_hash.size() != SHA256_DIGEST_LENGTH) {
- return false;
- }
- uint8_t early_data_hash[SHA256_DIGEST_LENGTH];
- if (!compute_quic_early_data_hash(config, early_data_hash) ||
- CRYPTO_memcmp(session->quic_early_data_hash.data(), early_data_hash,
- SHA256_DIGEST_LENGTH) != 0) {
+
+ if (session->quic_early_data_context.empty() ||
+ config->quic_early_data_context.size() !=
+ session->quic_early_data_context.size() ||
+ CRYPTO_memcmp(config->quic_early_data_context.data(),
+ session->quic_early_data_context.data(),
+ session->quic_early_data_context.size()) != 0) {
return false;
}
return true;
