Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1385746 - Add an application-selected token to tickets, r=ekr
--HG--
branch : NSS_TLS13_DRAFT19_BRANCH
  • Loading branch information
martinthomson committed Jul 31, 2017
1 parent 4050b48 commit 56d701d
Show file tree
Hide file tree
Showing 10 changed files with 197 additions and 40 deletions.
20 changes: 0 additions & 20 deletions gtests/ssl_gtest/libssl_internals.c
Expand Up @@ -225,26 +225,6 @@ PRBool SSLInt_SendAlert(PRFileDesc *fd, uint8_t level, uint8_t type) {
return PR_TRUE;
}

PRBool SSLInt_SendNewSessionTicket(PRFileDesc *fd) {
sslSocket *ss = ssl_FindSocket(fd);
if (!ss) {
return PR_FALSE;
}

ssl_GetSSL3HandshakeLock(ss);
ssl_GetXmitBufLock(ss);

SECStatus rv = tls13_SendNewSessionTicket(ss);
if (rv == SECSuccess) {
rv = ssl3_FlushHandshake(ss, 0);
}

ssl_ReleaseXmitBufLock(ss);
ssl_ReleaseSSL3HandshakeLock(ss);

return rv == SECSuccess;
}

SECStatus SSLInt_AdvanceReadSeqNum(PRFileDesc *fd, PRUint64 to) {
PRUint64 epoch;
sslSocket *ss;
Expand Down
1 change: 0 additions & 1 deletion gtests/ssl_gtest/libssl_internals.h
Expand Up @@ -35,7 +35,6 @@ PRBool SSLInt_DamageEarlyTrafficSecret(PRFileDesc *fd);
SECStatus SSLInt_Set0RttAlpn(PRFileDesc *fd, PRUint8 *data, unsigned int len);
PRBool SSLInt_HasCertWithAuthType(PRFileDesc *fd, SSLAuthType authType);
PRBool SSLInt_SendAlert(PRFileDesc *fd, uint8_t level, uint8_t type);
PRBool SSLInt_SendNewSessionTicket(PRFileDesc *fd);
SECStatus SSLInt_AdvanceWriteSeqNum(PRFileDesc *fd, PRUint64 to);
SECStatus SSLInt_AdvanceReadSeqNum(PRFileDesc *fd, PRUint64 to);
SECStatus SSLInt_AdvanceWriteSeqByAWindow(PRFileDesc *fd, PRInt32 extra);
Expand Down
142 changes: 141 additions & 1 deletion gtests/ssl_gtest/ssl_resumption_unittest.cc
Expand Up @@ -9,6 +9,7 @@
#include "secerr.h"
#include "ssl.h"
#include "sslerr.h"
#include "sslexp.h"
#include "sslproto.h"

extern "C" {
Expand Down Expand Up @@ -622,7 +623,7 @@ TEST_F(TlsConnectTest, TestTls13ResumptionDuplicateNST) {

// Clear the session ticket keys to invalidate the old ticket.
SSLInt_ClearSelfEncryptKey();
SSLInt_SendNewSessionTicket(server_->ssl_fd());
SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0);

SendReceive(); // Need to read so that we absorb the session tickets.
CheckKeys();
Expand All @@ -636,6 +637,145 @@ TEST_F(TlsConnectTest, TestTls13ResumptionDuplicateNST) {
SendReceive();
}

// Check that the value captured in a NewSessionTicket message matches the value
// captured from a pre_shared_key extension.
void NstTicketMatchesPskIdentity(const DataBuffer& nst, const DataBuffer& psk) {
uint32_t len;

size_t offset = 4 + 4; // Skip ticket_lifetime and ticket_age_add.
ASSERT_TRUE(nst.Read(offset, 1, &len));
offset += 1 + len; // Skip ticket_nonce.

ASSERT_TRUE(nst.Read(offset, 2, &len));
offset += 2; // Skip the ticket length.
ASSERT_LE(offset + len, nst.len());
DataBuffer nst_ticket(nst.data() + offset, static_cast<size_t>(len));

offset = 2; // Skip the identities length.
ASSERT_TRUE(psk.Read(offset, 2, &len));
offset += 2; // Skip the identity length.
ASSERT_LE(offset + len, psk.len());
DataBuffer psk_ticket(psk.data() + offset, static_cast<size_t>(len));

EXPECT_EQ(nst_ticket, psk_ticket);
}

TEST_F(TlsConnectTest, TestTls13ResumptionDuplicateNSTWithToken) {
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);

auto nst_capture = std::make_shared<TlsInspectorRecordHandshakeMessage>(
ssl_hs_new_session_ticket);
server_->SetTlsRecordFilter(nst_capture);
Connect();

// Clear the session ticket keys to invalidate the old ticket.
SSLInt_ClearSelfEncryptKey();
nst_capture->Reset();
uint8_t token[] = {0x20, 0x20, 0xff, 0x00};
EXPECT_EQ(SECSuccess,
SSL_SendSessionTicket(server_->ssl_fd(), token, sizeof(token)));

SendReceive(); // Need to read so that we absorb the session tickets.
CheckKeys();
EXPECT_LT(0U, nst_capture->buffer().len());

// Resume the connection.
Reset();
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
ExpectResumption(RESUME_TICKET);

auto psk_capture =
std::make_shared<TlsExtensionCapture>(ssl_tls13_pre_shared_key_xtn);
client_->SetPacketFilter(psk_capture);
Connect();
SendReceive();

NstTicketMatchesPskIdentity(nst_capture->buffer(), psk_capture->extension());
}

// Disable SSL_ENABLE_SESSION_TICKETS but ensure that tickets can still be sent
// by invoking SSL_SendSessionTicket directly (and that the ticket is usable).
TEST_F(TlsConnectTest, SendSessionTicketWithTicketsDisabled) {
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);

EXPECT_EQ(SECSuccess, SSL_OptionSet(server_->ssl_fd(),
SSL_ENABLE_SESSION_TICKETS, PR_FALSE));

auto nst_capture = std::make_shared<TlsInspectorRecordHandshakeMessage>(
ssl_hs_new_session_ticket);
server_->SetTlsRecordFilter(nst_capture);
Connect();

EXPECT_EQ(0U, nst_capture->buffer().len()) << "expect nothing captured yet";

EXPECT_EQ(SECSuccess, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0));
EXPECT_LT(0U, nst_capture->buffer().len()) << "should capture now";

SendReceive(); // Ensure that the client reads the ticket.

// Resume the connection.
Reset();
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
ExpectResumption(RESUME_TICKET);

auto psk_capture =
std::make_shared<TlsExtensionCapture>(ssl_tls13_pre_shared_key_xtn);
client_->SetPacketFilter(psk_capture);
Connect();
SendReceive();

NstTicketMatchesPskIdentity(nst_capture->buffer(), psk_capture->extension());
}

// Test calling SSL_SendSessionTicket in inappropriate conditions.
TEST_F(TlsConnectTest, SendSessionTicketInappropriate) {
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_2);

EXPECT_EQ(SECFailure, SSL_SendSessionTicket(client_->ssl_fd(), NULL, 0))
<< "clients can't send tickets";
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());

server_->StartConnect();
client_->StartConnect();

EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0))
<< "no ticket before the handshake has started";
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
Handshake();
EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0))
<< "no special tickets in TLS 1.2";
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
}

TEST_F(TlsConnectTest, SendSessionTicketMassiveToken) {
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
Connect();
// It should be safe to set length with a NULL token because the length should
// be checked before reading token.
EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0xffff))
<< "no special tickets in TLS 1.2";
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());

EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0x1ffff))
<< "no special tickets in TLS 1.2";
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
}

TEST_F(TlsConnectDatagram13, SendSessionTicketDtls) {
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
Connect();
EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0))
<< "no extra tickets in DTLS until we have Ack support";
EXPECT_EQ(SSL_ERROR_FEATURE_NOT_SUPPORTED_FOR_VERSION, PORT_GetError());
}

TEST_F(TlsConnectTest, TestTls13ResumptionDowngrade) {
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
Expand Down
2 changes: 2 additions & 0 deletions gtests/ssl_gtest/tls_filter.h
Expand Up @@ -209,6 +209,8 @@ class TlsInspectorRecordHandshakeMessage : public TlsHandshakeFilter {
const DataBuffer& input,
DataBuffer* output);

void Reset() { buffer_.Truncate(0); }

const DataBuffer& buffer() const { return buffer_; }

private:
Expand Down
4 changes: 2 additions & 2 deletions lib/ssl/ssl3con.c
Expand Up @@ -10138,8 +10138,8 @@ ssl3_SendNewSessionTicket(sslSocket *ss)
SECStatus rv;
NewSessionTicket nticket = { 0 };

rv = ssl3_EncodeSessionTicket(ss, &nticket, &ticket,
ss->ssl3.pwSpec->master_secret);
rv = ssl3_EncodeSessionTicket(ss, &nticket, NULL, 0,
ss->ssl3.pwSpec->master_secret, &ticket);
if (rv != SECSuccess)
goto loser;

Expand Down
31 changes: 27 additions & 4 deletions lib/ssl/ssl3exthandle.c
Expand Up @@ -660,9 +660,9 @@ PRUint32 ssl_ticket_lifetime = 2 * 24 * 60 * 60; /* 2 days in seconds */
* Called from ssl3_SendNewSessionTicket, tls13_SendNewSessionTicket
*/
SECStatus
ssl3_EncodeSessionTicket(sslSocket *ss,
const NewSessionTicket *ticket,
SECItem *ticket_data, PK11SymKey *secret)
ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket,
const PRUint8 *appToken, unsigned int appTokenLen,
PK11SymKey *secret, SECItem *ticket_data)
{
SECStatus rv;
SECItem plaintext;
Expand Down Expand Up @@ -745,7 +745,15 @@ ssl3_EncodeSessionTicket(sslSocket *ss,
+ sizeof(ticket->flags) /* ticket flags */
+ 1 + alpnSelection->len /* alpn value + length field */
+ 4 /* maxEarlyData */
+ 4; /* ticketAgeBaseline */
+ 4 /* ticketAgeBaseline */
+ 2 + appTokenLen; /* application token */

/* This really only happens if appTokenLen is too much, and that always
* comes from the using application. */
if (plaintext_length > 0xffff) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
goto loser;
}

if (SECITEM_AllocItem(NULL, &plaintext_item, plaintext_length) == NULL)
goto loser;
Expand Down Expand Up @@ -905,6 +913,14 @@ ssl3_EncodeSessionTicket(sslSocket *ss,
if (rv != SECSuccess)
goto loser;

/* Application token */
rv = ssl3_AppendNumberToItem(&plaintext, appTokenLen, 2);
if (rv != SECSuccess)
goto loser;
rv = ssl3_AppendToItem(&plaintext, appToken, appTokenLen);
if (rv != SECSuccess)
goto loser;

/* Check that we are totally full. */
PORT_Assert(plaintext.len == 0);

Expand Down Expand Up @@ -1178,6 +1194,13 @@ ssl_ParseSessionTicket(sslSocket *ss, const SECItem *decryptedTicket,
}
parsedTicket->ticketAgeBaseline = temp;

rv = ssl3_ExtConsumeHandshakeVariable(ss, &parsedTicket->applicationToken,
2, &buffer, &len);
if (rv != SECSuccess) {
PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
return SECFailure;
}

#ifndef UNSAFE_FUZZER_MODE
/* Done parsing. Check that all bytes have been consumed. */
if (len != 0) {
Expand Down
8 changes: 4 additions & 4 deletions lib/ssl/sslexp.h
Expand Up @@ -236,11 +236,11 @@ typedef SECStatus(PR_CALLBACK *SSLExtensionHandler)(
* Earlier versions of TLS do not support the spontaneous sending of the
* NewSessionTicket message.
*/
#define SSL_SendSessionTicket(fd, token, tokenLen) \
#define SSL_SendSessionTicket(fd, appToken, appTokenLen) \
SSL_EXPERIMENTAL_API("SSL_SendSessionTicket", \
(PRFiledesc * _fd, const PRUint8 *_token, \
unsigned int _tokenLen), \
(fd, token, tokenLen))
(PRFileDesc * _fd, const PRUint8 *_appToken, \
unsigned int _appTokenLen), \
(fd, appToken, appTokenLen))

SEC_END_PROTOS

Expand Down
5 changes: 4 additions & 1 deletion lib/ssl/sslimpl.h
Expand Up @@ -1002,6 +1002,7 @@ typedef struct SessionTicketStr {
SECItem alpnSelection;
PRUint32 maxEarlyData;
PRUint32 ticketAgeBaseline;
SECItem applicationToken;
} SessionTicket;

/*
Expand Down Expand Up @@ -1698,7 +1699,9 @@ extern void ssl3_SetSIDSessionTicket(sslSessionID *sid,
/*in/out*/ NewSessionTicket *session_ticket);
SECStatus ssl3_EncodeSessionTicket(sslSocket *ss,
const NewSessionTicket *ticket,
SECItem *ticket_data, PK11SymKey *secret);
const PRUint8 *appToken,
unsigned int appTokenLen,
PK11SymKey *secret, SECItem *ticket_data);
SECStatus SSLExp_SendSessionTicket(PRFileDesc *fd, const PRUint8 *token,
unsigned int tokenLen);

Expand Down
23 changes: 17 additions & 6 deletions lib/ssl/tls13con.c
Expand Up @@ -97,6 +97,9 @@ static SECStatus tls13_ClientHandleFinished(sslSocket *ss,
PRUint8 *b, PRUint32 length);
static SECStatus tls13_ServerHandleFinished(sslSocket *ss,
PRUint8 *b, PRUint32 length);
static SECStatus tls13_SendNewSessionTicket(sslSocket *ss,
const PRUint8 *appToken,
unsigned int appTokenLen);
static SECStatus tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b,
PRUint32 length);
static SECStatus tls13_ComputeHandshakeHashes(sslSocket *ss,
Expand Down Expand Up @@ -3721,7 +3724,7 @@ tls13_ServerHandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length)
}
ssl_GetXmitBufLock(ss);
if (ss->opt.enableSessionTickets) {
rv = tls13_SendNewSessionTicket(ss);
rv = tls13_SendNewSessionTicket(ss, NULL, 0);
if (rv != SECSuccess) {
ssl_ReleaseXmitBufLock(ss);
return SECFailure; /* Error code and alerts handled below */
Expand Down Expand Up @@ -3930,8 +3933,9 @@ tls13_SendClientSecondRound(sslSocket *ss)

PRUint32 ssl_max_early_data_size = (2 << 16); /* Arbitrary limit. */

SECStatus
tls13_SendNewSessionTicket(sslSocket *ss)
static SECStatus
tls13_SendNewSessionTicket(sslSocket *ss, const PRUint8 *appToken,
unsigned int appTokenLen)
{
PRUint16 message_length;
PK11SymKey *secret;
Expand Down Expand Up @@ -3971,7 +3975,8 @@ tls13_SendNewSessionTicket(sslSocket *ss)
goto loser;
}

rv = ssl3_EncodeSessionTicket(ss, &ticket, &ticket_data, secret);
rv = ssl3_EncodeSessionTicket(ss, &ticket, appToken, appTokenLen,
secret, &ticket_data);
PK11_FreeSymKey(secret);
if (rv != SECSuccess)
goto loser;
Expand Down Expand Up @@ -4052,15 +4057,21 @@ SSLExp_SendSessionTicket(PRFileDesc *fd, const PRUint8 *token,
return SECFailure;
}

if (IS_DTLS(ss)) {
PORT_SetError(SSL_ERROR_FEATURE_NOT_SUPPORTED_FOR_VERSION);
return SECFailure;
}

if (!ss->sec.isServer || !ss->firstHsDone ||
ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
ss->version < SSL_LIBRARY_VERSION_TLS_1_3 ||
tokenLen > 0xffff) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return SECFailure;
}

ssl_GetSSL3HandshakeLock(ss);
ssl_GetXmitBufLock(ss);
rv = tls13_SendNewSessionTicket(ss);
rv = tls13_SendNewSessionTicket(ss, token, tokenLen);
if (rv == SECSuccess) {
rv = ssl3_FlushHandshake(ss, 0);
}
Expand Down
1 change: 0 additions & 1 deletion lib/ssl/tls13con.h
Expand Up @@ -92,7 +92,6 @@ PRUint16 tls13_EncodeDraftVersion(SSL3ProtocolVersion version);
PRUint16 tls13_DecodeDraftVersion(PRUint16 version);
SECStatus tls13_NegotiateVersion(sslSocket *ss,
const TLSExtension *supported_versions);
SECStatus tls13_SendNewSessionTicket(sslSocket *ss);

PRBool tls13_IsReplay(const sslSocket *ss, const sslSessionID *sid);
void tls13_AntiReplayRollover(PRTime now);
Expand Down

0 comments on commit 56d701d

Please sign in to comment.