Switch tls13_enc.cc to spans. The callers become filled with MakeConstSpans, but the various TLS 1.3 secrets will get fixed in a subsequent CL. We do still need a better pattern for the EVP_MAX_MD_SIZE buffers. Change-Id: Ide9c173bf0760ecdb8cc45e63969457c20310de2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37125 Commit-Queue: Steven Valdez <svaldez@google.com> Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index e1a506a..e1b6afe 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -456,11 +456,13 @@ return ssl_hs_error; } - if (!tls13_init_early_key_schedule(hs, ssl->session->master_key, - ssl->session->master_key_length) || + if (!tls13_init_early_key_schedule( + hs, MakeConstSpan(ssl->session->master_key, + ssl->session->master_key_length)) || !tls13_derive_early_secrets(hs) || - !tls13_set_traffic_key(ssl, ssl_encryption_early_data, evp_aead_seal, - hs->early_traffic_secret, hs->hash_len)) { + !tls13_set_traffic_key( + ssl, ssl_encryption_early_data, evp_aead_seal, + MakeConstSpan(hs->early_traffic_secret, hs->hash_len))) { return ssl_hs_error; }
diff --git a/ssl/internal.h b/ssl/internal.h index 580cf6e..621052f 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1248,26 +1248,22 @@ // tls13_init_key_schedule initializes the handshake hash and key derivation // state, and incorporates the PSK. The cipher suite and PRF hash must have been // selected at this point. It returns true on success and false on error. -bool tls13_init_key_schedule(SSL_HANDSHAKE *hs, const uint8_t *psk, - size_t psk_len); +bool tls13_init_key_schedule(SSL_HANDSHAKE *hs, Span<const uint8_t> psk); // tls13_init_early_key_schedule initializes the handshake hash and key // derivation state from the resumption secret and incorporates the PSK to // derive the early secrets. It returns one on success and zero on error. -bool tls13_init_early_key_schedule(SSL_HANDSHAKE *hs, const uint8_t *psk, - size_t psk_len); +bool tls13_init_early_key_schedule(SSL_HANDSHAKE *hs, Span<const uint8_t> psk); // tls13_advance_key_schedule incorporates |in| into the key schedule with // HKDF-Extract. It returns true on success and false on error. -bool tls13_advance_key_schedule(SSL_HANDSHAKE *hs, const uint8_t *in, - size_t len); +bool tls13_advance_key_schedule(SSL_HANDSHAKE *hs, Span<const uint8_t> in); // tls13_set_traffic_key sets the read or write traffic keys to // |traffic_secret|. It returns true on success and false on error. bool tls13_set_traffic_key(SSL *ssl, enum ssl_encryption_level_t level, enum evp_aead_direction_t direction, - const uint8_t *traffic_secret, - size_t traffic_secret_len); + Span<const uint8_t> traffic_secret); // tls13_derive_early_secrets derives the early traffic secret. It returns true // on success and false on error.
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index f411e19..ba7dc55 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc
@@ -365,11 +365,12 @@ // Set up the key schedule and incorporate the PSK into the running secret. if (ssl->s3->session_reused) { - if (!tls13_init_key_schedule(hs, hs->new_session->master_key, - hs->new_session->master_key_length)) { + if (!tls13_init_key_schedule( + hs, MakeConstSpan(hs->new_session->master_key, + hs->new_session->master_key_length))) { return ssl_hs_error; } - } else if (!tls13_init_key_schedule(hs, kZeroes, hash_len)) { + } else if (!tls13_init_key_schedule(hs, MakeConstSpan(kZeroes, hash_len))) { return ssl_hs_error; } @@ -389,18 +390,21 @@ return ssl_hs_error; } - if (!tls13_advance_key_schedule(hs, dhe_secret.data(), dhe_secret.size()) || - !ssl_hash_message(hs, msg) || !tls13_derive_handshake_secrets(hs) || - !tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open, - hs->server_handshake_secret, hs->hash_len)) { + if (!tls13_advance_key_schedule(hs, dhe_secret) || + !ssl_hash_message(hs, msg) || + !tls13_derive_handshake_secrets(hs) || + !tls13_set_traffic_key( + ssl, ssl_encryption_handshake, evp_aead_open, + MakeConstSpan(hs->server_handshake_secret, hs->hash_len))) { return ssl_hs_error; } if (!hs->early_data_offered) { // If not sending early data, set client traffic keys now so that alerts are // encrypted. - if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal, - hs->client_handshake_secret, hs->hash_len)) { + if (!tls13_set_traffic_key( + ssl, ssl_encryption_handshake, evp_aead_seal, + MakeConstSpan(hs->client_handshake_secret, hs->hash_len))) { return ssl_hs_error; } } @@ -615,7 +619,7 @@ !tls13_process_finished(hs, msg, false /* don't use saved value */) || !ssl_hash_message(hs, msg) || // Update the secret to the master secret and derive traffic keys. - !tls13_advance_key_schedule(hs, kZeroes, hs->hash_len) || + !tls13_advance_key_schedule(hs, MakeConstSpan(kZeroes, hs->hash_len)) || !tls13_derive_application_secrets(hs)) { return ssl_hs_error; } @@ -640,8 +644,9 @@ } if (hs->early_data_offered) { - if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal, - hs->client_handshake_secret, hs->hash_len)) { + if (!tls13_set_traffic_key( + ssl, ssl_encryption_handshake, evp_aead_seal, + MakeConstSpan(hs->client_handshake_secret, hs->hash_len))) { return ssl_hs_error; } } @@ -735,10 +740,12 @@ } // Derive the final keys and enable them. - if (!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_open, - hs->server_traffic_secret_0, hs->hash_len) || - !tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal, - hs->client_traffic_secret_0, hs->hash_len) || + if (!tls13_set_traffic_key( + ssl, ssl_encryption_application, evp_aead_open, + MakeConstSpan(hs->server_traffic_secret_0, hs->hash_len)) || + !tls13_set_traffic_key( + ssl, ssl_encryption_application, evp_aead_seal, + MakeConstSpan(hs->client_traffic_secret_0, hs->hash_len)) || !tls13_derive_resumption_secret(hs)) { return ssl_hs_error; }
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc index 7a98128..19ae052 100644 --- a/ssl/tls13_enc.cc +++ b/ssl/tls13_enc.cc
@@ -33,7 +33,7 @@ BSSL_NAMESPACE_BEGIN static bool init_key_schedule(SSL_HANDSHAKE *hs, uint16_t version, - const SSL_CIPHER *cipher) { + const SSL_CIPHER *cipher) { if (!hs->transcript.InitHash(version, cipher)) { return false; } @@ -47,57 +47,58 @@ return true; } -bool tls13_init_key_schedule(SSL_HANDSHAKE *hs, const uint8_t *psk, - size_t psk_len) { +bool tls13_init_key_schedule(SSL_HANDSHAKE *hs, Span<const uint8_t> psk) { if (!init_key_schedule(hs, ssl_protocol_version(hs->ssl), hs->new_cipher)) { return false; } hs->transcript.FreeBuffer(); - return HKDF_extract(hs->secret, &hs->hash_len, hs->transcript.Digest(), psk, - psk_len, hs->secret, hs->hash_len); + return HKDF_extract(hs->secret, &hs->hash_len, hs->transcript.Digest(), + psk.data(), psk.size(), hs->secret, hs->hash_len); } -bool tls13_init_early_key_schedule(SSL_HANDSHAKE *hs, const uint8_t *psk, - size_t psk_len) { +bool tls13_init_early_key_schedule(SSL_HANDSHAKE *hs, Span<const uint8_t> psk) { SSL *const ssl = hs->ssl; return init_key_schedule(hs, ssl_session_protocol_version(ssl->session.get()), ssl->session->cipher) && - HKDF_extract(hs->secret, &hs->hash_len, hs->transcript.Digest(), psk, - psk_len, hs->secret, hs->hash_len); + HKDF_extract(hs->secret, &hs->hash_len, hs->transcript.Digest(), + psk.data(), psk.size(), hs->secret, hs->hash_len); } -static bool hkdf_expand_label(uint8_t *out, const EVP_MD *digest, - const uint8_t *secret, size_t secret_len, - const char *label, size_t label_len, - const uint8_t *hash, size_t hash_len, - size_t len) { - static const char kTLS13ProtocolLabel[] = "tls13 "; +static Span<const char> label_to_span(const char *label) { + return MakeConstSpan(label, strlen(label)); +} +static bool hkdf_expand_label(Span<uint8_t> out, const EVP_MD *digest, + Span<const uint8_t> secret, + Span<const char> label, + Span<const uint8_t> hash) { + Span<const char> protocol_label = label_to_span("tls13 "); ScopedCBB cbb; CBB child; Array<uint8_t> hkdf_label; - if (!CBB_init(cbb.get(), 2 + 1 + strlen(kTLS13ProtocolLabel) + label_len + 1 + - hash_len) || - !CBB_add_u16(cbb.get(), len) || + if (!CBB_init(cbb.get(), 2 + 1 + protocol_label.size() + label.size() + 1 + + hash.size()) || + !CBB_add_u16(cbb.get(), out.size()) || !CBB_add_u8_length_prefixed(cbb.get(), &child) || - !CBB_add_bytes(&child, (const uint8_t *)kTLS13ProtocolLabel, - strlen(kTLS13ProtocolLabel)) || - !CBB_add_bytes(&child, (const uint8_t *)label, label_len) || + !CBB_add_bytes(&child, + reinterpret_cast<const uint8_t *>(protocol_label.data()), + protocol_label.size()) || + !CBB_add_bytes(&child, reinterpret_cast<const uint8_t *>(label.data()), + label.size()) || !CBB_add_u8_length_prefixed(cbb.get(), &child) || - !CBB_add_bytes(&child, hash, hash_len) || + !CBB_add_bytes(&child, hash.data(), hash.size()) || !CBBFinishArray(cbb.get(), &hkdf_label)) { return false; } - return HKDF_expand(out, len, digest, secret, secret_len, hkdf_label.data(), - hkdf_label.size()); + return HKDF_expand(out.data(), out.size(), digest, secret.data(), + secret.size(), hkdf_label.data(), hkdf_label.size()); } static const char kTLS13LabelDerived[] = "derived"; -bool tls13_advance_key_schedule(SSL_HANDSHAKE *hs, const uint8_t *in, - size_t len) { +bool tls13_advance_key_schedule(SSL_HANDSHAKE *hs, Span<const uint8_t> in) { uint8_t derive_context[EVP_MAX_MD_SIZE]; unsigned derive_context_len; if (!EVP_Digest(nullptr, 0, derive_context, &derive_context_len, @@ -105,45 +106,41 @@ return false; } - if (!hkdf_expand_label(hs->secret, hs->transcript.Digest(), hs->secret, - hs->hash_len, kTLS13LabelDerived, - strlen(kTLS13LabelDerived), derive_context, - derive_context_len, hs->hash_len)) { + if (!hkdf_expand_label(MakeSpan(hs->secret, hs->hash_len), + hs->transcript.Digest(), + MakeConstSpan(hs->secret, hs->hash_len), + label_to_span(kTLS13LabelDerived), + MakeConstSpan(derive_context, derive_context_len))) { return false; } - return HKDF_extract(hs->secret, &hs->hash_len, hs->transcript.Digest(), in, - len, hs->secret, hs->hash_len); + return HKDF_extract(hs->secret, &hs->hash_len, hs->transcript.Digest(), + in.data(), in.size(), hs->secret, hs->hash_len); } -// derive_secret derives a secret of length |len| and writes the result in |out| -// with the given label and the current base secret and most recently-saved -// handshake context. It returns true on success and false on error. -static bool derive_secret(SSL_HANDSHAKE *hs, uint8_t *out, size_t len, - const char *label, size_t label_len) { +// derive_secret derives a secret of length |out.size()| and writes the result +// in |out| with the given label, the current base secret, and the most +// recently-saved handshake context. It returns true on success and false on +// error. +static bool derive_secret(SSL_HANDSHAKE *hs, Span<uint8_t> out, + Span<const char> label) { uint8_t context_hash[EVP_MAX_MD_SIZE]; size_t context_hash_len; if (!hs->transcript.GetHash(context_hash, &context_hash_len)) { return false; } - return hkdf_expand_label(out, hs->transcript.Digest(), hs->secret, - hs->hash_len, label, label_len, context_hash, - context_hash_len, len); + return hkdf_expand_label(out, hs->transcript.Digest(), + MakeConstSpan(hs->secret, hs->hash_len), label, + MakeConstSpan(context_hash, context_hash_len)); } bool tls13_set_traffic_key(SSL *ssl, enum ssl_encryption_level_t level, enum evp_aead_direction_t direction, - const uint8_t *traffic_secret, - size_t traffic_secret_len) { + Span<const uint8_t> traffic_secret) { const SSL_SESSION *session = SSL_get_session(ssl); uint16_t version = ssl_session_protocol_version(session); - if (traffic_secret_len > 0xff) { - OPENSSL_PUT_ERROR(SSL, ERR_R_OVERFLOW); - return false; - } - UniquePtr<SSLAEADContext> traffic_aead; if (ssl->quic_method == nullptr) { // Look up cipher suite properties. @@ -158,25 +155,26 @@ // Derive the key. size_t key_len = EVP_AEAD_key_length(aead); - uint8_t key[EVP_AEAD_MAX_KEY_LENGTH]; - if (!hkdf_expand_label(key, digest, traffic_secret, traffic_secret_len, - "key", 3, NULL, 0, key_len)) { + uint8_t key_buf[EVP_AEAD_MAX_KEY_LENGTH]; + auto key = MakeSpan(key_buf, key_len); + if (!hkdf_expand_label(key, digest, traffic_secret, label_to_span("key"), + {})) { return false; } // Derive the IV. size_t iv_len = EVP_AEAD_nonce_length(aead); - uint8_t iv[EVP_AEAD_MAX_NONCE_LENGTH]; - if (!hkdf_expand_label(iv, digest, traffic_secret, traffic_secret_len, "iv", - 2, NULL, 0, iv_len)) { + uint8_t iv_buf[EVP_AEAD_MAX_NONCE_LENGTH]; + auto iv = MakeSpan(iv_buf, iv_len); + if (!hkdf_expand_label(iv, digest, traffic_secret, label_to_span("iv"), + {})) { return false; } - traffic_aead = SSLAEADContext::Create( - direction, session->ssl_version, SSL_is_dtls(ssl), session->cipher, - MakeConstSpan(key, key_len), Span<const uint8_t>(), - MakeConstSpan(iv, iv_len)); + traffic_aead = SSLAEADContext::Create(direction, session->ssl_version, + SSL_is_dtls(ssl), session->cipher, + key, Span<const uint8_t>(), iv); } else { // Install a placeholder SSLAEADContext so that SSL accessors work. The // encryption itself will be handled by the SSL_QUIC_METHOD. @@ -199,15 +197,22 @@ } // Save the traffic secret. + if (traffic_secret.size() > + OPENSSL_ARRAY_SIZE(ssl->s3->read_traffic_secret) || + traffic_secret.size() > + OPENSSL_ARRAY_SIZE(ssl->s3->write_traffic_secret)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return false; + } if (direction == evp_aead_open) { - OPENSSL_memmove(ssl->s3->read_traffic_secret, traffic_secret, - traffic_secret_len); - ssl->s3->read_traffic_secret_len = traffic_secret_len; + OPENSSL_memmove(ssl->s3->read_traffic_secret, traffic_secret.data(), + traffic_secret.size()); + ssl->s3->read_traffic_secret_len = traffic_secret.size(); ssl->s3->read_level = level; } else { - OPENSSL_memmove(ssl->s3->write_traffic_secret, traffic_secret, - traffic_secret_len); - ssl->s3->write_traffic_secret_len = traffic_secret_len; + OPENSSL_memmove(ssl->s3->write_traffic_secret, traffic_secret.data(), + traffic_secret.size()); + ssl->s3->write_traffic_secret_len = traffic_secret.size(); ssl->s3->write_level = level; } @@ -225,9 +230,8 @@ bool tls13_derive_early_secrets(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (!derive_secret(hs, hs->early_traffic_secret, hs->hash_len, - kTLS13LabelClientEarlyTraffic, - strlen(kTLS13LabelClientEarlyTraffic)) || + if (!derive_secret(hs, MakeSpan(hs->early_traffic_secret, hs->hash_len), + label_to_span(kTLS13LabelClientEarlyTraffic)) || !ssl_log_secret(ssl, "CLIENT_EARLY_TRAFFIC_SECRET", hs->early_traffic_secret, hs->hash_len)) { return false; @@ -256,14 +260,12 @@ bool tls13_derive_handshake_secrets(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (!derive_secret(hs, hs->client_handshake_secret, hs->hash_len, - kTLS13LabelClientHandshakeTraffic, - strlen(kTLS13LabelClientHandshakeTraffic)) || + if (!derive_secret(hs, MakeSpan(hs->client_handshake_secret, hs->hash_len), + label_to_span(kTLS13LabelClientHandshakeTraffic)) || !ssl_log_secret(ssl, "CLIENT_HANDSHAKE_TRAFFIC_SECRET", hs->client_handshake_secret, hs->hash_len) || - !derive_secret(hs, hs->server_handshake_secret, hs->hash_len, - kTLS13LabelServerHandshakeTraffic, - strlen(kTLS13LabelServerHandshakeTraffic)) || + !derive_secret(hs, MakeSpan(hs->server_handshake_secret, hs->hash_len), + label_to_span(kTLS13LabelServerHandshakeTraffic)) || !ssl_log_secret(ssl, "SERVER_HANDSHAKE_TRAFFIC_SECRET", hs->server_handshake_secret, hs->hash_len)) { return false; @@ -293,18 +295,16 @@ bool tls13_derive_application_secrets(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; ssl->s3->exporter_secret_len = hs->hash_len; - if (!derive_secret(hs, hs->client_traffic_secret_0, hs->hash_len, - kTLS13LabelClientApplicationTraffic, - strlen(kTLS13LabelClientApplicationTraffic)) || + if (!derive_secret(hs, MakeSpan(hs->client_traffic_secret_0, hs->hash_len), + label_to_span(kTLS13LabelClientApplicationTraffic)) || !ssl_log_secret(ssl, "CLIENT_TRAFFIC_SECRET_0", hs->client_traffic_secret_0, hs->hash_len) || - !derive_secret(hs, hs->server_traffic_secret_0, hs->hash_len, - kTLS13LabelServerApplicationTraffic, - strlen(kTLS13LabelServerApplicationTraffic)) || + !derive_secret(hs, MakeSpan(hs->server_traffic_secret_0, hs->hash_len), + label_to_span(kTLS13LabelServerApplicationTraffic)) || !ssl_log_secret(ssl, "SERVER_TRAFFIC_SECRET_0", hs->server_traffic_secret_0, hs->hash_len) || - !derive_secret(hs, ssl->s3->exporter_secret, hs->hash_len, - kTLS13LabelExporter, strlen(kTLS13LabelExporter)) || + !derive_secret(hs, MakeSpan(ssl->s3->exporter_secret, hs->hash_len), + label_to_span(kTLS13LabelExporter)) || !ssl_log_secret(ssl, "EXPORTER_SECRET", ssl->s3->exporter_secret, hs->hash_len)) { return false; @@ -334,26 +334,20 @@ static const char kTLS13LabelApplicationTraffic[] = "traffic upd"; bool tls13_rotate_traffic_key(SSL *ssl, enum evp_aead_direction_t direction) { - uint8_t *secret; - size_t secret_len; + Span<uint8_t> secret; if (direction == evp_aead_open) { - secret = ssl->s3->read_traffic_secret; - secret_len = ssl->s3->read_traffic_secret_len; + secret = MakeSpan(ssl->s3->read_traffic_secret, + ssl->s3->read_traffic_secret_len); } else { - secret = ssl->s3->write_traffic_secret; - secret_len = ssl->s3->write_traffic_secret_len; + secret = MakeSpan(ssl->s3->write_traffic_secret, + ssl->s3->write_traffic_secret_len); } const EVP_MD *digest = ssl_session_get_digest(SSL_get_session(ssl)); - if (!hkdf_expand_label(secret, digest, secret, secret_len, - kTLS13LabelApplicationTraffic, - strlen(kTLS13LabelApplicationTraffic), NULL, 0, - secret_len)) { - return false; - } - - return tls13_set_traffic_key(ssl, ssl_encryption_application, direction, - secret, secret_len); + return hkdf_expand_label(secret, digest, secret, + label_to_span(kTLS13LabelApplicationTraffic), {}) && + tls13_set_traffic_key(ssl, ssl_encryption_application, direction, + secret); } static const char kTLS13LabelResumption[] = "res master"; @@ -364,24 +358,28 @@ return false; } hs->new_session->master_key_length = hs->hash_len; - return derive_secret(hs, hs->new_session->master_key, - hs->new_session->master_key_length, - kTLS13LabelResumption, strlen(kTLS13LabelResumption)); + return derive_secret( + hs, + MakeSpan(hs->new_session->master_key, hs->new_session->master_key_length), + label_to_span(kTLS13LabelResumption)); } static const char kTLS13LabelFinished[] = "finished"; // tls13_verify_data sets |out| to be the HMAC of |context| using a derived -// Finished key for both Finished messages and the PSK binder. -static bool tls13_verify_data(const EVP_MD *digest, uint16_t version, - uint8_t *out, size_t *out_len, - const uint8_t *secret, size_t hash_len, - const uint8_t *context, size_t context_len) { - uint8_t key[EVP_MAX_MD_SIZE]; +// Finished key for both Finished messages and the PSK binder. |out| must have +// space available for |EVP_MAX_MD_SIZE| bytes. +static bool tls13_verify_data(uint8_t *out, size_t *out_len, + const EVP_MD *digest, uint16_t version, + Span<const uint8_t> secret, + Span<const uint8_t> context) { + uint8_t key_buf[EVP_MAX_MD_SIZE]; + auto key = MakeSpan(key_buf, EVP_MD_size(digest)); unsigned len; - if (!hkdf_expand_label(key, digest, secret, hash_len, kTLS13LabelFinished, - strlen(kTLS13LabelFinished), NULL, 0, hash_len) || - HMAC(digest, key, hash_len, context, context_len, out, &len) == NULL) { + if (!hkdf_expand_label(key, digest, secret, + label_to_span(kTLS13LabelFinished), {}) || + HMAC(digest, key.data(), key.size(), context.data(), context.size(), out, + &len) == nullptr) { return false; } *out_len = len; @@ -390,19 +388,19 @@ bool tls13_finished_mac(SSL_HANDSHAKE *hs, uint8_t *out, size_t *out_len, bool is_server) { - const uint8_t *traffic_secret; + Span<const uint8_t> traffic_secret; if (is_server) { - traffic_secret = hs->server_handshake_secret; + traffic_secret = MakeConstSpan(hs->server_handshake_secret, hs->hash_len); } else { - traffic_secret = hs->client_handshake_secret; + traffic_secret = MakeConstSpan(hs->client_handshake_secret, hs->hash_len); } uint8_t context_hash[EVP_MAX_MD_SIZE]; size_t context_hash_len; if (!hs->transcript.GetHash(context_hash, &context_hash_len) || - !tls13_verify_data(hs->transcript.Digest(), hs->ssl->version, out, - out_len, traffic_secret, hs->hash_len, context_hash, - context_hash_len)) { + !tls13_verify_data(out, out_len, hs->transcript.Digest(), + hs->ssl->version, traffic_secret, + MakeConstSpan(context_hash, context_hash_len))) { return 0; } return 1; @@ -412,10 +410,11 @@ bool tls13_derive_session_psk(SSL_SESSION *session, Span<const uint8_t> nonce) { const EVP_MD *digest = ssl_session_get_digest(session); - return hkdf_expand_label(session->master_key, digest, session->master_key, - session->master_key_length, kTLS13LabelResumptionPSK, - strlen(kTLS13LabelResumptionPSK), nonce.data(), - nonce.size(), session->master_key_length); + // The session initially stores the resumption_master_secret, which we + // override with the PSK. + auto session_key = MakeSpan(session->master_key, session->master_key_length); + return hkdf_expand_label(session_key, digest, session_key, + label_to_span(kTLS13LabelResumptionPSK), nonce); } static const char kTLS13LabelExportKeying[] = "exporter"; @@ -432,31 +431,32 @@ const EVP_MD *digest = ssl_session_get_digest(SSL_get_session(ssl)); - uint8_t hash[EVP_MAX_MD_SIZE]; - uint8_t export_context[EVP_MAX_MD_SIZE]; - uint8_t derived_secret[EVP_MAX_MD_SIZE]; + uint8_t hash_buf[EVP_MAX_MD_SIZE]; + uint8_t export_context_buf[EVP_MAX_MD_SIZE]; unsigned hash_len; unsigned export_context_len; - unsigned derived_secret_len = EVP_MD_size(digest); - return EVP_Digest(context.data(), context.size(), hash, &hash_len, digest, - nullptr) && - EVP_Digest(nullptr, 0, export_context, &export_context_len, digest, - nullptr) && - hkdf_expand_label(derived_secret, digest, secret.data(), secret.size(), - label.data(), label.size(), export_context, - export_context_len, derived_secret_len) && - hkdf_expand_label(out.data(), digest, derived_secret, - derived_secret_len, kTLS13LabelExportKeying, - strlen(kTLS13LabelExportKeying), hash, hash_len, - out.size()); + if (!EVP_Digest(context.data(), context.size(), hash_buf, &hash_len, digest, + nullptr) || + !EVP_Digest(nullptr, 0, export_context_buf, &export_context_len, digest, + nullptr)) { + return false; + } + + auto hash = MakeConstSpan(hash_buf, hash_len); + auto export_context = MakeConstSpan(export_context_buf, export_context_len); + uint8_t derived_secret_buf[EVP_MAX_MD_SIZE]; + auto derived_secret = MakeSpan(derived_secret_buf, EVP_MD_size(digest)); + return hkdf_expand_label(derived_secret, digest, secret, label, + export_context) && + hkdf_expand_label(out, digest, derived_secret, + label_to_span(kTLS13LabelExportKeying), hash); } static const char kTLS13LabelPSKBinder[] = "res binder"; -static bool tls13_psk_binder(uint8_t *out, uint16_t version, - const EVP_MD *digest, const uint8_t *psk, - size_t psk_len, const uint8_t *context, - size_t context_len, size_t hash_len) { +static bool tls13_psk_binder(uint8_t *out, size_t *out_len, uint16_t version, + const EVP_MD *digest, Span<const uint8_t> psk, + Span<const uint8_t> context) { uint8_t binder_context[EVP_MAX_MD_SIZE]; unsigned binder_context_len; if (!EVP_Digest(NULL, 0, binder_context, &binder_context_len, digest, NULL)) { @@ -465,22 +465,22 @@ uint8_t early_secret[EVP_MAX_MD_SIZE] = {0}; size_t early_secret_len; - if (!HKDF_extract(early_secret, &early_secret_len, digest, psk, psk_len, NULL, - 0)) { + if (!HKDF_extract(early_secret, &early_secret_len, digest, psk.data(), + psk.size(), NULL, 0)) { return false; } - uint8_t binder_key[EVP_MAX_MD_SIZE] = {0}; - size_t len; - if (!hkdf_expand_label(binder_key, digest, early_secret, early_secret_len, - kTLS13LabelPSKBinder, strlen(kTLS13LabelPSKBinder), - binder_context, binder_context_len, hash_len) || - !tls13_verify_data(digest, version, out, &len, binder_key, hash_len, - context, context_len)) { + uint8_t binder_key_buf[EVP_MAX_MD_SIZE] = {0}; + auto binder_key = MakeSpan(binder_key_buf, EVP_MD_size(digest)); + if (!hkdf_expand_label(binder_key, digest, + MakeConstSpan(early_secret, early_secret_len), + label_to_span(kTLS13LabelPSKBinder), + MakeConstSpan(binder_context, binder_context_len)) || + !tls13_verify_data(out, out_len, digest, version, binder_key, context)) { return false; } - assert(len == hash_len); + assert(*out_len == EVP_MD_size(digest)); return true; } @@ -513,42 +513,50 @@ ScopedEVP_MD_CTX ctx; uint8_t context[EVP_MAX_MD_SIZE]; size_t context_len; - uint8_t verify_data[EVP_MAX_MD_SIZE] = {0}; + uint8_t verify_data[EVP_MAX_MD_SIZE]; + size_t verify_data_len; if (!hash_transcript_and_truncated_client_hello( hs, context, &context_len, digest, msg, 1 /* length prefix */ + hash_len) || - !tls13_psk_binder(verify_data, ssl->session->ssl_version, digest, - ssl->session->master_key, - ssl->session->master_key_length, context, context_len, - hash_len)) { + !tls13_psk_binder(verify_data, &verify_data_len, + ssl->session->ssl_version, digest, + MakeConstSpan(ssl->session->master_key, + ssl->session->master_key_length), + MakeConstSpan(context, context_len)) || + verify_data_len != hash_len) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return false; } - OPENSSL_memcpy(msg.data() + msg.size() - hash_len, verify_data, hash_len); + OPENSSL_memcpy(msg.data() + msg.size() - verify_data_len, verify_data, + verify_data_len); return true; } bool tls13_verify_psk_binder(SSL_HANDSHAKE *hs, SSL_SESSION *session, const SSLMessage &msg, CBS *binders) { - size_t hash_len = hs->transcript.DigestLen(); uint8_t context[EVP_MAX_MD_SIZE]; size_t context_len; - uint8_t verify_data[EVP_MAX_MD_SIZE] = {0}; + uint8_t verify_data[EVP_MAX_MD_SIZE]; + size_t verify_data_len; CBS binder; if (!hash_transcript_and_truncated_client_hello(hs, context, &context_len, hs->transcript.Digest(), msg.raw, CBS_len(binders)) || - !tls13_psk_binder(verify_data, hs->ssl->version, hs->transcript.Digest(), - session->master_key, session->master_key_length, - context, context_len, hash_len) || + !tls13_psk_binder( + verify_data, &verify_data_len, hs->ssl->version, + hs->transcript.Digest(), + MakeConstSpan(session->master_key, session->master_key_length), + MakeConstSpan(context, context_len)) || // 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); return false; } - bool binder_ok = CBS_len(&binder) == hash_len && - CRYPTO_memcmp(CBS_data(&binder), verify_data, hash_len) == 0; + bool binder_ok = + CBS_len(&binder) == verify_data_len && + CRYPTO_memcmp(CBS_data(&binder), verify_data, verify_data_len) == 0; #if defined(BORINGSSL_UNSAFE_FUZZER_MODE) binder_ok = true; #endif
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 1e87bb9..7b6f5df 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -87,7 +87,7 @@ return 0; } - return tls13_advance_key_schedule(hs, dhe_secret.data(), dhe_secret.size()); + return tls13_advance_key_schedule(hs, dhe_secret); } static int ssl_ext_supported_versions_add_serverhello(SSL_HANDSHAKE *hs, @@ -428,11 +428,12 @@ // Set up the key schedule and incorporate the PSK into the running secret. if (ssl->s3->session_reused) { - if (!tls13_init_key_schedule(hs, hs->new_session->master_key, - hs->new_session->master_key_length)) { + if (!tls13_init_key_schedule( + hs, MakeConstSpan(hs->new_session->master_key, + hs->new_session->master_key_length))) { return ssl_hs_error; } - } else if (!tls13_init_key_schedule(hs, kZeroes, hash_len)) { + } else if (!tls13_init_key_schedule(hs, MakeConstSpan(kZeroes, hash_len))) { return ssl_hs_error; } @@ -606,8 +607,9 @@ // Derive and enable the handshake traffic secrets. if (!tls13_derive_handshake_secrets(hs) || - !tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal, - hs->server_handshake_secret, hs->hash_len)) { + !tls13_set_traffic_key( + ssl, ssl_encryption_handshake, evp_aead_seal, + MakeConstSpan(hs->server_handshake_secret, hs->hash_len))) { return ssl_hs_error; } @@ -715,10 +717,11 @@ SSL *const ssl = hs->ssl; if (!tls13_add_finished(hs) || // Update the secret to the master secret and derive traffic keys. - !tls13_advance_key_schedule(hs, kZeroes, hs->hash_len) || + !tls13_advance_key_schedule(hs, MakeConstSpan(kZeroes, hs->hash_len)) || !tls13_derive_application_secrets(hs) || - !tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal, - hs->server_traffic_secret_0, hs->hash_len)) { + !tls13_set_traffic_key( + ssl, ssl_encryption_application, evp_aead_seal, + MakeConstSpan(hs->server_traffic_secret_0, hs->hash_len))) { return ssl_hs_error; } @@ -770,8 +773,9 @@ static enum ssl_hs_wait_t do_read_second_client_flight(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; if (ssl->s3->early_data_accepted) { - if (!tls13_set_traffic_key(ssl, ssl_encryption_early_data, evp_aead_open, - hs->early_traffic_secret, hs->hash_len)) { + if (!tls13_set_traffic_key( + ssl, ssl_encryption_early_data, evp_aead_open, + MakeConstSpan(hs->early_traffic_secret, hs->hash_len))) { return ssl_hs_error; } hs->can_early_write = true; @@ -805,8 +809,9 @@ ssl->method->next_message(ssl); } } - if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open, - hs->client_handshake_secret, hs->hash_len)) { + if (!tls13_set_traffic_key( + ssl, ssl_encryption_handshake, evp_aead_open, + MakeConstSpan(hs->client_handshake_secret, hs->hash_len))) { return ssl_hs_error; } hs->tls13_state = ssl->s3->early_data_accepted @@ -912,8 +917,9 @@ // and derived the resumption secret. !tls13_process_finished(hs, msg, ssl->s3->early_data_accepted) || // evp_aead_seal keys have already been switched. - !tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_open, - hs->client_traffic_secret_0, hs->hash_len)) { + !tls13_set_traffic_key( + ssl, ssl_encryption_application, evp_aead_open, + MakeConstSpan(hs->client_traffic_secret_0, hs->hash_len))) { return ssl_hs_error; }