From f077d65cb7ebd5354363e821c817f172c270fc37 Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Mon, 22 May 2017 10:38:02 +1000 Subject: [PATCH] Bug 1295163 - Limit time-variation when accepting 0-RTT, r=ekr --HG-- branch : NSS_TLS13_DRAFT19_BRANCH extra : rebase_source : 6d7b1f33b0da396706a01dedc0142868d727daf4 --- gtests/ssl_gtest/libssl_internals.c | 11 +++ gtests/ssl_gtest/libssl_internals.h | 1 + gtests/ssl_gtest/ssl_0rtt_unittest.cc | 53 ++++++++++++++ gtests/ssl_gtest/ssl_fuzz_unittest.cc | 4 +- gtests/ssl_gtest/tls_connect.cc | 9 +-- lib/ssl/authcert.c | 3 +- lib/ssl/ssl3con.c | 9 ++- lib/ssl/ssl3encode.c | 85 ++++++++++++++++++++++ lib/ssl/ssl3encode.h | 26 +++++++ lib/ssl/ssl3ext.h | 2 + lib/ssl/ssl3exthandle.c | 100 +++++++++++++++++++------- lib/ssl/sslencode.c | 34 ++++----- lib/ssl/sslencode.h | 6 +- lib/ssl/sslimpl.h | 31 +++++--- lib/ssl/sslinfo.c | 6 +- lib/ssl/sslnonce.c | 13 ++-- lib/ssl/sslsnce.c | 45 ++++++------ lib/ssl/sslsock.c | 1 + lib/ssl/tls13con.c | 28 ++++++-- lib/ssl/tls13exthandle.c | 21 ++++-- 20 files changed, 372 insertions(+), 116 deletions(-) create mode 100644 lib/ssl/ssl3encode.c create mode 100644 lib/ssl/ssl3encode.h diff --git a/gtests/ssl_gtest/libssl_internals.c b/gtests/ssl_gtest/libssl_internals.c index 823d61cd1b..1f2988db78 100644 --- a/gtests/ssl_gtest/libssl_internals.c +++ b/gtests/ssl_gtest/libssl_internals.c @@ -380,3 +380,14 @@ SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size) { return SECSuccess; } + +SECStatus SSLInt_SetTicketAgeTolerance(PRFileDesc *fd, PRUint16 tolerance) { + sslSocket *ss = ssl_FindSocket(fd); + + if (!ss) { + return SECFailure; + } + + ss->opt.ticketAgeTolerance = tolerance; + return SECSuccess; +} diff --git a/gtests/ssl_gtest/libssl_internals.h b/gtests/ssl_gtest/libssl_internals.h index c1b484aa83..d0ba5d0a8d 100644 --- a/gtests/ssl_gtest/libssl_internals.h +++ b/gtests/ssl_gtest/libssl_internals.h @@ -51,5 +51,6 @@ unsigned char *SSLInt_CipherSpecToIv(PRBool isServer, ssl3CipherSpec *spec); void SSLInt_SetTicketLifetime(uint32_t lifetime); void SSLInt_SetMaxEarlyDataSize(uint32_t size); SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size); +SECStatus SSLInt_SetTicketAgeTolerance(PRFileDesc *fd, PRUint16 tolerance); #endif // ndef libssl_internals_h_ diff --git a/gtests/ssl_gtest/ssl_0rtt_unittest.cc b/gtests/ssl_gtest/ssl_0rtt_unittest.cc index 9b34665ce3..9b66a7fa48 100644 --- a/gtests/ssl_gtest/ssl_0rtt_unittest.cc +++ b/gtests/ssl_gtest/ssl_0rtt_unittest.cc @@ -100,6 +100,59 @@ TEST_P(TlsConnectTls13, ZeroRttServerOnly) { CheckKeys(); } +// A small sleep after sending the ClientHello means that the ticket age that +// arrives at the server is too low. With a small tolerance for variation in +// ticket age, the server then rejects early data. +TEST_P(TlsConnectTls13, ZeroRttRejectOldTicket) { + SetupForZeroRtt(); + client_->Set0RttEnabled(true); + server_->Set0RttEnabled(true); + SSLInt_SetTicketAgeTolerance(server_->ssl_fd(), 1); + ExpectResumption(RESUME_TICKET); + ZeroRttSendReceive(true, false, []() { + PR_Sleep(PR_MillisecondsToInterval(10)); + return true; + }); + Handshake(); + ExpectEarlyDataAccepted(false); + CheckConnected(); + SendReceive(); +} + +// In this test, we falsely inflate the estimate of the RTT by delaying the +// ServerHello on the first handshake. This results in the server estimating a +// higher value of the ticket age than the client ultimately provides. Add a +// small tolerance for variation in ticket age and the ticket will appear to +// arrive prematurely, causing the server to reject early data. +TEST_P(TlsConnectTls13, ZeroRttRejectPrematureTicket) { + ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); + ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3); + server_->Set0RttEnabled(true); + client_->StartConnect(); + server_->StartConnect(); + client_->Handshake(); // ClientHello + server_->Handshake(); // ServerHello + PR_Sleep(PR_MillisecondsToInterval(10)); + Handshake(); // Remainder of handshake + CheckConnected(); + SendReceive(); + CheckKeys(); + + Reset(); + client_->Set0RttEnabled(true); + server_->Set0RttEnabled(true); + SSLInt_SetTicketAgeTolerance(server_->ssl_fd(), 1); + ExpectResumption(RESUME_TICKET); + ExpectEarlyDataAccepted(false); + + server_->StartConnect(); + client_->StartConnect(); + ZeroRttSendReceive(true, false); + Handshake(); + CheckConnected(); + SendReceive(); +} + TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpn) { EnableAlpn(); SetupForZeroRtt(); diff --git a/gtests/ssl_gtest/ssl_fuzz_unittest.cc b/gtests/ssl_gtest/ssl_fuzz_unittest.cc index 2a6e8e1346..b9d8bdf898 100644 --- a/gtests/ssl_gtest/ssl_fuzz_unittest.cc +++ b/gtests/ssl_gtest/ssl_fuzz_unittest.cc @@ -47,9 +47,9 @@ class TlsApplicationDataRecorder : public TlsRecordFilter { // Ensure that ssl_Time() returns a constant value. FUZZ_F(TlsFuzzTest, SSL_Time_Constant) { - PRUint32 now = ssl_Time(); + PRUint32 now = ssl_TimeSec(); PR_Sleep(PR_SecondsToInterval(2)); - EXPECT_EQ(ssl_Time(), now); + EXPECT_EQ(ssl_TimeSec(), now); } // Check that due to the deterministic PRNG we derive diff --git a/gtests/ssl_gtest/tls_connect.cc b/gtests/ssl_gtest/tls_connect.cc index 890e8a5d63..403b0c2905 100644 --- a/gtests/ssl_gtest/tls_connect.cc +++ b/gtests/ssl_gtest/tls_connect.cc @@ -552,20 +552,13 @@ void TlsConnectTestBase::SendReceive() { // Do a first connection so we can do 0-RTT on the second one. void TlsConnectTestBase::SetupForZeroRtt() { ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); - client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1, - SSL_LIBRARY_VERSION_TLS_1_3); - server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1, - SSL_LIBRARY_VERSION_TLS_1_3); + ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3); server_->Set0RttEnabled(true); // So we signal that we allow 0-RTT. Connect(); SendReceive(); // Need to read so that we absorb the session ticket. CheckKeys(); Reset(); - client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1, - SSL_LIBRARY_VERSION_TLS_1_3); - server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1, - SSL_LIBRARY_VERSION_TLS_1_3); server_->StartConnect(); client_->StartConnect(); } diff --git a/lib/ssl/authcert.c b/lib/ssl/authcert.c index 88c7c084ae..2765c83420 100644 --- a/lib/ssl/authcert.c +++ b/lib/ssl/authcert.c @@ -17,6 +17,7 @@ #include "nss.h" #include "ssl.h" #include "pk11func.h" /* for PK11_ function calls */ +#include "sslimpl.h" /* * This callback used by SSL to pull client sertificate upon @@ -63,7 +64,7 @@ NSS_GetClientAuthData(void *arg, if (!cert) continue; /* Only check unexpired certs */ - if (CERT_CheckCertValidTimes(cert, PR_Now(), PR_TRUE) != + if (CERT_CheckCertValidTimes(cert, ssl_TimeUsec(), PR_TRUE) != secCertTimeValid) { CERT_DestroyCertificate(cert); continue; diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 6faca879fd..7e14b65f8a 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -2949,9 +2949,8 @@ ssl3_FlushHandshake(sslSocket *ss, PRInt32 flags) { if (IS_DTLS(ss)) { return dtls_FlushHandshakeMessages(ss, flags); - } else { - return ssl3_FlushHandshakeMessages(ss, flags); } + return ssl3_FlushHandshakeMessages(ss, flags); } /* Attempt to send the content of sendBuf buffer in an SSL handshake record. @@ -10196,7 +10195,7 @@ ssl3_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length) * until it has verified the server's Finished message." See the comment in * ssl3_FinishHandshake for more details. */ - ss->ssl3.hs.newSessionTicket.received_timestamp = PR_Now(); + ss->ssl3.hs.newSessionTicket.received_timestamp = ssl_TimeUsec(); if (length < 4) { (void)SSL3_SendAlert(ss, alert_fatal, decode_error); PORT_SetError(SSL_ERROR_RX_MALFORMED_NEW_SESSION_TICKET); @@ -11488,8 +11487,8 @@ ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid, PK11SymKey *secret) sid->authKeyBits = ss->sec.authKeyBits; sid->keaType = ss->sec.keaType; sid->keaKeyBits = ss->sec.keaKeyBits; - sid->lastAccessTime = sid->creationTime = ssl_Time(); - sid->expirationTime = sid->creationTime + ssl3_sid_timeout; + sid->lastAccessTime = sid->creationTime = ssl_TimeUsec(); + sid->expirationTime = sid->creationTime + ssl3_sid_timeout * PR_USEC_PER_SEC; sid->localCert = CERT_DupCertificate(ss->sec.localCert); if (ss->sec.isServer) { sid->namedCurve = ss->sec.serverCert->namedCurve; diff --git a/lib/ssl/ssl3encode.c b/lib/ssl/ssl3encode.c new file mode 100644 index 0000000000..cbbd7ed242 --- /dev/null +++ b/lib/ssl/ssl3encode.c @@ -0,0 +1,85 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is PRIVATE to SSL. + * + * 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 "prnetdb.h" +#include "seccomon.h" +#include "secerr.h" +#include "ssl3encode.h" + +SECStatus +ssl3_AppendToItem(SECItem *item, const unsigned char *buf, PRUint32 bytes) +{ + if (bytes > item->len) { + PORT_SetError(SEC_ERROR_INVALID_ARGS); + return SECFailure; + } + + PORT_Memcpy(item->data, buf, bytes); + item->data += bytes; + item->len -= bytes; + return SECSuccess; +} + +SECStatus +ssl3_AppendNumberToItem(SECItem *item, PRUint64 num, PRInt32 lenSize) +{ + SECStatus rv; + PRUint8 b[sizeof(num)]; + + ssl_EncodeUintX(num, lenSize, b); + rv = ssl3_AppendToItem(item, &b[0], lenSize); + return rv; +} + +SECStatus +ssl3_ConsumeFromItem(SECItem *item, unsigned char **buf, PRUint32 bytes) +{ + if (bytes > item->len) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return SECFailure; + } + + *buf = item->data; + item->data += bytes; + item->len -= bytes; + return SECSuccess; +} + +SECStatus +ssl3_ConsumeNumberFromItem(SECItem *item, PRUint32 *num, PRUint32 bytes) +{ + int i; + + if (bytes > item->len || bytes > sizeof(*num)) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return SECFailure; + } + + *num = 0; + for (i = 0; i < bytes; i++) { + *num = (*num << 8) + item->data[i]; + } + + item->data += bytes; + item->len -= bytes; + + return SECSuccess; +} + +/* Helper function to encode an unsigned integer into a buffer. */ +PRUint8 * +ssl_EncodeUintX(PRUint64 value, unsigned int bytes, PRUint8 *to) +{ + PRUint64 encoded; + + PORT_Assert(bytes > 0 && bytes <= sizeof(encoded)); + + encoded = PR_htonll(value); + memcpy(to, ((unsigned char *)(&encoded)) + (sizeof(encoded) - bytes), bytes); + return to + bytes; +} diff --git a/lib/ssl/ssl3encode.h b/lib/ssl/ssl3encode.h new file mode 100644 index 0000000000..70c732e78d --- /dev/null +++ b/lib/ssl/ssl3encode.h @@ -0,0 +1,26 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is PRIVATE to SSL. + * + * 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/. */ + +#ifndef __ssl3encode_h_ +#define __ssl3encode_h_ + +#include "seccomon.h" + +/* All of these functions modify the underlying SECItem, and so should + * be performed on a shallow copy.*/ +SECStatus ssl3_AppendToItem(SECItem *item, + const unsigned char *buf, PRUint32 bytes); +SECStatus ssl3_AppendNumberToItem(SECItem *item, + PRUint64 num, PRInt32 lenSize); +SECStatus ssl3_ConsumeFromItem(SECItem *item, + unsigned char **buf, PRUint32 bytes); +SECStatus ssl3_ConsumeNumberFromItem(SECItem *item, + PRUint32 *num, PRUint32 bytes); +PRUint8 *ssl_EncodeUintX(PRUint64 value, unsigned int bytes, PRUint8 *to); + +#endif diff --git a/lib/ssl/ssl3ext.h b/lib/ssl/ssl3ext.h index d6bdad9113..3be091a277 100644 --- a/lib/ssl/ssl3ext.h +++ b/lib/ssl/ssl3ext.h @@ -90,6 +90,8 @@ struct TLSExtensionDataStr { SECItem pskBinder; /* The PSK binder for the first PSK (TLS 1.3) */ unsigned int pskBinderPrefixLen; /* The length of the binder input. */ PRCList remoteKeyShares; /* The other side's public keys (TLS 1.3) */ + /* This is used when deciding whether to accept early data. */ + PRUint32 ticketAge; }; typedef struct TLSExtensionStr { diff --git a/lib/ssl/ssl3exthandle.c b/lib/ssl/ssl3exthandle.c index cac2e9aa71..4fb1c761e3 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 (0x0105) +#define TLS_EX_SESS_TICKET_VERSION (0x0106) /* * Called from ssl3_SendNewSessionTicket, tls13_SendNewSessionTicket @@ -673,11 +673,12 @@ ssl3_EncodeSessionTicket(sslSocket *ss, unsigned char wrapped_ms[SSL3_MASTER_SECRET_LENGTH]; SECItem ms_item = { 0, NULL, 0 }; PRUint32 cert_length = 0; - PRUint32 now; + PRTime now; SECItem *srvName = NULL; CK_MECHANISM_TYPE msWrapMech = 0; /* dummy default value, * must be >= 0 */ SECItem *alpnSelection = NULL; + PRUint32 ticketAgeBaseline; SSL_TRC(3, ("%d: SSL3[%d]: send session_ticket handshake", SSL_GETPID(), ss->fd)); @@ -726,24 +727,25 @@ ssl3_EncodeSessionTicket(sslSocket *ss, alpnSelection = &ss->xtnData.nextProto; plaintext_length = - sizeof(PRUint16) /* ticket version */ - + sizeof(SSL3ProtocolVersion) /* ssl_version */ - + sizeof(ssl3CipherSuite) /* ciphersuite */ - + 1 /* compression */ - + 10 /* cipher spec parameters */ - + 1 /* certType arguments */ - + 1 /* SessionTicket.ms_is_wrapped */ - + 4 /* msWrapMech */ - + 2 /* master_secret.length */ - + ms_item.len /* master_secret */ - + 1 /* client_auth_type */ - + cert_length /* cert */ - + 2 + srvName->len /* name len + length field */ - + 1 /* extendedMasterSecretUsed */ - + sizeof(ticket->ticket_lifetime_hint) /* ticket lifetime hint */ - + sizeof(ticket->flags) /* ticket flags */ - + 1 + alpnSelection->len /* alpn value + length field */ - + 4; /* maxEarlyData */ + sizeof(PRUint16) /* ticket version */ + + sizeof(SSL3ProtocolVersion) /* ssl_version */ + + sizeof(ssl3CipherSuite) /* ciphersuite */ + + 1 /* compression */ + + 10 /* cipher spec parameters */ + + 1 /* certType arguments */ + + 1 /* SessionTicket.ms_is_wrapped */ + + 4 /* msWrapMech */ + + 2 /* master_secret.length */ + + ms_item.len /* master_secret */ + + 1 /* client_auth_type */ + + cert_length /* cert */ + + 2 + srvName->len /* name len + length field */ + + 1 /* extendedMasterSecretUsed */ + + sizeof(now) /* ticket lifetime hint */ + + sizeof(ticket->flags) /* ticket flags */ + + 1 + alpnSelection->len /* alpn value + length field */ + + 4 /* maxEarlyData */ + + 4; /* ticketAgeBaseline */ if (SECITEM_AllocItem(NULL, &plaintext_item, plaintext_length) == NULL) goto loser; @@ -836,9 +838,9 @@ ssl3_EncodeSessionTicket(sslSocket *ss, } /* timestamp */ - now = ssl_Time(); - rv = ssl3_AppendNumberToItem(&plaintext, now, - sizeof(ticket->ticket_lifetime_hint)); + now = ssl_TimeUsec(); + PORT_Assert(sizeof(now) == 8); + rv = ssl3_AppendNumberToItem(&plaintext, now, 8); if (rv != SECSuccess) goto loser; @@ -880,6 +882,29 @@ ssl3_EncodeSessionTicket(sslSocket *ss, if (rv != SECSuccess) goto loser; + /* + * We store this in the ticket: + * ticket_age_baseline = 1rtt - ticket_age_add + * + * When the client resumes, it will provide: + * obfuscated_age = ticket_age_client + ticket_age_add + * + * We expect to receive the ticket at: + * ticket_create + 1rtt + ticket_age_server + * + * We calculate the client's estimate of this as: + * ticket_create + ticket_age_baseline + obfuscated_age + * = ticket_create + 1rtt + ticket_age_client + * + * This is compared to the expected time, which should differ only as a + * result of clock errors or errors in the RTT estimate. + */ + ticketAgeBaseline = (ssl_TimeUsec() - ss->ssl3.hs.serverHelloTime) / PR_USEC_PER_MSEC; + ticketAgeBaseline -= ticket->ticket_age_add; + rv = ssl3_AppendNumberToItem(&plaintext, ticketAgeBaseline, 4); + if (rv != SECSuccess) + goto loser; + /* Check that we are totally full. */ PORT_Assert(plaintext.len == 0); @@ -1091,13 +1116,21 @@ ssl_ParseSessionTicket(sslSocket *ss, const SECItem *decryptedTicket, PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); return SECFailure; } - /* Read timestamp. */ + + /* Read timestamp. This is a 64-bit value and + * ssl3_ExtConsumeHandshakeNumber only reads 32-bits at a time. */ rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 4, &buffer, &len); if (rv != SECSuccess) { PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); return SECFailure; } - parsedTicket->timestamp = temp; + parsedTicket->timestamp = (PRTime)temp << 32; + rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 4, &buffer, &len); + if (rv != SECSuccess) { + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + return SECFailure; + } + parsedTicket->timestamp |= (PRTime)temp; /* Read server name */ rv = ssl3_ExtConsumeHandshakeVariable(ss, &parsedTicket->srvName, 2, @@ -1138,6 +1171,13 @@ ssl_ParseSessionTicket(sslSocket *ss, const SECItem *decryptedTicket, } parsedTicket->maxEarlyData = temp; + rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 4, &buffer, &len); + if (rv != SECSuccess) { + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + return SECFailure; + } + parsedTicket->ticketAgeBaseline = temp; + #ifndef UNSAFE_FUZZER_MODE /* Done parsing. Check that all bytes have been consumed. */ if (len != 0) { @@ -1164,6 +1204,7 @@ ssl_CreateSIDFromTicket(sslSocket *ss, const SECItem *rawTicket, /* Copy over parameters. */ sid->version = parsedTicket->ssl_version; + sid->creationTime = parsedTicket->timestamp; sid->u.ssl3.cipherSuite = parsedTicket->cipher_suite; sid->u.ssl3.compression = parsedTicket->compression_method; sid->authType = parsedTicket->authType; @@ -1279,8 +1320,8 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data) } /* Use the ticket if it is valid and unexpired. */ - if (parsedTicket.valid && - parsedTicket.timestamp + ssl_ticket_lifetime > ssl_Time()) { + if (parsedTicket.timestamp + ssl_ticket_lifetime * PR_USEC_PER_SEC > + ssl_TimeUsec()) { sslSessionID *sid; rv = ssl_CreateSIDFromTicket(ss, data, &parsedTicket, &sid); @@ -1289,6 +1330,11 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data) } ss->statelessResume = PR_TRUE; ss->sec.ci.sid = sid; + + /* We have the baseline value for the obfuscated ticket age here. Save + * that in xtnData temporarily. This value is updated in + * tls13_ServerHandlePreSharedKeyXtn with the final estimate. */ + ss->xtnData.ticketAge = parsedTicket.ticketAgeBaseline; } SECITEM_ZfreeItem(&decryptedTicket, PR_FALSE); diff --git a/lib/ssl/sslencode.c b/lib/ssl/sslencode.c index 61f18ee5c3..22b6e4d94d 100644 --- a/lib/ssl/sslencode.c +++ b/lib/ssl/sslencode.c @@ -110,61 +110,61 @@ sslBuffer_Clear(sslBuffer *b) } SECStatus -ssl3_AppendToItem(SECItem *item, const unsigned char *buf, PRUint32 bytes) +ssl3_AppendToItem(SECItem *item, const unsigned char *buf, unsigned int size) { - if (bytes > item->len) { + if (size > item->len) { PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; } - PORT_Memcpy(item->data, buf, bytes); - item->data += bytes; - item->len -= bytes; + PORT_Memcpy(item->data, buf, size); + item->data += size; + item->len -= size; return SECSuccess; } SECStatus -ssl3_AppendNumberToItem(SECItem *item, PRUint32 num, PRInt32 lenSize) +ssl3_AppendNumberToItem(SECItem *item, PRUint64 num, unsigned int size) { SECStatus rv; PRUint8 b[sizeof(num)]; - ssl_EncodeUintX(num, lenSize, b); - rv = ssl3_AppendToItem(item, &b[0], lenSize); + ssl_EncodeUintX(num, size, b); + rv = ssl3_AppendToItem(item, &b[0], size); return rv; } SECStatus -ssl3_ConsumeFromItem(SECItem *item, unsigned char **buf, PRUint32 bytes) +ssl3_ConsumeFromItem(SECItem *item, unsigned char **buf, unsigned int size) { - if (bytes > item->len) { + if (size > item->len) { PORT_SetError(SEC_ERROR_BAD_DATA); return SECFailure; } *buf = item->data; - item->data += bytes; - item->len -= bytes; + item->data += size; + item->len -= size; return SECSuccess; } SECStatus -ssl3_ConsumeNumberFromItem(SECItem *item, PRUint32 *num, PRUint32 bytes) +ssl3_ConsumeNumberFromItem(SECItem *item, PRUint32 *num, unsigned int size) { int i; - if (bytes > item->len || bytes > sizeof(*num)) { + if (size > item->len || size > sizeof(*num)) { PORT_SetError(SEC_ERROR_BAD_DATA); return SECFailure; } *num = 0; - for (i = 0; i < bytes; i++) { + for (i = 0; i < size; i++) { *num = (*num << 8) + item->data[i]; } - item->data += bytes; - item->len -= bytes; + item->data += size; + item->len -= size; return SECSuccess; } diff --git a/lib/ssl/sslencode.h b/lib/ssl/sslencode.h index 49c4397ee5..4b50fa3642 100644 --- a/lib/ssl/sslencode.h +++ b/lib/ssl/sslencode.h @@ -35,10 +35,10 @@ void sslBuffer_Clear(sslBuffer *b); SECStatus ssl3_AppendToItem(SECItem *item, const unsigned char *buf, PRUint32 bytes); SECStatus ssl3_AppendNumberToItem(SECItem *item, - PRUint32 num, PRInt32 lenSize); + PRUint64 num, unsigned int size); SECStatus ssl3_ConsumeFromItem(SECItem *item, - unsigned char **buf, PRUint32 bytes); + unsigned char **buf, unsigned int size); SECStatus ssl3_ConsumeNumberFromItem(SECItem *item, - PRUint32 *num, PRUint32 bytes); + PRUint32 *num, unsigned int size); #endif /* __sslencode_h_ */ diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index 61083ed577..1e0c47b4d1 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -262,6 +262,10 @@ typedef struct sslOptionsStr { * list of supported protocols. */ SECItem nextProtoNego; + /* The amount of tolerance to allow for relative clock drift and network + * delays when validating the age of a TLS 1.3. */ + PRUint16 ticketAgeTolerance; + unsigned int useSecurity : 1; unsigned int useSocks : 1; unsigned int requestCertificate : 1; @@ -518,7 +522,7 @@ struct sslSessionIDStr { sslSessionID *next; /* chain used for client sockets, only */ Cached cached; int references; - PRUint32 lastAccessTime; /* seconds since Jan 1, 1970 */ + PRTime lastAccessTime; /* The rest of the members, except for the members of u.ssl3.locked, may * be modified only when the sid is not in any cache. @@ -536,8 +540,8 @@ struct sslSessionIDStr { SSL3ProtocolVersion version; - PRUint32 creationTime; /* seconds since Jan 1, 1970 */ - PRUint32 expirationTime; /* seconds since Jan 1, 1970 */ + PRTime creationTime; + PRTime expirationTime; SSLAuthType authType; PRUint32 authKeyBits; @@ -853,17 +857,18 @@ typedef struct SSL3HandshakeStateStr { PK11SymKey *earlyExporterSecret; /* for 0-RTT exporters */ PK11SymKey *exporterSecret; /* for exporters */ PRCList cipherSpecs; /* The cipher specs in the sequence they - * will be applied. */ + * will be applied. */ sslZeroRttState zeroRttState; /* Are we doing a 0-RTT handshake? */ sslZeroRttIgnore zeroRttIgnore; /* Are we ignoring 0-RTT? */ ssl3CipherSuite zeroRttSuite; /* The cipher suite we used for 0-RTT. */ PRCList bufferedEarlyData; /* Buffered TLS 1.3 early data - * on server.*/ + * on server.*/ PRBool helloRetry; /* True if HelloRetryRequest has been sent - * or received. */ + * or received. */ PRBool clientCertRequested; /* True if CertificateRequest received. */ ssl3KEADef kea_def_mutable; /* Used to hold the writable kea_def - * we use for TLS 1.3 */ + * we use for TLS 1.3 */ + PRTime serverHelloTime; /* Time the ServerHello flight was sent. */ } SSL3HandshakeState; /* @@ -993,11 +998,12 @@ typedef struct SessionTicketStr { PRBool extendedMasterSecretUsed; ClientAuthenticationType client_auth_type; SECItem peer_cert; - PRUint32 timestamp; + PRTime timestamp; PRUint32 flags; SECItem srvName; /* negotiated server name */ SECItem alpnSelection; PRUint32 maxEarlyData; + PRUint32 ticketAgeBaseline; } SessionTicket; /* @@ -1694,7 +1700,7 @@ extern void ssl3_FreeSniNameArray(TLSExtensionData *xtnData); extern void ssl3_SetSIDSessionTicket(sslSessionID *sid, /*in/out*/ NewSessionTicket *session_ticket); SECStatus ssl3_EncodeSessionTicket(sslSocket *ss, - const NewSessionTicket *ticket_input, + const NewSessionTicket *ticket, SECItem *ticket_data, PK11SymKey *secret); SECStatus ssl_MaybeSetSelfEncryptKeyPair(const sslKeyPair *keyPair); @@ -1845,7 +1851,12 @@ extern void ssl3_CheckCipherSuiteOrderConsistency(); extern int ssl_MapLowLevelError(int hiLevelError); -extern PRUint32 ssl_Time(void); +extern PRUint32 ssl_TimeSec(void); +#ifdef UNSAFE_FUZZER_MODE +#define ssl_TimeUsec() ((PRTime)12345678) +#else +#define ssl_TimeUsec() (PR_Now()) +#endif extern PRBool ssl_TicketTimeValid(const NewSessionTicket *ticket); extern void SSL_AtomicIncrementLong(long *x); diff --git a/lib/ssl/sslinfo.c b/lib/ssl/sslinfo.c index bab97de079..f2b680d000 100644 --- a/lib/ssl/sslinfo.c +++ b/lib/ssl/sslinfo.c @@ -88,9 +88,9 @@ SSL_GetChannelInfo(PRFileDesc *fd, SSLChannelInfo *info, PRUintn len) if (sid) { unsigned int sidLen; - inf.creationTime = sid->creationTime; - inf.lastAccessTime = sid->lastAccessTime; - inf.expirationTime = sid->expirationTime; + inf.creationTime = sid->creationTime / PR_USEC_PER_SEC; + inf.lastAccessTime = sid->lastAccessTime / PR_USEC_PER_SEC; + inf.expirationTime = sid->expirationTime / PR_USEC_PER_SEC; inf.extendedMasterSecretUsed = (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3 || sid->u.ssl3.keys.extendedMasterSecretUsed) diff --git a/lib/ssl/sslnonce.c b/lib/ssl/sslnonce.c index 7ad1c6bc7a..228834e3db 100644 --- a/lib/ssl/sslnonce.c +++ b/lib/ssl/sslnonce.c @@ -256,7 +256,7 @@ ssl_LookupSID(const PRIPv6Addr *addr, PRUint16 port, const char *peerID, if (!urlSvrName) return NULL; - now = ssl_Time(); + now = ssl_TimeSec(); LOCK_CACHE; sidp = &cache; while ((sid = *sidp) != 0) { @@ -306,8 +306,6 @@ ssl_LookupSID(const PRIPv6Addr *addr, PRUint16 port, const char *peerID, static void CacheSID(sslSessionID *sid) { - PRUint32 expirationPeriod; - PORT_Assert(sid->cached == never_cached); SSL_TRC(8, ("SSL: Cache: sid=0x%x cached=%d addr=0x%08x%08x%08x%08x port=0x%04x " @@ -335,7 +333,6 @@ CacheSID(sslSessionID *sid) return; sid->u.ssl3.sessionIDLength = SSL3_SESSIONID_BYTES; } - expirationPeriod = ssl3_sid_timeout; PRINT_BUF(8, (0, "sessionID:", sid->u.ssl3.sessionID, sid->u.ssl3.sessionIDLength)); @@ -345,9 +342,9 @@ CacheSID(sslSessionID *sid) } PORT_Assert(sid->creationTime != 0 && sid->expirationTime != 0); if (!sid->creationTime) - sid->lastAccessTime = sid->creationTime = ssl_Time(); + sid->lastAccessTime = sid->creationTime = ssl_TimeUsec(); if (!sid->expirationTime) - sid->expirationTime = sid->creationTime + expirationPeriod; + sid->expirationTime = sid->creationTime + ssl3_sid_timeout * PR_USEC_PER_SEC; /* * Put sid into the cache. Bump reference count to indicate that @@ -438,7 +435,7 @@ SSL_ClearSessionCache(void) /* returns an unsigned int containing the number of seconds in PR_Now() */ PRUint32 -ssl_Time(void) +ssl_TimeSec(void) { #ifdef UNSAFE_FUZZER_MODE return 1234; @@ -471,7 +468,7 @@ ssl_TicketTimeValid(const NewSessionTicket *ticket) endTime = ticket->received_timestamp + (PRTime)(ticket->ticket_lifetime_hint * PR_USEC_PER_SEC); - return endTime > PR_Now(); + return endTime > ssl_TimeUsec(); } void diff --git a/lib/ssl/sslsnce.c b/lib/ssl/sslsnce.c index 3ef11f7a73..cf9b0b424c 100644 --- a/lib/ssl/sslsnce.c +++ b/lib/ssl/sslsnce.c @@ -85,11 +85,12 @@ /* ** Format of a cache entry in the shared memory. */ +PR_STATIC_ASSERT(sizeof(PRTime) == 8); struct sidCacheEntryStr { /* 16 */ PRIPv6Addr addr; /* client's IP address */ - /* 4 */ PRUint32 creationTime; - /* 4 */ PRUint32 lastAccessTime; - /* 4 */ PRUint32 expirationTime; + /* 8 */ PRTime creationTime; + /* 8 */ PRTime lastAccessTime; + /* 8 */ PRTime expirationTime; /* 2 */ PRUint16 version; /* 1 */ PRUint8 valid; /* 1 */ PRUint8 sessionIDLength; @@ -98,7 +99,7 @@ struct sidCacheEntryStr { /* 2 */ PRUint16 authKeyBits; /* 2 */ PRUint16 keaType; /* 2 */ PRUint16 keaKeyBits; - /* 72 - common header total */ + /* 84 - common header total */ union { struct { @@ -116,7 +117,7 @@ struct sidCacheEntryStr { /* force sizeof(sidCacheEntry) to be a multiple of cache line size */ struct { - /*120 */ PRUint8 filler[120]; /* 72+120==192, a multiple of 16 */ + /*124 */ PRUint8 filler[124]; /* 84+124==208, a multiple of 16 */ } forceSize; } u; }; @@ -282,7 +283,7 @@ LockSidCacheLock(sidCacheLock *lock, PRUint32 now) if (rv != SECSuccess) return 0; if (!now) - now = ssl_Time(); + now = ssl_TimeSec(); lock->timeStamp = now; lock->pid = myPid; return now; @@ -298,7 +299,7 @@ UnlockSidCacheLock(sidCacheLock *lock) return rv; } -/* returns the value of ssl_Time on success, zero on failure. */ +/* returns the value of ssl_TimeSec on success, zero on failure. */ static PRUint32 LockSet(cacheDesc *cache, PRUint32 set, PRUint32 now) { @@ -452,9 +453,10 @@ ConvertFromSID(sidCacheEntry *to, sslSessionID *from) SSL_TRC(8, ("%d: SSL3: ConvertSID: time=%d addr=0x%08x%08x%08x%08x " "cipherSuite=%d", - myPid, to->creationTime, to->addr.pr_s6_addr32[0], - to->addr.pr_s6_addr32[1], to->addr.pr_s6_addr32[2], - to->addr.pr_s6_addr32[3], to->u.ssl3.cipherSuite)); + myPid, to->creationTime / PR_USEC_PER_SEC, + to->addr.pr_s6_addr32[0], to->addr.pr_s6_addr32[1], + to->addr.pr_s6_addr32[2], to->addr.pr_s6_addr32[3], + to->u.ssl3.cipherSuite)); } /* @@ -748,17 +750,19 @@ ServerSessionIDCache(sslSessionID *sid) PORT_Assert(sid->creationTime != 0); if (!sid->creationTime) - sid->lastAccessTime = sid->creationTime = ssl_Time(); + sid->lastAccessTime = sid->creationTime = ssl_TimeUsec(); /* override caller's expiration time, which uses client timeout * duration, not server timeout duration. */ - sid->expirationTime = sid->creationTime + cache->ssl3Timeout; + sid->expirationTime = + sid->creationTime + cache->ssl3Timeout * PR_USEC_PER_SEC; SSL_TRC(8, ("%d: SSL: CacheMT: cached=%d addr=0x%08x%08x%08x%08x time=%x " "cipherSuite=%d", myPid, sid->cached, sid->addr.pr_s6_addr32[0], sid->addr.pr_s6_addr32[1], sid->addr.pr_s6_addr32[2], sid->addr.pr_s6_addr32[3], - sid->creationTime, sid->u.ssl3.cipherSuite)); + sid->creationTime / PR_USEC_PER_SEC, + sid->u.ssl3.cipherSuite)); PRINT_BUF(8, (0, "sessionID:", sid->u.ssl3.sessionID, sid->u.ssl3.sessionIDLength)); @@ -820,7 +824,8 @@ ServerSessionIDUncache(sslSessionID *sid) myPid, sid->cached, sid->addr.pr_s6_addr32[0], sid->addr.pr_s6_addr32[1], sid->addr.pr_s6_addr32[2], sid->addr.pr_s6_addr32[3], - sid->creationTime, sid->u.ssl3.cipherSuite)); + sid->creationTime / PR_USEC_PER_SEC, + sid->u.ssl3.cipherSuite)); PRINT_BUF(8, (0, "sessionID:", sessionID, sessionIDLength)); set = SIDindex(cache, &sid->addr, sessionID, sessionIDLength); now = LockSet(cache, set, 0); @@ -1086,7 +1091,7 @@ InitCache(cacheDesc *cache, int maxCacheEntries, int maxCertCacheEntries, cache->srvNameCacheData = (srvNameCacheEntry *)(cache->cacheMem + (ptrdiff_t)cache->srvNameCacheData); /* initialize the locks */ - init_time = ssl_Time(); + init_time = ssl_TimeSec(); pLock = cache->sidCacheLocks; for (locks_to_initialize = cache->numSIDCacheLocks + 3; locks_initialized < locks_to_initialize; @@ -1134,6 +1139,10 @@ SSL_SetMaxServerCacheLocks(PRUint32 maxLocks) return SECSuccess; } +PR_STATIC_ASSERT(sizeof(sidCacheEntry) % 16 == 0); +PR_STATIC_ASSERT(sizeof(certCacheEntry) == 4096); +PR_STATIC_ASSERT(sizeof(srvNameCacheEntry) == 1072); + static SECStatus ssl_ConfigServerSessionIDCacheInstanceWithOpt(cacheDesc *cache, PRUint32 ssl3_timeout, @@ -1145,10 +1154,6 @@ ssl_ConfigServerSessionIDCacheInstanceWithOpt(cacheDesc *cache, { SECStatus rv; - PORT_Assert(sizeof(sidCacheEntry) == 192); - PORT_Assert(sizeof(certCacheEntry) == 4096); - PORT_Assert(sizeof(srvNameCacheEntry) == 1072); - rv = ssl_Init(); if (rv != SECSuccess) { return rv; @@ -1519,7 +1524,7 @@ LockPoller(void *arg) if (sharedCache->stopPolling) break; - now = ssl_Time(); + now = ssl_TimeSec(); then = now - expiration; for (pLock = cache->sidCacheLocks, locks_polled = 0; locks_to_poll > locks_polled && !sharedCache->stopPolling; diff --git a/lib/ssl/sslsock.c b/lib/ssl/sslsock.c index 284731c246..b7c414b576 100644 --- a/lib/ssl/sslsock.c +++ b/lib/ssl/sslsock.c @@ -54,6 +54,7 @@ static const sslSocketOps ssl_secure_ops = { /* SSL. */ */ static sslOptions ssl_defaults = { { siBuffer, NULL, 0 }, /* nextProtoNego */ + 1000, /* ticketAgeTolerance (1s) */ PR_TRUE, /* useSecurity */ PR_FALSE, /* useSocks */ PR_FALSE, /* requestCertificate */ diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c index ef53595765..15b2256dfc 100644 --- a/lib/ssl/tls13con.c +++ b/lib/ssl/tls13con.c @@ -965,6 +965,8 @@ tls13_CanResume(sslSocket *ss, const sslSessionID *sid) static PRBool tls13_CanNegotiateZeroRtt(sslSocket *ss, const sslSessionID *sid) { + PRInt32 timeDelta; + PORT_Assert(ss->ssl3.hs.zeroRttState == ssl_0rtt_sent); if (!sid) @@ -982,6 +984,17 @@ tls13_CanNegotiateZeroRtt(sslSocket *ss, const sslSessionID *sid) &sid->u.ssl3.alpnSelection) != 0) return PR_FALSE; + /* Calculate the difference between the client's view of the age of the + * ticket (in |ss->xtnData.ticketAge|) and the server's view, which we now + * calculate. The result should be close to zero. timeDelta is signed to + * make the comparisons below easier. */ + timeDelta = ss->xtnData.ticketAge - + ((ssl_TimeUsec() - sid->creationTime) / PR_USEC_PER_MSEC); + if (timeDelta > ss->opt.ticketAgeTolerance || + timeDelta < (-1 * ss->opt.ticketAgeTolerance)) { + return PR_FALSE; + } + return PR_TRUE; } @@ -2024,6 +2037,7 @@ tls13_SendServerHelloSequence(sslSocket *ss) : wait_finished); } + ss->ssl3.hs.serverHelloTime = ssl_TimeUsec(); return SECSuccess; } @@ -3942,6 +3956,12 @@ tls13_SendNewSessionTicket(sslSocket *ss) } ticket.ticket_lifetime_hint = ssl_ticket_lifetime; + /* The ticket age obfuscator. */ + rv = PK11_GenerateRandom((PRUint8 *)&ticket.ticket_age_add, + sizeof(ticket.ticket_age_add)); + if (rv != SECSuccess) + goto loser; + rv = tls13_HkdfExpandLabel(ss->ssl3.hs.resumptionMasterSecret, tls13_GetHash(ss), NULL, 0, @@ -3976,12 +3996,6 @@ tls13_SendNewSessionTicket(sslSocket *ss) if (rv != SECSuccess) goto loser; - /* The ticket age obfuscator. */ - rv = PK11_GenerateRandom((PRUint8 *)&ticket.ticket_age_add, - sizeof(ticket.ticket_age_add)); - if (rv != SECSuccess) - goto loser; - rv = ssl3_AppendHandshakeNumber(ss, ticket.ticket_age_add, 4); if (rv != SECSuccess) goto loser; @@ -4052,7 +4066,7 @@ tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length) return SECFailure; } - ticket.received_timestamp = PR_Now(); + ticket.received_timestamp = ssl_TimeUsec(); rv = ssl3_ConsumeHandshakeNumber(ss, &ticket.ticket_lifetime_hint, 4, &b, &length); if (rv != SECSuccess) { diff --git a/lib/ssl/tls13exthandle.c b/lib/ssl/tls13exthandle.c index 47575dde6a..58a48c3559 100644 --- a/lib/ssl/tls13exthandle.c +++ b/lib/ssl/tls13exthandle.c @@ -418,7 +418,7 @@ tls13_ClientSendPreSharedKeyXtn(const sslSocket *ss, TLSExtensionData *xtnData, goto loser; /* Obfuscated age. */ - age = PR_Now() - session_ticket->received_timestamp; + age = ssl_TimeUsec() - session_ticket->received_timestamp; age /= PR_USEC_PER_MSEC; age += session_ticket->ticket_age_add; rv = sslBuffer_AppendNumber(buf, age, 4); @@ -476,7 +476,7 @@ tls13_ServerHandlePreSharedKeyXtn(const sslSocket *ss, TLSExtensionData *xtnData while (inner.len) { SECItem label; - PRUint32 utmp; + PRUint32 obfuscatedAge; rv = ssl3_ExtConsumeHandshakeVariable(ss, &label, 2, &inner.data, &inner.len); @@ -486,9 +486,8 @@ tls13_ServerHandlePreSharedKeyXtn(const sslSocket *ss, TLSExtensionData *xtnData goto alert_loser; } - /* Read and discard session ticket age. Bug 1295163 */ - rv = ssl3_ExtConsumeHandshake(ss, &utmp, 4, - &inner.data, &inner.len); + rv = ssl3_ExtConsumeHandshakeNumber(ss, &obfuscatedAge, 4, + &inner.data, &inner.len); if (rv != SECSuccess) return rv; @@ -502,6 +501,18 @@ tls13_ServerHandlePreSharedKeyXtn(const sslSocket *ss, TLSExtensionData *xtnData * and return SECSuccess. */ if (rv != SECSuccess) return SECFailure; + + if (ss->sec.ci.sid) { + /* xtnData->ticketAge contains the baseline we use for + * calculating the ticket age (i.e., our RTT estimate less the + * value of ticket_age_add). + * + * Add that to the obfuscated ticket age to recover the client's + * view of the ticket age plus the estimated RTT. + * + * See ssl3_EncodeSessionTicket() for details. */ + xtnData->ticketAge += obfuscatedAge; + } } ++numIdentities; }