Configure QUIC secrets inside set_{read,write}_state.
set_write_state flushes buffered handshake data, and we should finish
writing to a level before moving on to the next one.
I've moved the callback into set_{read,write}_state to ensure we still
update read_level and write_level after installing secrets, since that's
how we decide what level to write things and we should never write
alerts with keys we don't have. (I believe the only way this can come up
is if the QUIC callback itself fails, but it still seems prudent to
defer updating the levels.)
This does unfortunately mean a goofy secret_for_quic parameter, though
it is arguably more "correct" in that QUIC would ideally be a third
SSL_PROTOCOL_METHOD, rather than escape hatches over TLS. Probably a
cleaner abstraction would be for set_read_state and set_write_state to
take the secret and derive an SSLAEADContext internally.
Update-Note: See b/151142920#comment9
Change-Id: I4bbb76e15b5d95615ea643bccf796db87fae4989
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40244
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/ssl/tls_method.cc b/ssl/tls_method.cc
index 3868852..8165d1c 100644
--- a/ssl/tls_method.cc
+++ b/ssl/tls_method.cc
@@ -83,7 +83,8 @@
}
static bool tls_set_read_state(SSL *ssl, ssl_encryption_level_t level,
- UniquePtr<SSLAEADContext> aead_ctx) {
+ UniquePtr<SSLAEADContext> aead_ctx,
+ Span<const uint8_t> secret_for_quic) {
// Cipher changes are forbidden if the current epoch has leftover data.
if (tls_has_unprocessed_handshake_data(ssl)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_EXCESS_HANDSHAKE_DATA);
@@ -91,6 +92,21 @@
return false;
}
+ if (ssl->quic_method != nullptr) {
+ if (!ssl->quic_method->set_read_secret(ssl, level, aead_ctx->cipher(),
+ secret_for_quic.data(),
+ secret_for_quic.size())) {
+ return false;
+ }
+
+ // QUIC only uses |ssl| for handshake messages, which never use early data
+ // keys, so we return without installing anything. This avoids needing to
+ // have two secrets active at once in 0-RTT.
+ if (level == ssl_encryption_early_data) {
+ return true;
+ }
+ }
+
OPENSSL_memset(ssl->s3->read_sequence, 0, sizeof(ssl->s3->read_sequence));
ssl->s3->aead_read_ctx = std::move(aead_ctx);
ssl->s3->read_level = level;
@@ -98,11 +114,27 @@
}
static bool tls_set_write_state(SSL *ssl, ssl_encryption_level_t level,
- UniquePtr<SSLAEADContext> aead_ctx) {
+ UniquePtr<SSLAEADContext> aead_ctx,
+ Span<const uint8_t> secret_for_quic) {
if (!tls_flush_pending_hs_data(ssl)) {
return false;
}
+ if (ssl->quic_method != nullptr) {
+ if (!ssl->quic_method->set_write_secret(ssl, level, aead_ctx->cipher(),
+ secret_for_quic.data(),
+ secret_for_quic.size())) {
+ return false;
+ }
+
+ // QUIC only uses |ssl| for handshake messages, which never use early data
+ // keys, so we return without installing anything. This avoids needing to
+ // have two secrets active at once in 0-RTT.
+ if (level == ssl_encryption_early_data) {
+ return true;
+ }
+ }
+
OPENSSL_memset(ssl->s3->write_sequence, 0, sizeof(ssl->s3->write_sequence));
ssl->s3->aead_write_ctx = std::move(aead_ctx);
ssl->s3->write_level = level;