From 0f52cc4899a0b2b0fe2193f1d5703f9994b93b36 Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Fri, 27 Oct 2017 16:45:10 +1100 Subject: [PATCH] Bug 1411475 - Set the record layer version, r=ekr This refactors the code so that the cipher specs have a field that includes the record layer version number. This is used to cap the ClientHello record version, as well as set the 1.0/1.2 version as necessary for the two TLS 1.3 versions. --HG-- branch : NSS_TLS13_DRAFT19_BRANCH extra : rebase_source : 3b33a6c25c037d23e14270a4e8bf7da2d7f7d0a6 extra : amend_source : 19d43f91a6cd33d4baa16ad04adff5bcb22d86a8 extra : source : f5b94d346fdbad4e611435e37a0ee8090fce439f --- gtests/ssl_gtest/ssl_alths_unittest.cc | 17 ++- lib/ssl/ssl3con.c | 157 +++++++++++-------------- lib/ssl/ssl3gthr.c | 1 - lib/ssl/sslimpl.h | 2 - lib/ssl/sslspec.c | 20 ++++ lib/ssl/sslspec.h | 1 + lib/ssl/tls13con.c | 58 ++++++--- lib/ssl/tls13con.h | 1 + 8 files changed, 147 insertions(+), 110 deletions(-) diff --git a/gtests/ssl_gtest/ssl_alths_unittest.cc b/gtests/ssl_gtest/ssl_alths_unittest.cc index f3133313bc..2d6ae1d2dd 100644 --- a/gtests/ssl_gtest/ssl_alths_unittest.cc +++ b/gtests/ssl_gtest/ssl_alths_unittest.cc @@ -18,7 +18,7 @@ namespace nss_test { static const uint32_t kServerHelloVersionAlt = SSL_LIBRARY_VERSION_TLS_1_2; -static const uint32_t kServerHelloVersionRegular = +static const uint16_t kServerHelloVersionRegular = 0x7f00 | TLS_1_3_DRAFT_VERSION; class AltHandshakeTest : public TlsConnectStreamTls13 { @@ -27,6 +27,8 @@ class AltHandshakeTest : public TlsConnectStreamTls13 { 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_ = @@ -42,11 +44,17 @@ class AltHandshakeTest : public TlsConnectStreamTls13 { void InstallFilters() { client_->SetPacketFilter(client_ccs_recorder_); auto chain = std::make_shared(ChainedPacketFilterInit( - {server_ccs_recorder_, server_hello_recorder_})); + {server_handshake_recorder_, server_ccs_recorder_, + server_hello_recorder_})); server_->SetPacketFilter(chain); } - void CheckServerHelloVersion(uint32_t server_hello_version) { + 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); @@ -56,15 +64,18 @@ class AltHandshakeTest : public TlsConnectStreamTls13 { 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_; }; diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 5a8e43ac02..0d27bbfce2 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -919,6 +919,19 @@ Null_Cipher(void *ctx, unsigned char *output, int *outputLen, int maxOutputLen, * SSL3 Utility functions */ +static void +ssl_SetSpecVersions(sslSocket *ss, ssl3CipherSpec *spec) +{ + spec->version = ss->version; + if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) { + tls13_SetSpecRecordVersion(ss, spec); + } else if (IS_DTLS(ss)) { + spec->recordVersion = dtls_TLSVersionToDTLSVersion(ss->version); + } else { + spec->recordVersion = ss->version; + } +} + /* allowLargerPeerVersion controls whether the function will select the * highest enabled SSL version or fail when peerVersion is greater than the * highest enabled version. @@ -1405,6 +1418,7 @@ ssl3_SetupPendingCipherSpec(sslSocket *ss, CipherSpecDirection direction, if (IS_DTLS(ss) && direction == CipherSpecRead) { dtls_InitRecvdRecords(&spec->recvdRecords); } + ssl_SetSpecVersions(ss, spec); ssl_SaveCipherSpec(ss, spec); *specp = spec; @@ -1517,13 +1531,7 @@ ssl3_BuildRecordPseudoHeader(DTLSEpoch epoch, /* SSL3 MAC doesn't include the record's version field. */ if (includesVersion) { /* TLS MAC and AEAD additional data include version. */ - if (isDTLS) { - rv = sslBuffer_AppendNumber(buf, - dtls_TLSVersionToDTLSVersion(version), - 2); - } else { - rv = sslBuffer_AppendNumber(buf, version, 2); - } + rv = sslBuffer_AppendNumber(buf, version, 2); if (rv != SECSuccess) { return SECFailure; } @@ -1983,7 +1991,6 @@ SECStatus ssl3_MACEncryptRecord(ssl3CipherSpec *cwSpec, PRBool isServer, PRBool isDTLS, - PRBool capRecordVersion, SSL3ContentType type, const PRUint8 *pIn, PRUint32 contentLen, @@ -2028,7 +2035,7 @@ ssl3_MACEncryptRecord(ssl3CipherSpec *cwSpec, rv = ssl3_BuildRecordPseudoHeader( cwSpec->epoch, cwSpec->seqNum, type, - cwSpec->version >= SSL_LIBRARY_VERSION_TLS_1_0, cwSpec->version, + cwSpec->version >= SSL_LIBRARY_VERSION_TLS_1_0, cwSpec->recordVersion, isDTLS, contentLen, &pseudoHeader); PORT_Assert(rv == SECSuccess); if (cwSpec->cipherDef->type == type_aead) { @@ -2142,35 +2149,29 @@ ssl3_MACEncryptRecord(ssl3CipherSpec *cwSpec, } /* Note: though this can report failure, it shouldn't. */ -SECStatus +static SECStatus ssl_InsertRecordHeader(const sslSocket *ss, ssl3CipherSpec *cwSpec, - PRBool capRecordVersion, SSL3ContentType type, - unsigned int len, sslBuffer *wrBuf) + SSL3ContentType contentType, unsigned int len, + sslBuffer *wrBuf) { - SSL3ProtocolVersion version = cwSpec->version; - PRBool isTLS13 = (PRBool)(version >= SSL_LIBRARY_VERSION_TLS_1_3); SECStatus rv; #ifndef UNSAFE_FUZZER_MODE - if (isTLS13 && cwSpec->cipherDef->calg != ssl_calg_null) { - rv = sslBuffer_AppendNumber(wrBuf, content_application_data, 1); - } else -#endif - { - rv = sslBuffer_AppendNumber(wrBuf, type, 1); + if (cwSpec->version >= SSL_LIBRARY_VERSION_TLS_1_3 && + cwSpec->cipherDef->calg != ssl_calg_null) { + contentType = content_application_data; } +#endif + rv = sslBuffer_AppendNumber(wrBuf, contentType, 1); if (rv != SECSuccess) { return SECFailure; } + rv = sslBuffer_AppendNumber(wrBuf, cwSpec->recordVersion, 2); + if (rv != SECSuccess) { + return SECFailure; + } if (IS_DTLS(ss)) { - version = isTLS13 ? SSL_LIBRARY_VERSION_TLS_1_1 : version; - version = dtls_TLSVersionToDTLSVersion(version); - - rv = sslBuffer_AppendNumber(wrBuf, version, 2); - if (rv != SECSuccess) { - return SECFailure; - } rv = sslBuffer_AppendNumber(wrBuf, cwSpec->epoch, 2); if (rv != SECSuccess) { return SECFailure; @@ -2179,14 +2180,6 @@ ssl_InsertRecordHeader(const sslSocket *ss, ssl3CipherSpec *cwSpec, if (rv != SECSuccess) { return SECFailure; } - } else { - if (capRecordVersion || isTLS13) { - version = PR_MIN(SSL_LIBRARY_VERSION_TLS_1_0, version); - } - rv = sslBuffer_AppendNumber(wrBuf, version, 2); - if (rv != SECSuccess) { - return SECFailure; - } } rv = sslBuffer_AppendNumber(wrBuf, len, 2); if (rv != SECSuccess) { @@ -2197,8 +2190,7 @@ ssl_InsertRecordHeader(const sslSocket *ss, ssl3CipherSpec *cwSpec, } SECStatus -ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, - PRBool capRecordVersion, SSL3ContentType type, +ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, SSL3ContentType type, const PRUint8 *pIn, PRUint32 contentLen, sslBuffer *wrBuf) { unsigned int headerLen = IS_DTLS(ss) ? DTLS_RECORD_HEADER_LENGTH @@ -2235,8 +2227,7 @@ ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, if (isTLS13) { rv = tls13_ProtectRecord(ss, cwSpec, type, pIn, contentLen, &protBuf); } else { - rv = ssl3_MACEncryptRecord(cwSpec, ss->sec.isServer, - IS_DTLS(ss), capRecordVersion, type, + rv = ssl3_MACEncryptRecord(cwSpec, ss->sec.isServer, IS_DTLS(ss), type, pIn, contentLen, &protBuf); } #endif @@ -2246,8 +2237,8 @@ ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, PORT_Assert(protBuf.len <= MAX_FRAGMENT_LENGTH + (isTLS13 ? 256 : 1024)); - rv = ssl_InsertRecordHeader(ss, cwSpec, capRecordVersion, type, - SSL_BUFFER_LEN(&protBuf), wrBuf); + rv = ssl_InsertRecordHeader(ss, cwSpec, type, SSL_BUFFER_LEN(&protBuf), + wrBuf); if (rv != SECSuccess) { return SECFailure; } @@ -2264,8 +2255,7 @@ ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, } SECStatus -ssl_ProtectNextRecord(sslSocket *ss, ssl3CipherSpec *spec, - SSL3ContentType type, PRBool capRecordVersion, +ssl_ProtectNextRecord(sslSocket *ss, ssl3CipherSpec *spec, SSL3ContentType type, const PRUint8 *pIn, unsigned int nIn, unsigned int *written) { @@ -2289,8 +2279,7 @@ ssl_ProtectNextRecord(sslSocket *ss, ssl3CipherSpec *spec, } } - rv = ssl_ProtectRecord(ss, spec, capRecordVersion, - type, pIn, contentLen, wrBuf); + rv = ssl_ProtectRecord(ss, spec, type, pIn, contentLen, wrBuf); if (rv != SECSuccess) { return SECFailure; } @@ -2320,16 +2309,6 @@ ssl_ProtectNextRecord(sslSocket *ss, ssl3CipherSpec *spec, * all ciphertext into the pending ciphertext buffer. * ssl_SEND_FLAG_USE_EPOCH (for DTLS) * Forces the use of the provided epoch - * ssl_SEND_FLAG_CAP_RECORD_VERSION - * Caps the record layer version number of TLS ClientHello to { 3, 1 } - * (TLS 1.0). Some TLS 1.0 servers (which seem to use F5 BIG-IP) ignore - * ClientHello.client_version and use the record layer version number - * (TLSPlaintext.version) instead when negotiating protocol versions. In - * addition, if the record layer version number of ClientHello is { 3, 2 } - * (TLS 1.1) or higher, these servers reset the TCP connections. Lastly, - * some F5 BIG-IP servers hang if a record containing a ClientHello has a - * version greater than { 3, 1 } and a length greater than 255. Set this - * flag to work around such servers. */ PRInt32 ssl3_SendRecord(sslSocket *ss, @@ -2343,7 +2322,6 @@ ssl3_SendRecord(sslSocket *ss, ssl3CipherSpec *spec; SECStatus rv; PRInt32 totalSent = 0; - PRBool capRecordVersion; SSL_TRC(3, ("%d: SSL3[%d] SendRecord type: %s nIn=%d", SSL_GETPID(), ss->fd, ssl3_DecodeContentType(type), @@ -2358,17 +2336,6 @@ ssl3_SendRecord(sslSocket *ss, return SECFailure; } - capRecordVersion = ((flags & ssl_SEND_FLAG_CAP_RECORD_VERSION) != 0); - - if (capRecordVersion) { - /* ssl_SEND_FLAG_CAP_RECORD_VERSION can only be used with the - * TLS initial ClientHello. */ - PORT_Assert(!IS_DTLS(ss)); - PORT_Assert(!ss->firstHsDone); - PORT_Assert(type == content_handshake); - PORT_Assert(ss->ssl3.hs.ws == wait_server_hello); - } - if (ss->ssl3.initialized == PR_FALSE) { /* This can happen on a server if the very first incoming record ** looks like a defective ssl3 record (e.g. too long), and we're @@ -2401,8 +2368,7 @@ ssl3_SendRecord(sslSocket *ss, unsigned int written = 0; ssl_GetSpecReadLock(ss); - rv = ssl_ProtectNextRecord(ss, spec, type, capRecordVersion, - pIn, nIn, &written); + rv = ssl_ProtectNextRecord(ss, spec, type, pIn, nIn, &written); ssl_ReleaseSpecReadLock(ss); if (rv != SECSuccess) { return SECFailure; @@ -2624,8 +2590,7 @@ ssl3_FlushHandshake(sslSocket *ss, PRInt32 flags) static SECStatus ssl3_FlushHandshakeMessages(sslSocket *ss, PRInt32 flags) { - static const PRInt32 allowedFlags = ssl_SEND_FLAG_FORCE_INTO_BUFFER | - ssl_SEND_FLAG_CAP_RECORD_VERSION; + static const PRInt32 allowedFlags = ssl_SEND_FLAG_FORCE_INTO_BUFFER; PRInt32 count = -1; SECStatus rv; @@ -3086,7 +3051,6 @@ ssl3_SendChangeCipherSpecsInt(sslSocket *ss) static SECStatus ssl3_SendChangeCipherSpecs(sslSocket *ss) { - ssl3CipherSpec *pwSpec; SECStatus rv; PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss)); @@ -4792,7 +4756,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type) if (sid->version < SSL_LIBRARY_VERSION_TLS_1_3) { length += sid->u.ssl3.sessionIDLength; } else if (ss->opt.enableAltHandshaketype && !IS_DTLS(ss)) { - length += SSL3_RANDOM_LENGTH; + length += SSL3_SESSIONID_BYTES; } if (IS_DTLS(ss)) { length += 1 + cookieLen; @@ -4956,9 +4920,6 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type) } flags = 0; - if (!ss->firstHsDone && !IS_DTLS(ss)) { - flags |= ssl_SEND_FLAG_CAP_RECORD_VERSION; - } rv = ssl3_FlushHandshake(ss, flags); if (rv != SECSuccess) { return rv; /* error code set by ssl3_FlushHandshake */ @@ -6139,7 +6100,6 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) int errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO; SECStatus rv; SECItem sidBytes = { siBuffer, NULL, 0 }; - PRBool isTLS = PR_FALSE; SSL3AlertDescription desc = illegal_parameter; TLSExtension *versionExtension; #ifndef TLS_1_3_DRAFT_VERSION @@ -6192,7 +6152,7 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) goto loser; /* alert has been sent */ } if (sidBytes.len > SSL3_SESSIONID_BYTES) { - if (isTLS) + if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_0) desc = decode_error; goto alert_loser; /* malformed. */ } @@ -6283,8 +6243,6 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) errCode = SSL_ERROR_UNSUPPORTED_VERSION; goto alert_loser; } - ss->ssl3.hs.preliminaryInfo |= ssl_preinfo_version; - isTLS = (ss->version > SSL_LIBRARY_VERSION_3_0); #ifndef TLS_1_3_DRAFT_VERSION /* Check the ServerHello.random per @@ -6320,6 +6278,15 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) } #endif + /* 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) { + ssl_GetSpecWriteLock(ss); + ssl_SetSpecVersions(ss, ss->ssl3.cwSpec); + ssl_ReleaseSpecWriteLock(ss); + } + /* Check that the session ID is as expected. */ if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) { PRUint8 buf[SSL3_SESSIONID_BYTES]; @@ -7977,8 +7944,21 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length) goto alert_loser; } } + + if (ss->firstHsDone && ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) { + desc = unexpected_message; + errCode = SSL_ERROR_RENEGOTIATION_NOT_ALLOWED; + goto alert_loser; + } + isTLS13 = ss->version >= SSL_LIBRARY_VERSION_TLS_1_3; ss->ssl3.hs.preliminaryInfo |= ssl_preinfo_version; + /* Update the write spec to match the selected version. */ + if (!ss->firstHsDone) { + ssl_GetSpecWriteLock(ss); + ssl_SetSpecVersions(ss, ss->ssl3.cwSpec); + ssl_ReleaseSpecWriteLock(ss); + } if (ss->ssl3.hs.altHandshakeType) { rv = SECITEM_CopyItem(NULL, &ss->ssl3.hs.fakeSid, &sidBytes); @@ -8093,16 +8073,8 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length) } } - /* This is a second check for TLS 1.3 and re-handshake to stop us - * from re-handshake up to TLS 1.3, so it happens after version - * negotiation. */ - if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) { - if (ss->firstHsDone) { - desc = unexpected_message; - errCode = SSL_ERROR_RENEGOTIATION_NOT_ALLOWED; - goto alert_loser; - } - } else { + /* The check for renegotiation in TLS 1.3 is earlier. */ + if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) { if (ss->firstHsDone && (ss->opt.enableRenegotiation == SSL_RENEGOTIATE_REQUIRES_XTN || ss->opt.enableRenegotiation == SSL_RENEGOTIATE_TRANSITIONAL) && @@ -8642,6 +8614,11 @@ ssl3_HandleV2ClientHello(sslSocket *ss, unsigned char *buffer, int length, goto alert_loser; } ss->ssl3.hs.preliminaryInfo |= ssl_preinfo_version; + if (!ss->firstHsDone) { + ssl_GetSpecWriteLock(ss); + ssl_SetSpecVersions(ss, ss->ssl3.cwSpec); + ssl_ReleaseSpecWriteLock(ss); + } /* if we get a non-zero SID, just ignore it. */ if (length != total) { diff --git a/lib/ssl/ssl3gthr.c b/lib/ssl/ssl3gthr.c index 9106aa4df1..20404f4da2 100644 --- a/lib/ssl/ssl3gthr.c +++ b/lib/ssl/ssl3gthr.c @@ -495,7 +495,6 @@ ssl3_GatherCompleteHandshake(sslSocket *ss, int flags) if (IS_DTLS(ss)) { sslSequenceNumber seq_num; - cText.version = dtls_DTLSVersionToTLSVersion(cText.version); /* DTLS sequence number */ PORT_Memcpy(&seq_num, &ss->gs.hdr[3], sizeof(seq_num)); cText.seq_num = PR_ntohll(seq_num); diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index 1359f647eb..bcf5882e90 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -199,8 +199,6 @@ struct sslSocketOpsStr { #define ssl_SEND_FLAG_FORCE_INTO_BUFFER 0x40000000 #define ssl_SEND_FLAG_NO_BUFFER 0x20000000 #define ssl_SEND_FLAG_NO_RETRANSMIT 0x08000000 /* DTLS only */ -#define ssl_SEND_FLAG_CAP_RECORD_VERSION \ - 0x04000000 /* TLS only */ #define ssl_SEND_FLAG_MASK 0x7f000000 /* diff --git a/lib/ssl/sslspec.c b/lib/ssl/sslspec.c index 1d2d2f9937..440ab0b99a 100644 --- a/lib/ssl/sslspec.c +++ b/lib/ssl/sslspec.c @@ -174,6 +174,26 @@ ssl_SetupNullCipherSpec(sslSocket *ss, CipherSpecDirection dir) * own rules about construction of unencrypted records. For instance, TLS * 1.3 will set the record version number to TLS 1.0. */ spec->version = ss->vrange.max; + if (IS_DTLS(ss)) { + spec->recordVersion = SSL_LIBRARY_VERSION_DTLS_1_0_WIRE; + } else { + /* For new connections, cap the record layer version number of TLS + * ClientHello to { 3, 1 } (TLS 1.0). Some TLS 1.0 servers (which seem + * to use F5 BIG-IP) ignore ClientHello.client_version and use the + * record layer version number (TLSPlaintext.version) instead when + * negotiating protocol versions. In addition, if the record layer + * version number of ClientHello is { 3, 2 } (TLS 1.1) or higher, these + * servers reset the TCP connections. Lastly, some F5 BIG-IP servers + * hang if a record containing a ClientHello has a version greater than + * { 3, 1 } and a length greater than 255. Set this flag to work around + * such servers. + * + * We bump the version up when we settle on a version. Before producing + * an initial ServerHello, or when processing it. + */ + spec->recordVersion = PR_MIN(SSL_LIBRARY_VERSION_TLS_1_0, + ss->vrange.max); + } spec->cipherDef = &ssl_bulk_cipher_defs[cipher_null]; PORT_Assert(spec->cipherDef->cipher == cipher_null); diff --git a/lib/ssl/sslspec.h b/lib/ssl/sslspec.h index 72a0b948b8..729ac1006f 100644 --- a/lib/ssl/sslspec.h +++ b/lib/ssl/sslspec.h @@ -148,6 +148,7 @@ struct ssl3CipherSpecStr { CipherSpecDirection direction; SSL3ProtocolVersion version; + SSL3ProtocolVersion recordVersion; const ssl3BulkCipherDef *cipherDef; const ssl3MACDef *macDef; diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c index 84886fa889..7a71344166 100644 --- a/lib/ssl/tls13con.c +++ b/lib/ssl/tls13con.c @@ -132,9 +132,6 @@ const char keylogLabelExporterSecret[] = "EXPORTER_SECRET"; ? ss->ssl3.hs.client##name \ : ss->ssl3.hs.server##name) -const SSL3ProtocolVersion kTlsRecordVersion = SSL_LIBRARY_VERSION_TLS_1_0; -const SSL3ProtocolVersion kDtlsRecordVersion = SSL_LIBRARY_VERSION_TLS_1_1; - /* Belt and suspenders in case we ever add a TLS 1.4. */ PR_STATIC_ASSERT(SSL_LIBRARY_VERSION_MAX_SUPPORTED <= SSL_LIBRARY_VERSION_TLS_1_3); @@ -2961,12 +2958,40 @@ tls13_DeriveTrafficKeys(sslSocket *ss, ssl3CipherSpec *spec, return SECFailure; } +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. */ + 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; + } else { + spec->recordVersion = kTlsRecordVersion; + } + SSL_TRC(10, ("%d: TLS13[%d]: Set record version to 0x%04x", + SSL_GETPID(), ss->fd, spec->recordVersion)); +} + static SECStatus tls13_SetupPendingCipherSpec(sslSocket *ss, ssl3CipherSpec *spec) { ssl3CipherSuite suite = ss->ssl3.hs.cipher_suite; PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); + PORT_Assert(spec->epoch); /* Version isn't set when we send 0-RTT data. */ spec->version = PR_MAX(SSL_LIBRARY_VERSION_TLS_1_3, ss->version); @@ -2994,6 +3019,13 @@ tls13_SetupPendingCipherSpec(sslSocket *ss, ssl3CipherSpec *spec) PORT_Assert(0); return SECFailure; } + + if (spec->epoch == TrafficKeyEarlyApplicationData) { + spec->earlyDataRemaining = + ss->sec.ci.sid->u.ssl3.locked.sessionTicket.max_early_data_size; + } + + tls13_SetSpecRecordVersion(ss, spec); return SECSuccess; } @@ -3053,9 +3085,6 @@ tls13_SetCipherSpec(sslSocket *ss, TrafficKeyType type, if (!spec) { return SECFailure; } - rv = tls13_SetupPendingCipherSpec(ss, spec); - if (rv != SECSuccess) - return SECFailure; specp = (direction == CipherSpecRead) ? &ss->ssl3.crSpec : &ss->ssl3.cwSpec; @@ -3069,9 +3098,11 @@ tls13_SetCipherSpec(sslSocket *ss, TrafficKeyType type, if (IS_DTLS(ss)) { dtls_InitRecvdRecords(&spec->recvdRecords); } - if (type == TrafficKeyEarlyApplicationData) { - spec->earlyDataRemaining = - ss->sec.ci.sid->u.ssl3.locked.sessionTicket.max_early_data_size; + + /* This depends on spec having a valid direction and epoch. */ + rv = tls13_SetupPendingCipherSpec(ss, spec); + if (rv != SECSuccess) { + return SECFailure; } rv = tls13_DeriveTrafficKeys(ss, spec, type, deleteSecret); @@ -4679,9 +4710,8 @@ tls13_UnprotectRecord(sslSocket *ss, return SECFailure; } - /* Check the version number in the record */ - if ((IS_DTLS(ss) && cText->version != kDtlsRecordVersion) || - (!IS_DTLS(ss) && cText->version != kTlsRecordVersion)) { + /* Check the version number in the record. */ + if (cText->version != spec->recordVersion) { /* Do we need a better error here? */ SSL_TRC(3, ("%d: TLS13[%d]: record has bogus version", @@ -5017,14 +5047,14 @@ tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supported_versions) if (ss->opt.enableAltHandshaketype && !IS_DTLS(ss) && supported == alt_wire) { - ss->version = version; - ss->ssl3.hs.altHandshakeType = PR_TRUE; 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; } } diff --git a/lib/ssl/tls13con.h b/lib/ssl/tls13con.h index c3ada89fa0..5ca033921a 100644 --- a/lib/ssl/tls13con.h +++ b/lib/ssl/tls13con.h @@ -110,5 +110,6 @@ SECStatus SSLExp_HelloRetryRequestCallback(PRFileDesc *fd, void *arg); SECStatus SSLExp_UseAltHandshakeType(PRFileDesc *fd, PRBool enable); PRBool tls13_MaybeTls13(sslSocket *ss); +void tls13_SetSpecRecordVersion(sslSocket *ss, ssl3CipherSpec *spec); #endif /* __tls13con_h_ */