Implement DTLS 1.25 Change-Id: Ib3ae75c55da8f4e71af201627810e01cfb348687 Bug: 715 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68027 Reviewed-by: David Benjamin <davidben@google.com> Auto-Submit: Nick Harper <nharper@chromium.org> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 10898a9..2f14754 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -651,6 +651,17 @@ #define DTLS1_VERSION 0xfeff #define DTLS1_2_VERSION 0xfefd +// DTLS1_3_EXPERIMENTAL_VERSION gates experimental, in-progress code for DTLS +// 1.3. +// +// WARNING: Do not use this value. BoringSSL's DTLS 1.3 implementation is still +// under development. The code enabled by this value is neither stable nor +// secure. It does not correspond to any real protocol. It is also incompatible +// with other DTLS implementations, and it is not compatible with future or past +// versions of BoringSSL. +// +// When the DTLS 1.3 implementation is complete, this symbol will be replaced. +#define DTLS1_3_EXPERIMENTAL_VERSION 0xfc25 // SSL_CTX_set_min_proto_version sets the minimum protocol version for |ctx| to // |version|. If |version| is zero, the default minimum version is used. It @@ -3635,13 +3646,13 @@ // holds for any application protocol state remembered for 0-RTT, e.g. HTTP/3 // SETTINGS. -// ssl_encryption_level_t represents a specific QUIC encryption level used to -// transmit handshake messages. +// ssl_encryption_level_t represents an encryption level in TLS 1.3. Values in +// this enum match the first 4 epochs used in DTLS 1.3 (section 6.1). enum ssl_encryption_level_t BORINGSSL_ENUM_INT { ssl_encryption_initial = 0, - ssl_encryption_early_data, - ssl_encryption_handshake, - ssl_encryption_application, + ssl_encryption_early_data = 1, + ssl_encryption_handshake = 2, + ssl_encryption_application = 3, }; // ssl_quic_method_st (aka |SSL_QUIC_METHOD|) describes custom QUIC hooks.
diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc index 1a68d93..a4499a7 100644 --- a/ssl/d1_both.cc +++ b/ssl/d1_both.cc
@@ -624,12 +624,6 @@ assert(ssl->d1->outgoing_written < ssl->d1->outgoing_messages_len); assert(msg == &ssl->d1->outgoing_messages[ssl->d1->outgoing_written]); - if (msg->epoch != ssl->d1->w_epoch && - (ssl->d1->w_epoch == 0 || msg->epoch != ssl->d1->w_epoch - 1)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return seal_error; - } - size_t overhead = dtls_max_seal_overhead(ssl, msg->epoch); size_t prefix = dtls_seal_prefix_len(ssl, msg->epoch);
diff --git a/ssl/d1_lib.cc b/ssl/d1_lib.cc index 52fbfae..f6a4de8 100644 --- a/ssl/d1_lib.cc +++ b/ssl/d1_lib.cc
@@ -95,6 +95,12 @@ return false; } + d1->initial_aead_write_ctx = SSLAEADContext::CreateNullCipher(true); + if (!d1->initial_aead_write_ctx) { + tls_free(ssl); + return false; + } + ssl->d1 = d1.release(); // Set the version to the highest supported version.
diff --git a/ssl/dtls_method.cc b/ssl/dtls_method.cc index e3d7eaf..ac42e0b 100644 --- a/ssl/dtls_method.cc +++ b/ssl/dtls_method.cc
@@ -88,8 +88,20 @@ return false; } - ssl->d1->r_epoch++; - ssl->d1->bitmap = DTLS1_BITMAP(); + if (ssl_protocol_version(ssl) > TLS1_2_VERSION) { + // TODO(crbug.com/boringssl/715): Handle the additional epochs used for key + // update. + // TODO(crbug.com/boringssl/715): If we want to gracefully handle packet + // reordering around KeyUpdate (i.e. accept records from both epochs), we'll + // need a separate bitmap for each epoch. + ssl->d1->r_epoch = level; + // |ssl->d1->bitmap| incorporates epochs into sequence numbers, so it + // doesn't need to be reset. Preserving it allows |SSL_get_read_sequence| to + // query the maximum sequence number received. + } else { + ssl->d1->r_epoch++; + ssl->d1->bitmap = DTLS1_BITMAP(); + } ssl->s3->read_sequence = 0; ssl->s3->aead_read_ctx = std::move(aead_ctx); @@ -106,6 +118,9 @@ ssl->d1->last_write_sequence = ssl->s3->write_sequence; ssl->s3->write_sequence = 0; + if (ssl_protocol_version(ssl) > TLS1_2_VERSION) { + ssl->d1->w_epoch = level; + } ssl->d1->last_aead_write_ctx = std::move(ssl->s3->aead_write_ctx); ssl->s3->aead_write_ctx = std::move(aead_ctx); ssl->s3->write_level = level;
diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc index 0b2a51b..f7aeac2 100644 --- a/ssl/dtls_record.cc +++ b/ssl/dtls_record.cc
@@ -258,14 +258,17 @@ return ssl_open_record_success; } -static const SSLAEADContext *get_write_aead(const SSL *ssl, - uint16_t epoch) { +static SSLAEADContext *get_write_aead(const SSL *ssl, uint16_t epoch) { + if (epoch == 0) { + return ssl->d1->initial_aead_write_ctx.get(); + } + if (epoch < ssl->d1->w_epoch) { - assert(epoch + 1 == ssl->d1->w_epoch); + BSSL_CHECK(epoch + 1 == ssl->d1->w_epoch); return ssl->d1->last_aead_write_ctx.get(); } - assert(epoch == ssl->d1->w_epoch); + BSSL_CHECK(epoch == ssl->d1->w_epoch); return ssl->s3->aead_write_ctx.get(); } @@ -296,15 +299,13 @@ } // Determine the parameters for the current epoch. - SSLAEADContext *aead = ssl->s3->aead_write_ctx.get(); + SSLAEADContext *aead = get_write_aead(ssl, epoch); uint64_t *seq = &ssl->s3->write_sequence; if (epoch < ssl->d1->w_epoch) { - assert(epoch + 1 == ssl->d1->w_epoch); - aead = ssl->d1->last_aead_write_ctx.get(); seq = &ssl->d1->last_write_sequence; - } else { - assert(epoch == ssl->d1->w_epoch); } + // TODO(crbug.com/boringssl/715): If epoch is initial or handshake, the value + // of seq is probably wrong for a retransmission. const size_t record_header_len = dtls_record_header_write_len(ssl, epoch); if (max_out < record_header_len) {
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index f674515..c532345 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -372,9 +372,13 @@ static bool parse_server_version(const SSL_HANDSHAKE *hs, uint16_t *out_version, uint8_t *out_alert, const ParsedServerHello &server_hello) { + uint16_t legacy_version = TLS1_2_VERSION; + if (SSL_is_dtls(hs->ssl)) { + legacy_version = DTLS1_2_VERSION; + } // If the outer version is not TLS 1.2, use it. // TODO(davidben): This function doesn't quite match the RFC8446 formulation. - if (server_hello.legacy_version != TLS1_2_VERSION) { + if (server_hello.legacy_version != legacy_version) { *out_version = server_hello.legacy_version; return true; } @@ -618,10 +622,6 @@ assert(SSL_is_dtls(ssl)); - // When implementing DTLS 1.3, we need to handle the interactions between - // HelloVerifyRequest, DTLS 1.3's HelloVerifyRequest removal, and ECH. - assert(hs->max_version < TLS1_3_VERSION); - SSLMessage msg; if (!ssl->method->get_message(ssl, &msg)) { return ssl_hs_read_message; @@ -632,6 +632,12 @@ return ssl_hs_ok; } + // TODO(crbug.com/boringssl/715): At the point when we read an HVR, we don't + // know whether the connection is DTLS 1.2 (or earlier) or DTLS 1.3 - that's + // determined when we read the supported_versions in the ServerHello. If we + // receive HVR and then the ServerHello selects DTLS 1.3, that is an error and + // we should close the connection. + CBS hello_verify_request = msg.body, cookie; uint16_t server_version; if (!CBS_get_u16(&hello_verify_request, &server_version) || @@ -716,6 +722,15 @@ return ssl_hs_error; } + // TODO(crbug.com/boringssl/715): Check that if the server picked DTLS 1.3, + // that it didn't also previously send an HVR, as that is not allowed by RFC + // 9147. (DTLS 1.25 still uses HVR instead of HRR.) Also add a runner test to + // test that we handle that case properly. + // + // See + // https://boringssl-review.googlesource.com/c/boringssl/+/68027/3/ssl/handshake_client.cc + // for an example of what this check might look like. + assert(ssl->s3->have_version == ssl->s3->initial_handshake_complete); if (!ssl->s3->have_version) { ssl->version = server_version;
diff --git a/ssl/internal.h b/ssl/internal.h index 4a84883..62c16d0 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -3015,6 +3015,11 @@ uint64_t last_write_sequence = 0; UniquePtr<SSLAEADContext> last_aead_write_ctx; + + // In DTLS 1.3, this contains the write AEAD for the initial encryption level. + // TODO(crbug.com/boringssl/715): Drop this when it is no longer needed. + UniquePtr<SSLAEADContext> initial_aead_write_ctx; + // incoming_messages is a ring buffer of incoming handshake messages that have // yet to be processed. The front of the ring buffer is message number // |handshake_read_seq|, at position |handshake_read_seq| %
diff --git a/ssl/ssl_aead_ctx.cc b/ssl/ssl_aead_ctx.cc index 2556922..85617a4 100644 --- a/ssl/ssl_aead_ctx.cc +++ b/ssl/ssl_aead_ctx.cc
@@ -175,7 +175,7 @@ return version_; } - return TLS1_2_VERSION; + return is_dtls_ ? DTLS1_2_VERSION : TLS1_2_VERSION; } size_t SSLAEADContext::ExplicitNonceLen() const {
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 206a016..c86b51b 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -2954,8 +2954,18 @@ uint64_t SSL_get_read_sequence(const SSL *ssl) { if (SSL_is_dtls(ssl)) { - // max_seq_num already includes the epoch. - assert(ssl->d1->r_epoch == (ssl->d1->bitmap.max_seq_num >> 48)); + // TODO(crbug.com/42290608): The API for read sequences in DTLS 1.3 needs to + // reworked. In DTLS 1.3, the read epoch is updated once new keys are + // derived (before we receive a message encrypted with those keys), which + // results in the read epoch being ahead of the highest record received. + // Additionally, when we process a KeyUpdate, we will install new read keys + // for the new epoch, but we may receive messages from the old epoch for + // some time if the ACK gets lost or there is reordering. + + // max_seq_num already includes the epoch. However, the current epoch may + // be one ahead of the highest record received, immediately after a key + // change. + assert(ssl->d1->r_epoch >= ssl->d1->bitmap.max_seq_num >> 48); return ssl->d1->bitmap.max_seq_num; } return ssl->s3->read_sequence;
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index c344277..731116a 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -92,6 +92,7 @@ {TLS1_3_VERSION, VersionParam::is_tls, "TLS1_3"}, {DTLS1_VERSION, VersionParam::is_dtls, "DTLS1"}, {DTLS1_2_VERSION, VersionParam::is_dtls, "DTLS1_2"}, + {DTLS1_3_EXPERIMENTAL_VERSION, VersionParam::is_dtls, "DTLS1_3"}, }; struct ExpectedCipher { @@ -2817,11 +2818,19 @@ uint64_t server_write_seq = SSL_get_write_sequence(server_.get()); if (is_dtls()) { - // Both client and server must be at epoch 1. - EXPECT_EQ(EpochFromSequence(client_read_seq), 1); - EXPECT_EQ(EpochFromSequence(client_write_seq), 1); - EXPECT_EQ(EpochFromSequence(server_read_seq), 1); - EXPECT_EQ(EpochFromSequence(server_write_seq), 1); + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // Client and server write epochs should be at 3 (application data). + EXPECT_EQ(EpochFromSequence(client_write_seq), 3); + EXPECT_EQ(EpochFromSequence(server_write_seq), 3); + // TODO(crbug.com/42290608): The read sequences aren't checked because the + // SSL_get_read_sequence API needs to be reworked for DTLS 1.3. + } else { + // Both client and server must be at epoch 1. + EXPECT_EQ(EpochFromSequence(client_read_seq), 1); + EXPECT_EQ(EpochFromSequence(client_write_seq), 1); + EXPECT_EQ(EpochFromSequence(server_read_seq), 1); + EXPECT_EQ(EpochFromSequence(server_write_seq), 1); + } // The next record to be written should exceed the largest received. EXPECT_GT(client_write_seq, server_read_seq); @@ -2837,6 +2846,14 @@ EXPECT_EQ(SSL_write(client_.get(), &byte, 1), 1); EXPECT_EQ(SSL_read(server_.get(), &byte, 1), 1); + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/42290608): Write an appropriate test for incrementing both + // sequence number and epoch in the following test. The server read seq was + // in epoch 2, but after the write it's in epoch 3, so adding 1 doesn't work + // any more. + return; + } + // The client write and server read sequence numbers should have // incremented. EXPECT_EQ(client_write_seq + 1, SSL_get_write_sequence(client_.get())); @@ -3491,6 +3508,11 @@ } TEST_P(SSLVersionTest, SessionIDContext) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } static const uint8_t kContext1[] = {1}; static const uint8_t kContext2[] = {2}; @@ -3627,6 +3649,11 @@ } TEST_P(SSLVersionTest, SessionTimeout) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } for (bool server_test : {false, true}) { SCOPED_TRACE(server_test); @@ -3763,6 +3790,11 @@ } TEST_P(SSLVersionTest, DefaultTicketKeyRotation) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } static const time_t kStartTime = 1001; g_current_time.tv_sec = kStartTime; @@ -3986,6 +4018,8 @@ return "DTLSv1"; case DTLS1_2_VERSION: return "DTLSv1.2"; + case DTLS1_3_EXPERIMENTAL_VERSION: + return "DTLSv1.3"; default: return "???"; } @@ -4045,7 +4079,8 @@ TEST_P(SSLVersionTest, SSLClearSessionResumption) { // Skip this for TLS 1.3. TLS 1.3's ticket mechanism is incompatible with this // API pattern. - if (version() == TLS1_3_VERSION) { + if (version() == TLS1_3_VERSION || + version() == DTLS1_3_EXPERIMENTAL_VERSION) { return; } @@ -4390,11 +4425,15 @@ uint16_t record_version, length; ASSERT_TRUE(CBS_get_u8(&cbs, &type)); ASSERT_TRUE(CBS_get_u16(&cbs, &record_version)); - EXPECT_EQ(record_version & 0xff00, version() & 0xff00); + EXPECT_EQ(record_version >> 8, is_dtls() ? 0xfe : 0x03); if (is_dtls()) { uint16_t epoch; ASSERT_TRUE(CBS_get_u16(&cbs, &epoch)); - EXPECT_TRUE(epoch == 0 || epoch == 1) << "Invalid epoch: " << epoch; + uint16_t max_epoch = 1; + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + max_epoch = 3; + } + EXPECT_LE(epoch, max_epoch) << "Invalid epoch: " << epoch; ASSERT_TRUE(CBS_skip(&cbs, 6)); } ASSERT_TRUE(CBS_get_u16(&cbs, &length)); @@ -4442,6 +4481,11 @@ bssl::UniquePtr<SSL_SESSION> session = CreateClientSession(client_ctx_.get(), server_ctx_.get(), config); + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } // If the client resumes a session with a different name, |SSL_get_servername| // must return the new name. ASSERT_TRUE(session); @@ -4454,6 +4498,11 @@ // Test that session cache mode bits are honored in the client session callback. TEST_P(SSLVersionTest, ClientSessionCacheMode) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_OFF); EXPECT_FALSE(CreateClientSession(client_ctx_.get(), server_ctx_.get())); @@ -5413,6 +5462,11 @@ } TEST_P(SSLVersionTest, SessionVersion) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH); SSL_CTX_set_session_cache_mode(server_ctx_.get(), SSL_SESS_CACHE_BOTH); @@ -6065,6 +6119,11 @@ // Test that ticket-based sessions on the client get fake session IDs. TEST_P(SSLVersionTest, FakeIDsForTickets) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH); SSL_CTX_set_session_cache_mode(server_ctx_.get(), SSL_SESS_CACHE_BOTH); @@ -6086,7 +6145,8 @@ SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH); SSL_CTX_set_session_cache_mode(server_ctx_.get(), SSL_SESS_CACHE_BOTH); - if (version() == TLS1_3_VERSION) { + if (version() == TLS1_3_VERSION || + version() == DTLS1_3_EXPERIMENTAL_VERSION) { // Our TLS 1.3 implementation does not support stateful resumption. ASSERT_FALSE(CreateClientSession(client_ctx_.get(), server_ctx_.get())); return; @@ -6195,6 +6255,11 @@ } TEST_P(SSLVersionTest, SessionTicketThreads) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } for (bool renew_ticket : {false, true}) { SCOPED_TRACE(renew_ticket); ASSERT_NO_FATAL_FAILURE(ResetContexts()); @@ -6264,7 +6329,8 @@ // performing stateful resumption will share an underlying SSL_SESSION object, // potentially across threads. TEST_P(SSLVersionTest, SessionPropertiesThreads) { - if (version() == TLS1_3_VERSION) { + if (version() == TLS1_3_VERSION || + version() == DTLS1_3_EXPERIMENTAL_VERSION) { // Our TLS 1.3 implementation does not support stateful resumption. ASSERT_FALSE(CreateClientSession(client_ctx_.get(), server_ctx_.get())); return; @@ -7859,6 +7925,11 @@ } TEST_P(SSLVersionTest, SameKeyResume) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } uint8_t key[48]; RAND_bytes(key, sizeof(key)); @@ -7896,6 +7967,11 @@ } TEST_P(SSLVersionTest, DifferentKeyNoResume) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } uint8_t key1[48], key2[48]; RAND_bytes(key1, sizeof(key1)); RAND_bytes(key2, sizeof(key2)); @@ -7934,6 +8010,11 @@ } TEST_P(SSLVersionTest, UnrelatedServerNoResume) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } bssl::UniquePtr<SSL_CTX> server_ctx2 = CreateContext(); ASSERT_TRUE(server_ctx2); ASSERT_TRUE(UseCertAndKey(server_ctx2.get())); @@ -7971,6 +8052,11 @@ } TEST_P(SSLVersionTest, TicketSessionIDsMatch) { + if (version() == DTLS1_3_EXPERIMENTAL_VERSION) { + // TODO(crbug.com/boringssl/715): Enable the rest of this test for DTLS 1.3 + // once it supports NewSessionTickets. + return; + } // This checks that the session IDs at client and server match after a ticket // resumption. It's unclear whether this should be true, but Envoy depends // on it in their tests so this will give an early signal if we break it. @@ -9446,7 +9532,8 @@ ASSERT_TRUE(Connect()); // Check that we logged the secrets we expected to log. - if (version() == TLS1_3_VERSION) { + if (version() == TLS1_3_VERSION || + version() == DTLS1_3_EXPERIMENTAL_VERSION) { EXPECT_THAT(client_log, ElementsAre(Key("CLIENT_HANDSHAKE_TRAFFIC_SECRET"), Key("CLIENT_TRAFFIC_SECRET_0"), Key("EXPORTER_SECRET"),
diff --git a/ssl/ssl_versions.cc b/ssl/ssl_versions.cc index 8aa7e3f..c521a61 100644 --- a/ssl/ssl_versions.cc +++ b/ssl/ssl_versions.cc
@@ -46,6 +46,10 @@ *out = TLS1_2_VERSION; return true; + case DTLS1_3_EXPERIMENTAL_VERSION: + *out = TLS1_3_VERSION; + return true; + default: return false; } @@ -62,6 +66,7 @@ }; static const uint16_t kDTLSVersions[] = { + DTLS1_3_EXPERIMENTAL_VERSION, DTLS1_2_VERSION, DTLS1_VERSION, }; @@ -99,6 +104,7 @@ {TLS1_VERSION, "TLSv1"}, {DTLS1_VERSION, "DTLSv1"}, {DTLS1_2_VERSION, "DTLSv1.2"}, + {DTLS1_3_EXPERIMENTAL_VERSION, "DTLSv1.3"}, }; static const char *ssl_version_to_string(uint16_t version) {
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index 6c9ea75..73e307c 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc
@@ -107,9 +107,13 @@ if (!ssl_parse_server_hello(out, out_alert, msg)) { return false; } + uint16_t server_hello_version = TLS1_2_VERSION; + if (SSL_is_dtls(hs->ssl)) { + server_hello_version = DTLS1_2_VERSION; + } // The RFC8446 version of the structure fixes some legacy values. // Additionally, the session ID must echo the original one. - if (out->legacy_version != TLS1_2_VERSION || + if (out->legacy_version != server_hello_version || out->compression_method != 0 || !CBS_mem_equal(&out->session_id, hs->session_id, hs->session_id_len) || CBS_len(&out->extensions) == 0) {
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 67e1f78..d3147ea 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -792,11 +792,15 @@ } } + uint16_t server_hello_version = TLS1_2_VERSION; + if (SSL_is_dtls(ssl)) { + server_hello_version = DTLS1_2_VERSION; + } Array<uint8_t> server_hello; ScopedCBB cbb; CBB body, extensions, session_id; if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) || - !CBB_add_u16(&body, TLS1_2_VERSION) || + !CBB_add_u16(&body, server_hello_version) || !CBB_add_bytes(&body, ssl->s3->server_random, sizeof(ssl->s3->server_random)) || !CBB_add_u8_length_prefixed(&body, &session_id) ||