Allow a single SSL to consume and produce handshake hints This CL splits the `hints` field on SSL into two separate fields for hints provided by the caller from SSL_set_handshake_hints, and pending hints being recorded for the current handshake (if requested via SSL_request_handshake_hints). Some handshake values applied from the provided hints may end up being emitted to the pending hints upon SSL_serialize_handshake_hints. Bug: 512856871 Change-Id: I5bbe8f10bc21e9c7258495a608207dcc6a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/97127 Reviewed-by: David Benjamin <davidben@google.com> Auto-Submit: Lily Chen <chlily@google.com> Commit-Queue: Lily Chen <chlily@google.com>
diff --git a/ssl/extensions.cc b/ssl/extensions.cc index a6c1f7c..64152ec 100644 --- a/ssl/extensions.cc +++ b/ssl/extensions.cc
@@ -5003,22 +5003,24 @@ Array<uint8_t> plaintext; enum ssl_ticket_aead_result_t result; - SSL_HANDSHAKE_HINTS *const hints = hs->hints.get(); - if (is_psk && hints && !hs->hints_requested && - !hints->decrypted_psk.empty()) { - result = plaintext.CopyFrom(hints->decrypted_psk) ? ssl_ticket_aead_success - : ssl_ticket_aead_error; - } else if (is_psk && hints && !hs->hints_requested && hints->ignore_psk) { + if (is_psk && hs->provided_hints != nullptr && + !hs->provided_hints->decrypted_psk.empty()) { + result = plaintext.CopyFrom(hs->provided_hints->decrypted_psk) + ? ssl_ticket_aead_success + : ssl_ticket_aead_error; + } else if (is_psk && hs->provided_hints != nullptr && + hs->provided_hints->ignore_psk) { result = ssl_ticket_aead_ignore_ticket; - } else if (!is_psk && hints && !hs->hints_requested && - !hints->decrypted_ticket.empty()) { - if (plaintext.CopyFrom(hints->decrypted_ticket)) { + } else if (!is_psk && hs->provided_hints != nullptr && + !hs->provided_hints->decrypted_ticket.empty()) { + if (plaintext.CopyFrom(hs->provided_hints->decrypted_ticket)) { result = ssl_ticket_aead_success; - *out_renew_ticket = hints->renew_ticket; + *out_renew_ticket = hs->provided_hints->renew_ticket; } else { result = ssl_ticket_aead_error; } - } else if (!is_psk && hints && !hs->hints_requested && hints->ignore_ticket) { + } else if (!is_psk && hs->provided_hints != nullptr && + hs->provided_hints->ignore_ticket) { result = ssl_ticket_aead_ignore_ticket; } else if (ssl->session_ctx->ticket_aead_method != nullptr) { result = ssl_decrypt_ticket_with_method(hs, &plaintext, out_renew_ticket, @@ -5038,23 +5040,23 @@ } } - if (hints && hs->hints_requested) { + if (hs->pending_hints != nullptr) { if (result == ssl_ticket_aead_ignore_ticket) { if (is_psk) { - hints->ignore_psk = true; + hs->pending_hints->ignore_psk = true; } else { - hints->ignore_ticket = true; + hs->pending_hints->ignore_ticket = true; } } else if (result == ssl_ticket_aead_success) { if (is_psk) { - if (!hints->decrypted_psk.CopyFrom(plaintext)) { + if (!hs->pending_hints->decrypted_psk.CopyFrom(plaintext)) { return ssl_ticket_aead_error; } } else { - if (!hints->decrypted_ticket.CopyFrom(plaintext)) { + if (!hs->pending_hints->decrypted_ticket.CopyFrom(plaintext)) { return ssl_ticket_aead_error; } - hints->renew_ticket = *out_renew_ticket; + hs->pending_hints->renew_ticket = *out_renew_ticket; } } }
diff --git a/ssl/handoff.cc b/ssl/handoff.cc index 5b87821..7f346c3 100644 --- a/ssl/handoff.cc +++ b/ssl/handoff.cc
@@ -828,9 +828,10 @@ CBS cbs, seq; CBS_init(&cbs, capabilities, capabilities_len); - UniquePtr<SSL_HANDSHAKE_HINTS> hints = MakeUnique<SSL_HANDSHAKE_HINTS>(); - if (hints == nullptr || // - !CBS_get_asn1(&cbs, &seq, CBS_ASN1_SEQUENCE) || // + UniquePtr<SSL_HANDSHAKE_HINTS> pending_hints = + MakeUnique<SSL_HANDSHAKE_HINTS>(); + if (pending_hints == nullptr || + !CBS_get_asn1(&cbs, &seq, CBS_ASN1_SEQUENCE) || !apply_remote_features(ssl, &seq)) { return 0; } @@ -856,8 +857,7 @@ return 0; } - s3->hs->hints_requested = true; - s3->hs->hints = std::move(hints); + s3->hs->pending_hints = std::move(pending_hints); return 1; } @@ -937,12 +937,12 @@ int SSL_serialize_handshake_hints(const SSL *ssl, CBB *out) { const SSL_HANDSHAKE *hs = ssl->s3->hs.get(); - if (!ssl->server || !hs->hints_requested) { + const SSL_HANDSHAKE_HINTS *const hints = hs->pending_hints.get(); + if (!ssl->server || hints == nullptr) { OPENSSL_PUT_ERROR(SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - const SSL_HANDSHAKE_HINTS *hints = hs->hints.get(); CBB seq, child; if (!CBB_add_asn1(out, &seq, CBS_ASN1_SEQUENCE)) { return 0; @@ -1195,6 +1195,6 @@ return 0; } - ssl->s3->hs->hints = std::move(hints_obj); + ssl->s3->hs->provided_hints = std::move(hints_obj); return 1; }
diff --git a/ssl/handshake.cc b/ssl/handshake.cc index 1e6d35d..09a1763 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc
@@ -55,7 +55,6 @@ extended_master_secret(false), pending_private_key_op(false), handback(false), - hints_requested(false), cert_compression_negotiated(false), apply_jdk11_workaround(false), can_release_private_key(false),
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 31b7492..2046db0 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -930,10 +930,10 @@ hs->channel_id_negotiated = false; } - SSL_HANDSHAKE_HINTS *const hints = hs->hints.get(); - if (hints && !hs->hints_requested && - hints->server_random_tls12.size() == SSL3_RANDOM_SIZE) { - OPENSSL_memcpy(ssl->s3->server_random, hints->server_random_tls12.data(), + if (hs->provided_hints != nullptr && + hs->provided_hints->server_random_tls12.size() == SSL3_RANDOM_SIZE) { + OPENSSL_memcpy(ssl->s3->server_random, + hs->provided_hints->server_random_tls12.data(), SSL3_RANDOM_SIZE); } else { OPENSSL_timeval now = ssl_ctx_get_current_time(ssl->ctx.get()); @@ -942,10 +942,11 @@ if (!RAND_bytes(ssl->s3->server_random + 4, SSL3_RANDOM_SIZE - 4)) { return ssl_hs_error; } - if (hints && hs->hints_requested && - !hints->server_random_tls12.CopyFrom(ssl->s3->server_random)) { - return ssl_hs_error; - } + } + if (hs->pending_hints != nullptr && + !hs->pending_hints->server_random_tls12.CopyFrom( + ssl->s3->server_random)) { + return ssl_hs_error; } // Implement the TLS 1.3 anti-downgrade feature. @@ -991,7 +992,7 @@ if (ssl->session != nullptr) { // No additional hints to generate in resumption. - if (hs->hints_requested) { + if (hs->pending_hints != nullptr) { return ssl_hs_hints_ready; } hs->state = state12_send_server_finished; @@ -1067,19 +1068,18 @@ return ssl_hs_error; } - SSL_HANDSHAKE_HINTS *const hints = hs->hints.get(); - bool hint_ok = false; - if (hints && !hs->hints_requested && - hints->ecdhe_group_id == hs->new_session->group_id && - !hints->ecdhe_public_key.empty() && - !hints->ecdhe_private_key.empty()) { - CBS cbs = CBS(hints->ecdhe_private_key); - hint_ok = hs->key_shares[0]->DeserializePrivateKey(&cbs); + bool provided_hint_ok = false; + if (hs->provided_hints != nullptr && + hs->provided_hints->ecdhe_group_id == hs->new_session->group_id && + !hs->provided_hints->ecdhe_public_key.empty() && + !hs->provided_hints->ecdhe_private_key.empty()) { + CBS cbs = CBS(hs->provided_hints->ecdhe_private_key); + provided_hint_ok = hs->key_shares[0]->DeserializePrivateKey(&cbs); } - if (hint_ok) { + if (provided_hint_ok) { // Reuse the ECDH key from handshake hints. - if (!CBB_add_bytes(&child, hints->ecdhe_public_key.data(), - hints->ecdhe_public_key.size())) { + if (!CBB_add_bytes(&child, hs->provided_hints->ecdhe_public_key.data(), + hs->provided_hints->ecdhe_public_key.size())) { return ssl_hs_error; } } else { @@ -1087,18 +1087,18 @@ if (!hs->key_shares[0]->Generate(&child)) { return ssl_hs_error; } - // If generating hints, save the ECDHE key. - if (hints && hs->hints_requested) { - bssl::ScopedCBB private_key_cbb; - if (!hints->ecdhe_public_key.CopyFrom(CBBAsSpan(&child)) || - !CBB_init(private_key_cbb.get(), 32) || - !hs->key_shares[0]->SerializePrivateKey(private_key_cbb.get()) || - !CBBFinishArray(private_key_cbb.get(), - &hints->ecdhe_private_key)) { - return ssl_hs_error; - } - hints->ecdhe_group_id = hs->new_session->group_id; + } + // If generating hints, save the ECDHE key. + if (hs->pending_hints != nullptr) { + bssl::ScopedCBB private_key_cbb; + if (!hs->pending_hints->ecdhe_public_key.CopyFrom(CBBAsSpan(&child)) || + !CBB_init(private_key_cbb.get(), 32) || + !hs->key_shares[0]->SerializePrivateKey(private_key_cbb.get()) || + !CBBFinishArray(private_key_cbb.get(), + &hs->pending_hints->ecdhe_private_key)) { + return ssl_hs_error; } + hs->pending_hints->ecdhe_group_id = hs->new_session->group_id; } } else { assert(alg_k & SSL_kPSK); @@ -1185,7 +1185,7 @@ static enum ssl_hs_wait_t do_send_server_hello_done(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (hs->hints_requested) { + if (hs->pending_hints != nullptr) { return ssl_hs_hints_ready; }
diff --git a/ssl/internal.h b/ssl/internal.h index 5cd13e1..c8d5330 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1985,12 +1985,15 @@ // key_block is the record-layer key block for TLS 1.2 and earlier. Array<uint8_t> key_block; - // hints contains the handshake hints for this connection. If - // `hints_requested` is true, this field is non-null and contains the pending - // hints to filled as the predicted handshake progresses. Otherwise, this - // field, if non-null, contains hints configured by the caller and will - // influence the handshake on match. - UniquePtr<SSL_HANDSHAKE_HINTS> hints; + // pending_hints, if non-null, contains the pending hints to filled as the + // predicted handshake progresses. If non-null, only the first round-trip of + // the handshake will complete, after which the `pending_hints` structure can + // be serialized. + UniquePtr<SSL_HANDSHAKE_HINTS> pending_hints; + + // provided_hints, if non-null, contains hints configured by the caller + // and will influence the (real or predicted) handshake on match. + UniquePtr<SSL_HANDSHAKE_HINTS> provided_hints; // ech_is_inner, on the server, indicates whether the ClientHello contained an // inner ECH extension. @@ -2070,11 +2073,6 @@ // `SSL_apply_handoff`. bool handback : 1; - // hints_requested indicates the caller has requested handshake hints. Only - // the first round-trip of the handshake will complete, after which the - // `hints` structure can be serialized. - bool hints_requested : 1; - // cert_compression_negotiated is true iff `cert_compression_alg_id` is valid. bool cert_compression_negotiated : 1;
diff --git a/ssl/s3_both.cc b/ssl/s3_both.cc index 142a879..c08e381 100644 --- a/ssl/s3_both.cc +++ b/ssl/s3_both.cc
@@ -155,7 +155,7 @@ auto data = Span(reinterpret_cast<const uint8_t *>(pending_hs_data->data), pending_hs_data->length); if (SSL_is_quic(ssl)) { - if ((ssl->s3->hs == nullptr || !ssl->s3->hs->hints_requested) && + if ((ssl->s3->hs == nullptr || ssl->s3->hs->pending_hints == nullptr) && !ssl->quic_method->add_handshake_data(ssl, ssl->s3->quic_write_level, data.data(), data.size())) { OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_INTERNAL_ERROR);
diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 8c52d7c..4e43717 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc
@@ -231,9 +231,8 @@ uint16_t sigalg, Span<const uint8_t> in) { SSL *const ssl = hs->ssl; const SSLCredential *const cred = hs->credential.get(); - SSL_HANDSHAKE_HINTS *const hints = hs->hints.get(); Array<uint8_t> spki; - if (hints) { + if (hs->provided_hints != nullptr || hs->pending_hints != nullptr) { ScopedCBB spki_cbb; if (!CBB_init(spki_cbb.get(), 64) || !EVP_marshal_public_key(spki_cbb.get(), cred->pubkey.get()) || @@ -244,60 +243,61 @@ } // Replay the signature from handshake hints if available. - if (hints && !hs->hints_requested && // - sigalg == hints->signature_algorithm && // - in == hints->signature_input && // - Span(spki) == hints->signature_spki && // - !hints->signature.empty() && // - hints->signature.size() <= max_out) { + bool hint_applicable = hs->provided_hints != nullptr && + sigalg == hs->provided_hints->signature_algorithm && + in == hs->provided_hints->signature_input && + Span(spki) == hs->provided_hints->signature_spki && + !hs->provided_hints->signature.empty() && + hs->provided_hints->signature.size() <= max_out; + if (hint_applicable) { // Signature algorithm and input both match. Reuse the signature from hints. - *out_len = hints->signature.size(); - OPENSSL_memcpy(out, hints->signature.data(), hints->signature.size()); - return ssl_private_key_success; - } + *out_len = hs->provided_hints->signature.size(); + OPENSSL_memcpy(out, hs->provided_hints->signature.data(), + hs->provided_hints->signature.size()); + } else { + const SSL_PRIVATE_KEY_METHOD *key_method = cred->key_method; + EVP_PKEY *privkey = cred->privkey.get(); + assert(!hs->can_release_private_key); - const SSL_PRIVATE_KEY_METHOD *key_method = cred->key_method; - EVP_PKEY *privkey = cred->privkey.get(); - assert(!hs->can_release_private_key); - - if (key_method != nullptr) { - if (key_method->sign == nullptr) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return ssl_private_key_failure; - } - enum ssl_private_key_result_t ret; - if (hs->pending_private_key_op) { - if (key_method->complete == nullptr) { + if (key_method != nullptr) { + if (key_method->sign == nullptr) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return ssl_private_key_failure; } - ret = key_method->complete(ssl, out, out_len, max_out); + enum ssl_private_key_result_t ret; + if (hs->pending_private_key_op) { + if (key_method->complete == nullptr) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return ssl_private_key_failure; + } + ret = key_method->complete(ssl, out, out_len, max_out); + } else { + ret = key_method->sign(ssl, out, out_len, max_out, sigalg, in.data(), + in.size()); + } + if (ret == ssl_private_key_failure) { + OPENSSL_PUT_ERROR(SSL, SSL_R_PRIVATE_KEY_OPERATION_FAILED); + } + hs->pending_private_key_op = ret == ssl_private_key_retry; + if (ret != ssl_private_key_success) { + return ret; + } } else { - ret = key_method->sign(ssl, out, out_len, max_out, sigalg, in.data(), - in.size()); - } - if (ret == ssl_private_key_failure) { - OPENSSL_PUT_ERROR(SSL, SSL_R_PRIVATE_KEY_OPERATION_FAILED); - } - hs->pending_private_key_op = ret == ssl_private_key_retry; - if (ret != ssl_private_key_success) { - return ret; - } - } else { - *out_len = max_out; - ScopedEVP_MD_CTX ctx; - if (!setup_ctx(ssl, ctx.get(), privkey, sigalg, false /* sign */) || - !EVP_DigestSign(ctx.get(), out, out_len, in.data(), in.size())) { - return ssl_private_key_failure; + *out_len = max_out; + ScopedEVP_MD_CTX ctx; + if (!setup_ctx(ssl, ctx.get(), privkey, sigalg, false /* sign */) || + !EVP_DigestSign(ctx.get(), out, out_len, in.data(), in.size())) { + return ssl_private_key_failure; + } } } - // Save the hint if applicable. - if (hints && hs->hints_requested) { - hints->signature_algorithm = sigalg; - hints->signature_spki = std::move(spki); - if (!hints->signature_input.CopyFrom(in) || - !hints->signature.CopyFrom(Span(out, *out_len))) { + // Save the hint if requested. + if (hs->pending_hints != nullptr) { + hs->pending_hints->signature_algorithm = sigalg; + hs->pending_hints->signature_spki = std::move(spki); + if (!hs->pending_hints->signature_input.CopyFrom(in) || + !hs->pending_hints->signature.CopyFrom(Span(out, *out_len))) { return ssl_private_key_failure; } }
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 7b7ce71..7975325 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -11815,5 +11815,151 @@ EXPECT_EQ(SSL_get_signature_algorithm_used(server.get()), 0u); } +struct HintTestState { + std::vector<uint8_t> client_hello; + std::vector<uint8_t> capabilities; + std::optional<std::vector<uint8_t>> hints; +}; + +// This callback either saves the ClientHello to the HintTestState, or resumes +// the handshake with hints read from the HintTestState. If `hints` is +// populated, the contents will be passed to `SSL_set_handshake_hints` and the +// handshake will be continued. Otherwise, `client_hello` and `capabilities` +// will be filled in and the handshake will be paused. +static enum ssl_select_cert_result_t SaveClientHelloCallback( + const SSL_CLIENT_HELLO *client_hello) { + SSL *ssl = client_hello->ssl; + HintTestState *state = + reinterpret_cast<HintTestState *>(SSL_get_app_data(ssl)); + if (state == nullptr) { + return ssl_select_cert_success; + } + if (state->hints.has_value()) { + SSL_set_handshake_hints(ssl, state->hints->data(), state->hints->size()); + return ssl_select_cert_success; + } + + state->client_hello.assign( + client_hello->client_hello, + client_hello->client_hello + client_hello->client_hello_len); + + bssl::ScopedCBB cbb; + if (!CBB_init(cbb.get(), 64) || !SSL_serialize_capabilities(ssl, cbb.get())) { + return ssl_select_cert_error; + } + Span<const uint8_t> capabilities = CBBAsSpan(cbb.get()); + state->capabilities.assign(capabilities.begin(), capabilities.end()); + return ssl_select_cert_retry; +} + +// Helper to optionally apply `hints_in`, run the server side of a handshake to +// the first round-trip, and obtain hints. Returns std::nullopt on failure. +std::optional<std::vector<uint8_t>> GetHandshakeHints( + SSL *ssl, const HintTestState &test_state, Span<const uint8_t> hints_in) { + SSL_set_accept_state(ssl); + if (!SSL_request_handshake_hints( + ssl, test_state.client_hello.data(), test_state.client_hello.size(), + test_state.capabilities.data(), test_state.capabilities.size()) || + (!hints_in.empty() && + !SSL_set_handshake_hints(ssl, hints_in.data(), hints_in.size()))) { + return std::nullopt; + } + int ret = SSL_do_handshake(ssl); + if (ret != -1 || SSL_get_error(ssl, ret) != SSL_ERROR_HANDSHAKE_HINTS_READY) { + return std::nullopt; + } + bssl::ScopedCBB hints; + if (!CBB_init(hints.get(), 256) || + !SSL_serialize_handshake_hints(ssl, hints.get())) { + return std::nullopt; + } + return std::vector<uint8_t>(CBB_data(hints.get()), + CBB_data(hints.get()) + CBB_len(hints.get())); +} + +TEST(SSLTest, HandshakeHints) { + bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(client_ctx); + + bssl::UniquePtr<SSL_CTX> server_rsa_ctx( + CreateContextWithTestCertificate(TLS_method())); + ASSERT_TRUE(server_rsa_ctx); + + bssl::UniquePtr<SSL_CTX> server_ecdsa_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(server_ecdsa_ctx); + bssl::UniquePtr<X509> ecdsa_cert = GetECDSATestCertificate(); + bssl::UniquePtr<EVP_PKEY> ecdsa_key = GetECDSATestKey(); + ASSERT_TRUE(ecdsa_cert); + ASSERT_TRUE(ecdsa_key); + ASSERT_TRUE( + SSL_CTX_use_certificate(server_ecdsa_ctx.get(), ecdsa_cert.get())); + ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ecdsa_ctx.get(), ecdsa_key.get())); + + SSL_CTX_set_custom_verify(client_ctx.get(), SSL_VERIFY_PEER, + AcceptAnyCertificate); + SSL_CTX_set_custom_verify(server_rsa_ctx.get(), SSL_VERIFY_PEER, + AcceptAnyCertificate); + SSL_CTX_set_custom_verify(server_ecdsa_ctx.get(), SSL_VERIFY_PEER, + AcceptAnyCertificate); + + // Capture ClientHello and capabilities, pausing the handshake to resume later + // with hints. + HintTestState test_state; + SSL_CTX_set_select_certificate_cb(server_rsa_ctx.get(), + SaveClientHelloCallback); + bssl::UniquePtr<SSL> client, server; + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_rsa_ctx.get())); + SSL_set_app_data(server.get(), &test_state); + ASSERT_FALSE(CompleteHandshakes(client.get(), server.get())); + ASSERT_FALSE(test_state.client_hello.empty()); + ASSERT_FALSE(test_state.capabilities.empty()); + + // Generate RSA hints from scratch. + bssl::UniquePtr<SSL> rsa1(SSL_new(server_rsa_ctx.get())); + ASSERT_TRUE(rsa1); + std::optional<std::vector<uint8_t>> rsa_hints1 = + GetHandshakeHints(rsa1.get(), test_state, + /*hints_in=*/Span<const uint8_t>()); + ASSERT_TRUE(rsa_hints1.has_value()); + + // Generate RSA hints after applying rsa_hints1. + bssl::UniquePtr<SSL> rsa2(SSL_new(server_rsa_ctx.get())); + ASSERT_TRUE(rsa2); + std::optional<std::vector<uint8_t>> rsa_hints2 = + GetHandshakeHints(rsa2.get(), test_state, + /*hints_in=*/*rsa_hints1); + ASSERT_TRUE(rsa_hints2.has_value()); + + EXPECT_EQ(*rsa_hints1, *rsa_hints2); + + // Generate ECDSA hints after providing rsa_hints1. + bssl::UniquePtr<SSL> ecdsa1(SSL_new(server_ecdsa_ctx.get())); + ASSERT_TRUE(ecdsa1); + std::optional<std::vector<uint8_t>> ecdsa_hints1 = + GetHandshakeHints(ecdsa1.get(), test_state, + /*hints_in=*/*rsa_hints1); + ASSERT_TRUE(ecdsa_hints1.has_value()); + + // The ECDSA hints are different because the ECDSA handshake could not apply + // the RSA hints. + EXPECT_NE(*ecdsa_hints1, *rsa_hints1); + + // Generate ECDSA hints after applying ecdsa_hints1. + bssl::UniquePtr<SSL> ecdsa2(SSL_new(server_ecdsa_ctx.get())); + ASSERT_TRUE(ecdsa2); + std::optional<std::vector<uint8_t>> ecdsa_hints2 = + GetHandshakeHints(ecdsa2.get(), test_state, + /*hints_in=*/*ecdsa_hints1); + ASSERT_TRUE(ecdsa_hints2.has_value()); + + EXPECT_EQ(*ecdsa_hints1, *ecdsa_hints2); + + // Complete the original handshake while providing ecdsa_hints1. (The hints + // will not apply, but the handshake should still succeed.) + test_state.hints = *std::move(ecdsa_hints1); + ASSERT_TRUE(CompleteHandshakes(client.get(), server.get())); +} + } // namespace BSSL_NAMESPACE_END
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc index fb9d05e..62a2a70 100644 --- a/ssl/tls13_both.cc +++ b/ssl/tls13_both.cc
@@ -579,13 +579,14 @@ return false; } - SSL_HANDSHAKE_HINTS *const hints = hs->hints.get(); - if (hints && !hs->hints_requested && - hints->cert_compression_alg_id == hs->cert_compression_alg_id && - hints->cert_compression_input == Span(msg) && - !hints->cert_compression_output.empty()) { - if (!CBB_add_bytes(&compressed, hints->cert_compression_output.data(), - hints->cert_compression_output.size())) { + if (hs->provided_hints != nullptr && + hs->provided_hints->cert_compression_alg_id == + hs->cert_compression_alg_id && + hs->provided_hints->cert_compression_input == Span(msg) && + !hs->provided_hints->cert_compression_output.empty()) { + if (!CBB_add_bytes(&compressed, + hs->provided_hints->cert_compression_output.data(), + hs->provided_hints->cert_compression_output.size())) { return false; } } else { @@ -593,12 +594,13 @@ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return false; } - if (hints && hs->hints_requested) { - hints->cert_compression_alg_id = hs->cert_compression_alg_id; - if (!hints->cert_compression_input.CopyFrom(msg) || - !hints->cert_compression_output.CopyFrom(CBBAsSpan(&compressed))) { - return false; - } + } + if (hs->pending_hints != nullptr) { + hs->pending_hints->cert_compression_alg_id = hs->cert_compression_alg_id; + if (!hs->pending_hints->cert_compression_input.CopyFrom(msg) || + !hs->pending_hints->cert_compression_output.CopyFrom( + CBBAsSpan(&compressed))) { + return false; } }
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 730e5a4..d09aa53 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -89,12 +89,13 @@ } Array<uint8_t> secret; - SSL_HANDSHAKE_HINTS *const hints = hs->hints.get(); - if (hints && !hs->hints_requested && hints->key_share_group_id == group_id && - !hints->key_share_secret.empty()) { + if (hs->provided_hints != nullptr && + hs->provided_hints->key_share_group_id == group_id && + !hs->provided_hints->key_share_secret.empty()) { // Copy the key_share secret from hints. - if (!hs->key_share_ciphertext.CopyFrom(hints->key_share_ciphertext) || - !secret.CopyFrom(hints->key_share_secret)) { + if (!hs->key_share_ciphertext.CopyFrom( + hs->provided_hints->key_share_ciphertext) || + !secret.CopyFrom(hs->provided_hints->key_share_secret)) { ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return false; } @@ -108,13 +109,14 @@ ssl_send_alert(ssl, SSL3_AL_FATAL, alert); return false; } - if (hints && hs->hints_requested) { - hints->key_share_group_id = group_id; - if (!hints->key_share_ciphertext.CopyFrom(hs->key_share_ciphertext) || - !hints->key_share_secret.CopyFrom(secret)) { - ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return false; - } + } + if (hs->pending_hints != nullptr) { + hs->pending_hints->key_share_group_id = group_id; + if (!hs->pending_hints->key_share_ciphertext.CopyFrom( + hs->key_share_ciphertext) || + !hs->pending_hints->key_share_secret.CopyFrom(secret)) { + ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + return false; } } @@ -824,7 +826,7 @@ static enum ssl_hs_wait_t do_send_hello_retry_request(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (hs->hints_requested) { + if (hs->pending_hints != nullptr) { return ssl_hs_hints_ready; } @@ -1000,17 +1002,17 @@ Span<uint8_t> random(ssl->s3->server_random); - SSL_HANDSHAKE_HINTS *const hints = hs->hints.get(); - if (hints && !hs->hints_requested && - hints->server_random_tls13.size() == random.size()) { - OPENSSL_memcpy(random.data(), hints->server_random_tls13.data(), + if (hs->provided_hints != nullptr && + hs->provided_hints->server_random_tls13.size() == random.size()) { + OPENSSL_memcpy(random.data(), + hs->provided_hints->server_random_tls13.data(), random.size()); } else { RAND_bytes(random.data(), random.size()); - if (hints && hs->hints_requested && - !hints->server_random_tls13.CopyFrom(random)) { - return ssl_hs_error; - } + } + if (hs->pending_hints != nullptr && + !hs->pending_hints->server_random_tls13.CopyFrom(random)) { + return ssl_hs_error; } Array<uint8_t> server_hello; @@ -1146,7 +1148,7 @@ static enum ssl_hs_wait_t do_send_server_finished(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (hs->hints_requested) { + if (hs->pending_hints != nullptr) { return ssl_hs_hints_ready; }
diff --git a/ssl/tls_method.cc b/ssl/tls_method.cc index b458f84..0e5d918 100644 --- a/ssl/tls_method.cc +++ b/ssl/tls_method.cc
@@ -51,7 +51,7 @@ } if (SSL_is_quic(ssl)) { - if ((ssl->s3->hs == nullptr || !ssl->s3->hs->hints_requested) && + if ((ssl->s3->hs == nullptr || ssl->s3->hs->pending_hints == nullptr) && !ssl->quic_method->set_read_secret(ssl, level, aead_ctx->cipher(), traffic_secret.data(), traffic_secret.size())) { @@ -80,7 +80,7 @@ } if (SSL_is_quic(ssl)) { - if ((ssl->s3->hs == nullptr || !ssl->s3->hs->hints_requested) && + if ((ssl->s3->hs == nullptr || ssl->s3->hs->pending_hints == nullptr) && !ssl->quic_method->set_write_secret(ssl, level, aead_ctx->cipher(), traffic_secret.data(), traffic_secret.size())) {