Skip to content

Commit

Permalink
Add SSL_parse_client_hello
Browse files Browse the repository at this point in the history
Applications sometimes need to create SSL_CLIENT_HELLO objects for
testing. We already have this function internally. Just expose it as
public API.

The only non-trivial change here is I've moved the error queue bits from
the caller of ssl_client_hello_init to inside SSL_parse_client_hello.
This matches our conventions a bit better (usually functions are
responsible for originating the error), and keeps ssl.h's public APIs
consistently using the error queue here.

Bug: 395069491
Change-Id: I16fb35772a61e98d2621f781e475aa5611b13582
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76128
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
  • Loading branch information
davidben authored and Boringssl LUCI CQ committed Feb 11, 2025
1 parent ce25d4b commit 9439870
Show file tree
Hide file tree
Showing 10 changed files with 211 additions and 31 deletions.
3 changes: 2 additions & 1 deletion fuzz/decode_client_hello_inner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) {

// Use the remaining bytes in |reader| as the ClientHelloOuter.
SSL_CLIENT_HELLO client_hello_outer;
if (!bssl::ssl_client_hello_init(ssl.get(), &client_hello_outer, reader)) {
if (!SSL_parse_client_hello(ssl.get(), &client_hello_outer, CBS_data(&reader),
CBS_len(&reader))) {
return 0;
}

Expand Down
13 changes: 13 additions & 0 deletions include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4897,6 +4897,19 @@ OPENSSL_EXPORT int SSL_used_hello_retry_request(const SSL *ssl);
// https://bugs.openjdk.java.net/browse/JDK-8213202
OPENSSL_EXPORT void SSL_set_jdk11_workaround(SSL *ssl, int enable);

// SSL_parse_client_hello decodes a ClientHello structure from |len| bytes in
// |in|. On success, it returns one and writes the result to |*out|. Otherwise,
// it returns zero. |ssl| will be saved into |*out| and determines how the
// ClientHello is parsed, notably TLS vs DTLS. The fields in |*out| will alias
// |in| and are only valid as long as |in| is valid and unchanged.
//
// |in| should contain just the ClientHello structure (RFC 8446 and RFC 9147),
// excluding the handshake header and already reassembled from record layer.
// That is, |in| should begin with the legacy_version field, not the
// client_hello HandshakeType constant or the handshake ContentType constant.
OPENSSL_EXPORT int SSL_parse_client_hello(const SSL *ssl, SSL_CLIENT_HELLO *out,
const uint8_t *in, size_t len);


// Deprecated functions.

Expand Down
3 changes: 1 addition & 2 deletions ssl/encrypted_client_hello.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ static bool is_valid_client_hello_inner(SSL *ssl, uint8_t *out_alert,
// See draft-ietf-tls-esni-13, section 7.1.
SSL_CLIENT_HELLO client_hello;
CBS extension;
if (!ssl_client_hello_init(ssl, &client_hello, body) ||
if (!SSL_parse_client_hello(ssl, &client_hello, body.data(), body.size()) ||
!ssl_client_hello_get_extension(&client_hello, &extension,
TLSEXT_TYPE_encrypted_client_hello) ||
CBS_len(&extension) != 1 || //
Expand Down Expand Up @@ -139,7 +139,6 @@ bool ssl_decode_client_hello_inner(
CBS cbs = encoded_client_hello_inner;
if (!ssl_parse_client_hello_with_trailing_data(ssl, &cbs,
&client_hello_inner)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return false;
}
// The remaining data is padding.
Expand Down
27 changes: 17 additions & 10 deletions ssl/extensions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,6 @@ static bool is_post_quantum_group(uint16_t id) {
}
}

bool ssl_client_hello_init(const SSL *ssl, SSL_CLIENT_HELLO *out,
Span<const uint8_t> body) {
CBS cbs = body;
if (!ssl_parse_client_hello_with_trailing_data(ssl, &cbs, out) ||
CBS_len(&cbs) != 0) {
return false;
}
return true;
}

bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
SSL_CLIENT_HELLO *out) {
OPENSSL_memset(out, 0, sizeof(*out));
Expand All @@ -142,6 +132,7 @@ bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
!CBS_get_bytes(cbs, &random, SSL3_RANDOM_SIZE) ||
!CBS_get_u8_length_prefixed(cbs, &session_id) ||
CBS_len(&session_id) > SSL_MAX_SSL_SESSION_ID_LENGTH) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return false;
}

Expand All @@ -153,6 +144,7 @@ bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
if (SSL_is_dtls(out->ssl)) {
CBS cookie;
if (!CBS_get_u8_length_prefixed(cbs, &cookie)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return false;
}
out->dtls_cookie = CBS_data(&cookie);
Expand All @@ -167,6 +159,7 @@ bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
CBS_len(&cipher_suites) < 2 || (CBS_len(&cipher_suites) & 1) != 0 ||
!CBS_get_u8_length_prefixed(cbs, &compression_methods) ||
CBS_len(&compression_methods) < 1) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return false;
}

Expand All @@ -185,6 +178,7 @@ bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
CBS extensions;
if (!CBS_get_u16_length_prefixed(cbs, &extensions) ||
!tls1_check_duplicate_extensions(&extensions)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return false;
}
out->extensions = CBS_data(&extensions);
Expand Down Expand Up @@ -4559,6 +4553,19 @@ BSSL_NAMESPACE_END

using namespace bssl;

int SSL_parse_client_hello(const SSL *ssl, SSL_CLIENT_HELLO *out,
const uint8_t *in, size_t len) {
CBS cbs = Span(in, len);
if (!ssl_parse_client_hello_with_trailing_data(ssl, &cbs, out)) {
return 0;
}
if (CBS_len(&cbs) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return 0;
}
return 1;
}

int SSL_early_callback_ctx_extension_get(const SSL_CLIENT_HELLO *client_hello,
uint16_t extension_type,
const uint8_t **out_data,
Expand Down
3 changes: 2 additions & 1 deletion ssl/handoff.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ bool SSL_serialize_handoff(const SSL *ssl, CBB *out,
s3->hs_buf->length) ||
!serialize_features(&seq) || !CBB_flush(out) ||
!ssl->method->get_message(ssl, &msg) ||
!ssl_client_hello_init(ssl, out_hello, msg.body)) {
!SSL_parse_client_hello(ssl, out_hello, CBS_data(&msg.body),
CBS_len(&msg.body))) {
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion ssl/handshake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ bool SSL_HANDSHAKE::GetClientHello(SSLMessage *out_msg,
return false;
}

if (!ssl_client_hello_init(ssl, out_client_hello, out_msg->body)) {
if (!SSL_parse_client_hello(ssl, out_client_hello, CBS_data(&out_msg->body),
CBS_len(&out_msg->body))) {
OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return false;
Expand Down
5 changes: 3 additions & 2 deletions ssl/handshake_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,8 @@ static enum ssl_hs_wait_t do_read_client_hello(SSL_HANDSHAKE *hs) {
}

SSL_CLIENT_HELLO client_hello;
if (!ssl_client_hello_init(ssl, &client_hello, msg.body)) {
if (!SSL_parse_client_hello(ssl, &client_hello, CBS_data(&msg.body),
CBS_len(&msg.body))) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return ssl_hs_error;
Expand Down Expand Up @@ -764,7 +765,7 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) {
hs->new_cipher = params.cipher;
hs->signature_algorithm = params.signature_algorithm;

// |ssl_client_hello_init| checks that |client_hello.session_id| is not too
// |SSL_parse_client_hello| checks that |client_hello.session_id| is not too
// large.
hs->session_id.CopyFrom(
Span(client_hello.session_id, client_hello.session_id_len));
Expand Down
6 changes: 0 additions & 6 deletions ssl/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -2676,12 +2676,6 @@ bool ssl_log_secret(const SSL *ssl, const char *label,

// ClientHello functions.

// ssl_client_hello_init parses |body| as a ClientHello message, excluding the
// message header, and writes the result to |*out|. It returns true on success
// and false on error. This function is exported for testing.
OPENSSL_EXPORT bool ssl_client_hello_init(const SSL *ssl, SSL_CLIENT_HELLO *out,
Span<const uint8_t> body);

bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
SSL_CLIENT_HELLO *out);

Expand Down
176 changes: 169 additions & 7 deletions ssl/ssl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2189,13 +2189,14 @@ static bool GetECHLength(SSL_CTX *ctx, size_t *out_client_hello_len,
std::vector<uint8_t> client_hello;
SSL_CLIENT_HELLO parsed;
const uint8_t *unused;
if (!GetClientHello(ssl.get(), &client_hello) ||
!ssl_client_hello_init(
ssl.get(), &parsed,
// Skip record and handshake headers. This assumes the ClientHello
// fits in one record.
Span(client_hello)
.subspan(SSL3_RT_HEADER_LENGTH + SSL3_HM_HEADER_LENGTH)) ||
if (!GetClientHello(ssl.get(), &client_hello)) {
return false;
}
// Skip record and handshake headers. This assumes the ClientHello
// fits in one record.
auto body =
Span(client_hello).subspan(SSL3_RT_HEADER_LENGTH + SSL3_HM_HEADER_LENGTH);
if (!SSL_parse_client_hello(ssl.get(), &parsed, body.data(), body.size()) ||
!SSL_early_callback_ctx_extension_get(
&parsed, TLSEXT_TYPE_encrypted_client_hello, &unused, out_ech_len)) {
return false;
Expand Down Expand Up @@ -9856,6 +9857,167 @@ TEST(SSLTest, SetGetCompliancePolicy) {
}
}

TEST(SSLTest, ParseClientHello) {
for (bool dtls : {false, true}) {
SCOPED_TRACE(dtls);
bssl::UniquePtr<SSL_CTX> ctx(
SSL_CTX_new(dtls ? DTLS_method() : TLS_method()));
ASSERT_TRUE(ctx);
bssl::UniquePtr<SSL> ssl(SSL_new(ctx.get()));
ASSERT_TRUE(ssl);

// From draft-ietf-tls-tls13-vectors-06, Section 7, annotated.
static const uint8_t kClientHelloTLS[] = {
// legacy_version
0x03, 0x03,
// random
0x37, 0xb0, 0x76, 0xd2, 0xfa, 0x50, 0x94, 0x39, 0x5e, 0x99, 0x71, 0xd7,
0x53, 0xc3, 0xc4, 0xcf, 0x07, 0x56, 0xb9, 0x40, 0x70, 0x13, 0xcb, 0xca,
0xc7, 0xf4, 0x4a, 0xc3, 0x28, 0x13, 0xf6, 0x0f,
// legacy_session_id
0x20, 0x91, 0x41, 0xb7, 0x89, 0x83, 0xd3, 0x67, 0xa0, 0xfe, 0x97, 0x08,
0xdf, 0x32, 0xf5, 0xb9, 0x88, 0x8f, 0xe5, 0x9e, 0xde, 0x4e, 0x61, 0x2c,
0xf6, 0xbd, 0xb1, 0xfb, 0xbe, 0xe6, 0xf9, 0xef, 0xfe,
// cipher_suites
0x00, 0x06, 0x13, 0x01, 0x13, 0x03, 0x13, 0x02,
// legacy_compression_methods
0x01, 0x00,
// extensions
0x00, 0x8d,
// server_name
0x00, 0x00, 0x00, 0x0b, 0x00, 0x09, 0x00, 0x00, 0x06, 0x73, 0x65, 0x72,
0x76, 0x65, 0x72,
// renegotiation_info
0xff, 0x01, 0x00, 0x01, 0x00,
// supported_groups
0x00, 0x0a, 0x00, 0x14, 0x00, 0x12, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18,
0x00, 0x19, 0x01, 0x00, 0x01, 0x01, 0x01, 0x02, 0x01, 0x03, 0x01, 0x04,
// key_share
0x00, 0x33, 0x00, 0x26, 0x00, 0x24, 0x00, 0x1d, 0x00, 0x20, 0xd5, 0x15,
0x42, 0x62, 0x5f, 0x25, 0xa9, 0x2d, 0x44, 0xa3, 0xaa, 0xde, 0xf5, 0x9c,
0xa8, 0x49, 0xad, 0x2f, 0x8e, 0xfa, 0x9f, 0x04, 0xb8, 0xf5, 0xda, 0xb4,
0x02, 0xac, 0xbc, 0x57, 0x1f, 0x16,
// supported_versions
0x00, 0x2b, 0x00, 0x03, 0x02, 0x03, 0x04,
// signature_algorithms
0x00, 0x0d, 0x00, 0x20, 0x00, 0x1e, 0x04, 0x03, 0x05, 0x03, 0x06, 0x03,
0x02, 0x03, 0x08, 0x04, 0x08, 0x05, 0x08, 0x06, 0x04, 0x01, 0x05, 0x01,
0x06, 0x01, 0x02, 0x01, 0x04, 0x02, 0x05, 0x02, 0x06, 0x02, 0x02, 0x02,
// psk_key_exchange_modes
0x00, 0x2d, 0x00, 0x02, 0x01, 0x01,
// record_size_limit
0x00, 0x1c, 0x00, 0x02, 0x40, 0x01};
// The above, modified for DTLS 1.3. (Versions switched to DTLS spelling, a
// cookie added.)
static const uint8_t kClientHelloDTLS[] = {
// legacy_version
0xfe, 0xfd,
// random
0x37, 0xb0, 0x76, 0xd2, 0xfa, 0x50, 0x94, 0x39, 0x5e, 0x99, 0x71, 0xd7,
0x53, 0xc3, 0xc4, 0xcf, 0x07, 0x56, 0xb9, 0x40, 0x70, 0x13, 0xcb, 0xca,
0xc7, 0xf4, 0x4a, 0xc3, 0x28, 0x13, 0xf6, 0x0f,
// legacy_session_id
0x20, 0x91, 0x41, 0xb7, 0x89, 0x83, 0xd3, 0x67, 0xa0, 0xfe, 0x97, 0x08,
0xdf, 0x32, 0xf5, 0xb9, 0x88, 0x8f, 0xe5, 0x9e, 0xde, 0x4e, 0x61, 0x2c,
0xf6, 0xbd, 0xb1, 0xfb, 0xbe, 0xe6, 0xf9, 0xef, 0xfe,
// legacy_cookie
0x04, 0x01, 0x02, 0x03, 0x04,
// cipher_suites
0x00, 0x06, 0x13, 0x01, 0x13, 0x03, 0x13, 0x02,
// legacy_compression_methods
0x01, 0x00,
// extensions
0x00, 0x8d,
// server_name
0x00, 0x00, 0x00, 0x0b, 0x00, 0x09, 0x00, 0x00, 0x06, 0x73, 0x65, 0x72,
0x76, 0x65, 0x72,
// renegotiation_info
0xff, 0x01, 0x00, 0x01, 0x00,
// supported_groups
0x00, 0x0a, 0x00, 0x14, 0x00, 0x12, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18,
0x00, 0x19, 0x01, 0x00, 0x01, 0x01, 0x01, 0x02, 0x01, 0x03, 0x01, 0x04,
// key_share
0x00, 0x33, 0x00, 0x26, 0x00, 0x24, 0x00, 0x1d, 0x00, 0x20, 0xd5, 0x15,
0x42, 0x62, 0x5f, 0x25, 0xa9, 0x2d, 0x44, 0xa3, 0xaa, 0xde, 0xf5, 0x9c,
0xa8, 0x49, 0xad, 0x2f, 0x8e, 0xfa, 0x9f, 0x04, 0xb8, 0xf5, 0xda, 0xb4,
0x02, 0xac, 0xbc, 0x57, 0x1f, 0x16,
// supported_versions
0x00, 0x2b, 0x00, 0x03, 0x02, 0xfe, 0xfc,
// signature_algorithms
0x00, 0x0d, 0x00, 0x20, 0x00, 0x1e, 0x04, 0x03, 0x05, 0x03, 0x06, 0x03,
0x02, 0x03, 0x08, 0x04, 0x08, 0x05, 0x08, 0x06, 0x04, 0x01, 0x05, 0x01,
0x06, 0x01, 0x02, 0x01, 0x04, 0x02, 0x05, 0x02, 0x06, 0x02, 0x02, 0x02,
// psk_key_exchange_modes
0x00, 0x2d, 0x00, 0x02, 0x01, 0x01,
// record_size_limit
0x00, 0x1c, 0x00, 0x02, 0x40, 0x01};

auto in = dtls ? Span(kClientHelloDTLS) : Span(kClientHelloTLS);
SSL_CLIENT_HELLO client_hello;
ASSERT_TRUE(
SSL_parse_client_hello(ssl.get(), &client_hello, in.data(), in.size()));
EXPECT_EQ(client_hello.ssl, ssl.get());
EXPECT_EQ(Bytes(client_hello.client_hello, client_hello.client_hello_len),
Bytes(in));
EXPECT_EQ(client_hello.version, dtls ? DTLS1_2_VERSION : TLS1_2_VERSION);
static const uint8_t kRandom[] = {
0x37, 0xb0, 0x76, 0xd2, 0xfa, 0x50, 0x94, 0x39, 0x5e, 0x99, 0x71,
0xd7, 0x53, 0xc3, 0xc4, 0xcf, 0x07, 0x56, 0xb9, 0x40, 0x70, 0x13,
0xcb, 0xca, 0xc7, 0xf4, 0x4a, 0xc3, 0x28, 0x13, 0xf6, 0x0f};
EXPECT_EQ(Bytes(client_hello.random, client_hello.random_len),
Bytes(kRandom));
static const uint8_t kSessionID[] = {
0x91, 0x41, 0xb7, 0x89, 0x83, 0xd3, 0x67, 0xa0, 0xfe, 0x97, 0x08,
0xdf, 0x32, 0xf5, 0xb9, 0x88, 0x8f, 0xe5, 0x9e, 0xde, 0x4e, 0x61,
0x2c, 0xf6, 0xbd, 0xb1, 0xfb, 0xbe, 0xe6, 0xf9, 0xef, 0xfe};
EXPECT_EQ(Bytes(client_hello.session_id, client_hello.session_id_len),
Bytes(kSessionID));
if (dtls) {
static const uint8_t kCookie[] = {0x01, 0x02, 0x03, 0x04};
EXPECT_EQ(Bytes(client_hello.dtls_cookie, client_hello.dtls_cookie_len),
Bytes(kCookie));
} else {
EXPECT_EQ(client_hello.dtls_cookie, nullptr);
EXPECT_EQ(client_hello.dtls_cookie_len, 0u);
}
static const uint8_t kCipherSuites[] = {0x13, 0x01, 0x13, 0x03, 0x13, 0x02};
EXPECT_EQ(Bytes(client_hello.cipher_suites, client_hello.cipher_suites_len),
Bytes(kCipherSuites));
static const uint8_t kCompressionMethods[] = {0x00};
EXPECT_EQ(Bytes(client_hello.compression_methods,
client_hello.compression_methods_len),
Bytes(kCompressionMethods));
auto extensions = in.last(141);
EXPECT_EQ(Bytes(client_hello.extensions, client_hello.extensions_len),
Bytes(extensions));

static const uint8_t kServerName[] = {0x00, 0x09, 0x00, 0x00, 0x06, 0x73,
0x65, 0x72, 0x76, 0x65, 0x72};
const uint8_t *data;
size_t len;
ASSERT_TRUE(SSL_early_callback_ctx_extension_get(
&client_hello, TLSEXT_TYPE_server_name, &data, &len));
EXPECT_EQ(Bytes(data, len), Bytes(kServerName));
EXPECT_FALSE(SSL_early_callback_ctx_extension_get(
&client_hello, TLSEXT_TYPE_encrypted_client_hello, &data, &len));

// Trailing data should be rejected.
std::vector<uint8_t> trailing_data(in.begin(), in.end());
trailing_data.push_back(0);
EXPECT_FALSE(SSL_parse_client_hello(
ssl.get(), &client_hello, trailing_data.data(), trailing_data.size()));
EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_SSL, SSL_R_DECODE_ERROR));
ERR_clear_error();

// Other invalid inputs.
static const uint8_t kInvalid[] = {'n', 'o', 'p', 'e'};
EXPECT_FALSE(SSL_parse_client_hello(ssl.get(), &client_hello, kInvalid,
sizeof(kInvalid)));
EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_SSL, SSL_R_DECODE_ERROR));
ERR_clear_error();
}
}

class SSLPAKETest : public testing::Test {
public:
static Span<const uint8_t> pake_context() {
Expand Down
3 changes: 2 additions & 1 deletion ssl/tls13_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,8 @@ static enum ssl_hs_wait_t do_read_second_client_hello(SSL_HANDSHAKE *hs) {
return ssl_hs_error;
}
SSL_CLIENT_HELLO client_hello;
if (!ssl_client_hello_init(ssl, &client_hello, msg.body)) {
if (!SSL_parse_client_hello(ssl, &client_hello, CBS_data(&msg.body),
CBS_len(&msg.body))) {
OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return ssl_hs_error;
Expand Down

0 comments on commit 9439870

Please sign in to comment.