From 875149a9724d9217da0e64efe7ce4038c18c931c Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Wed, 22 Nov 2017 20:33:29 +1100 Subject: [PATCH] Bug 1418862 - Make HelloRetryRequest look like ServerHello, r=ekr Update TLS 1.3 implementation for draft-22. This makes the changes from Bug 1411475 the default mode of operation. A new option, SSL_ENABLE_TLS13_COMPAT_MODE, is added to control whether a client attempts to force the server into compatibility mode. When enabled, clients will send a fake session_id in the ClientHello and send a ChangeCipherSpec message before sending any encrypted records. This patch also includes changes to make a HelloRetryRequest look like a ServerHello. This includes the version number change to draft-22. --HG-- branch : NSS_TLS13_DRAFT19_BRANCH extra : rebase_source : 0a2868314e7fed0930be029352a0824ec1eb4b46 extra : amend_source : 8a39378587292bd9acaed5de1da105806f3d0522 extra : histedit_source : b2a5f8f4439eba4c41347fea267af6a320c16e52%2C2b1453083a95b9dcfa95f54e70a197f3d870d885 --- gtests/ssl_gtest/manifest.mn | 2 +- gtests/ssl_gtest/ssl_agent_unittest.cc | 15 +- gtests/ssl_gtest/ssl_alths_unittest.cc | 188 ----------- gtests/ssl_gtest/ssl_auth_unittest.cc | 6 +- gtests/ssl_gtest/ssl_dhe_unittest.cc | 44 +-- gtests/ssl_gtest/ssl_drop_unittest.cc | 14 +- gtests/ssl_gtest/ssl_ecdh_unittest.cc | 20 +- gtests/ssl_gtest/ssl_extension_unittest.cc | 14 +- gtests/ssl_gtest/ssl_gtest.gyp | 2 +- gtests/ssl_gtest/ssl_hrr_unittest.cc | 79 +++-- gtests/ssl_gtest/ssl_loopback_unittest.cc | 8 +- gtests/ssl_gtest/ssl_resumption_unittest.cc | 25 +- gtests/ssl_gtest/ssl_tls13compat_unittest.cc | 337 +++++++++++++++++++ gtests/ssl_gtest/ssl_version_unittest.cc | 14 +- gtests/ssl_gtest/tls_agent.cc | 7 - gtests/ssl_gtest/tls_agent.h | 1 - gtests/ssl_gtest/tls_connect.cc | 12 + gtests/ssl_gtest/tls_connect.h | 2 + gtests/ssl_gtest/tls_filter.cc | 75 +++-- gtests/ssl_gtest/tls_filter.h | 48 +-- lib/ssl/ssl.h | 6 + lib/ssl/ssl3con.c | 328 ++++++++++-------- lib/ssl/ssl3ext.c | 1 + lib/ssl/ssl3exthandle.c | 17 +- lib/ssl/ssl3prot.h | 2 +- lib/ssl/sslexp.h | 12 +- lib/ssl/sslimpl.h | 14 +- lib/ssl/sslsock.c | 17 +- lib/ssl/tls13con.c | 253 ++++++-------- lib/ssl/tls13con.h | 3 +- lib/ssl/tls13exthandle.c | 12 +- lib/ssl/tls13hashstate.c | 2 +- 32 files changed, 854 insertions(+), 726 deletions(-) delete mode 100644 gtests/ssl_gtest/ssl_alths_unittest.cc create mode 100644 gtests/ssl_gtest/ssl_tls13compat_unittest.cc diff --git a/gtests/ssl_gtest/manifest.mn b/gtests/ssl_gtest/manifest.mn index 4407293805..86d6ddd6f1 100644 --- a/gtests/ssl_gtest/manifest.mn +++ b/gtests/ssl_gtest/manifest.mn @@ -15,7 +15,6 @@ CPPSRCS = \ bloomfilter_unittest.cc \ ssl_0rtt_unittest.cc \ ssl_agent_unittest.cc \ - ssl_alths_unittest.cc \ ssl_auth_unittest.cc \ ssl_cert_ext_unittest.cc \ ssl_ciphersuite_unittest.cc \ @@ -40,6 +39,7 @@ CPPSRCS = \ ssl_renegotiation_unittest.cc \ ssl_skip_unittest.cc \ ssl_staticrsa_unittest.cc \ + ssl_tls13compat_unittest.cc \ ssl_v2_client_hello_unittest.cc \ ssl_version_unittest.cc \ ssl_versionpolicy_unittest.cc \ diff --git a/gtests/ssl_gtest/ssl_agent_unittest.cc b/gtests/ssl_gtest/ssl_agent_unittest.cc index 5035a338d2..d703e8e785 100644 --- a/gtests/ssl_gtest/ssl_agent_unittest.cc +++ b/gtests/ssl_gtest/ssl_agent_unittest.cc @@ -44,13 +44,14 @@ const static uint8_t kCannedTls13ClientHello[] = { 0x02, 0x05, 0x02, 0x06, 0x02, 0x02, 0x02}; const static uint8_t kCannedTls13ServerHello[] = { - 0x7f, kD13, 0x9c, 0xbc, 0x14, 0x9b, 0x0e, 0x2e, 0xfa, 0x0d, 0xf3, 0xf0, - 0x5c, 0x70, 0x7a, 0xe0, 0xd1, 0x9b, 0x3e, 0x5a, 0x44, 0x6b, 0xdf, 0xe5, - 0xc2, 0x28, 0x64, 0xf7, 0x00, 0xc1, 0x9c, 0x08, 0x76, 0x08, 0x13, 0x01, - 0x00, 0x28, 0x00, 0x28, 0x00, 0x24, 0x00, 0x1d, 0x00, 0x20, 0xc2, 0xcf, - 0x23, 0x17, 0x64, 0x23, 0x03, 0xf0, 0xfb, 0x45, 0x98, 0x26, 0xd1, 0x65, - 0x24, 0xa1, 0x6c, 0xa9, 0x80, 0x8f, 0x2c, 0xac, 0x0a, 0xea, 0x53, 0x3a, - 0xcb, 0xe3, 0x08, 0x84, 0xae, 0x19}; + 0x03, 0x03, 0x9c, 0xbc, 0x14, 0x9b, 0x0e, 0x2e, 0xfa, 0x0d, 0xf3, + 0xf0, 0x5c, 0x70, 0x7a, 0xe0, 0xd1, 0x9b, 0x3e, 0x5a, 0x44, 0x6b, + 0xdf, 0xe5, 0xc2, 0x28, 0x64, 0xf7, 0x00, 0xc1, 0x9c, 0x08, 0x76, + 0x08, 0x00, 0x13, 0x01, 0x00, 0x00, 0x2e, 0x00, 0x28, 0x00, 0x24, + 0x00, 0x1d, 0x00, 0x20, 0xc2, 0xcf, 0x23, 0x17, 0x64, 0x23, 0x03, + 0xf0, 0xfb, 0x45, 0x98, 0x26, 0xd1, 0x65, 0x24, 0xa1, 0x6c, 0xa9, + 0x80, 0x8f, 0x2c, 0xac, 0x0a, 0xea, 0x53, 0x3a, 0xcb, 0xe3, 0x08, + 0x84, 0xae, 0x19, 0x00, 0x2b, 0x00, 0x02, 0x7f, kD13}; static const char *k0RttData = "ABCDEF"; TEST_P(TlsAgentTest, EarlyFinished) { diff --git a/gtests/ssl_gtest/ssl_alths_unittest.cc b/gtests/ssl_gtest/ssl_alths_unittest.cc deleted file mode 100644 index 2d6ae1d2dd..0000000000 --- a/gtests/ssl_gtest/ssl_alths_unittest.cc +++ /dev/null @@ -1,188 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include -#include -#include "ssl.h" -#include "sslerr.h" -#include "sslproto.h" - -#include "gtest_utils.h" -#include "tls_connect.h" -#include "tls_filter.h" -#include "tls_parser.h" - -namespace nss_test { - -static const uint32_t kServerHelloVersionAlt = SSL_LIBRARY_VERSION_TLS_1_2; -static const uint16_t kServerHelloVersionRegular = - 0x7f00 | TLS_1_3_DRAFT_VERSION; - -class AltHandshakeTest : public TlsConnectStreamTls13 { - protected: - void SetUp() { - TlsConnectStreamTls13::SetUp(); - client_ccs_recorder_ = - std::make_shared(kTlsChangeCipherSpecType); - server_handshake_recorder_ = - std::make_shared(kTlsHandshakeType); - server_ccs_recorder_ = - std::make_shared(kTlsChangeCipherSpecType); - server_hello_recorder_ = - std::make_shared( - kTlsHandshakeServerHello); - } - - void SetAltHandshakeTypeEnabled() { - client_->SetAltHandshakeTypeEnabled(); - server_->SetAltHandshakeTypeEnabled(); - } - - void InstallFilters() { - client_->SetPacketFilter(client_ccs_recorder_); - auto chain = std::make_shared(ChainedPacketFilterInit( - {server_handshake_recorder_, server_ccs_recorder_, - server_hello_recorder_})); - server_->SetPacketFilter(chain); - } - - void CheckServerHelloRecordVersion(uint16_t record_version) { - ASSERT_EQ(record_version, - server_handshake_recorder_->record(0).header.version()); - } - - void CheckServerHelloVersion(uint16_t server_hello_version) { - uint32_t ver; - ASSERT_TRUE(server_hello_recorder_->buffer().Read(0, 2, &ver)); - ASSERT_EQ(server_hello_version, ver); - } - - void CheckForRegularHandshake() { - EXPECT_EQ(0U, client_ccs_recorder_->count()); - EXPECT_EQ(0U, server_ccs_recorder_->count()); - CheckServerHelloVersion(kServerHelloVersionRegular); - CheckServerHelloRecordVersion(SSL_LIBRARY_VERSION_TLS_1_0); - } - - void CheckForAltHandshake() { - EXPECT_EQ(1U, client_ccs_recorder_->count()); - EXPECT_EQ(1U, server_ccs_recorder_->count()); - CheckServerHelloVersion(kServerHelloVersionAlt); - CheckServerHelloRecordVersion(SSL_LIBRARY_VERSION_TLS_1_2); - } - - std::shared_ptr client_ccs_recorder_; - std::shared_ptr server_handshake_recorder_; - std::shared_ptr server_ccs_recorder_; - std::shared_ptr server_hello_recorder_; -}; - -TEST_F(AltHandshakeTest, ClientOnly) { - client_->SetAltHandshakeTypeEnabled(); - InstallFilters(); - Connect(); - CheckForRegularHandshake(); -} - -TEST_F(AltHandshakeTest, ServerOnly) { - server_->SetAltHandshakeTypeEnabled(); - InstallFilters(); - Connect(); - CheckForRegularHandshake(); -} - -TEST_F(AltHandshakeTest, Enabled) { - SetAltHandshakeTypeEnabled(); - InstallFilters(); - Connect(); - CheckForAltHandshake(); -} - -TEST_F(AltHandshakeTest, ZeroRtt) { - SetAltHandshakeTypeEnabled(); - SetupForZeroRtt(); - SetAltHandshakeTypeEnabled(); - client_->Set0RttEnabled(true); - server_->Set0RttEnabled(true); - - InstallFilters(); - - ExpectResumption(RESUME_TICKET); - ZeroRttSendReceive(true, true); - Handshake(); - ExpectEarlyDataAccepted(true); - CheckConnected(); - - CheckForAltHandshake(); -} - -// Neither client nor server has the extension prior to resumption, so the -// client doesn't send a CCS before its 0-RTT data. -TEST_F(AltHandshakeTest, DisabledBeforeZeroRtt) { - SetupForZeroRtt(); - SetAltHandshakeTypeEnabled(); - client_->Set0RttEnabled(true); - server_->Set0RttEnabled(true); - - InstallFilters(); - - ExpectResumption(RESUME_TICKET); - ZeroRttSendReceive(true, true); - Handshake(); - ExpectEarlyDataAccepted(true); - CheckConnected(); - - EXPECT_EQ(0U, client_ccs_recorder_->count()); - EXPECT_EQ(1U, server_ccs_recorder_->count()); - CheckServerHelloVersion(kServerHelloVersionAlt); -} - -// Both use the alternative in the initial handshake but only the server enables -// it on resumption. -TEST_F(AltHandshakeTest, ClientDisabledAfterZeroRtt) { - SetAltHandshakeTypeEnabled(); - SetupForZeroRtt(); - server_->SetAltHandshakeTypeEnabled(); - client_->Set0RttEnabled(true); - server_->Set0RttEnabled(true); - - InstallFilters(); - - ExpectResumption(RESUME_TICKET); - ZeroRttSendReceive(true, true); - Handshake(); - ExpectEarlyDataAccepted(true); - CheckConnected(); - - CheckForRegularHandshake(); -} - -// If the alternative handshake isn't negotiated after 0-RTT, and the client has -// it enabled, it will send a ChangeCipherSpec. The server chokes on it if it -// hasn't negotiated the alternative handshake. -TEST_F(AltHandshakeTest, ServerDisabledAfterZeroRtt) { - SetAltHandshakeTypeEnabled(); - SetupForZeroRtt(); - client_->SetAltHandshakeTypeEnabled(); - client_->Set0RttEnabled(true); - server_->Set0RttEnabled(true); - - client_->Handshake(); // Send ClientHello (and CCS) - - server_->Handshake(); // Consume the ClientHello, which is OK. - client_->ExpectResumption(); - client_->Handshake(); // Read the server handshake. - EXPECT_EQ(TlsAgent::STATE_CONNECTED, client_->state()); - - // Now the server reads the CCS instead of more handshake messages. - ExpectAlert(server_, kTlsAlertBadRecordMac); - server_->Handshake(); - EXPECT_EQ(TlsAgent::STATE_ERROR, server_->state()); - client_->Handshake(); // Consume the alert. - EXPECT_EQ(TlsAgent::STATE_ERROR, client_->state()); -} - -} // nss_test diff --git a/gtests/ssl_gtest/ssl_auth_unittest.cc b/gtests/ssl_gtest/ssl_auth_unittest.cc index 34892f9e7b..dbcdd92eae 100644 --- a/gtests/ssl_gtest/ssl_auth_unittest.cc +++ b/gtests/ssl_gtest/ssl_auth_unittest.cc @@ -159,13 +159,11 @@ TEST_P(TlsConnectTls12, ClientAuthBigRsaCheckSigAlg) { class TlsZeroCertificateRequestSigAlgsFilter : public TlsHandshakeFilter { public: + TlsZeroCertificateRequestSigAlgsFilter() + : TlsHandshakeFilter({kTlsHandshakeCertificateRequest}) {} virtual PacketFilter::Action FilterHandshake( const TlsHandshakeFilter::HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() != kTlsHandshakeCertificateRequest) { - return KEEP; - } - TlsParser parser(input); std::cerr << "Zeroing CertReq.supported_signature_algorithms" << std::endl; diff --git a/gtests/ssl_gtest/ssl_dhe_unittest.cc b/gtests/ssl_gtest/ssl_dhe_unittest.cc index dc824647ed..4aa3bb6392 100644 --- a/gtests/ssl_gtest/ssl_dhe_unittest.cc +++ b/gtests/ssl_gtest/ssl_dhe_unittest.cc @@ -103,14 +103,11 @@ TEST_P(TlsConnectGenericPre13, ConnectFfdheServer) { class TlsDheServerKeyExchangeDamager : public TlsHandshakeFilter { public: - TlsDheServerKeyExchangeDamager() {} + TlsDheServerKeyExchangeDamager() + : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}) {} virtual PacketFilter::Action FilterHandshake( const TlsHandshakeFilter::HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() != kTlsHandshakeServerKeyExchange) { - return KEEP; - } - // Damage the first octet of dh_p. Anything other than the known prime will // be rejected as "weak" when we have SSL_REQUIRE_DH_NAMED_GROUPS enabled. *output = input; @@ -144,7 +141,8 @@ class TlsDheSkeChangeY : public TlsHandshakeFilter { kYZeroPad }; - TlsDheSkeChangeY(ChangeYTo change) : change_Y_(change) {} + TlsDheSkeChangeY(uint8_t handshake_type, ChangeYTo change) + : TlsHandshakeFilter({handshake_type}), change_Y_(change) {} protected: void ChangeY(const DataBuffer& input, DataBuffer* output, size_t offset, @@ -210,7 +208,9 @@ class TlsDheSkeChangeY : public TlsHandshakeFilter { class TlsDheSkeChangeYServer : public TlsDheSkeChangeY { public: TlsDheSkeChangeYServer(ChangeYTo change, bool modify) - : TlsDheSkeChangeY(change), modify_(modify), p_() {} + : TlsDheSkeChangeY(kTlsHandshakeServerKeyExchange, change), + modify_(modify), + p_() {} const DataBuffer& prime() const { return p_; } @@ -218,10 +218,6 @@ class TlsDheSkeChangeYServer : public TlsDheSkeChangeY { virtual PacketFilter::Action FilterHandshake( const TlsHandshakeFilter::HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) override { - if (header.handshake_type() != kTlsHandshakeServerKeyExchange) { - return KEEP; - } - size_t offset = 2; // Read dh_p uint32_t dh_len = 0; @@ -251,16 +247,13 @@ class TlsDheSkeChangeYClient : public TlsDheSkeChangeY { TlsDheSkeChangeYClient( ChangeYTo change, std::shared_ptr server_filter) - : TlsDheSkeChangeY(change), server_filter_(server_filter) {} + : TlsDheSkeChangeY(kTlsHandshakeClientKeyExchange, change), + server_filter_(server_filter) {} protected: virtual PacketFilter::Action FilterHandshake( const TlsHandshakeFilter::HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) override { - if (header.handshake_type() != kTlsHandshakeClientKeyExchange) { - return KEEP; - } - ChangeY(input, output, 0, server_filter_->prime()); return CHANGE; } @@ -365,13 +358,10 @@ INSTANTIATE_TEST_CASE_P( class TlsDheSkeMakePEven : public TlsHandshakeFilter { public: + TlsDheSkeMakePEven() : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}) {} virtual PacketFilter::Action FilterHandshake( const TlsHandshakeFilter::HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() != kTlsHandshakeServerKeyExchange) { - return KEEP; - } - // Find the end of dh_p uint32_t dh_len = 0; EXPECT_TRUE(input.Read(0, 2, &dh_len)); @@ -399,13 +389,10 @@ TEST_P(TlsConnectGenericPre13, MakeDhePEven) { class TlsDheSkeZeroPadP : public TlsHandshakeFilter { public: + TlsDheSkeZeroPadP() : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}) {} virtual PacketFilter::Action FilterHandshake( const TlsHandshakeFilter::HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() != kTlsHandshakeServerKeyExchange) { - return KEEP; - } - *output = input; uint32_t dh_len = 0; EXPECT_TRUE(input.Read(0, 2, &dh_len)); @@ -559,16 +546,15 @@ TEST_P(TlsConnectTls13, ResumeFfdhe) { class TlsDheSkeChangeSignature : public TlsHandshakeFilter { public: TlsDheSkeChangeSignature(uint16_t version, const uint8_t* data, size_t len) - : version_(version), data_(data), len_(len) {} + : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}), + version_(version), + data_(data), + len_(len) {} protected: virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() != kTlsHandshakeServerKeyExchange) { - return KEEP; - } - TlsParser parser(input); EXPECT_TRUE(parser.SkipVariable(2)); // dh_p EXPECT_TRUE(parser.SkipVariable(2)); // dh_g diff --git a/gtests/ssl_gtest/ssl_drop_unittest.cc b/gtests/ssl_gtest/ssl_drop_unittest.cc index bc8703eeef..da4f6626c8 100644 --- a/gtests/ssl_gtest/ssl_drop_unittest.cc +++ b/gtests/ssl_gtest/ssl_drop_unittest.cc @@ -105,7 +105,7 @@ class TlsDropDatagram13 : public TlsConnectDatagram13 { server_->ssl_fd(), 1, [](PRFileDesc* fd, SSLHandshakeType message, PRUint8* data, unsigned int* len, unsigned int maxLen, void* arg) -> PRBool { - SSLInt_SetMTU(fd, 400); // Splits the certificate. + SSLInt_SetMTU(fd, 500); // Splits the certificate. return PR_FALSE; }, nullptr, @@ -654,9 +654,9 @@ TEST_F(TlsDropDatagram13, SendOutOfOrderAppWithHandshakeKey) { auto spec = capturer.spec(0); ASSERT_NE(nullptr, spec.get()); ASSERT_EQ(2, spec->epoch()); - ASSERT_TRUE(client_->SendEncryptedRecord(spec, 0xfeff, 0x0002000000000002, - kTlsApplicationDataType, - DataBuffer(buf, sizeof(buf)))); + ASSERT_TRUE(client_->SendEncryptedRecord( + spec, SSL_LIBRARY_VERSION_DTLS_1_2_WIRE, 0x0002000000000002, + kTlsApplicationDataType, DataBuffer(buf, sizeof(buf)))); // Now have the server consume the bogus message. server_->ExpectSendAlert(illegal_parameter, kTlsAlertFatal); @@ -680,9 +680,9 @@ TEST_F(TlsDropDatagram13, SendOutOfOrderHsNonsenseWithHandshakeKey) { auto spec = capturer.spec(0); ASSERT_NE(nullptr, spec.get()); ASSERT_EQ(2, spec->epoch()); - ASSERT_TRUE(client_->SendEncryptedRecord(spec, 0xfeff, 0x0002000000000002, - kTlsHandshakeType, - DataBuffer(buf, sizeof(buf)))); + ASSERT_TRUE(client_->SendEncryptedRecord( + spec, SSL_LIBRARY_VERSION_DTLS_1_2_WIRE, 0x0002000000000002, + kTlsHandshakeType, DataBuffer(buf, sizeof(buf)))); server_->Handshake(); EXPECT_EQ(2UL, server_filters_.ack_->count()); CheckAcks(server_filters_, 0, {0x0002000000000000ULL}); diff --git a/gtests/ssl_gtest/ssl_ecdh_unittest.cc b/gtests/ssl_gtest/ssl_ecdh_unittest.cc index f14a498177..e0f8b1f55f 100644 --- a/gtests/ssl_gtest/ssl_ecdh_unittest.cc +++ b/gtests/ssl_gtest/ssl_ecdh_unittest.cc @@ -193,7 +193,9 @@ TEST_P(TlsConnectGenericPre13, P384PriorityFromModelSocket) { class TlsKeyExchangeGroupCapture : public TlsHandshakeFilter { public: - TlsKeyExchangeGroupCapture() : group_(ssl_grp_none) {} + TlsKeyExchangeGroupCapture() + : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}), + group_(ssl_grp_none) {} SSLNamedGroup group() const { return group_; } @@ -201,10 +203,6 @@ class TlsKeyExchangeGroupCapture : public TlsHandshakeFilter { virtual PacketFilter::Action FilterHandshake(const HandshakeHeader &header, const DataBuffer &input, DataBuffer *output) { - if (header.handshake_type() != kTlsHandshakeServerKeyExchange) { - return KEEP; - } - uint32_t value = 0; EXPECT_TRUE(input.Read(0, 1, &value)); EXPECT_EQ(3U, value) << "curve type has to be 3"; @@ -518,16 +516,12 @@ TEST_P(TlsKeyExchangeTest13, MultipleClientShares) { // Replace the point in the client key exchange message with an empty one class ECCClientKEXFilter : public TlsHandshakeFilter { public: - ECCClientKEXFilter() {} + ECCClientKEXFilter() : TlsHandshakeFilter({kTlsHandshakeClientKeyExchange}) {} protected: virtual PacketFilter::Action FilterHandshake(const HandshakeHeader &header, const DataBuffer &input, DataBuffer *output) { - if (header.handshake_type() != kTlsHandshakeClientKeyExchange) { - return KEEP; - } - // Replace the client key exchange message with an empty point output->Allocate(1); output->Write(0, 0U, 1); // set point length 0 @@ -538,16 +532,12 @@ class ECCClientKEXFilter : public TlsHandshakeFilter { // Replace the point in the server key exchange message with an empty one class ECCServerKEXFilter : public TlsHandshakeFilter { public: - ECCServerKEXFilter() {} + ECCServerKEXFilter() : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}) {} protected: virtual PacketFilter::Action FilterHandshake(const HandshakeHeader &header, const DataBuffer &input, DataBuffer *output) { - if (header.handshake_type() != kTlsHandshakeServerKeyExchange) { - return KEEP; - } - // Replace the server key exchange message with an empty point output->Allocate(4); output->Write(0, 3U, 1); // named curve diff --git a/gtests/ssl_gtest/ssl_extension_unittest.cc b/gtests/ssl_gtest/ssl_extension_unittest.cc index decb1f7556..4142ab07af 100644 --- a/gtests/ssl_gtest/ssl_extension_unittest.cc +++ b/gtests/ssl_gtest/ssl_extension_unittest.cc @@ -64,15 +64,11 @@ class TlsExtensionDamager : public TlsExtensionFilter { class TlsExtensionAppender : public TlsHandshakeFilter { public: TlsExtensionAppender(uint8_t handshake_type, uint16_t ext, DataBuffer& data) - : handshake_type_(handshake_type), extension_(ext), data_(data) {} + : TlsHandshakeFilter({handshake_type}), extension_(ext), data_(data) {} virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() != handshake_type_) { - return KEEP; - } - TlsParser parser(input); if (!TlsExtensionFilter::FindExtensions(&parser, header)) { return KEEP; @@ -117,7 +113,6 @@ class TlsExtensionAppender : public TlsHandshakeFilter { return true; } - const uint8_t handshake_type_; const uint16_t extension_; const DataBuffer data_; }; @@ -1054,13 +1049,6 @@ TEST_P(TlsBogusExtensionTest13, AddVersionExtensionCertificateRequest) { Run(kTlsHandshakeCertificateRequest, ssl_tls13_supported_versions_xtn); } -TEST_P(TlsBogusExtensionTest13, AddVersionExtensionHelloRetryRequest) { - static const std::vector groups = {ssl_grp_ec_secp384r1}; - server_->ConfigNamedGroups(groups); - - Run(kTlsHandshakeHelloRetryRequest, ssl_tls13_supported_versions_xtn); -} - // NewSessionTicket allows unknown extensions AND it isn't protected by the // Finished. So adding an unknown extension doesn't cause an error. TEST_P(TlsBogusExtensionTest13, AddBogusExtensionNewSessionTicket) { diff --git a/gtests/ssl_gtest/ssl_gtest.gyp b/gtests/ssl_gtest/ssl_gtest.gyp index 371df06991..03cad2e894 100644 --- a/gtests/ssl_gtest/ssl_gtest.gyp +++ b/gtests/ssl_gtest/ssl_gtest.gyp @@ -16,7 +16,6 @@ 'selfencrypt_unittest.cc', 'ssl_0rtt_unittest.cc', 'ssl_agent_unittest.cc', - 'ssl_alths_unittest.cc', 'ssl_auth_unittest.cc', 'ssl_cert_ext_unittest.cc', 'ssl_ciphersuite_unittest.cc', @@ -41,6 +40,7 @@ 'ssl_renegotiation_unittest.cc', 'ssl_skip_unittest.cc', 'ssl_staticrsa_unittest.cc', + 'ssl_tls13compat_unittest.cc', 'ssl_v2_client_hello_unittest.cc', 'ssl_version_unittest.cc', 'ssl_versionpolicy_unittest.cc', diff --git a/gtests/ssl_gtest/ssl_hrr_unittest.cc b/gtests/ssl_gtest/ssl_hrr_unittest.cc index 2251584c1f..93e19a7203 100644 --- a/gtests/ssl_gtest/ssl_hrr_unittest.cc +++ b/gtests/ssl_gtest/ssl_hrr_unittest.cc @@ -559,17 +559,6 @@ TEST_F(TlsConnectStreamTls13, RetryCallbackWithSessionTicketToken) { EXPECT_TRUE(cb_run); } -std::shared_ptr MakeNewServer(std::shared_ptr& client) { - auto server = std::make_shared(TlsAgent::kServerRsa, - TlsAgent::SERVER, client->variant()); - server->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1, - SSL_LIBRARY_VERSION_TLS_1_3); - client->SetPeer(server); - server->SetPeer(client); - server->StartConnect(); - return server; -} - void TriggerHelloRetryRequest(std::shared_ptr& client, std::shared_ptr& server) { size_t cb_called = 0; @@ -589,7 +578,7 @@ TEST_P(TlsConnectTls13, RetryStateless) { EnsureTlsSetup(); TriggerHelloRetryRequest(client_, server_); - server_ = MakeNewServer(client_); + MakeNewServer(); Handshake(); SendReceive(); @@ -618,7 +607,7 @@ TEST_F(TlsConnectStreamTls13, RetryStatelessDamageFirstClientHello) { client_->SetPacketFilter(damage_ch); TriggerHelloRetryRequest(client_, server_); - server_ = MakeNewServer(client_); + MakeNewServer(); // Key exchange fails when the handshake continues because client and server // disagree about the transcript. @@ -634,7 +623,7 @@ TEST_F(TlsConnectStreamTls13, RetryStatelessDamageSecondClientHello) { EnsureTlsSetup(); TriggerHelloRetryRequest(client_, server_); - server_ = MakeNewServer(client_); + MakeNewServer(); auto damage_ch = std::make_shared(0xfff3, DataBuffer()); client_->SetPacketFilter(damage_ch); @@ -648,6 +637,22 @@ TEST_F(TlsConnectStreamTls13, RetryStatelessDamageSecondClientHello) { client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); } +// Read the cipher suite from the HRR and disable it on the identified agent. +static void DisableSuiteFromHrr( + std::shared_ptr& agent, + std::shared_ptr& capture_hrr) { + uint32_t tmp; + size_t offset = 2 + 32; // skip version + server_random + ASSERT_TRUE( + capture_hrr->buffer().Read(offset, 1, &tmp)); // session_id length + EXPECT_EQ(0U, tmp); + offset += 1 + tmp; + ASSERT_TRUE(capture_hrr->buffer().Read(offset, 2, &tmp)); // suite + EXPECT_EQ( + SECSuccess, + SSL_CipherPrefSet(agent->ssl_fd(), static_cast(tmp), PR_FALSE)); +} + TEST_P(TlsConnectTls13, RetryStatelessDisableSuiteClient) { ConfigureSelfEncrypt(); EnsureTlsSetup(); @@ -657,20 +662,15 @@ TEST_P(TlsConnectTls13, RetryStatelessDisableSuiteClient) { server_->SetPacketFilter(capture_hrr); TriggerHelloRetryRequest(client_, server_); - server_ = MakeNewServer(client_); + MakeNewServer(); - // Read the cipher suite from the HRR and disable it on the client. - uint32_t suite; - ASSERT_TRUE(capture_hrr->buffer().Read(2, 2, &suite)); - EXPECT_EQ(SECSuccess, - SSL_CipherPrefSet(client_->ssl_fd(), static_cast(suite), - PR_FALSE)); + DisableSuiteFromHrr(client_, capture_hrr); // The client thinks that the HelloRetryRequest is bad, even though its // because it changed its mind about the cipher suite. ExpectAlert(client_, kTlsAlertIllegalParameter); Handshake(); - client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_HELLO_RETRY_REQUEST); + client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP); server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT); } @@ -683,14 +683,9 @@ TEST_P(TlsConnectTls13, RetryStatelessDisableSuiteServer) { server_->SetPacketFilter(capture_hrr); TriggerHelloRetryRequest(client_, server_); - server_ = MakeNewServer(client_); + MakeNewServer(); - // Read the cipher suite from the HRR and disable it on the server. - uint32_t suite; - ASSERT_TRUE(capture_hrr->buffer().Read(2, 2, &suite)); - EXPECT_EQ(SECSuccess, - SSL_CipherPrefSet(server_->ssl_fd(), static_cast(suite), - PR_FALSE)); + DisableSuiteFromHrr(server_, capture_hrr); ExpectAlert(server_, kTlsAlertIllegalParameter); Handshake(); @@ -703,7 +698,7 @@ TEST_P(TlsConnectTls13, RetryStatelessDisableGroupClient) { EnsureTlsSetup(); TriggerHelloRetryRequest(client_, server_); - server_ = MakeNewServer(client_); + MakeNewServer(); static const std::vector groups = {ssl_grp_ec_secp384r1}; client_->ConfigNamedGroups(groups); @@ -723,7 +718,7 @@ TEST_P(TlsConnectTls13, RetryStatelessDisableGroupServer) { EnsureTlsSetup(); TriggerHelloRetryRequest(client_, server_); - server_ = MakeNewServer(client_); + MakeNewServer(); static const std::vector groups = {ssl_grp_ec_secp384r1}; server_->ConfigNamedGroups(groups); @@ -751,7 +746,7 @@ TEST_P(TlsConnectTls13, RetryStatelessBadCookie) { ASSERT_NE(nullptr, hmac_key); SSLInt_SetSelfEncryptMacKey(hmac_key); // Passes ownership. - server_ = MakeNewServer(client_); + MakeNewServer(); ExpectAlert(server_, kTlsAlertIllegalParameter); Handshake(); @@ -912,16 +907,30 @@ class HelloRetryRequestAgentTest : public TlsAgentTestClient { void MakeCannedHrr(const uint8_t* body, size_t len, DataBuffer* hrr_record, uint32_t seq_num = 0) const { DataBuffer hrr_data; + const uint8_t ssl_hello_retry_random[] = { + 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, 0x1D, 0x8C, + 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, + 0x8C, 0x5E, 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33, 0x9C}; + hrr_data.Allocate(len + 6); size_t i = 0; - i = hrr_data.Write(i, 0x7f00 | TLS_1_3_DRAFT_VERSION, 2); + i = hrr_data.Write(i, 0x0303, 2); + i = hrr_data.Write(i, ssl_hello_retry_random, + sizeof(ssl_hello_retry_random)); + i = hrr_data.Write(i, static_cast(0), 1); // session_id i = hrr_data.Write(i, TLS_AES_128_GCM_SHA256, 2); - i = hrr_data.Write(i, static_cast(len), 2); + i = hrr_data.Write(i, ssl_compression_null, 1); + // Add extensions. First a length, which includes the supported version. + i = hrr_data.Write(i, static_cast(len) + 6, 2); + // Now the supported version. + i = hrr_data.Write(i, ssl_tls13_supported_versions_xtn, 2); + i = hrr_data.Write(i, 2, 2); + i = hrr_data.Write(i, 0x7f00 | TLS_1_3_DRAFT_VERSION, 2); if (len) { hrr_data.Write(i, body, len); } DataBuffer hrr; - MakeHandshakeMessage(kTlsHandshakeHelloRetryRequest, hrr_data.data(), + MakeHandshakeMessage(kTlsHandshakeServerHello, hrr_data.data(), hrr_data.len(), &hrr, seq_num); MakeRecord(kTlsHandshakeType, SSL_LIBRARY_VERSION_TLS_1_3, hrr.data(), hrr.len(), hrr_record, seq_num); diff --git a/gtests/ssl_gtest/ssl_loopback_unittest.cc b/gtests/ssl_gtest/ssl_loopback_unittest.cc index 4cdfffba76..5e5c556047 100644 --- a/gtests/ssl_gtest/ssl_loopback_unittest.cc +++ b/gtests/ssl_gtest/ssl_loopback_unittest.cc @@ -85,13 +85,13 @@ class TlsAlertRecorder : public TlsRecordFilter { }; class HelloTruncator : public TlsHandshakeFilter { + public: + HelloTruncator() + : TlsHandshakeFilter( + {kTlsHandshakeClientHello, kTlsHandshakeServerHello}) {} PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) override { - if (header.handshake_type() != kTlsHandshakeClientHello && - header.handshake_type() != kTlsHandshakeServerHello) { - return KEEP; - } output->Assign(input.data(), input.len() - 1); return CHANGE; } diff --git a/gtests/ssl_gtest/ssl_resumption_unittest.cc b/gtests/ssl_gtest/ssl_resumption_unittest.cc index 7deaf7124b..a413caf2c2 100644 --- a/gtests/ssl_gtest/ssl_resumption_unittest.cc +++ b/gtests/ssl_gtest/ssl_resumption_unittest.cc @@ -490,16 +490,13 @@ TEST_P(TlsConnectStream, TestResumptionOverrideCipher) { class SelectedVersionReplacer : public TlsHandshakeFilter { public: - SelectedVersionReplacer(uint16_t version) : version_(version) {} + SelectedVersionReplacer(uint16_t version) + : TlsHandshakeFilter({kTlsHandshakeServerHello}), version_(version) {} protected: PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) override { - if (header.handshake_type() != kTlsHandshakeServerHello) { - return KEEP; - } - *output = input; output->Write(0, static_cast(version_), 2); return CHANGE; @@ -825,12 +822,26 @@ TEST_F(TlsConnectTest, TestTls13ResumptionForcedDowngrade) { TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256)); filters.push_back( std::make_shared(SSL_LIBRARY_VERSION_TLS_1_2)); + + // Drop a bunch of extensions so that we get past the SH processing. The + // version extension says TLS 1.3, which is counter to our goal, the others + // are not permitted in TLS 1.2 handshakes. + filters.push_back( + std::make_shared(ssl_tls13_supported_versions_xtn)); + filters.push_back( + std::make_shared(ssl_tls13_key_share_xtn)); + filters.push_back( + std::make_shared(ssl_tls13_pre_shared_key_xtn)); server_->SetPacketFilter(std::make_shared(filters)); - client_->ExpectSendAlert(kTlsAlertDecodeError); + // The client here generates an unexpected_message alert when it receives an + // encrypted handshake message from the server (EncryptedExtension). The + // client expects to receive an unencrypted TLS 1.2 Certificate message. + // The server can't decrypt the alert. + client_->ExpectSendAlert(kTlsAlertUnexpectedMessage); server_->ExpectSendAlert(kTlsAlertBadRecordMac); // Server can't read ConnectExpectFail(); - client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO); + client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_APPLICATION_DATA); server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); } diff --git a/gtests/ssl_gtest/ssl_tls13compat_unittest.cc b/gtests/ssl_gtest/ssl_tls13compat_unittest.cc new file mode 100644 index 0000000000..75cee52fc9 --- /dev/null +++ b/gtests/ssl_gtest/ssl_tls13compat_unittest.cc @@ -0,0 +1,337 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include +#include +#include "ssl.h" +#include "sslerr.h" +#include "sslproto.h" + +#include "gtest_utils.h" +#include "tls_connect.h" +#include "tls_filter.h" +#include "tls_parser.h" + +namespace nss_test { + +class Tls13CompatTest : public TlsConnectStreamTls13 { + protected: + void EnableCompatMode() { + client_->SetOption(SSL_ENABLE_TLS13_COMPAT_MODE, PR_TRUE); + } + + void InstallFilters() { + EnsureTlsSetup(); + client_recorders_.Install(client_); + server_recorders_.Install(server_); + } + + void CheckRecordVersions() { + ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_0, + client_recorders_.records_->record(0).header.version()); + CheckRecordsAreTls12("client", client_recorders_.records_, 1); + CheckRecordsAreTls12("server", server_recorders_.records_, 0); + } + + void CheckHelloVersions() { + uint32_t ver; + ASSERT_TRUE(server_recorders_.hello_->buffer().Read(0, 2, &ver)); + ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, static_cast(ver)); + ASSERT_TRUE(client_recorders_.hello_->buffer().Read(0, 2, &ver)); + ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, static_cast(ver)); + } + + void CheckForCCS(bool expected_client, bool expected_server) { + client_recorders_.CheckForCCS(expected_client); + server_recorders_.CheckForCCS(expected_server); + } + + void CheckForRegularHandshake() { + CheckRecordVersions(); + CheckHelloVersions(); + EXPECT_EQ(0U, client_recorders_.session_id_length()); + EXPECT_EQ(0U, server_recorders_.session_id_length()); + CheckForCCS(false, false); + } + + void CheckForCompatHandshake() { + CheckRecordVersions(); + CheckHelloVersions(); + EXPECT_EQ(32U, client_recorders_.session_id_length()); + EXPECT_EQ(32U, server_recorders_.session_id_length()); + CheckForCCS(true, true); + } + + private: + struct Recorders { + Recorders() + : records_(new TlsRecordRecorder()), + hello_(new TlsInspectorRecordHandshakeMessage(std::set( + {kTlsHandshakeClientHello, kTlsHandshakeServerHello}))) {} + + uint8_t session_id_length() const { + // session_id is always after version (2) and random (32). + uint32_t len = 0; + EXPECT_TRUE(hello_->buffer().Read(2 + 32, 1, &len)); + return static_cast(len); + } + + void CheckForCCS(bool expected) const { + EXPECT_LT(0U, records_->count()); + for (size_t i = 0; i < records_->count(); ++i) { + // Only the second record can be a CCS. + bool expected_match = expected && (i == 1); + EXPECT_EQ(expected_match, + kTlsChangeCipherSpecType == + records_->record(i).header.content_type()); + } + } + + void Install(std::shared_ptr& agent) { + agent->SetPacketFilter(std::make_shared( + ChainedPacketFilterInit({records_, hello_}))); + } + + std::shared_ptr records_; + std::shared_ptr hello_; + }; + + void CheckRecordsAreTls12(const std::string& agent, + const std::shared_ptr& records, + size_t start) { + EXPECT_LE(start, records->count()); + for (size_t i = start; i < records->count(); ++i) { + EXPECT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, + records->record(i).header.version()) + << agent << ": record " << i << " has wrong version"; + } + } + + Recorders client_recorders_; + Recorders server_recorders_; +}; + +TEST_F(Tls13CompatTest, Disabled) { + InstallFilters(); + Connect(); + CheckForRegularHandshake(); +} + +TEST_F(Tls13CompatTest, Enabled) { + EnableCompatMode(); + InstallFilters(); + Connect(); + CheckForCompatHandshake(); +} + +TEST_F(Tls13CompatTest, EnabledZeroRtt) { + SetupForZeroRtt(); + EnableCompatMode(); + InstallFilters(); + + client_->Set0RttEnabled(true); + server_->Set0RttEnabled(true); + ExpectResumption(RESUME_TICKET); + ZeroRttSendReceive(true, true); + CheckForCCS(true, true); + Handshake(); + ExpectEarlyDataAccepted(true); + CheckConnected(); + + CheckForCompatHandshake(); +} + +TEST_F(Tls13CompatTest, EnabledHrr) { + EnableCompatMode(); + InstallFilters(); + + // Force a HelloRetryRequest. The server sends CCS immediately. + server_->ConfigNamedGroups({ssl_grp_ec_secp384r1}); + client_->StartConnect(); + server_->StartConnect(); + client_->Handshake(); + server_->Handshake(); + CheckForCCS(false, true); + + Handshake(); + CheckConnected(); + CheckForCompatHandshake(); +} + +TEST_F(Tls13CompatTest, EnabledStatelessHrr) { + EnableCompatMode(); + InstallFilters(); + + // Force a HelloRetryRequest + server_->ConfigNamedGroups({ssl_grp_ec_secp384r1}); + client_->StartConnect(); + server_->StartConnect(); + client_->Handshake(); + server_->Handshake(); + CheckForCCS(false, true); + + // A new server should just work, but not send another CCS. + MakeNewServer(); + InstallFilters(); + server_->ConfigNamedGroups({ssl_grp_ec_secp384r1}); + + Handshake(); + CheckConnected(); + CheckForCompatHandshake(); +} + +TEST_F(Tls13CompatTest, EnabledHrrZeroRtt) { + SetupForZeroRtt(); + EnableCompatMode(); + InstallFilters(); + server_->ConfigNamedGroups({ssl_grp_ec_secp384r1}); + + // With 0-RTT, the client sends CCS immediately. With HRR, the server sends + // CCS immediately too. + client_->Set0RttEnabled(true); + server_->Set0RttEnabled(true); + ExpectResumption(RESUME_TICKET); + ZeroRttSendReceive(true, false); + CheckForCCS(true, true); + + Handshake(); + ExpectEarlyDataAccepted(false); + CheckConnected(); + CheckForCompatHandshake(); +} + +static const uint8_t kCannedCcs[] = { + kTlsChangeCipherSpecType, + SSL_LIBRARY_VERSION_TLS_1_2 >> 8, + SSL_LIBRARY_VERSION_TLS_1_2 & 0xff, + 0, + 1, // length + 1 // change_cipher_spec_choice +}; + +// A ChangeCipherSpec is ignored by a server because we have to tolerate it for +// compatibility mode. That doesn't mean that we have to tolerate it +// unconditionally. If we negotiate 1.3, we expect to see a cookie extension. +TEST_F(TlsConnectStreamTls13, ChangeCipherSpecBeforeClientHello13) { + EnsureTlsSetup(); + server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, + SSL_LIBRARY_VERSION_TLS_1_3); + client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, + SSL_LIBRARY_VERSION_TLS_1_3); + // Client sends CCS before starting the handshake. + client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + ConnectExpectAlert(server_, kTlsAlertUnexpectedMessage); + server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER); + client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT); +} + +// A ChangeCipherSpec is ignored by a server because we have to tolerate it for +// compatibility mode. That doesn't mean that we have to tolerate it +// unconditionally. If we negotiate 1.3, we expect to see a cookie extension. +TEST_F(TlsConnectStreamTls13, ChangeCipherSpecBeforeClientHelloTwice) { + EnsureTlsSetup(); + server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, + SSL_LIBRARY_VERSION_TLS_1_3); + client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, + SSL_LIBRARY_VERSION_TLS_1_3); + // Client sends CCS before starting the handshake. + client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + ConnectExpectAlert(server_, kTlsAlertUnexpectedMessage); + server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER); + client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT); +} + +// If we negotiate 1.2, we abort. +TEST_F(TlsConnectStreamTls13, ChangeCipherSpecBeforeClientHello12) { + EnsureTlsSetup(); + server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, + SSL_LIBRARY_VERSION_TLS_1_3); + client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, + SSL_LIBRARY_VERSION_TLS_1_2); + // Client sends CCS before starting the handshake. + client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + ConnectExpectAlert(server_, kTlsAlertUnexpectedMessage); + server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER); + client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT); +} + +TEST_F(TlsConnectDatagram13, CompatModeDtlsClient) { + EnsureTlsSetup(); + client_->SetOption(SSL_ENABLE_TLS13_COMPAT_MODE, PR_TRUE); + auto client_records = std::make_shared(); + client_->SetPacketFilter(client_records); + auto server_records = std::make_shared(); + server_->SetPacketFilter(server_records); + Connect(); + + ASSERT_EQ(2U, client_records->count()); // CH, Fin + EXPECT_EQ(kTlsHandshakeType, client_records->record(0).header.content_type()); + EXPECT_EQ(kTlsApplicationDataType, + client_records->record(1).header.content_type()); + + ASSERT_EQ(6U, server_records->count()); // SH, EE, CT, CV, Fin, Ack + EXPECT_EQ(kTlsHandshakeType, server_records->record(0).header.content_type()); + for (size_t i = 1; i < server_records->count(); ++i) { + EXPECT_EQ(kTlsApplicationDataType, + server_records->record(i).header.content_type()); + } +} + +class AddSessionIdFilter : public TlsHandshakeFilter { + public: + AddSessionIdFilter() : TlsHandshakeFilter({ssl_hs_client_hello}) {} + + protected: + PacketFilter::Action FilterHandshake(const HandshakeHeader& header, + const DataBuffer& input, + DataBuffer* output) override { + uint32_t session_id_len = 0; + EXPECT_TRUE(input.Read(2 + 32, 1, &session_id_len)); + EXPECT_EQ(0U, session_id_len); + uint8_t session_id[33] = {32}; // 32 for length, the rest zero. + *output = input; + output->Splice(session_id, sizeof(session_id), 34, 1); + return CHANGE; + } +}; + +// Adding a session ID to a DTLS ClientHello should not trigger compatibility +// mode. It should be ignored instead. +TEST_F(TlsConnectDatagram13, CompatModeDtlsServer) { + EnsureTlsSetup(); + auto client_records = std::make_shared(); + client_->SetPacketFilter( + std::make_shared(ChainedPacketFilterInit( + {client_records, std::make_shared()}))); + auto server_hello = std::make_shared( + kTlsHandshakeServerHello); + auto server_records = std::make_shared(); + server_->SetPacketFilter(std::make_shared( + ChainedPacketFilterInit({server_records, server_hello}))); + StartConnect(); + client_->Handshake(); + server_->Handshake(); + // The client will consume the ServerHello, but discard everything else + // because it doesn't decrypt. And don't wait around for the client to ACK. + client_->Handshake(); + + ASSERT_EQ(1U, client_records->count()); + EXPECT_EQ(kTlsHandshakeType, client_records->record(0).header.content_type()); + + ASSERT_EQ(5U, server_records->count()); // SH, EE, CT, CV, Fin + EXPECT_EQ(kTlsHandshakeType, server_records->record(0).header.content_type()); + for (size_t i = 1; i < server_records->count(); ++i) { + EXPECT_EQ(kTlsApplicationDataType, + server_records->record(i).header.content_type()); + } + + uint32_t session_id_len = 0; + EXPECT_TRUE(server_hello->buffer().Read(2 + 32, 1, &session_id_len)); + EXPECT_EQ(0U, session_id_len); +} + +} // nss_test diff --git a/gtests/ssl_gtest/ssl_version_unittest.cc b/gtests/ssl_gtest/ssl_version_unittest.cc index 9f208592cd..9db293b07c 100644 --- a/gtests/ssl_gtest/ssl_version_unittest.cc +++ b/gtests/ssl_gtest/ssl_version_unittest.cc @@ -217,20 +217,20 @@ TEST_F(TlsConnectStreamTls13, Tls14ClientHelloWithSupportedVersions) { client_->SetPacketFilter( std::make_shared( SSL_LIBRARY_VERSION_TLS_1_3 + 1)); - auto capture = std::make_shared( - kTlsHandshakeServerHello); + auto capture = + std::make_shared(ssl_tls13_supported_versions_xtn); server_->SetPacketFilter(capture); client_->ExpectSendAlert(kTlsAlertBadRecordMac); server_->ExpectSendAlert(kTlsAlertBadRecordMac); ConnectExpectFail(); client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); - const DataBuffer& server_hello = capture->buffer(); - ASSERT_GT(server_hello.len(), 2U); - uint32_t ver; - ASSERT_TRUE(server_hello.Read(0, 2, &ver)); + + ASSERT_EQ(2U, capture->extension().len()); + uint32_t version = 0; + ASSERT_TRUE(capture->extension().Read(0, 2, &version)); // This way we don't need to change with new draft version. - ASSERT_LT(static_cast(SSL_LIBRARY_VERSION_TLS_1_2), ver); + ASSERT_LT(static_cast(SSL_LIBRARY_VERSION_TLS_1_2), version); } } // namespace nss_test diff --git a/gtests/ssl_gtest/tls_agent.cc b/gtests/ssl_gtest/tls_agent.cc index de19b16837..3b939bba86 100644 --- a/gtests/ssl_gtest/tls_agent.cc +++ b/gtests/ssl_gtest/tls_agent.cc @@ -376,13 +376,6 @@ void TlsAgent::Set0RttEnabled(bool en) { SetOption(SSL_ENABLE_0RTT_DATA, en ? PR_TRUE : PR_FALSE); } -void TlsAgent::SetAltHandshakeTypeEnabled() { - EXPECT_TRUE(EnsureTlsSetup()); - - SECStatus rv = SSL_UseAltHandshakeType(ssl_fd(), PR_TRUE); - EXPECT_EQ(SECSuccess, rv); -} - void TlsAgent::SetVersionRange(uint16_t minver, uint16_t maxver) { vrange_.min = minver; vrange_.max = maxver; diff --git a/gtests/ssl_gtest/tls_agent.h b/gtests/ssl_gtest/tls_agent.h index dbe2f7f67e..b3fd892ae4 100644 --- a/gtests/ssl_gtest/tls_agent.h +++ b/gtests/ssl_gtest/tls_agent.h @@ -127,7 +127,6 @@ class TlsAgent : public PollTarget { void ConfigureSessionCache(SessionResumptionMode mode); void Set0RttEnabled(bool en); void SetFallbackSCSVEnabled(bool en); - void SetAltHandshakeTypeEnabled(); void SetVersionRange(uint16_t minver, uint16_t maxver); void GetVersionRange(uint16_t* minver, uint16_t* maxver); void CheckPreliminaryInfo(); diff --git a/gtests/ssl_gtest/tls_connect.cc b/gtests/ssl_gtest/tls_connect.cc index bcf89e96a5..c23dc3bd14 100644 --- a/gtests/ssl_gtest/tls_connect.cc +++ b/gtests/ssl_gtest/tls_connect.cc @@ -224,6 +224,18 @@ void TlsConnectTestBase::Reset(const std::string& server_name, Init(); } +void TlsConnectTestBase::MakeNewServer() { + auto replacement = std::make_shared( + server_->name(), TlsAgent::SERVER, server_->variant()); + server_ = replacement; + if (version_) { + server_->SetVersionRange(version_, version_); + } + client_->SetPeer(server_); + server_->SetPeer(client_); + server_->StartConnect(); +} + void TlsConnectTestBase::ExpectResumption(SessionResumptionMode expected, uint8_t num_resumptions) { expected_resumption_mode_ = expected; diff --git a/gtests/ssl_gtest/tls_connect.h b/gtests/ssl_gtest/tls_connect.h index b2ec7b7478..c1f23c2f29 100644 --- a/gtests/ssl_gtest/tls_connect.h +++ b/gtests/ssl_gtest/tls_connect.h @@ -61,6 +61,8 @@ class TlsConnectTestBase : public ::testing::Test { // Reset, and update the certificate names on both peers void Reset(const std::string& server_name, const std::string& client_name = "client"); + // Replace the server. + void MakeNewServer(); // Set up void StartConnect(); diff --git a/gtests/ssl_gtest/tls_filter.cc b/gtests/ssl_gtest/tls_filter.cc index 6706d2252e..89f2012950 100644 --- a/gtests/ssl_gtest/tls_filter.cc +++ b/gtests/ssl_gtest/tls_filter.cc @@ -272,6 +272,30 @@ bool TlsRecordFilter::Protect(const TlsRecordHeader& header, return cipher_spec_->Protect(header, padded, ciphertext); } +bool IsHelloRetry(const DataBuffer& body) { + static const uint8_t ssl_hello_retry_random[] = { + 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, 0x1D, 0x8C, + 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, + 0x8C, 0x5E, 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33, 0x9C}; + return memcmp(body.data() + 2, ssl_hello_retry_random, + sizeof(ssl_hello_retry_random)) == 0; +} + +bool TlsHandshakeFilter::IsFilteredType(const HandshakeHeader& header, + const DataBuffer& body) { + if (handshake_types_.empty()) { + return true; + } + + uint8_t type = header.handshake_type(); + if (type == kTlsHandshakeServerHello) { + if (IsHelloRetry(body)) { + type = kTlsHandshakeHelloRetryRequest; + } + } + return handshake_types_.count(type) > 0U; +} + PacketFilter::Action TlsHandshakeFilter::FilterRecord( const TlsRecordHeader& record_header, const DataBuffer& input, DataBuffer* output) { @@ -306,7 +330,12 @@ PacketFilter::Action TlsHandshakeFilter::FilterRecord( preceding_fragment_.Truncate(0); DataBuffer filtered; - PacketFilter::Action action = FilterHandshake(header, handshake, &filtered); + PacketFilter::Action action; + if (!IsFilteredType(header, handshake)) { + action = KEEP; + } else { + action = FilterHandshake(header, handshake, &filtered); + } if (action == DROP) { changed = true; std::cerr << "handshake drop: " << handshake << std::endl; @@ -431,21 +460,15 @@ PacketFilter::Action TlsInspectorRecordHandshakeMessage::FilterHandshake( return KEEP; } - if (header.handshake_type() == handshake_type_) { - buffer_ = input; - } + buffer_ = input; return KEEP; } PacketFilter::Action TlsInspectorReplaceHandshakeMessage::FilterHandshake( const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() == handshake_type_) { - *output = buffer_; - return CHANGE; - } - - return KEEP; + *output = buffer_; + return CHANGE; } PacketFilter::Action TlsRecordRecorder::FilterRecord( @@ -540,14 +563,6 @@ bool FindServerHelloExtensions(TlsParser* parser, const TlsVersioned& header) { return true; } -static bool FindHelloRetryExtensions(TlsParser* parser, - const TlsVersioned& header) { - if (!parser->Skip(4)) { // version (2) + cipher suite (2) - return false; - } - return true; -} - bool FindEncryptedExtensions(TlsParser* parser, const TlsVersioned& header) { return true; } @@ -592,7 +607,6 @@ static bool FindNewSessionTicketExtensions(TlsParser* parser, static const std::map kExtensionFinders = { {kTlsHandshakeClientHello, FindClientHelloExtensions}, {kTlsHandshakeServerHello, FindServerHelloExtensions}, - {kTlsHandshakeHelloRetryRequest, FindHelloRetryExtensions}, {kTlsHandshakeEncryptedExtensions, FindEncryptedExtensions}, {kTlsHandshakeCertificateRequest, FindCertReqExtensions}, {kTlsHandshakeCertificate, FindCertificateExtensions}, @@ -610,10 +624,6 @@ bool TlsExtensionFilter::FindExtensions(TlsParser* parser, PacketFilter::Action TlsExtensionFilter::FilterHandshake( const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (handshake_types_.count(header.handshake_type()) == 0) { - return KEEP; - } - TlsParser parser(input); if (!FindExtensions(&parser, header)) { return KEEP; @@ -765,10 +775,8 @@ PacketFilter::Action AfterRecordN::FilterRecord(const TlsRecordHeader& header, PacketFilter::Action TlsInspectorClientHelloVersionChanger::FilterHandshake( const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() == kTlsHandshakeClientKeyExchange) { - EXPECT_EQ(SECSuccess, - SSLInt_IncrementClientHandshakeVersion(server_.lock()->ssl_fd())); - } + EXPECT_EQ(SECSuccess, + SSLInt_IncrementClientHandshakeVersion(server_.lock()->ssl_fd())); return KEEP; } @@ -803,21 +811,14 @@ PacketFilter::Action SelectiveRecordDropFilter::FilterRecord( PacketFilter::Action TlsInspectorClientHelloVersionSetter::FilterHandshake( const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() == kTlsHandshakeClientHello) { - *output = input; - output->Write(0, version_, 2); - return CHANGE; - } - return KEEP; + *output = input; + output->Write(0, version_, 2); + return CHANGE; } PacketFilter::Action SelectedCipherSuiteReplacer::FilterHandshake( const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output) { - if (header.handshake_type() != kTlsHandshakeServerHello) { - return KEEP; - } - *output = input; uint32_t temp = 0; EXPECT_TRUE(input.Read(0, 2, &temp)); diff --git a/gtests/ssl_gtest/tls_filter.h b/gtests/ssl_gtest/tls_filter.h index 5bd804f182..1db3b90f69 100644 --- a/gtests/ssl_gtest/tls_filter.h +++ b/gtests/ssl_gtest/tls_filter.h @@ -175,7 +175,16 @@ inline std::ostream& operator<<(std::ostream& stream, // records and that they don't span records or anything crazy like that. class TlsHandshakeFilter : public TlsRecordFilter { public: - TlsHandshakeFilter() : preceding_fragment_() {} + TlsHandshakeFilter() : handshake_types_(), preceding_fragment_() {} + TlsHandshakeFilter(const std::set& types) + : handshake_types_(types), preceding_fragment_() {} + + // This filter can be set to be selective based on handshake message type. If + // this function isn't used (or the set is empty), then all handshake messages + // will be filtered. + void SetHandshakeTypes(const std::set& types) { + handshake_types_ = types; + } class HandshakeHeader : public TlsVersioned { public: @@ -212,6 +221,10 @@ class TlsHandshakeFilter : public TlsRecordFilter { DataBuffer* output) = 0; private: + bool IsFilteredType(const HandshakeHeader& header, + const DataBuffer& handshake); + + std::set handshake_types_; DataBuffer preceding_fragment_; }; @@ -219,7 +232,9 @@ class TlsHandshakeFilter : public TlsRecordFilter { class TlsInspectorRecordHandshakeMessage : public TlsHandshakeFilter { public: TlsInspectorRecordHandshakeMessage(uint8_t handshake_type) - : handshake_type_(handshake_type), buffer_() {} + : TlsHandshakeFilter({handshake_type}), buffer_() {} + TlsInspectorRecordHandshakeMessage(const std::set& handshake_types) + : TlsHandshakeFilter(handshake_types), buffer_() {} virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, @@ -230,7 +245,6 @@ class TlsInspectorRecordHandshakeMessage : public TlsHandshakeFilter { const DataBuffer& buffer() const { return buffer_; } private: - uint8_t handshake_type_; DataBuffer buffer_; }; @@ -239,14 +253,13 @@ class TlsInspectorReplaceHandshakeMessage : public TlsHandshakeFilter { public: TlsInspectorReplaceHandshakeMessage(uint8_t handshake_type, const DataBuffer& replacement) - : handshake_type_(handshake_type), buffer_(replacement) {} + : TlsHandshakeFilter({handshake_type}), buffer_(replacement) {} virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, DataBuffer* output); private: - uint8_t handshake_type_; DataBuffer buffer_; }; @@ -325,18 +338,13 @@ typedef std::function class TlsExtensionFilter : public TlsHandshakeFilter { public: - TlsExtensionFilter() : handshake_types_() { - handshake_types_.insert(kTlsHandshakeClientHello); - handshake_types_.insert(kTlsHandshakeServerHello); - handshake_types_.insert(kTlsHandshakeEncryptedExtensions); - } + TlsExtensionFilter() + : TlsHandshakeFilter({kTlsHandshakeClientHello, kTlsHandshakeServerHello, + kTlsHandshakeHelloRetryRequest, + kTlsHandshakeEncryptedExtensions}) {} TlsExtensionFilter(const std::set& types) - : handshake_types_(types) {} - - void SetHandshakeTypes(const std::set& types) { - handshake_types_ = types; - } + : TlsHandshakeFilter(types) {} static bool FindExtensions(TlsParser* parser, const HandshakeHeader& header); @@ -353,8 +361,6 @@ class TlsExtensionFilter : public TlsHandshakeFilter { PacketFilter::Action FilterExtensions(TlsParser* parser, const DataBuffer& input, DataBuffer* output); - - std::set handshake_types_; }; class TlsExtensionCapture : public TlsExtensionFilter { @@ -441,7 +447,7 @@ class AfterRecordN : public TlsRecordFilter { class TlsInspectorClientHelloVersionChanger : public TlsHandshakeFilter { public: TlsInspectorClientHelloVersionChanger(std::shared_ptr& server) - : server_(server) {} + : TlsHandshakeFilter({kTlsHandshakeClientKeyExchange}), server_(server) {} virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, @@ -505,7 +511,8 @@ class SelectiveRecordDropFilter : public TlsRecordFilter { // Set the version number in the ClientHello. class TlsInspectorClientHelloVersionSetter : public TlsHandshakeFilter { public: - TlsInspectorClientHelloVersionSetter(uint16_t version) : version_(version) {} + TlsInspectorClientHelloVersionSetter(uint16_t version) + : TlsHandshakeFilter({kTlsHandshakeClientHello}), version_(version) {} virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header, const DataBuffer& input, @@ -538,7 +545,8 @@ class TlsLastByteDamager : public TlsHandshakeFilter { class SelectedCipherSuiteReplacer : public TlsHandshakeFilter { public: - SelectedCipherSuiteReplacer(uint16_t suite) : cipher_suite_(suite) {} + SelectedCipherSuiteReplacer(uint16_t suite) + : TlsHandshakeFilter({kTlsHandshakeServerHello}), cipher_suite_(suite) {} protected: PacketFilter::Action FilterHandshake(const HandshakeHeader& header, diff --git a/lib/ssl/ssl.h b/lib/ssl/ssl.h index 9be8a60039..25aabbaa21 100644 --- a/lib/ssl/ssl.h +++ b/lib/ssl/ssl.h @@ -248,6 +248,12 @@ SSL_IMPORT PRFileDesc *DTLS_ImportFD(PRFileDesc *model, PRFileDesc *fd); */ #define SSL_ENABLE_0RTT_DATA 33 +/* Enables TLS 1.3 compatibility mode. In this mode, the client includes a fake + * session ID in the handshake and sends a ChangeCipherSpec. A server will + * always use the setting chosen by the client, so the value of this option has + * no effect for a server. This setting is ignored for DTLS. */ +#define SSL_ENABLE_TLS13_COMPAT_MODE 35 + #ifdef SSL_DEPRECATED_FUNCTION /* Old deprecated function names */ SSL_IMPORT SECStatus SSL_Enable(PRFileDesc *fd, int option, PRIntn on); diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 39295ef2de..274dd104e2 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -65,6 +65,14 @@ static CK_MECHANISM_TYPE ssl3_GetHashMechanismByHashType(SSLHashType hashType); static CK_MECHANISM_TYPE ssl3_GetMgfMechanismByHashType(SSLHashType hash); PRBool ssl_IsRsaPssSignatureScheme(SSLSignatureScheme scheme); +const PRUint8 ssl_hello_retry_random[] = { + 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, + 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, + 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, + 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33, 0x9C +}; +PR_STATIC_ASSERT(PR_ARRAY_SIZE(ssl_hello_retry_random) == SSL3_RANDOM_LENGTH); + /* This list of SSL3 cipher suites is sorted in descending order of * precedence (desirability). It only includes cipher suites we implement. * This table is modified by SSL3_SetPolicy(). The ordering of cipher suites @@ -991,14 +999,6 @@ ssl_ClientReadVersion(sslSocket *ss, PRUint8 **b, unsigned int *len, } if (temp == tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3)) { v = SSL_LIBRARY_VERSION_TLS_1_3; - } else if (temp == tls13_EncodeAltDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3)) { - if (!ss->opt.enableAltHandshaketype || IS_DTLS(ss)) { - (void)SSL3_SendAlert(ss, alert_fatal, protocol_version); - PORT_SetError(SSL_ERROR_UNSUPPORTED_VERSION); - return SECFailure; - } - ss->ssl3.hs.altHandshakeType = PR_TRUE; - v = SSL_LIBRARY_VERSION_TLS_1_3; } else { v = (SSL3ProtocolVersion)temp; } @@ -3087,19 +3087,18 @@ ssl3_HandleChangeCipherSpecs(sslSocket *ss, sslBuffer *buf) SSL_TRC(3, ("%d: SSL3[%d]: handle change_cipher_spec record", SSL_GETPID(), ss->fd)); - if (ws != wait_change_cipher) { - if (IS_DTLS(ss)) { - /* Ignore this because it's out of order. */ - SSL_TRC(3, ("%d: SSL3[%d]: discard out of order " - "DTLS change_cipher_spec", - SSL_GETPID(), ss->fd)); - buf->len = 0; - return SECSuccess; - } - (void)SSL3_SendAlert(ss, alert_fatal, unexpected_message); - PORT_SetError(SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER); - return SECFailure; + /* For DTLS: Ignore this if we aren't expecting it. Don't kill a connection + * as a result of receiving trash. + * For TLS: Maybe ignore, but only after checking format. */ + if (ws != wait_change_cipher && IS_DTLS(ss)) { + /* Ignore this because it's out of order. */ + SSL_TRC(3, ("%d: SSL3[%d]: discard out of order " + "DTLS change_cipher_spec", + SSL_GETPID(), ss->fd)); + buf->len = 0; + return SECSuccess; } + /* Handshake messages should not span ChangeCipherSpec. */ if (ss->ssl3.hs.header_bytes) { (void)SSL3_SendAlert(ss, alert_fatal, unexpected_message); @@ -3118,7 +3117,33 @@ ssl3_HandleChangeCipherSpecs(sslSocket *ss, sslBuffer *buf) PORT_SetError(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER); return SECFailure; } + buf->len = 0; + if (ws != wait_change_cipher) { + /* Ignore a CCS for TLS 1.3. This only happens if the server sends a + * HelloRetryRequest. In other cases, the CCS will fail decryption and + * will be discarded by ssl3_HandleRecord(). */ + if (ws == wait_server_hello && + ss->version >= SSL_LIBRARY_VERSION_TLS_1_3 && + ss->ssl3.hs.helloRetry) { + PORT_Assert(!ss->sec.isServer); + return SECSuccess; + } + /* Note: For a server, we can't test ss->ssl3.hs.helloRetry or + * ss->version because the server might be stateless (and so it won't + * have set either value yet). Set a flag so that at least we will + * guarantee that the server will treat any ClientHello properly. */ + if (ws == wait_client_hello && + ss->vrange.max >= SSL_LIBRARY_VERSION_TLS_1_3 && + !ss->ssl3.hs.receivedCcs) { + PORT_Assert(ss->sec.isServer); + ss->ssl3.hs.receivedCcs = PR_TRUE; + return SECSuccess; + } + (void)SSL3_SendAlert(ss, alert_fatal, unexpected_message); + PORT_SetError(SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER); + return SECFailure; + } SSL_TRC(3, ("%d: SSL3[%d] Set Current Read Cipher Suite to Pending", SSL_GETPID(), ss->fd)); @@ -4772,7 +4797,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type) 1 + 1 /* compression methods */; if (sid->version < SSL_LIBRARY_VERSION_TLS_1_3) { length += sid->u.ssl3.sessionIDLength; - } else if (ss->opt.enableAltHandshaketype && !IS_DTLS(ss)) { + } else if (ss->opt.enableTls13CompatMode && !IS_DTLS(ss)) { length += SSL3_SESSIONID_BYTES; } if (IS_DTLS(ss)) { @@ -4826,7 +4851,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type) if (sid->version < SSL_LIBRARY_VERSION_TLS_1_3) { rv = ssl3_AppendHandshakeVariable( ss, sid->u.ssl3.sessionID, sid->u.ssl3.sessionIDLength, 1); - } else if (ss->opt.enableAltHandshaketype && !IS_DTLS(ss)) { + } else if (ss->opt.enableTls13CompatMode && !IS_DTLS(ss)) { /* We're faking session resumption, so rather than create new * randomness, just mix up the client random a little. */ PRUint8 buf[SSL3_SESSIONID_BYTES]; @@ -6115,10 +6140,14 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) { PRUint32 cipher; int errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO; + PRUint32 compression; SECStatus rv; SECItem sidBytes = { siBuffer, NULL, 0 }; + PRBool isHelloRetry; SSL3AlertDescription desc = illegal_parameter; TLSExtension *versionExtension; + const PRUint8 *savedMsg = b; + const PRUint32 savedLength = length; #ifndef TLS_1_3_DRAFT_VERSION SSL3ProtocolVersion downgradeCheckVersion; #endif @@ -6148,9 +6177,8 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) ss->ssl3.clientPrivateKey = NULL; } - /* Note that if we are doing the alternative handshake for TLS 1.3, this - * will set the version to TLS 1.2. We will amend that once all other - * fields have been read. */ + /* Note that if the server selects TLS 1.3, this will set the version to TLS + * 1.2. We will amend that once all other fields have been read. */ rv = ssl_ClientReadVersion(ss, &b, &length, &ss->version); if (rv != SECSuccess) { goto loser; /* alert has been sent */ @@ -6161,17 +6189,17 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) if (rv != SECSuccess) { goto loser; /* alert has been sent */ } + isHelloRetry = !PORT_Memcmp(ss->ssl3.hs.server_random, + ssl_hello_retry_random, SSL3_RANDOM_LENGTH); - if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { - rv = ssl3_ConsumeHandshakeVariable(ss, &sidBytes, 1, &b, &length); - if (rv != SECSuccess) { - goto loser; /* alert has been sent */ - } - if (sidBytes.len > SSL3_SESSIONID_BYTES) { - if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_0) - desc = decode_error; - goto alert_loser; /* malformed. */ - } + rv = ssl3_ConsumeHandshakeVariable(ss, &sidBytes, 1, &b, &length); + if (rv != SECSuccess) { + goto loser; /* alert has been sent */ + } + if (sidBytes.len > SSL3_SESSIONID_BYTES) { + if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_0) + desc = decode_error; + goto alert_loser; /* malformed. */ } /* Read the cipher suite. */ @@ -6181,17 +6209,14 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) } /* Compression method. */ - if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { - PRUint32 compression; - rv = ssl3_ConsumeHandshakeNumber(ss, &compression, 1, &b, &length); - if (rv != SECSuccess) { - goto loser; /* alert has been sent */ - } - if (compression != ssl_compression_null) { - desc = illegal_parameter; - errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO; - goto alert_loser; - } + rv = ssl3_ConsumeHandshakeNumber(ss, &compression, 1, &b, &length); + if (rv != SECSuccess) { + goto loser; /* alert has been sent */ + } + if (compression != ssl_compression_null) { + desc = illegal_parameter; + errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO; + goto alert_loser; } /* Parse extensions. */ @@ -6233,6 +6258,14 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) goto alert_loser; } + if (isHelloRetry && ss->ssl3.hs.helloRetry) { + SSL_TRC(3, ("%d: SSL3[%d]: received a second hello_retry_request", + SSL_GETPID(), ss->fd)); + desc = unexpected_message; + errCode = SSL_ERROR_RX_UNEXPECTED_HELLO_RETRY_REQUEST; + goto alert_loser; + } + /* The server didn't pick 1.3 although we either received a * HelloRetryRequest, or we prepared to send early app data. */ if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { @@ -6296,8 +6329,9 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) /* Finally, now all the version-related checks have passed. */ ss->ssl3.hs.preliminaryInfo |= ssl_preinfo_version; - /* Update the write cipher spec to match the version. */ - if (!ss->firstHsDone) { + /* Update the write cipher spec to match the version. But not after + * HelloRetryRequest, because cwSpec might be a 0-RTT cipher spec. */ + if (!ss->firstHsDone && !ss->ssl3.hs.helloRetry) { ssl_GetSpecWriteLock(ss); ssl_SetSpecVersions(ss, ss->ssl3.cwSpec); ssl_ReleaseSpecWriteLock(ss); @@ -6307,34 +6341,51 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) { PRUint8 buf[SSL3_SESSIONID_BYTES]; unsigned int expectedSidLen; - if (ss->ssl3.hs.altHandshakeType) { + if (ss->opt.enableTls13CompatMode && !IS_DTLS(ss)) { expectedSidLen = SSL3_SESSIONID_BYTES; ssl_MakeFakeSid(ss, buf); } else { expectedSidLen = 0; } if (sidBytes.len != expectedSidLen || - PORT_Memcmp(buf, sidBytes.data, expectedSidLen) != 0) { + (expectedSidLen > 0 && + PORT_Memcmp(buf, sidBytes.data, expectedSidLen) != 0)) { desc = illegal_parameter; errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO; goto alert_loser; } } + /* Only initialize hashes if this isn't a Hello Retry. */ rv = ssl_ClientSetCipherSuite(ss, ss->version, cipher, - PR_TRUE /* init hashes */); + !isHelloRetry); if (rv != SECSuccess) { + desc = illegal_parameter; errCode = PORT_GetError(); - goto loser; + goto alert_loser; + } + + dtls_ReceivedFirstMessageInFlight(ss); + + if (isHelloRetry) { + rv = tls13_HandleHelloRetryRequest(ss, savedMsg, savedLength); + if (rv != SECSuccess) { + goto loser; + } + return SECSuccess; } rv = ssl3_HandleParsedExtensions(ss, ssl_hs_server_hello); ssl3_DestroyRemoteExtensions(&ss->ssl3.hs.remoteExtensions); if (rv != SECSuccess) { - goto alert_loser; + goto loser; } - dtls_ReceivedFirstMessageInFlight(ss); + rv = ssl_HashHandshakeMessage(ss, ssl_hs_server_hello, + savedMsg, savedLength); + if (rv != SECSuccess) { + goto loser; + } if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) { rv = tls13_HandleServerHelloPart2(ss); @@ -7455,7 +7506,6 @@ ssl3_NewSessionID(sslSocket *ss, PRBool is_server) sid->u.ssl3.clientWriteKey = NULL; sid->u.ssl3.serverWriteKey = NULL; sid->u.ssl3.keys.extendedMasterSecretUsed = PR_FALSE; - sid->u.ssl3.altHandshakeType = ss->ssl3.hs.altHandshakeType; if (is_server) { SECStatus rv; @@ -7975,7 +8025,8 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length) ssl_ReleaseSpecWriteLock(ss); } - if (ss->ssl3.hs.altHandshakeType) { + if (isTLS13 && sidBytes.len > 0 && !IS_DTLS(ss)) { + SECITEM_FreeItem(&ss->ssl3.hs.fakeSid, PR_FALSE); rv = SECITEM_CopyItem(NULL, &ss->ssl3.hs.fakeSid, &sidBytes); if (rv != SECSuccess) { desc = internal_error; @@ -8039,6 +8090,17 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length) ss->ssl3.hs.helloRetry = PR_TRUE; } + if (ss->ssl3.hs.receivedCcs) { + /* This is only valid if we sent HelloRetryRequest, so we should have + * negotiated TLS 1.3 and there should be a cookie extension. */ + if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3 || + !ss->ssl3.hs.helloRetry) { + desc = unexpected_message; + errCode = SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER; + goto alert_loser; + } + } + /* Now parse the rest of the extensions. */ rv = ssl3_HandleParsedExtensions(ss, ssl_hs_client_hello); ssl3_DestroyRemoteExtensions(&ss->ssl3.hs.remoteExtensions); @@ -8770,6 +8832,64 @@ ssl3_HandleV2ClientHello(sslSocket *ss, unsigned char *buffer, int length, return SECFailure; } +SECStatus +ssl_ConstructServerHello(sslSocket *ss, PRBool helloRetry, + const sslBuffer *extensionBuf, sslBuffer *messageBuf) +{ + SECStatus rv; + SSL3ProtocolVersion version; + sslSessionID *sid = ss->sec.ci.sid; + + if (IS_DTLS(ss) && ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { + version = dtls_TLSVersionToDTLSVersion(ss->version); + } else { + version = PR_MIN(ss->version, SSL_LIBRARY_VERSION_TLS_1_2); + } + + rv = sslBuffer_AppendNumber(messageBuf, version, 2); + if (rv != SECSuccess) { + return SECFailure; + } + /* Random already generated in ssl3_HandleClientHello */ + rv = sslBuffer_Append(messageBuf, helloRetry ? ssl_hello_retry_random : ss->ssl3.hs.server_random, + SSL3_RANDOM_LENGTH); + if (rv != SECSuccess) { + return SECFailure; + } + + if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { + if (sid) { + rv = sslBuffer_AppendVariable(messageBuf, sid->u.ssl3.sessionID, + sid->u.ssl3.sessionIDLength, 1); + } else { + rv = sslBuffer_AppendNumber(messageBuf, 0, 1); + } + } else { + rv = sslBuffer_AppendVariable(messageBuf, ss->ssl3.hs.fakeSid.data, + ss->ssl3.hs.fakeSid.len, 1); + } + if (rv != SECSuccess) { + return SECFailure; + } + + rv = sslBuffer_AppendNumber(messageBuf, ss->ssl3.hs.cipher_suite, 2); + if (rv != SECSuccess) { + return SECFailure; + } + rv = sslBuffer_AppendNumber(messageBuf, ssl_compression_null, 1); + if (rv != SECSuccess) { + return SECFailure; + } + if (SSL_BUFFER_LEN(extensionBuf)) { + rv = sslBuffer_AppendBufferVariable(messageBuf, extensionBuf, 2); + if (rv != SECSuccess) { + return SECFailure; + } + } + + return SECSuccess; +} + /* The negotiated version number has been already placed in ss->version. ** ** Called from: ssl3_HandleClientHello (resuming session), @@ -8779,11 +8899,9 @@ ssl3_HandleV2ClientHello(sslSocket *ss, unsigned char *buffer, int length, SECStatus ssl3_SendServerHello(sslSocket *ss) { - sslSessionID *sid; SECStatus rv; sslBuffer extensionBuf = SSL_BUFFER_EMPTY; - PRUint32 length; - SSL3ProtocolVersion version; + sslBuffer messageBuf = SSL_BUFFER_EMPTY; SSL_TRC(3, ("%d: SSL3[%d]: send server_hello handshake", SSL_GETPID(), ss->fd)); @@ -8797,91 +8915,27 @@ ssl3_SendServerHello(sslSocket *ss) return SECFailure; } - sid = ss->sec.ci.sid; - rv = ssl_ConstructExtensions(ss, &extensionBuf, ssl_hs_server_hello); if (rv != SECSuccess) { - return SECFailure; - } - - length = sizeof(SSL3ProtocolVersion) + SSL3_RANDOM_LENGTH; - - /* TLS 1.3 proper doesn't include the session_id in the ServerHello. */ - if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3 || - ss->ssl3.hs.altHandshakeType) { - length += 1; /* Session ID Length */ - if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3 && sid) { - length += sid->u.ssl3.sessionIDLength; - } else if (ss->ssl3.hs.altHandshakeType) { - length += ss->ssl3.hs.fakeSid.len; - } - } - length += sizeof(ssl3CipherSuite); - if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3 || - ss->ssl3.hs.altHandshakeType) { - length += 1; /* Compression. */ - } - if (extensionBuf.len) { - length += 2 + extensionBuf.len; - } - - rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_server_hello, length); - if (rv != SECSuccess) { - goto loser; /* err set by AppendHandshake. */ - } - - if (IS_DTLS(ss) && ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { - version = dtls_TLSVersionToDTLSVersion(ss->version); - } else if (ss->ssl3.hs.altHandshakeType) { - version = SSL_LIBRARY_VERSION_TLS_1_2; - } else { - version = tls13_EncodeDraftVersion(ss->version); + goto loser; } - rv = ssl3_AppendHandshakeNumber(ss, version, 2); - if (rv != SECSuccess) { - goto loser; /* err set by AppendHandshake. */ - } - /* Random already generated in ssl3_HandleClientHello */ - rv = ssl3_AppendHandshake(ss, ss->ssl3.hs.server_random, - SSL3_RANDOM_LENGTH); + rv = ssl_ConstructServerHello(ss, PR_FALSE, &extensionBuf, &messageBuf); if (rv != SECSuccess) { - goto loser; /* err set by AppendHandshake. */ + goto loser; } - if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { - if (sid) { - rv = ssl3_AppendHandshakeVariable( - ss, sid->u.ssl3.sessionID, sid->u.ssl3.sessionIDLength, 1); - } else { - rv = ssl3_AppendHandshakeNumber(ss, 0, 1); - } - } else if (ss->ssl3.hs.altHandshakeType) { - rv = ssl3_AppendHandshakeVariable(ss, ss->ssl3.hs.fakeSid.data, - ss->ssl3.hs.fakeSid.len, 1); - SECITEM_FreeItem(&ss->ssl3.hs.fakeSid, PR_FALSE); - } + rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_server_hello, + SSL_BUFFER_LEN(&messageBuf)); if (rv != SECSuccess) { goto loser; /* err set by AppendHandshake. */ } - rv = ssl3_AppendHandshakeNumber(ss, ss->ssl3.hs.cipher_suite, 2); + rv = ssl3_AppendHandshake(ss, SSL_BUFFER_BASE(&messageBuf), + SSL_BUFFER_LEN(&messageBuf)); if (rv != SECSuccess) { goto loser; /* err set by AppendHandshake. */ } - if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3 || - ss->ssl3.hs.altHandshakeType) { - rv = ssl3_AppendHandshakeNumber(ss, ssl_compression_null, 1); - if (rv != SECSuccess) { - goto loser; /* err set by AppendHandshake. */ - } - } - if (extensionBuf.len) { - rv = ssl3_AppendBufferToHandshakeVariable(ss, &extensionBuf, 2); - if (rv != SECSuccess) { - goto loser; /* err set by ssl3_AppendHandshakeVariable */ - } - } if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { rv = ssl3_SetupBothPendingCipherSpecs(ss); @@ -8891,10 +8945,12 @@ ssl3_SendServerHello(sslSocket *ss) } sslBuffer_Clear(&extensionBuf); + sslBuffer_Clear(&messageBuf); return SECSuccess; loser: sslBuffer_Clear(&extensionBuf); + sslBuffer_Clear(&messageBuf); return SECFailure; } @@ -11099,7 +11155,6 @@ ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid, PK11SymKey *secret) return SECFailure; /* error already set. */ } } - sid->u.ssl3.altHandshakeType = ss->ssl3.hs.altHandshakeType; /* Copy the master secret (wrapped or unwrapped) into the sid */ return ssl3_CacheWrappedSecret(ss, ss->sec.ci.sid, secret); @@ -11160,6 +11215,8 @@ ssl_HashHandshakeMessageInt(sslSocket *ss, SSLHandshakeType type, PRUint8 dtlsData[8]; SECStatus rv; + PRINT_BUF(50, (ss, "Hash handshake message:", b, length)); + hdr[0] = (PRUint8)type; hdr[1] = (PRUint8)(length >> 16); hdr[2] = (PRUint8)(length >> 8); @@ -11237,7 +11294,7 @@ ssl3_HandleHandshakeMessage(sslSocket *ss, PRUint8 *b, PRUint32 length, /* Defer hashing of these messages until the message handlers. */ case ssl_hs_client_hello: - case ssl_hs_hello_retry_request: + case ssl_hs_server_hello: case ssl_hs_certificate_verify: case ssl_hs_finished: break; @@ -11331,12 +11388,6 @@ ssl3_HandlePostHelloHandshakeMessage(sslSocket *ss, PRUint8 *b, rv = ssl3_HandleHelloRequest(ss); break; - case ssl_hs_hello_retry_request: - /* This arrives here because - as a client - we haven't received a - * final decision on the version from the server. */ - rv = tls13_HandleHelloRetryRequest(ss, b, length); - break; - case ssl_hs_hello_verify_request: if (!IS_DTLS(ss) || ss->sec.isServer) { (void)SSL3_SendAlert(ss, alert_fatal, unexpected_message); @@ -12193,7 +12244,6 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Ciphertext *cText, sslBuffer *databuf) if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3 && cText->type == content_change_cipher_spec && ss->ssl3.hs.ws != idle_handshake && - ss->ssl3.hs.altHandshakeType && cText->buf->len == 1 && cText->buf->buf[0] == change_cipher_spec_choice) { /* Ignore the CCS. */ diff --git a/lib/ssl/ssl3ext.c b/lib/ssl/ssl3ext.c index b57f95f628..d490836a5c 100644 --- a/lib/ssl/ssl3ext.c +++ b/lib/ssl/ssl3ext.c @@ -156,6 +156,7 @@ static const sslExtensionBuilder tls13_cert_req_senders[] = { static const sslExtensionBuilder tls13_hrr_senders[] = { { ssl_tls13_key_share_xtn, &tls13_ServerSendHrrKeyShareXtn }, { ssl_tls13_cookie_xtn, &tls13_ServerSendHrrCookieXtn }, + { ssl_tls13_supported_versions_xtn, &tls13_ServerSendSupportedVersionsXtn }, { 0, NULL } }; diff --git a/lib/ssl/ssl3exthandle.c b/lib/ssl/ssl3exthandle.c index 9ad9e70a38..9476f4fd1e 100644 --- a/lib/ssl/ssl3exthandle.c +++ b/lib/ssl/ssl3exthandle.c @@ -654,7 +654,7 @@ ssl3_ClientHandleStatusRequestXtn(const sslSocket *ss, TLSExtensionData *xtnData } PRUint32 ssl_ticket_lifetime = 2 * 24 * 60 * 60; /* 2 days in seconds */ -#define TLS_EX_SESS_TICKET_VERSION (0x0109) +#define TLS_EX_SESS_TICKET_VERSION (0x010a) /* * Called from ssl3_SendNewSessionTicket, tls13_SendNewSessionTicket @@ -821,12 +821,6 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, if (rv != SECSuccess) goto loser; - /* Alternative handshake type. */ - rv = sslBuffer_AppendNumber( - &plaintext, ss->sec.ci.sid->u.ssl3.altHandshakeType, 1); - if (rv != SECSuccess) - goto loser; - rv = sslBuffer_AppendNumber(&plaintext, ssl_max_early_data_size, 4); if (rv != SECSuccess) goto loser; @@ -1126,14 +1120,6 @@ ssl_ParseSessionTicket(sslSocket *ss, const SECItem *decryptedTicket, return SECFailure; } - rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 1, &buffer, &len); - if (rv != SECSuccess) { - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); - return SECFailure; - } - PORT_Assert(temp == PR_TRUE || temp == PR_FALSE); - parsedTicket->altHandshakeType = temp; - rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 4, &buffer, &len); if (rv != SECSuccess) { PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); @@ -1211,7 +1197,6 @@ ssl_CreateSIDFromTicket(sslSocket *ss, const SECItem *rawTicket, sid->u.ssl3.masterValid = PR_TRUE; sid->u.ssl3.keys.resumable = PR_TRUE; sid->u.ssl3.keys.extendedMasterSecretUsed = parsedTicket->extendedMasterSecretUsed; - sid->u.ssl3.altHandshakeType = parsedTicket->altHandshakeType; /* Copy over client cert from session ticket if there is one. */ if (parsedTicket->peer_cert.data != NULL) { diff --git a/lib/ssl/ssl3prot.h b/lib/ssl/ssl3prot.h index 7ba1a095ce..6d27dfd7ca 100644 --- a/lib/ssl/ssl3prot.h +++ b/lib/ssl/ssl3prot.h @@ -16,7 +16,7 @@ typedef PRUint16 SSL3ProtocolVersion; /* The TLS 1.3 draft version. Used to avoid negotiating * between incompatible pre-standard TLS 1.3 drafts. * TODO(ekr@rtfm.com): Remove when TLS 1.3 is published. */ -#define TLS_1_3_DRAFT_VERSION 21 +#define TLS_1_3_DRAFT_VERSION 22 typedef PRUint16 ssl3CipherSuite; /* The cipher suites are defined in sslproto.h */ diff --git a/lib/ssl/sslexp.h b/lib/ssl/sslexp.h index 3dd37ad62d..cf71913cb1 100644 --- a/lib/ssl/sslexp.h +++ b/lib/ssl/sslexp.h @@ -21,6 +21,8 @@ SEC_BEGIN_PROTOS (SSL_GetExperimentalAPI(name) \ ? ((SECStatus(*) arglist)SSL_GetExperimentalAPI(name))args \ : SECFailure) +#define SSL_DEPRECATED_EXPERIMENTAL_API \ + (PR_SetError(SSL_ERROR_UNSUPPORTED_EXPERIMENTAL_API, 0), SECFailure) /* * SSL_GetExtensionSupport() returns whether NSS supports a particular TLS @@ -338,14 +340,8 @@ typedef SSLHelloRetryRequestAction(PR_CALLBACK *SSLHelloRetryRequestCallback)( SSLHelloRetryRequestCallback _cb, void *_arg), \ (fd, cb, arg)) -/* Make the TLS 1.3 handshake mimic TLS 1.2 session resumption. - * This will either become part of the standard or be disabled - * after we have tested it. - */ -#define SSL_UseAltHandshakeType(fd, enable) \ - SSL_EXPERIMENTAL_API("SSL_UseAltHandshakeType", \ - (PRFileDesc * _fd, PRBool _enable), \ - (fd, enable)) +#define SSL_UseAltHandshakeType(fd, enable) \ + SSL_DEPRECATED_EXPERIMENTAL_API SEC_END_PROTOS diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index 62b144edf6..44345e3f2e 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -256,8 +256,7 @@ typedef struct sslOptionsStr { unsigned int enableSignedCertTimestamps : 1; unsigned int requireDHENamedGroups : 1; unsigned int enable0RttData : 1; - unsigned int enableShortHeaders : 1; - unsigned int enableAltHandshaketype : 1; + unsigned int enableTls13CompatMode : 1; } sslOptions; typedef enum { sslHandshakingUndetermined = 0, @@ -444,8 +443,6 @@ struct sslSessionIDStr { * Used for TLS 1.3. */ SECItem alpnSelection; - PRBool altHandshakeType; - /* This lock is lazily initialized by CacheSID when a sid is first * cached. Before then, there is no need to lock anything because * the sid isn't being shared by anything. @@ -688,19 +685,20 @@ typedef struct SSL3HandshakeStateStr { * on server.*/ PRBool helloRetry; /* True if HelloRetryRequest has been sent * or received. */ + PRBool receivedCcs; /* A server received ChangeCipherSpec + * before the handshake started. */ PRBool clientCertRequested; /* True if CertificateRequest received. */ ssl3KEADef kea_def_mutable; /* Used to hold the writable kea_def * we use for TLS 1.3 */ PRTime serverHelloTime; /* Time the ServerHello flight was sent. */ PRUint16 ticketNonce; /* A counter we use for tickets. */ - PRBool altHandshakeType; /* Alternative ServerHello content type. */ SECItem fakeSid; /* ... (server) the SID the client used. */ PRBool endOfFlight; /* Processed a full flight (DTLS 1.3). */ /* The following lists contain DTLSHandshakeRecordEntry */ PRCList dtlsSentHandshake; /* Used to map records to handshake fragments. */ PRCList dtlsRcvdHandshake; /* Handshake records we have received - * used to generate ACKs. */ + * used to generate ACKs. */ } SSL3HandshakeState; #define SSL_ASSERT_HASHES_EMPTY(ss) \ @@ -839,7 +837,6 @@ typedef struct SessionTicketStr { PRUint32 flags; SECItem srvName; /* negotiated server name */ SECItem alpnSelection; - PRBool altHandshakeType; PRUint32 maxEarlyData; PRUint32 ticketAgeBaseline; SECItem applicationToken; @@ -1610,6 +1607,9 @@ SECStatus ssl3_ParseCertificateRequestCAs(sslSocket *ss, PRUint8 **b, SECStatus ssl3_CompleteHandleCertificateRequest( sslSocket *ss, const SSLSignatureScheme *signatureSchemes, unsigned int signatureSchemeCount, CERTDistNames *ca_list); +SECStatus ssl_ConstructServerHello(sslSocket *ss, PRBool helloRetry, + const sslBuffer *extensionBuf, + sslBuffer *messageBuf); SECStatus ssl3_SendServerHello(sslSocket *ss); SECStatus ssl3_SendChangeCipherSpecsInt(sslSocket *ss); SECStatus ssl3_ComputeHandshakeHashes(sslSocket *ss, diff --git a/lib/ssl/sslsock.c b/lib/ssl/sslsock.c index 711f79d816..3e344068d7 100644 --- a/lib/ssl/sslsock.c +++ b/lib/ssl/sslsock.c @@ -80,7 +80,7 @@ static sslOptions ssl_defaults = { PR_FALSE, /* enableSignedCertTimestamps */ PR_FALSE, /* requireDHENamedGroups */ PR_FALSE, /* enable0RttData */ - PR_FALSE /* enableAltHandshakeType */ + PR_FALSE /* enableTls13CompatMode */ }; /* @@ -803,6 +803,10 @@ SSL_OptionSet(PRFileDesc *fd, PRInt32 which, PRIntn val) ss->opt.enable0RttData = val; break; + case SSL_ENABLE_TLS13_COMPAT_MODE: + ss->opt.enableTls13CompatMode = val; + break; + default: PORT_SetError(SEC_ERROR_INVALID_ARGS); rv = SECFailure; @@ -936,6 +940,9 @@ SSL_OptionGet(PRFileDesc *fd, PRInt32 which, PRIntn *pVal) case SSL_ENABLE_0RTT_DATA: val = ss->opt.enable0RttData; break; + case SSL_ENABLE_TLS13_COMPAT_MODE: + val = ss->opt.enableTls13CompatMode; + break; default: PORT_SetError(SEC_ERROR_INVALID_ARGS); rv = SECFailure; @@ -1053,6 +1060,9 @@ SSL_OptionGetDefault(PRInt32 which, PRIntn *pVal) case SSL_ENABLE_0RTT_DATA: val = ssl_defaults.enable0RttData; break; + case SSL_ENABLE_TLS13_COMPAT_MODE: + val = ssl_defaults.enableTls13CompatMode; + break; default: PORT_SetError(SEC_ERROR_INVALID_ARGS); rv = SECFailure; @@ -1232,6 +1242,10 @@ SSL_OptionSetDefault(PRInt32 which, PRIntn val) ssl_defaults.enable0RttData = val; break; + case SSL_ENABLE_TLS13_COMPAT_MODE: + ssl_defaults.enableTls13CompatMode = val; + break; + default: PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; @@ -3922,7 +3936,6 @@ struct { EXP(InstallExtensionHooks), EXP(SendSessionTicket), EXP(SetupAntiReplay), - EXP(UseAltHandshakeType), #endif { "", NULL } }; diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c index 18a330eba9..33f60e9e74 100644 --- a/lib/ssl/tls13con.c +++ b/lib/ssl/tls13con.c @@ -1663,24 +1663,13 @@ tls13_ConstructHelloRetryRequest(sslSocket *ss, sslBuffer extensionsBuf = SSL_BUFFER_EMPTY; PORT_Assert(buffer->len == 0); - rv = sslBuffer_AppendNumber(buffer, - tls13_EncodeDraftVersion(ss->version), 2); - - if (rv != SECSuccess) { - goto loser; - } - - rv = sslBuffer_AppendNumber(buffer, cipherSuite, 2); - if (rv != SECSuccess) { - goto loser; - } - /* Note: cookie is pointing to a stack variable, so is only valid * now. */ ss->xtnData.selectedGroup = selectedGroup; ss->xtnData.cookie.data = cookie; ss->xtnData.cookie.len = cookieLen; - rv = ssl_ConstructExtensions(ss, &extensionsBuf, ssl_hs_hello_retry_request); + rv = ssl_ConstructExtensions(ss, &extensionsBuf, + ssl_hs_hello_retry_request); if (rv != SECSuccess) { goto loser; } @@ -1691,15 +1680,15 @@ tls13_ConstructHelloRetryRequest(sslSocket *ss, ss->xtnData.cookie.data = NULL; ss->xtnData.cookie.len = 0; - rv = sslBuffer_AppendBufferVariable(buffer, &extensionsBuf, 2); - sslBuffer_Clear(&extensionsBuf); + rv = ssl_ConstructServerHello(ss, PR_TRUE, &extensionsBuf, buffer); if (rv != SECSuccess) { goto loser; } - + sslBuffer_Clear(&extensionsBuf); return SECSuccess; loser: + sslBuffer_Clear(&extensionsBuf); sslBuffer_Clear(buffer); return SECFailure; } @@ -1739,7 +1728,7 @@ tls13_SendHelloRetryRequest(sslSocket *ss, /* And send it. */ ssl_GetXmitBufLock(ss); - rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_hello_retry_request, + rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_server_hello, SSL_BUFFER_LEN(&messageBuf)); if (rv != SECSuccess) { goto loser; @@ -1749,10 +1738,29 @@ tls13_SendHelloRetryRequest(sslSocket *ss, goto loser; } sslBuffer_Clear(&messageBuf); /* Done with messageBuf */ - rv = ssl3_FlushHandshake(ss, 0); - if (rv != SECSuccess) { - goto loser; /* error code set by ssl3_FlushHandshake */ + + if (ss->ssl3.hs.fakeSid.len) { + PRInt32 sent; + + PORT_Assert(!IS_DTLS(ss)); + rv = ssl3_SendChangeCipherSpecsInt(ss); + if (rv != SECSuccess) { + goto loser; + } + /* ssl3_SendChangeCipherSpecsInt() only flushes to the output buffer, so we + * have to force a send. */ + sent = ssl_SendSavedWriteData(ss); + if (sent < 0 && PORT_GetError() != PR_WOULD_BLOCK_ERROR) { + PORT_SetError(SSL_ERROR_SOCKET_WRITE_FAILURE); + goto loser; + } + } else { + rv = ssl3_FlushHandshake(ss, 0); + if (rv != SECSuccess) { + goto loser; /* error code set by ssl3_FlushHandshake */ + } } + /* We depend on this being exactly one record and one message. */ PORT_Assert(!IS_DTLS(ss) || (ss->ssl3.hs.sendMessageSeq == 1 && ss->ssl3.cwSpec->seqNum == 1)); @@ -1915,14 +1923,22 @@ tls13_ReinjectHandshakeTranscript(sslSocket *ss) return SECSuccess; } +static unsigned int +ssl_ListCount(PRCList *list) +{ + unsigned int c = 0; + PRCList *cur; + for (cur = PR_NEXT_LINK(list); cur != list; cur = PR_NEXT_LINK(cur)) { + ++c; + } + return c; +} + SECStatus -tls13_HandleHelloRetryRequest(sslSocket *ss, PRUint8 *b, PRUint32 length) +tls13_HandleHelloRetryRequest(sslSocket *ss, const PRUint8 *savedMsg, + PRUint32 savedLength) { SECStatus rv; - PRUint32 tmp; - SSL3ProtocolVersion version; - const PRUint8 *savedMsg = b; - const PRUint32 savedLength = length; SSL_TRC(3, ("%d: TLS13[%d]: handle hello retry request", SSL_GETPID(), ss->fd)); @@ -1935,20 +1951,7 @@ tls13_HandleHelloRetryRequest(sslSocket *ss, PRUint8 *b, PRUint32 length) unexpected_message); return SECFailure; } - - /* Client only. */ - rv = TLS13_CHECK_HS_STATE(ss, SSL_ERROR_RX_UNEXPECTED_HELLO_RETRY_REQUEST, - wait_server_hello); - if (rv != SECSuccess) { - return SECFailure; - } - - /* Fool me once, shame on you; fool me twice... */ - if (ss->ssl3.hs.helloRetry) { - FATAL_ERROR(ss, SSL_ERROR_RX_UNEXPECTED_HELLO_RETRY_REQUEST, - unexpected_message); - return SECFailure; - } + PORT_Assert(ss->ssl3.hs.ws == wait_server_hello); if (ss->ssl3.hs.zeroRttState == ssl_0rtt_sent) { ss->ssl3.hs.zeroRttState = ssl_0rtt_ignored; @@ -1963,50 +1966,20 @@ tls13_HandleHelloRetryRequest(sslSocket *ss, PRUint8 *b, PRUint32 length) PORT_Assert(ss->ssl3.hs.zeroRttState == ssl_0rtt_none); } - dtls_ReceivedFirstMessageInFlight(ss); - - /* Version. */ - rv = ssl_ClientReadVersion(ss, &b, &length, &version); - if (rv != SECSuccess) { - FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_HELLO_RETRY_REQUEST, - protocol_version); - return SECFailure; - } - if (version > ss->vrange.max || version < SSL_LIBRARY_VERSION_TLS_1_3) { - FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_HELLO_RETRY_REQUEST, - protocol_version); - return SECFailure; - } - - rv = ssl3_ConsumeHandshakeNumber(ss, &tmp, 2, &b, &length); - if (rv != SECSuccess) { - return SECFailure; /* error code already set */ - } - rv = ssl_ClientSetCipherSuite(ss, version, tmp, - PR_FALSE /* don't initHashes */); - if (rv != SECSuccess) { - FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_HELLO_RETRY_REQUEST, - illegal_parameter); - return SECFailure; /* error code already set */ - } - - /* Extensions. */ - rv = ssl3_ConsumeHandshakeNumber(ss, &tmp, 2, &b, &length); - if (rv != SECSuccess) { - return SECFailure; /* error code already set */ - } - /* Extensions must be non-empty and use the remainder of the message. - * This means that a HelloRetryRequest cannot be a no-op: we must have an - * extension, it must be one that we understand and recognize as being valid - * for HelloRetryRequest, and all the extensions we permit cause us to - * modify our ClientHello in some way. */ - if (!tmp || tmp != length) { + /* Extensions must contain more than just supported_versions. This will + * ensure that a HelloRetryRequest isn't a no-op: we must have at least two + * extensions, supported_versions plus one other. That other must be one + * that we understand and recognize as being valid for HelloRetryRequest, + * and all the extensions we permit cause us to modify our second + * ClientHello in some meaningful way. */ + if (ssl_ListCount(&ss->ssl3.hs.remoteExtensions) <= 1) { FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_HELLO_RETRY_REQUEST, decode_error); return SECFailure; } - rv = ssl3_HandleExtensions(ss, &b, &length, ssl_hs_hello_retry_request); + rv = ssl3_HandleParsedExtensions(ss, ssl_hs_hello_retry_request); + ssl3_DestroyRemoteExtensions(&ss->ssl3.hs.remoteExtensions); if (rv != SECSuccess) { return SECFailure; /* Error code set below */ } @@ -2017,21 +1990,31 @@ tls13_HandleHelloRetryRequest(sslSocket *ss, PRUint8 *b, PRUint32 length) return rv; } - rv = ssl_HashHandshakeMessage(ss, ssl_hs_hello_retry_request, + rv = ssl_HashHandshakeMessage(ss, ssl_hs_server_hello, savedMsg, savedLength); if (rv != SECSuccess) { - return rv; + return SECFailure; } ssl_GetXmitBufLock(ss); - + if (ss->opt.enableTls13CompatMode && !IS_DTLS(ss) && + ss->ssl3.hs.zeroRttState == ssl_0rtt_none) { + rv = ssl3_SendChangeCipherSpecsInt(ss); + if (rv != SECSuccess) { + goto loser; + } + } rv = ssl3_SendClientHello(ss, client_hello_retry); - ssl_ReleaseXmitBufLock(ss); if (rv != SECSuccess) { - return SECFailure; + goto loser; } + ssl_ReleaseXmitBufLock(ss); return SECSuccess; + +loser: + ssl_ReleaseXmitBufLock(ss); + return SECFailure; } static SECStatus @@ -2177,15 +2160,26 @@ tls13_SendServerHelloSequence(sslSocket *ss) PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss)); + rv = ssl3_RegisterExtensionSender(ss, &ss->xtnData, + ssl_tls13_supported_versions_xtn, + tls13_ServerSendSupportedVersionsXtn); + if (rv != SECSuccess) { + return SECFailure; + } + rv = ssl3_SendServerHello(ss); if (rv != SECSuccess) { return rv; /* err code is set. */ } - if (ss->ssl3.hs.altHandshakeType) { - rv = ssl3_SendChangeCipherSpecsInt(ss); - if (rv != SECSuccess) { - return rv; + if (ss->ssl3.hs.fakeSid.len) { + PORT_Assert(!IS_DTLS(ss)); + SECITEM_FreeItem(&ss->ssl3.hs.fakeSid, PR_FALSE); + if (!ss->ssl3.hs.helloRetry) { + rv = ssl3_SendChangeCipherSpecsInt(ss); + if (rv != SECSuccess) { + return rv; + } } } @@ -2966,28 +2960,14 @@ tls13_DeriveTrafficKeys(sslSocket *ss, ssl3CipherSpec *spec, void tls13_SetSpecRecordVersion(sslSocket *ss, ssl3CipherSpec *spec) { - const SSL3ProtocolVersion kTlsRecordVersion = SSL_LIBRARY_VERSION_TLS_1_0; - const SSL3ProtocolVersion kTlsAltRecordVersion = SSL_LIBRARY_VERSION_TLS_1_2; - const SSL3ProtocolVersion kDtlsRecordVersion = SSL_LIBRARY_VERSION_DTLS_1_0_WIRE; - - /* Set the record version. */ + /* Set the record version to pretend to be (D)TLS 1.2. */ if (IS_DTLS(ss)) { - spec->recordVersion = kDtlsRecordVersion; - } else if (spec->epoch == TrafficKeyEarlyApplicationData) { - /* For early data, the previous session determines the record type that - * is used (and not what this session might negotiate). */ - if (ss->sec.ci.sid && ss->sec.ci.sid->u.ssl3.altHandshakeType) { - spec->recordVersion = kTlsAltRecordVersion; - } else { - spec->recordVersion = kTlsRecordVersion; - } - } else if (ss->ssl3.hs.altHandshakeType) { - spec->recordVersion = kTlsAltRecordVersion; + spec->recordVersion = SSL_LIBRARY_VERSION_DTLS_1_2_WIRE; } else { - spec->recordVersion = kTlsRecordVersion; + spec->recordVersion = SSL_LIBRARY_VERSION_TLS_1_2; } - SSL_TRC(10, ("%d: TLS13[%d]: Set record version to 0x%04x", - SSL_GETPID(), ss->fd, spec->recordVersion)); + SSL_TRC(10, ("%d: TLS13[%d]: set spec=%d record version to 0x%04x", + SSL_GETPID(), ss->fd, spec, spec->recordVersion)); } static SECStatus @@ -4125,8 +4105,9 @@ tls13_SendClientSecondRound(sslSocket *ss) if (rv != SECSuccess) { return SECFailure; /* Error code already set. */ } - } else if (ss->ssl3.hs.zeroRttState == ssl_0rtt_none && - ss->ssl3.hs.altHandshakeType) { + } else if (ss->opt.enableTls13CompatMode && !IS_DTLS(ss) && + ss->ssl3.hs.zeroRttState == ssl_0rtt_none && + !ss->ssl3.hs.helloRetry) { ssl_GetXmitBufLock(ss); /*******************************/ rv = ssl3_SendChangeCipherSpecsInt(ss); ssl_ReleaseXmitBufLock(ss); /*******************************/ @@ -4515,7 +4496,8 @@ static const struct { certificate) }, { ssl_tls13_cookie_xtn, _M2(client_hello, hello_retry_request) }, { ssl_tls13_certificate_authorities_xtn, _M1(certificate_request) }, - { ssl_tls13_supported_versions_xtn, _M2(client_hello, server_hello) } + { ssl_tls13_supported_versions_xtn, _M3(client_hello, server_hello, + hello_retry_request) } }; tls13ExtensionStatus @@ -4844,10 +4826,12 @@ tls13_MaybeDo0RTTHandshake(sslSocket *ss) } } - /* If the alternative handshake type option is enabled and the last session - * had the alternative handshake type, then send CCS. */ - if (ss->opt.enableAltHandshaketype && - ss->sec.ci.sid->u.ssl3.altHandshakeType) { + if (ss->opt.enableTls13CompatMode && !IS_DTLS(ss)) { + /* Pretend that this is a proper ChangeCipherSpec even though it is sent + * before receiving the ServerHello. */ + ssl_GetSpecWriteLock(ss); + tls13_SetSpecRecordVersion(ss, ss->ssl3.cwSpec); + ssl_ReleaseSpecWriteLock(ss); ssl_GetXmitBufLock(ss); rv = ssl3_SendChangeCipherSpecsInt(ss); ssl_ReleaseXmitBufLock(ss); @@ -5007,17 +4991,6 @@ tls13_EncodeDraftVersion(SSL3ProtocolVersion version) return (PRUint16)version; } -PRUint16 -tls13_EncodeAltDraftVersion(SSL3ProtocolVersion version) -{ -#ifdef TLS_1_3_DRAFT_VERSION - if (version == SSL_LIBRARY_VERSION_TLS_1_3) { - return 0x7e02; - } -#endif - return (PRUint16)version; -} - /* Pick the highest version we support that is also advertised. */ SECStatus tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supported_versions) @@ -5039,7 +5012,6 @@ tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supported_versions) } for (version = ss->vrange.max; version >= ss->vrange.min; --version) { PRUint16 wire = tls13_EncodeDraftVersion(version); - PRUint16 alt_wire = tls13_EncodeAltDraftVersion(version); unsigned long offset; for (offset = 0; offset < versions.len; offset += 2) { @@ -5049,19 +5021,6 @@ tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supported_versions) ss->version = version; return SECSuccess; } - if (ss->opt.enableAltHandshaketype && - !IS_DTLS(ss) && - supported == alt_wire) { - rv = ssl3_RegisterExtensionSender(ss, &ss->xtnData, - ssl_tls13_supported_versions_xtn, - tls13_ServerSendSupportedVersionsXtn); - if (rv != SECSuccess) { - return SECFailure; - } - ss->ssl3.hs.altHandshakeType = PR_TRUE; - ss->version = version; - return SECSuccess; - } } } @@ -5069,24 +5028,6 @@ tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supported_versions) return SECFailure; } -SECStatus -SSLExp_UseAltHandshakeType(PRFileDesc *fd, PRBool enable) -{ - sslSocket *ss; - - ss = ssl_FindSocket(fd); - if (!ss || IS_DTLS(ss)) { - SSL_DBG(("%d: SSL[%d]: bad socket in SSLExp_UseAltHandshakeType", - SSL_GETPID(), fd)); - PORT_SetError(SEC_ERROR_INVALID_ARGS); - return SECFailure; - } - - ss->opt.enableAltHandshaketype = enable; - - return SECSuccess; -} - /* This is TLS 1.3 or might negotiate to it. */ PRBool tls13_MaybeTls13(sslSocket *ss) diff --git a/lib/ssl/tls13con.h b/lib/ssl/tls13con.h index 5ca033921a..d4583ee2e6 100644 --- a/lib/ssl/tls13con.h +++ b/lib/ssl/tls13con.h @@ -77,7 +77,7 @@ SECStatus tls13_ConstructHelloRetryRequest(sslSocket *ss, PRUint8 *cookie, unsigned int cookieLen, sslBuffer *buffer); -SECStatus tls13_HandleHelloRetryRequest(sslSocket *ss, PRUint8 *b, +SECStatus tls13_HandleHelloRetryRequest(sslSocket *ss, const PRUint8 *b, PRUint32 length); void tls13_DestroyKeyShareEntry(TLS13KeyShareEntry *entry); void tls13_DestroyKeyShares(PRCList *list); @@ -96,7 +96,6 @@ PRInt32 tls13_Read0RttData(sslSocket *ss, void *buf, PRInt32 len); SECStatus tls13_HandleEarlyApplicationData(sslSocket *ss, sslBuffer *origBuf); PRBool tls13_ClientAllow0Rtt(const sslSocket *ss, const sslSessionID *sid); PRUint16 tls13_EncodeDraftVersion(SSL3ProtocolVersion version); -PRUint16 tls13_EncodeAltDraftVersion(SSL3ProtocolVersion version); SECStatus tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supported_versions); diff --git a/lib/ssl/tls13exthandle.c b/lib/ssl/tls13exthandle.c index 7c7341480c..899f238276 100644 --- a/lib/ssl/tls13exthandle.c +++ b/lib/ssl/tls13exthandle.c @@ -750,13 +750,6 @@ tls13_ClientSendSupportedVersionsXtn(const sslSocket *ss, TLSExtensionData *xtnD return SECFailure; } - if (ss->opt.enableAltHandshaketype && !IS_DTLS(ss)) { - rv = sslBuffer_AppendNumber( - buf, tls13_EncodeAltDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3), 2); - if (rv != SECSuccess) { - return SECFailure; - } - } for (version = ss->vrange.max; version >= ss->vrange.min; --version) { rv = sslBuffer_AppendNumber(buf, tls13_EncodeDraftVersion(version), 2); if (rv != SECSuccess) { @@ -782,15 +775,12 @@ tls13_ServerSendSupportedVersionsXtn(const sslSocket *ss, TLSExtensionData *xtnD if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { return SECSuccess; } - if (!ss->ssl3.hs.altHandshakeType) { - return SECSuccess; - } SSL_TRC(3, ("%d: TLS13[%d]: server send supported_versions extension", SSL_GETPID(), ss->fd)); rv = sslBuffer_AppendNumber( - buf, tls13_EncodeAltDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3), 2); + buf, tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3), 2); if (rv != SECSuccess) { return SECFailure; } diff --git a/lib/ssl/tls13hashstate.c b/lib/ssl/tls13hashstate.c index 07969147f5..e3232f524e 100644 --- a/lib/ssl/tls13hashstate.c +++ b/lib/ssl/tls13hashstate.c @@ -167,7 +167,7 @@ tls13_RecoverHashState(sslSocket *ss, return SECFailure; } - rv = ssl_HashHandshakeMessageInt(ss, ssl_hs_hello_retry_request, 0, + rv = ssl_HashHandshakeMessageInt(ss, ssl_hs_server_hello, 0, SSL_BUFFER_BASE(&messageBuf), SSL_BUFFER_LEN(&messageBuf)); sslBuffer_Clear(&messageBuf);