From b34ee8106923683ccb5128a5560a3b1dae105687 Mon Sep 17 00:00:00 2001 From: EKR Date: Thu, 15 Mar 2018 12:01:09 +0000 Subject: Bug 1446643 - Update to TLS 1.3 draft-26. r=mt - Update version number - Forbid negotiating < TLS 1.3 with supported_versions - Change to version number 0303 after HRR. Plus test - Update AAD. https://phabricator.services.mozilla.com/D753 --- cpputil/tls_parser.cc | 15 ++++ cpputil/tls_parser.h | 1 + gtests/ssl_gtest/ssl_agent_unittest.cc | 49 +++++------- gtests/ssl_gtest/ssl_hrr_unittest.cc | 28 ++++++- gtests/ssl_gtest/tls_agent.cc | 18 +++++ gtests/ssl_gtest/tls_agent.h | 6 ++ gtests/ssl_gtest/tls_filter.cc | 39 +++++++--- gtests/ssl_gtest/tls_filter.h | 12 ++- gtests/ssl_gtest/tls_protect.cc | 31 +++++--- gtests/ssl_gtest/tls_protect.h | 15 ++-- lib/ssl/ssl3con.c | 55 ++++++-------- lib/ssl/ssl3gthr.c | 5 +- lib/ssl/ssl3prot.h | 2 +- lib/ssl/sslimpl.h | 9 ++- lib/ssl/tls13con.c | 134 +++++++++++++++++++++++++++------ lib/ssl/tls13con.h | 1 + 16 files changed, 300 insertions(+), 120 deletions(-) diff --git a/cpputil/tls_parser.cc b/cpputil/tls_parser.cc index e4c06aa91..efedd7a65 100644 --- a/cpputil/tls_parser.cc +++ b/cpputil/tls_parser.cc @@ -46,6 +46,21 @@ bool TlsParser::Read(DataBuffer* val, size_t len) { return true; } +bool TlsParser::ReadFromMark(DataBuffer* val, size_t len, size_t mark) { + auto saved = offset_; + offset_ = mark; + + if (remaining() < len) { + offset_ = saved; + return false; + } + + val->Assign(ptr(), len); + + offset_ = saved; + return true; +} + bool TlsParser::ReadVariable(DataBuffer* val, size_t len_size) { uint32_t len; if (!Read(&len, len_size)) { diff --git a/cpputil/tls_parser.h b/cpputil/tls_parser.h index 436c11e76..56f562e07 100644 --- a/cpputil/tls_parser.h +++ b/cpputil/tls_parser.h @@ -123,6 +123,7 @@ class TlsParser { bool Read(uint32_t* val, size_t size); // Reads len bytes into dest buffer, overwriting it. bool Read(DataBuffer* dest, size_t len); + bool ReadFromMark(DataBuffer* val, size_t len, size_t mark); // Reads bytes into dest buffer, overwriting it. The number of bytes is // determined by reading from len_size bytes from the stream first. bool ReadVariable(DataBuffer* dest, size_t len_size); diff --git a/gtests/ssl_gtest/ssl_agent_unittest.cc b/gtests/ssl_gtest/ssl_agent_unittest.cc index f0c57e8b1..f97e9051e 100644 --- a/gtests/ssl_gtest/ssl_agent_unittest.cc +++ b/gtests/ssl_gtest/ssl_agent_unittest.cc @@ -8,9 +8,6 @@ #include "sslerr.h" #include "sslproto.h" -// This is an internal header, used to get TLS_1_3_DRAFT_VERSION. -#include "ssl3prot.h" - #include #include "databuffer.h" @@ -21,7 +18,6 @@ namespace nss_test { -static const uint8_t kD13 = TLS_1_3_DRAFT_VERSION; // This is a 1-RTT ClientHello with ECDHE. const static uint8_t kCannedTls13ClientHello[] = { 0x01, 0x00, 0x00, 0xcf, 0x03, 0x03, 0x6c, 0xb3, 0x46, 0x81, 0xc8, 0x1a, @@ -42,16 +38,7 @@ const static uint8_t kCannedTls13ClientHello[] = { 0x1e, 0x04, 0x03, 0x05, 0x03, 0x06, 0x03, 0x02, 0x03, 0x08, 0x04, 0x08, 0x05, 0x08, 0x06, 0x04, 0x01, 0x05, 0x01, 0x06, 0x01, 0x02, 0x01, 0x04, 0x02, 0x05, 0x02, 0x06, 0x02, 0x02, 0x02}; - -const static uint8_t kCannedTls13ServerHello[] = { - 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, 0x33, 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 size_t kFirstFragmentSize = 20; static const char *k0RttData = "ABCDEF"; TEST_P(TlsAgentTest, EarlyFinished) { @@ -74,8 +61,9 @@ TEST_P(TlsAgentTestClient13, CannedHello) { DataBuffer buffer; EnsureInit(); DataBuffer server_hello; - MakeHandshakeMessage(kTlsHandshakeServerHello, kCannedTls13ServerHello, - sizeof(kCannedTls13ServerHello), &server_hello); + auto sh = MakeCannedTls13ServerHello(); + MakeHandshakeMessage(kTlsHandshakeServerHello, sh.data(), sh.len(), + &server_hello); MakeRecord(kTlsHandshakeType, SSL_LIBRARY_VERSION_TLS_1_3, server_hello.data(), server_hello.len(), &buffer); ProcessMessage(buffer, TlsAgent::STATE_CONNECTING); @@ -83,8 +71,9 @@ TEST_P(TlsAgentTestClient13, CannedHello) { TEST_P(TlsAgentTestClient13, EncryptedExtensionsInClear) { DataBuffer server_hello; - MakeHandshakeMessage(kTlsHandshakeServerHello, kCannedTls13ServerHello, - sizeof(kCannedTls13ServerHello), &server_hello); + auto sh = MakeCannedTls13ServerHello(); + MakeHandshakeMessage(kTlsHandshakeServerHello, sh.data(), sh.len(), + &server_hello); DataBuffer encrypted_extensions; MakeHandshakeMessage(kTlsHandshakeEncryptedExtensions, nullptr, 0, &encrypted_extensions, 1); @@ -100,19 +89,21 @@ TEST_P(TlsAgentTestClient13, EncryptedExtensionsInClear) { TEST_F(TlsAgentStreamTestClient, EncryptedExtensionsInClearTwoPieces) { DataBuffer server_hello; - MakeHandshakeMessage(kTlsHandshakeServerHello, kCannedTls13ServerHello, - sizeof(kCannedTls13ServerHello), &server_hello); + auto sh = MakeCannedTls13ServerHello(); + MakeHandshakeMessage(kTlsHandshakeServerHello, sh.data(), sh.len(), + &server_hello); DataBuffer encrypted_extensions; MakeHandshakeMessage(kTlsHandshakeEncryptedExtensions, nullptr, 0, &encrypted_extensions, 1); server_hello.Append(encrypted_extensions); DataBuffer buffer; MakeRecord(kTlsHandshakeType, SSL_LIBRARY_VERSION_TLS_1_3, - server_hello.data(), 20, &buffer); + server_hello.data(), kFirstFragmentSize, &buffer); DataBuffer buffer2; MakeRecord(kTlsHandshakeType, SSL_LIBRARY_VERSION_TLS_1_3, - server_hello.data() + 20, server_hello.len() - 20, &buffer2); + server_hello.data() + kFirstFragmentSize, + server_hello.len() - kFirstFragmentSize, &buffer2); EnsureInit(); agent_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_3, @@ -124,15 +115,15 @@ TEST_F(TlsAgentStreamTestClient, EncryptedExtensionsInClearTwoPieces) { } TEST_F(TlsAgentDgramTestClient, EncryptedExtensionsInClearTwoPieces) { + auto sh = MakeCannedTls13ServerHello(); DataBuffer server_hello_frag1; - MakeHandshakeMessageFragment( - kTlsHandshakeServerHello, kCannedTls13ServerHello, - sizeof(kCannedTls13ServerHello), &server_hello_frag1, 0, 0, 20); + MakeHandshakeMessageFragment(kTlsHandshakeServerHello, sh.data(), sh.len(), + &server_hello_frag1, 0, 0, kFirstFragmentSize); DataBuffer server_hello_frag2; - MakeHandshakeMessageFragment( - kTlsHandshakeServerHello, kCannedTls13ServerHello + 20, - sizeof(kCannedTls13ServerHello), &server_hello_frag2, 0, 20, - sizeof(kCannedTls13ServerHello) - 20); + MakeHandshakeMessageFragment(kTlsHandshakeServerHello, + sh.data() + kFirstFragmentSize, sh.len(), + &server_hello_frag2, 0, kFirstFragmentSize, + sh.len() - kFirstFragmentSize); DataBuffer encrypted_extensions; MakeHandshakeMessage(kTlsHandshakeEncryptedExtensions, nullptr, 0, &encrypted_extensions, 1); diff --git a/gtests/ssl_gtest/ssl_hrr_unittest.cc b/gtests/ssl_gtest/ssl_hrr_unittest.cc index c78b328d8..68b5aa537 100644 --- a/gtests/ssl_gtest/ssl_hrr_unittest.cc +++ b/gtests/ssl_gtest/ssl_hrr_unittest.cc @@ -568,6 +568,28 @@ void TriggerHelloRetryRequest(std::shared_ptr& client, client->Handshake(); server->Handshake(); EXPECT_EQ(1U, cb_called); + // Stop the callback from being called in future handshakes. + EXPECT_EQ(SECSuccess, + SSL_HelloRetryRequestCallback(server->ssl_fd(), nullptr, nullptr)); +} + +TEST_P(TlsConnectTls13, VersionNumbersAfterRetry) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + auto r = MakeTlsFilter(client_); + TriggerHelloRetryRequest(client_, server_); + Handshake(); + ASSERT_GT(r->count(), 1UL); + auto ch1 = r->record(0); + if (ch1.header.is_dtls()) { + ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_1, ch1.header.version()); + } else { + ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_0, ch1.header.version()); + } + auto ch2 = r->record(1); + ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, ch2.header.version()); + + CheckConnected(); } TEST_P(TlsConnectTls13, RetryStateless) { @@ -578,6 +600,7 @@ TEST_P(TlsConnectTls13, RetryStateless) { MakeNewServer(); Handshake(); + CheckConnected(); SendReceive(); } @@ -908,7 +931,10 @@ class HelloRetryRequestAgentTest : public TlsAgentTestClient { hrr_data.Allocate(len + 6); size_t i = 0; - i = hrr_data.Write(i, 0x0303, 2); + i = hrr_data.Write(i, variant_ == ssl_variant_datagram + ? SSL_LIBRARY_VERSION_DTLS_1_2_WIRE + : SSL_LIBRARY_VERSION_TLS_1_2, + 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 diff --git a/gtests/ssl_gtest/tls_agent.cc b/gtests/ssl_gtest/tls_agent.cc index 084da0ab5..3ee0b68f4 100644 --- a/gtests/ssl_gtest/tls_agent.cc +++ b/gtests/ssl_gtest/tls_agent.cc @@ -44,6 +44,16 @@ const std::string TlsAgent::kServerEcdhRsa = "ecdh_rsa"; const std::string TlsAgent::kServerEcdhEcdsa = "ecdh_ecdsa"; const std::string TlsAgent::kServerDsa = "dsa"; +static const uint8_t kCannedTls13ServerHello[] = { + 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, 0x33, 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}; + TlsAgent::TlsAgent(const std::string& nm, Role rl, SSLProtocolVariant var) : name_(nm), variant_(var), @@ -1146,4 +1156,12 @@ void TlsAgentTestBase::MakeTrivialHandshakeRecord(uint8_t hs_type, } } +DataBuffer TlsAgentTestBase::MakeCannedTls13ServerHello() { + DataBuffer sh(kCannedTls13ServerHello, sizeof(kCannedTls13ServerHello)); + if (variant_ == ssl_variant_datagram) { + sh.Write(0, SSL_LIBRARY_VERSION_DTLS_1_2_WIRE, 2); + } + return sh; +} + } // namespace nss_test diff --git a/gtests/ssl_gtest/tls_agent.h b/gtests/ssl_gtest/tls_agent.h index 5ce5e6280..a00bd4be1 100644 --- a/gtests/ssl_gtest/tls_agent.h +++ b/gtests/ssl_gtest/tls_agent.h @@ -10,6 +10,9 @@ #include "prio.h" #include "ssl.h" +// This is an internal header, used to get TLS_1_3_DRAFT_VERSION. +#include "ssl3prot.h" + #include #include @@ -57,6 +60,8 @@ typedef std::function SniCallbackFunction; +static const uint8_t kD13 = TLS_1_3_DRAFT_VERSION; + class TlsAgent : public PollTarget { public: enum Role { CLIENT, SERVER }; @@ -442,6 +447,7 @@ class TlsAgentTestBase : public ::testing::Test { size_t hs_len, DataBuffer* out, uint64_t seq_num, uint32_t fragment_offset, uint32_t fragment_length) const; + DataBuffer MakeCannedTls13ServerHello(); static void MakeTrivialHandshakeRecord(uint8_t hs_type, size_t hs_len, DataBuffer* out); static inline TlsAgent::Role ToRole(const std::string& str) { diff --git a/gtests/ssl_gtest/tls_filter.cc b/gtests/ssl_gtest/tls_filter.cc index 10b1e31d0..5b49d9851 100644 --- a/gtests/ssl_gtest/tls_filter.cc +++ b/gtests/ssl_gtest/tls_filter.cc @@ -213,14 +213,14 @@ PacketFilter::Action TlsRecordFilter::FilterRecord( } size_t TlsRecordHeader::header_length() const { - if (!is_dtls()) { - return 5; + // If we have a header, return it's length. + if (header_.len()) { + return header_.len(); } - if (version() >= SSL_LIBRARY_VERSION_TLS_1_3 && - content_type_ == kTlsApplicationDataType) { - return 7; - } - return 13; + + // Otherwise make a dummy header and return the length. + DataBuffer buf; + return WriteHeader(&buf, 0, 0); } uint64_t TlsRecordHeader::RecoverSequenceNumber(uint64_t expected, @@ -265,6 +265,8 @@ uint64_t TlsRecordHeader::ParseSequenceNumber(uint64_t expected, uint32_t raw, bool TlsRecordHeader::Parse(bool is_dtls13, uint64_t seqno, TlsParser* parser, DataBuffer* body) { + auto mark = parser->consumed(); + if (!parser->Read(&content_type_)) { return false; } @@ -281,6 +283,10 @@ bool TlsRecordHeader::Parse(bool is_dtls13, uint64_t seqno, TlsParser* parser, return false; } sequence_number_ = ParseSequenceNumber(seqno, tmp, 30, 2); + if (!parser->ReadFromMark(&header_, parser->consumed() + 2 - mark, + mark)) { + return false; + } return parser->ReadVariable(body, 2); } @@ -294,6 +300,10 @@ bool TlsRecordHeader::Parse(bool is_dtls13, uint64_t seqno, TlsParser* parser, tmp |= (content_type_ & 0x1f) << 8; content_type_ = kTlsApplicationDataType; sequence_number_ = ParseSequenceNumber(seqno, tmp, 12, 1); + + if (!parser->ReadFromMark(&header_, parser->consumed() - mark, mark)) { + return false; + } return parser->Read(body, parser->remaining()); } @@ -327,11 +337,14 @@ bool TlsRecordHeader::Parse(bool is_dtls13, uint64_t seqno, TlsParser* parser, } else { sequence_number_ = seqno; } + if (!parser->ReadFromMark(&header_, parser->consumed() + 2 - mark, mark)) { + return false; + } return parser->ReadVariable(body, 2); } -size_t TlsRecordHeader::Write(DataBuffer* buffer, size_t offset, - const DataBuffer& body) const { +size_t TlsRecordHeader::WriteHeader(DataBuffer* buffer, size_t offset, + size_t body_len) const { offset = buffer->Write(offset, content_type_, 1); if (is_dtls() && version_ >= SSL_LIBRARY_VERSION_TLS_1_3 && content_type() == kTlsApplicationDataType) { @@ -349,7 +362,13 @@ size_t TlsRecordHeader::Write(DataBuffer* buffer, size_t offset, offset = buffer->Write(offset, sequence_number_ & 0xffffffff, 4); } } - offset = buffer->Write(offset, body.len(), 2); + offset = buffer->Write(offset, body_len, 2); + return offset; +} + +size_t TlsRecordHeader::Write(DataBuffer* buffer, size_t offset, + const DataBuffer& body) const { + offset = WriteHeader(buffer, offset, body.len()); offset = buffer->Write(offset, body); return offset; } diff --git a/gtests/ssl_gtest/tls_filter.h b/gtests/ssl_gtest/tls_filter.h index 296c0a9ee..90520361c 100644 --- a/gtests/ssl_gtest/tls_filter.h +++ b/gtests/ssl_gtest/tls_filter.h @@ -44,10 +44,14 @@ class TlsVersioned { class TlsRecordHeader : public TlsVersioned { public: - TlsRecordHeader() : TlsVersioned(), content_type_(0), sequence_number_(0) {} + TlsRecordHeader() + : TlsVersioned(), content_type_(0), sequence_number_(0), header_() {} TlsRecordHeader(SSLProtocolVariant var, uint16_t ver, uint8_t ct, uint64_t seqno) - : TlsVersioned(var, ver), content_type_(ct), sequence_number_(seqno) {} + : TlsVersioned(var, ver), + content_type_(ct), + sequence_number_(seqno), + header_() {} uint8_t content_type() const { return content_type_; } uint64_t sequence_number() const { return sequence_number_; } @@ -55,12 +59,15 @@ class TlsRecordHeader : public TlsVersioned { return static_cast(sequence_number_ >> 48); } size_t header_length() const; + const DataBuffer& header() const { return header_; } + // Parse the header; return true if successful; body in an outparam if OK. bool Parse(bool is_dtls13, uint64_t sequence_number, TlsParser* parser, DataBuffer* body); // Write the header and body to a buffer at the given offset. // Return the offset of the end of the write. size_t Write(DataBuffer* buffer, size_t offset, const DataBuffer& body) const; + size_t WriteHeader(DataBuffer* buffer, size_t offset, size_t body_len) const; private: static uint64_t RecoverSequenceNumber(uint64_t expected, uint32_t partial, @@ -70,6 +77,7 @@ class TlsRecordHeader : public TlsVersioned { uint8_t content_type_; uint64_t sequence_number_; + DataBuffer header_; }; struct TlsRecord { diff --git a/gtests/ssl_gtest/tls_protect.cc b/gtests/ssl_gtest/tls_protect.cc index 7606e034d..c715a36a6 100644 --- a/gtests/ssl_gtest/tls_protect.cc +++ b/gtests/ssl_gtest/tls_protect.cc @@ -54,17 +54,17 @@ bool AeadCipher::AeadInner(bool decrypt, void *params, size_t param_length, return rv == SECSuccess; } -bool AeadCipherAesGcm::Aead(bool decrypt, uint64_t seq, const uint8_t *in, - size_t inlen, uint8_t *out, size_t *outlen, - size_t maxlen) { +bool AeadCipherAesGcm::Aead(bool decrypt, const uint8_t *hdr, size_t hdr_len, + uint64_t seq, const uint8_t *in, size_t inlen, + uint8_t *out, size_t *outlen, size_t maxlen) { CK_GCM_PARAMS aeadParams; unsigned char nonce[12]; memset(&aeadParams, 0, sizeof(aeadParams)); aeadParams.pIv = nonce; aeadParams.ulIvLen = sizeof(nonce); - aeadParams.pAAD = NULL; - aeadParams.ulAADLen = 0; + aeadParams.pAAD = const_cast(hdr); + aeadParams.ulAADLen = hdr_len; aeadParams.ulTagBits = 128; FormatNonce(seq, nonce); @@ -72,7 +72,8 @@ bool AeadCipherAesGcm::Aead(bool decrypt, uint64_t seq, const uint8_t *in, in, inlen, out, outlen, maxlen); } -bool AeadCipherChacha20Poly1305::Aead(bool decrypt, uint64_t seq, +bool AeadCipherChacha20Poly1305::Aead(bool decrypt, const uint8_t *hdr, + size_t hdr_len, uint64_t seq, const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen, size_t maxlen) { @@ -82,8 +83,8 @@ bool AeadCipherChacha20Poly1305::Aead(bool decrypt, uint64_t seq, memset(&aeadParams, 0, sizeof(aeadParams)); aeadParams.pNonce = nonce; aeadParams.ulNonceLen = sizeof(nonce); - aeadParams.pAAD = NULL; - aeadParams.ulAADLen = 0; + aeadParams.pAAD = const_cast(hdr); + aeadParams.ulAADLen = hdr_len; aeadParams.ulTagLen = 16; FormatNonce(seq, nonce); @@ -114,10 +115,12 @@ bool TlsCipherSpec::Unprotect(const TlsRecordHeader &header, // Make space. plaintext->Allocate(ciphertext.len()); + auto header_bytes = header.header(); size_t len; bool ret = - aead_->Aead(true, header.sequence_number(), ciphertext.data(), - ciphertext.len(), plaintext->data(), &len, plaintext->len()); + aead_->Aead(true, header_bytes.data(), header_bytes.len(), + header.sequence_number(), ciphertext.data(), ciphertext.len(), + plaintext->data(), &len, plaintext->len()); if (!ret) return false; plaintext->Truncate(len); @@ -133,9 +136,13 @@ bool TlsCipherSpec::Protect(const TlsRecordHeader &header, ciphertext->Allocate(plaintext.len() + 32); // Room for any plausible auth tag size_t len; + + DataBuffer header_bytes; + (void)header.WriteHeader(&header_bytes, 0, plaintext.len() + 16); bool ret = - aead_->Aead(false, header.sequence_number(), plaintext.data(), - plaintext.len(), ciphertext->data(), &len, ciphertext->len()); + aead_->Aead(false, header_bytes.data(), header_bytes.len(), + header.sequence_number(), plaintext.data(), plaintext.len(), + ciphertext->data(), &len, ciphertext->len()); if (!ret) return false; ciphertext->Truncate(len); diff --git a/gtests/ssl_gtest/tls_protect.h b/gtests/ssl_gtest/tls_protect.h index 93ffd6322..6f129a4eb 100644 --- a/gtests/ssl_gtest/tls_protect.h +++ b/gtests/ssl_gtest/tls_protect.h @@ -23,8 +23,9 @@ class AeadCipher { virtual ~AeadCipher(); bool Init(PK11SymKey *key, const uint8_t *iv); - virtual bool Aead(bool decrypt, uint64_t seq, const uint8_t *in, size_t inlen, - uint8_t *out, size_t *outlen, size_t maxlen) = 0; + virtual bool Aead(bool decrypt, const uint8_t *hdr, size_t hdr_len, + uint64_t seq, const uint8_t *in, size_t inlen, uint8_t *out, + size_t *outlen, size_t maxlen) = 0; protected: void FormatNonce(uint64_t seq, uint8_t *nonce); @@ -42,8 +43,9 @@ class AeadCipherChacha20Poly1305 : public AeadCipher { AeadCipherChacha20Poly1305() : AeadCipher(CKM_NSS_CHACHA20_POLY1305) {} protected: - bool Aead(bool decrypt, uint64_t seq, const uint8_t *in, size_t inlen, - uint8_t *out, size_t *outlen, size_t maxlen); + bool Aead(bool decrypt, const uint8_t *hdr, size_t hdr_len, uint64_t seq, + const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen, + size_t maxlen); }; class AeadCipherAesGcm : public AeadCipher { @@ -51,8 +53,9 @@ class AeadCipherAesGcm : public AeadCipher { AeadCipherAesGcm() : AeadCipher(CKM_AES_GCM) {} protected: - bool Aead(bool decrypt, uint64_t seq, const uint8_t *in, size_t inlen, - uint8_t *out, size_t *outlen, size_t maxlen); + bool Aead(bool decrypt, const uint8_t *hdr, size_t hdr_len, uint64_t seq, + const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen, + size_t maxlen); }; // Our analog of ssl3CipherSpec diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 22fdaf5b1..add561e24 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -990,27 +990,22 @@ ssl_ClientReadVersion(sslSocket *ss, PRUint8 **b, unsigned int *len, if (rv != SECSuccess) { return SECFailure; /* alert has been sent */ } - -#ifdef TLS_1_3_DRAFT_VERSION - if (temp == SSL_LIBRARY_VERSION_TLS_1_3) { - (void)SSL3_SendAlert(ss, alert_fatal, protocol_version); - PORT_SetError(SSL_ERROR_UNSUPPORTED_VERSION); - return SECFailure; - } - if (temp == tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3)) { - v = SSL_LIBRARY_VERSION_TLS_1_3; - } else { - v = (SSL3ProtocolVersion)temp; - } -#else v = (SSL3ProtocolVersion)temp; -#endif if (IS_DTLS(ss)) { - /* If this fails, we get 0 back and the next check to fails. */ v = dtls_DTLSVersionToTLSVersion(v); + /* Check for failure. */ + if (!v || v > SSL_LIBRARY_VERSION_MAX_SUPPORTED) { + SSL3_SendAlert(ss, alert_fatal, illegal_parameter); + return SECFailure; + } } + /* You can't negotiate TLS 1.3 this way. */ + if (v >= SSL_LIBRARY_VERSION_TLS_1_3) { + SSL3_SendAlert(ss, alert_fatal, illegal_parameter); + return SECFailure; + } *version = v; return SECSuccess; } @@ -2159,7 +2154,7 @@ ssl3_MACEncryptRecord(ssl3CipherSpec *cwSpec, } /* Note: though this can report failure, it shouldn't. */ -static SECStatus +SECStatus ssl_InsertRecordHeader(const sslSocket *ss, ssl3CipherSpec *cwSpec, SSL3ContentType contentType, sslBuffer *wrBuf, PRBool *needsLength) @@ -6176,7 +6171,6 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) 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 @@ -6267,16 +6261,10 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) } } - /* Update the version based on the extension, as necessary. */ - versionExtension = ssl3_FindExtension(ss, ssl_tls13_supported_versions_xtn); - if (versionExtension) { - rv = ssl_ClientReadVersion(ss, &versionExtension->data.data, - &versionExtension->data.len, - &ss->version); - if (rv != SECSuccess) { - errCode = PORT_GetError(); - goto loser; /* An alert is sent by ssl_ClientReadVersion */ - } + /* Read supported_versions if present. */ + rv = tls13_ClientReadSupportedVersion(ss); + if (rv != SECSuccess) { + goto loser; } PORT_Assert(!SSL_ALL_VERSIONS_DISABLED(&ss->vrange)); @@ -6361,8 +6349,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. But not after - * HelloRetryRequest, because cwSpec might be a 0-RTT cipher spec. */ - if (!ss->firstHsDone && !ss->ssl3.hs.helloRetry) { + * HelloRetryRequest, because cwSpec might be a 0-RTT cipher spec, + * in which case this is a no-op. */ + if (!ss->firstHsDone && !isHelloRetry) { ssl_GetSpecWriteLock(ss); ssl_SetSpecVersions(ss, ss->ssl3.cwSpec); ssl_ReleaseSpecWriteLock(ss); @@ -8859,12 +8848,10 @@ ssl_ConstructServerHello(sslSocket *ss, PRBool helloRetry, 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); + version = PR_MIN(ss->version, SSL_LIBRARY_VERSION_TLS_1_2); + if (IS_DTLS(ss)) { + version = dtls_TLSVersionToDTLSVersion(version); } - rv = sslBuffer_AppendNumber(messageBuf, version, 2); if (rv != SECSuccess) { return SECFailure; diff --git a/lib/ssl/ssl3gthr.c b/lib/ssl/ssl3gthr.c index b0dd7315f..0f01c0107 100644 --- a/lib/ssl/ssl3gthr.c +++ b/lib/ssl/ssl3gthr.c @@ -158,6 +158,7 @@ ssl3_GatherData(sslSocket *ss, sslGather *gs, int flags, ssl2Gather *ssl2gs) * the length of the following encrypted data, and then * read in the rest of the record into gs->inbuf. */ gs->remainder = (gs->hdr[3] << 8) | gs->hdr[4]; + gs->hdrLen = SSL3_RECORD_HEADER_LENGTH; } else { /* Probably an SSLv2 record header. No need to handle any * security escapes (gs->hdr[0] & 0x40) as we wouldn't get @@ -340,6 +341,7 @@ dtls_GatherData(sslSocket *ss, sslGather *gs, int flags) } memcpy(gs->hdr, SSL_BUFFER_BASE(&gs->dtlsPacket) + gs->dtlsPacketOffset, headerLen); + gs->hdrLen = headerLen; gs->dtlsPacketOffset += headerLen; /* Have received SSL3 record header in gs->hdr. */ @@ -515,11 +517,12 @@ ssl3_GatherCompleteHandshake(sslSocket *ss, int flags) * If it's application data, ss->gs.buf will not be empty upon return. * If it's a change cipher spec, alert, or handshake message, * ss->gs.buf.len will be 0 when ssl3_HandleRecord returns SECSuccess. - * + * * cText only needs to be valid for this next function call, so * it can borrow gs.hdr. */ cText.hdr = ss->gs.hdr; + cText.hdrLen = ss->gs.hdrLen; cText.buf = &ss->gs.inbuf; rv = ssl3_HandleRecord(ss, &cText); } diff --git a/lib/ssl/ssl3prot.h b/lib/ssl/ssl3prot.h index d1f46db97..7f98dd6dc 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 23 +#define TLS_1_3_DRAFT_VERSION 26 typedef PRUint16 ssl3CipherSuite; /* The cipher suites are defined in sslproto.h */ diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index 799049c32..3c01ab436 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -326,9 +326,11 @@ struct sslGatherStr { ** than into buf or inbuf, while in the GS_HEADER state. ** The portion of the SSL record header put here always comes off the wire ** as plaintext, never ciphertext. - ** For SSL3/TLS, the plaintext portion is 5 bytes long. For DTLS it is 13. + ** For SSL3/TLS, the plaintext portion is 5 bytes long. For DTLS it + ** varies based on version and header type. */ unsigned char hdr[13]; + unsigned int hdrLen; /* Buffer for DTLS data read off the wire as a single datagram */ sslBuffer dtlsPacket; @@ -785,6 +787,8 @@ typedef struct { sslSequenceNumber seqNum; /* The header of the cipherText. */ const PRUint8 *hdr; + unsigned int hdrLen; + /* |buf| is the payload of the ciphertext. */ sslBuffer *buf; } SSL3Ciphertext; @@ -1642,6 +1646,9 @@ SSLHashType ssl_SignatureSchemeToHashType(SSLSignatureScheme scheme); KeyType ssl_SignatureSchemeToKeyType(SSLSignatureScheme scheme); SECStatus ssl3_SetupCipherSuite(sslSocket *ss, PRBool initHashes); +SECStatus ssl_InsertRecordHeader(const sslSocket *ss, ssl3CipherSpec *cwSpec, + SSL3ContentType contentType, sslBuffer *wrBuf, + PRBool *needsLength); /* Pull in DTLS functions */ #include "dtlscon.h" diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c index ed0bb5ecb..99652ac37 100644 --- a/lib/ssl/tls13con.c +++ b/lib/ssl/tls13con.c @@ -2209,6 +2209,8 @@ tls13_HandleHelloRetryRequest(sslSocket *ss, const PRUint8 *savedMsg, } else { PORT_Assert(ss->ssl3.hs.zeroRttState == ssl_0rtt_none); } + /* Set the spec version, because we want to send CH now with 0303 */ + tls13_SetSpecRecordVersion(ss, ss->ssl3.cwSpec); /* 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 @@ -2248,6 +2250,7 @@ tls13_HandleHelloRetryRequest(sslSocket *ss, const PRUint8 *savedMsg, goto loser; } } + rv = ssl3_SendClientHello(ss, client_hello_retry); if (rv != SECSuccess) { goto loser; @@ -3536,14 +3539,15 @@ tls13_AESGCM(ssl3KeyMaterial *keys, CK_GCM_PARAMS gcmParams; unsigned char nonce[12]; + PORT_Assert(additionalDataLen > 8); memset(&gcmParams, 0, sizeof(gcmParams)); gcmParams.pIv = nonce; gcmParams.ulIvLen = sizeof(nonce); - gcmParams.pAAD = NULL; - gcmParams.ulAADLen = 0; + gcmParams.pAAD = (PRUint8 *)(additionalData + 8); + gcmParams.ulAADLen = additionalDataLen - 8; gcmParams.ulTagBits = 128; /* GCM measures tag length in bits. */ - tls13_WriteNonce(keys, additionalData, additionalDataLen, + tls13_WriteNonce(keys, additionalData, 8, nonce, sizeof(nonce)); return tls13_AEAD(keys, doDecrypt, out, outlen, maxout, in, inlen, CKM_AES_GCM, @@ -3560,14 +3564,15 @@ tls13_ChaCha20Poly1305(ssl3KeyMaterial *keys, PRBool doDecrypt, CK_NSS_AEAD_PARAMS aeadParams; unsigned char nonce[12]; + PORT_Assert(additionalDataLen > 8); memset(&aeadParams, 0, sizeof(aeadParams)); aeadParams.pNonce = nonce; aeadParams.ulNonceLen = sizeof(nonce); - aeadParams.pAAD = NULL; /* No AAD in TLS 1.3. */ - aeadParams.ulAADLen = 0; + aeadParams.pAAD = (PRUint8 *)(additionalData + 8); + aeadParams.ulAADLen = additionalDataLen - 8; aeadParams.ulTagLen = 16; /* The Poly1305 tag is 16 octets. */ - tls13_WriteNonce(keys, additionalData, additionalDataLen, + tls13_WriteNonce(keys, additionalData, 8, nonce, sizeof(nonce)); return tls13_AEAD(keys, doDecrypt, out, outlen, maxout, in, inlen, CKM_NSS_CHACHA20_POLY1305, @@ -4780,19 +4785,20 @@ tls13_ExtensionStatus(PRUint16 extension, SSLHandshakeType message) #undef _M2 #undef _M3 -/* TLS 1.3 doesn't actually have additional data but the aead function - * signature overloads additional data to carry the record sequence - * number and that's what we put here. The TLS 1.3 AEAD functions - * just use this input as the sequence number and not as additional - * data. */ +/* We cheat a bit on additional data because the AEAD interface + * which doesn't have room for the record number. The AAD we + * format is serialized record number followed by the true AD + * (i.e., the record header) plus the serialized record number. */ static SECStatus -tls13_FormatAdditionalData(sslSocket *ss, PRUint8 *aad, unsigned int length, - DTLSEpoch epoch, sslSequenceNumber seqNum) +tls13_FormatAdditionalData( + sslSocket *ss, + const PRUint8 *header, unsigned int headerLen, + DTLSEpoch epoch, sslSequenceNumber seqNum, + PRUint8 *aad, unsigned int *aadLength, unsigned int maxLength) { SECStatus rv; - sslBuffer buf = SSL_BUFFER_FIXED(aad, length); + sslBuffer buf = SSL_BUFFER_FIXED(aad, maxLength); - PORT_Assert(length == 8); if (IS_DTLS(ss)) { rv = sslBuffer_AppendNumber(&buf, epoch, 2); if (rv != SECSuccess) { @@ -4803,6 +4809,14 @@ tls13_FormatAdditionalData(sslSocket *ss, PRUint8 *aad, unsigned int length, if (rv != SECSuccess) { return SECFailure; } + + rv = sslBuffer_Append(&buf, header, headerLen); + if (rv != SECSuccess) { + return SECFailure; + } + + *aadLength = buf.len; + return SECSuccess; } @@ -4859,15 +4873,35 @@ tls13_ProtectRecord(sslSocket *ss, rv = sslBuffer_Skip(wrBuf, contentLen, NULL); PORT_Assert(rv == SECSuccess); } else { - PRUint8 aad[8]; + PRUint8 hdr[13]; + sslBuffer buf = SSL_BUFFER_FIXED(hdr, sizeof(hdr)); + PRBool needsLength; + PRUint8 aad[21]; + unsigned int aadLen; int len; + PORT_Assert(cipher_def->type == type_aead); /* Add the content type at the end. */ *(SSL_BUFFER_NEXT(wrBuf) + contentLen) = type; - rv = tls13_FormatAdditionalData(ss, aad, sizeof(aad), cwSpec->epoch, - cwSpec->nextSeqNum); + /* Create the header (ugly that we have to do it twice). */ + rv = ssl_InsertRecordHeader(ss, cwSpec, content_application_data, + &buf, &needsLength); + if (rv != SECSuccess) { + return SECFailure; + } + if (needsLength) { + rv = sslBuffer_AppendNumber(&buf, contentLen + 1 + + cwSpec->cipherDef->tag_size, + 2); + if (rv != SECSuccess) { + return SECFailure; + } + } + rv = tls13_FormatAdditionalData(ss, SSL_BUFFER_BASE(&buf), SSL_BUFFER_LEN(&buf), + cwSpec->epoch, cwSpec->nextSeqNum, + aad, &aadLen, sizeof(aad)); if (rv != SECSuccess) { return SECFailure; } @@ -4878,7 +4912,7 @@ tls13_ProtectRecord(sslSocket *ss, SSL_BUFFER_SPACE(wrBuf), /* max out */ SSL_BUFFER_NEXT(wrBuf), /* input */ contentLen + 1, /* input len */ - aad, sizeof(aad)); + aad, aadLen); if (rv != SECSuccess) { PORT_SetError(SSL_ERROR_ENCRYPTION_FAILURE); return SECFailure; @@ -4908,7 +4942,8 @@ tls13_UnprotectRecord(sslSocket *ss, SSL3AlertDescription *alert) { const ssl3BulkCipherDef *cipher_def = spec->cipherDef; - PRUint8 aad[8]; + PRUint8 aad[21]; + unsigned int aadLen; SECStatus rv; *alert = bad_record_mac; /* Default alert for most issues. */ @@ -4957,8 +4992,9 @@ tls13_UnprotectRecord(sslSocket *ss, /* Decrypt */ PORT_Assert(cipher_def->type == type_aead); - rv = tls13_FormatAdditionalData(ss, aad, sizeof(aad), spec->epoch, - cText->seqNum); + rv = tls13_FormatAdditionalData(ss, cText->hdr, cText->hdrLen, + spec->epoch, cText->seqNum, + aad, &aadLen, sizeof(aad)); if (rv != SECSuccess) { return SECFailure; } @@ -4969,7 +5005,7 @@ tls13_UnprotectRecord(sslSocket *ss, plaintext->space, /* maxout */ cText->buf->buf, /* in */ cText->buf->len, /* inlen */ - aad, sizeof(aad)); + aad, aadLen); if (rv != SECSuccess) { SSL_TRC(3, ("%d: TLS13[%d]: record has bogus MAC", @@ -5234,6 +5270,58 @@ tls13_EncodeDraftVersion(SSL3ProtocolVersion version) return (PRUint16)version; } +SECStatus +tls13_ClientReadSupportedVersion(sslSocket *ss) +{ + PRUint32 temp; + SSL3ProtocolVersion v; + TLSExtension *versionExtension; + SECItem it; + SECStatus rv; + + /* Update the version based on the extension, as necessary. */ + versionExtension = ssl3_FindExtension(ss, ssl_tls13_supported_versions_xtn); + if (!versionExtension) { + return SECSuccess; + } + + /* Struct copy so we don't damage the extension. */ + it = versionExtension->data; + + rv = ssl3_ConsumeHandshakeNumber(ss, &temp, 2, &it.data, &it.len); + if (rv != SECSuccess) { + return SECFailure; + } + if (it.len) { + FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_SERVER_HELLO, illegal_parameter); + return SECFailure; + } + v = (SSL3ProtocolVersion)temp; + + /* You cannot negotiate < TLS 1.3 with supported_versions. */ + if (v < SSL_LIBRARY_VERSION_TLS_1_3) { + FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_SERVER_HELLO, illegal_parameter); + return SECFailure; + } + +#ifdef TLS_1_3_DRAFT_VERSION + if (temp == SSL_LIBRARY_VERSION_TLS_1_3) { + FATAL_ERROR(ss, SSL_ERROR_UNSUPPORTED_VERSION, protocol_version); + return SECFailure; + } + if (temp == tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3)) { + v = SSL_LIBRARY_VERSION_TLS_1_3; + } else { + v = (SSL3ProtocolVersion)temp; + } +#else + v = (SSL3ProtocolVersion)temp; +#endif + + ss->version = v; + return SECSuccess; +} + /* Pick the highest version we support that is also advertised. */ SECStatus tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supportedVersions) diff --git a/lib/ssl/tls13con.h b/lib/ssl/tls13con.h index 7ef1fabc2..f35b20023 100644 --- a/lib/ssl/tls13con.h +++ b/lib/ssl/tls13con.h @@ -102,6 +102,7 @@ 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); +SECStatus tls13_ClientReadSupportedVersion(sslSocket *ss); SECStatus tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supported_versions); -- cgit v1.2.1