Fold tls13_write_psk_binder into the caller When we support multiple PSKs, we'll need to do a bit more work to fill all the fields in. Cut the abstraction at a slightly different place so the calling code can handle the serialization. Bug: 369963041 Change-Id: I726d25e621686476917afc950eb86811cd9db57f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/89147 Reviewed-by: Lily Chen <chlily@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/extensions.cc b/ssl/extensions.cc index daf6435..d06f013 100644 --- a/ssl/extensions.cc +++ b/ssl/extensions.cc
@@ -1940,7 +1940,8 @@ uint32_t ticket_age = 1000 * (now.tv_sec - ssl->session->time); uint32_t obfuscated_ticket_age = ticket_age + ssl->session->ticket_age_add; - size_t binder_len = EVP_MD_size(ssl_session_get_digest(ssl->session.get())); + const size_t one_binder_len = + EVP_MD_size(ssl_session_get_digest(ssl->session.get())); const size_t len_before = CBB_len(out_extensions); CBB contents, identity, ticket, binders, binder; if (!CBB_add_u16(out_extensions, TLSEXT_TYPE_pre_shared_key) || @@ -1954,21 +1955,31 @@ !CBB_add_u8_length_prefixed(&binders, &binder) || // Fill in a placeholder zero binder of the appropriate length. It will be // computed and filled in later after length prefixes are computed. - !CBB_add_zeros(&binder, binder_len) || // + !CBB_add_zeros(&binder, one_binder_len) || // !CBB_flush(out_extensions)) { return false; } // Sample the length of the PSK extension. *out_psk_len = CBB_len(out_extensions) - len_before; - // Close |out_extensions| and fill in the binder. - const auto &transcript = - type == ssl_client_hello_inner ? hs->inner_transcript : hs->transcript; - if (!CBB_flush(out_client_hello) || - !tls13_write_psk_binder(hs, transcript, CBBAsSpan(out_client_hello))) { + // Fill in |out_extensions|'s length prefix. + if (!CBB_flush(out_client_hello)) { return false; } + // Fill in the binder. + const auto &transcript = + type == ssl_client_hello_inner ? hs->inner_transcript : hs->transcript; + auto client_hello = CBBAsSpan(out_client_hello); + auto binder_out = client_hello.last(one_binder_len); + size_t binders_len = 3 + one_binder_len; // u16 and u8 length prefix + size_t actual_binder_len; + if (!tls13_psk_binder(hs, binder_out, &actual_binder_len, ssl->session.get(), + transcript, client_hello, binders_len)) { + return false; + } + assert(actual_binder_len == one_binder_len); + return true; }
diff --git a/ssl/internal.h b/ssl/internal.h index 01f8623..c0d38f5 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1212,12 +1212,15 @@ bool tls13_derive_session_psk(SSL_SESSION *session, Span<const uint8_t> nonce, bool is_dtls); -// tls13_write_psk_binder calculates the PSK binder value over |transcript| and -// |msg|, and replaces the last bytes of |msg| with the resulting value. It -// returns true on success, and false on failure. |msg| should contain the body -// of a ClientHello, but not the message header. -bool tls13_write_psk_binder(const SSL_HANDSHAKE *hs, - const SSLTranscript &transcript, Span<uint8_t> msg); +// tls13_psk_binder calculates PSK binder value for |session| over +// |transcript| and |client_hello|. On success, it writes the result to |out|, +// sets |*out_len| to the length, and returns true. Otherwise, it returns false. +// |binders_len| must be the length of the binders field, covering all binders +// and the overall length prefix, in |client_hello|. +bool tls13_psk_binder(const SSL_HANDSHAKE *hs, Span<uint8_t> out, + size_t *out_len, const SSL_SESSION *session, + const SSLTranscript &transcript, + Span<const uint8_t> client_hello, size_t binders_len); // tls13_verify_psk_binder verifies that the handshake transcript, truncated up // to the binders has a valid signature using the value of |session|'s
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc index 59e0e0a..adb6ac8 100644 --- a/ssl/tls13_enc.cc +++ b/ssl/tls13_enc.cc
@@ -505,11 +505,10 @@ static const char kTLS13LabelPSKBinder[] = "res binder"; -static bool tls13_psk_binder(uint8_t *out, size_t *out_len, - const SSL_SESSION *session, - const SSLTranscript &transcript, - Span<const uint8_t> client_hello, - size_t binders_len, bool is_dtls) { +bool tls13_psk_binder(const SSL_HANDSHAKE *hs, Span<uint8_t> out, + size_t *out_len, const SSL_SESSION *session, + const SSLTranscript &transcript, + Span<const uint8_t> client_hello, size_t binders_len) { const EVP_MD *digest = ssl_session_get_digest(session); // Compute the binder key. @@ -527,10 +526,10 @@ !HKDF_extract(early_secret, &early_secret_len, digest, session->secret.data(), session->secret.size(), nullptr, 0) || - !hkdf_expand_label(binder_key, digest, - Span(early_secret, early_secret_len), - kTLS13LabelPSKBinder, - Span(binder_context, binder_context_len), is_dtls)) { + !hkdf_expand_label( + binder_key, digest, Span(early_secret, early_secret_len), + kTLS13LabelPSKBinder, Span(binder_context, binder_context_len), + SSL_is_dtls(hs->ssl))) { return false; } @@ -557,8 +556,10 @@ return false; } - if (!tls13_verify_data(out, out_len, digest, session->ssl_version, binder_key, - Span(context, context_len), is_dtls)) { + BSSL_CHECK(out.size() >= EVP_MD_size(digest)); + if (!tls13_verify_data(out.data(), out_len, digest, session->ssl_version, + binder_key, Span(context, context_len), + SSL_is_dtls(hs->ssl))) { return false; } @@ -566,30 +567,6 @@ return true; } -bool tls13_write_psk_binder(const SSL_HANDSHAKE *hs, - const SSLTranscript &transcript, - Span<uint8_t> msg) { - const SSL *const ssl = hs->ssl; - const EVP_MD *digest = ssl_session_get_digest(ssl->session.get()); - const size_t hash_len = EVP_MD_size(digest); - // We only offer one PSK, so the binders are a u16 and u8 length - // prefix, followed by the binder. The caller is assumed to have constructed - // |msg| with placeholder binders. - const size_t binders_len = 3 + hash_len; - uint8_t verify_data[EVP_MAX_MD_SIZE]; - size_t verify_data_len; - if (!tls13_psk_binder(verify_data, &verify_data_len, ssl->session.get(), - transcript, msg, binders_len, SSL_is_dtls(hs->ssl)) || - verify_data_len != hash_len) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return false; - } - - auto msg_binder = msg.last(verify_data_len); - OPENSSL_memcpy(msg_binder.data(), verify_data, verify_data_len); - return true; -} - bool tls13_verify_psk_binder(const SSL_HANDSHAKE *hs, const SSL_SESSION *session, const SSLMessage &msg, CBS *binders) { @@ -599,8 +576,8 @@ // The binders are computed over |msg| with |binders| and its u16 length // prefix removed. The caller is assumed to have parsed |msg|, extracted // |binders|, and verified the PSK extension is last. - if (!tls13_psk_binder(verify_data, &verify_data_len, session, hs->transcript, - msg.body, 2 + CBS_len(binders), SSL_is_dtls(hs->ssl)) || + if (!tls13_psk_binder(hs, verify_data, &verify_data_len, session, + hs->transcript, msg.body, 2 + CBS_len(binders)) || // We only consider the first PSK, so compare against the first binder. !CBS_get_u8_length_prefixed(binders, &binder)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);