Commit e533ed60 authored by Martin Thomson's avatar Martin Thomson

Bug 1493769 - Set session_id for external resumption tokens, r=franziskus

This also includes some cleanup that I performed when looking into this.

It turns out that the hacks that we were using for managing the reference count
on sids was unnecessary.  Daiki added a much neater solution in D7493 that I
stole.

The error handling in SSLExp_SetResumptionToken looks nicer after a
spring-clean too.

--HG--
extra : rebase_source : a4aeff32ce0cee61743d98234a21d7726a8dc496
extra : amend_source : 9b3492f78154ebad6216e9a0dacd7e498d906927
parent 7d0f4c6e
......@@ -1126,6 +1126,34 @@ void CheckGetInfoResult(uint32_t alpnSize, uint32_t earlyDataSize,
ASSERT_EQ(earlyDataSize, token->maxEarlyDataSize);
}
// The client should generate a new, randomized session_id
// when resuming using an external token.
TEST_P(TlsConnectGenericResumptionToken, CheckSessionId) {
ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
auto original_sid = MakeTlsFilter<CaptureSessionId>(client_);
Connect();
SendReceive();
Reset();
ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
ExpectResumption(RESUME_TICKET);
StartConnect();
ASSERT_TRUE(client_->MaybeSetResumptionToken());
auto resumed_sid = MakeTlsFilter<CaptureSessionId>(client_);
Handshake();
CheckConnected();
SendReceive();
if (version_ < SSL_LIBRARY_VERSION_TLS_1_3) {
EXPECT_NE(resumed_sid->sid(), original_sid->sid());
EXPECT_EQ(32U, resumed_sid->sid().len());
} else {
EXPECT_EQ(0U, resumed_sid->sid().len());
}
}
TEST_P(TlsConnectGenericResumptionToken, ConnectResumeGetInfo) {
ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
Connect();
......
......@@ -229,6 +229,7 @@ bool TlsAgent::EnsureTlsSetup(PRFileDesc* modelSocket) {
bool TlsAgent::MaybeSetResumptionToken() {
if (!resumption_token_.empty()) {
LOG("setting external resumption token");
SECStatus rv = SSL_SetResumptionToken(ssl_fd(), resumption_token_.data(),
resumption_token_.size());
......
......@@ -559,13 +559,15 @@ void TlsConnectTestBase::CheckResumption(SessionResumptionMode expected) {
EXPECT_EQ(stateless_count, stats->hsh_sid_stateless_resumes);
if (expected != RESUME_NONE) {
if (client_->version() < SSL_LIBRARY_VERSION_TLS_1_3) {
if (client_->version() < SSL_LIBRARY_VERSION_TLS_1_3 &&
client_->GetResumptionToken().size() == 0) {
// Check that the last two session ids match.
ASSERT_EQ(1U + expected_resumptions_, session_ids_.size());
EXPECT_EQ(session_ids_[session_ids_.size() - 1],
session_ids_[session_ids_.size() - 2]);
} else {
// TLS 1.3 only uses tickets.
// We've either chosen TLS 1.3 or are using an external resumption token,
// both of which only use tickets.
EXPECT_TRUE(expected & RESUME_TICKET);
}
}
......
......@@ -4771,7 +4771,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
* If we have an sid and it comes from an external cache, we use it. */
if (ss->sec.ci.sid && ss->sec.ci.sid->cached == in_external_cache) {
PORT_Assert(!ss->sec.isServer);
sid = ss->sec.ci.sid;
sid = ssl_ReferenceSID(ss->sec.ci.sid);
SSL_TRC(3, ("%d: SSL3[%d]: using external resumption token in ClientHello",
SSL_GETPID(), ss->fd));
} else if (!ss->opt.noCache) {
......@@ -4919,9 +4919,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
}
ssl_ReleaseSpecWriteLock(ss);
if (ss->sec.ci.sid != NULL) {
ssl_FreeSID(ss->sec.ci.sid); /* decrement ref count, free if zero */
}
ssl_FreeSID(ss->sec.ci.sid); /* release the old sid */
ss->sec.ci.sid = sid;
/* HACK for SCSV in SSL 3.0. On initial handshake, prepend SCSV,
......
......@@ -1184,6 +1184,7 @@ extern sslSessionID *ssl_LookupSID(const PRIPv6Addr *addr, PRUint16 port,
const char *peerID, const char *urlSvrName);
extern void ssl_FreeSID(sslSessionID *sid);
extern void ssl_DestroySID(sslSessionID *sid, PRBool freeIt);
extern sslSessionID *ssl_ReferenceSID(sslSessionID *sid);
extern int ssl3_SendApplicationData(sslSocket *ss, const PRUint8 *in,
int len, int flags);
......@@ -1726,7 +1727,7 @@ void ssl_Trace(const char *format, ...);
void ssl_CacheExternalToken(sslSocket *ss);
SECStatus ssl_DecodeResumptionToken(sslSessionID *sid, const PRUint8 *encodedTicket,
PRUint32 encodedTicketLen);
PRBool ssl_IsResumptionTokenValid(sslSocket *ss);
PRBool ssl_IsResumptionTokenUsable(sslSocket *ss, sslSessionID *sid);
/* Remove when stable. */
......
......@@ -233,10 +233,21 @@ ssl_FreeLockedSID(sslSessionID *sid)
*/
void
ssl_FreeSID(sslSessionID *sid)
{
if (sid) {
LOCK_CACHE;
ssl_FreeLockedSID(sid);
UNLOCK_CACHE;
}
}
sslSessionID *
ssl_ReferenceSID(sslSessionID *sid)
{
LOCK_CACHE;
ssl_FreeLockedSID(sid);
sid->references++;
UNLOCK_CACHE;
return sid;
}
/************************************************************************/
......@@ -704,10 +715,9 @@ ssl_DecodeResumptionToken(sslSessionID *sid, const PRUint8 *encodedToken,
}
PRBool
ssl_IsResumptionTokenValid(sslSocket *ss)
ssl_IsResumptionTokenUsable(sslSocket *ss, sslSessionID *sid)
{
PORT_Assert(ss);
sslSessionID *sid = ss->sec.ci.sid;
PORT_Assert(sid);
// Check that the ticket didn't expire.
......
......@@ -18,6 +18,7 @@
#include "private/pprio.h"
#include "nss.h"
#include "pk11pqg.h"
#include "pk11pub.h"
#include "tls13esni.h"
static const sslSocketOps ssl_default_ops = { /* No SSL. */
......@@ -4102,6 +4103,7 @@ SSLExp_SetResumptionToken(PRFileDesc *fd, const PRUint8 *token,
unsigned int len)
{
sslSocket *ss = ssl_FindSocket(fd);
sslSessionID *sid = NULL;
if (!ss) {
SSL_DBG(("%d: SSL[%d]: bad socket in SSL_SetResumptionToken",
......@@ -4115,7 +4117,7 @@ SSLExp_SetResumptionToken(PRFileDesc *fd, const PRUint8 *token,
if (ss->firstHsDone || ss->ssl3.hs.ws != idle_handshake ||
ss->sec.isServer || len == 0 || !token) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
goto done;
goto loser;
}
// We override any previously set session.
......@@ -4126,41 +4128,44 @@ SSLExp_SetResumptionToken(PRFileDesc *fd, const PRUint8 *token,
PRINT_BUF(50, (ss, "incoming resumption token", token, len));
ss->sec.ci.sid = ssl3_NewSessionID(ss, PR_FALSE);
if (!ss->sec.ci.sid) {
goto done;
sid = ssl3_NewSessionID(ss, PR_FALSE);
if (!sid) {
goto loser;
}
/* Populate NewSessionTicket values */
SECStatus rv = ssl_DecodeResumptionToken(ss->sec.ci.sid, token, len);
SECStatus rv = ssl_DecodeResumptionToken(sid, token, len);
if (rv != SECSuccess) {
// If decoding fails, we assume the token is bad.
PORT_SetError(SSL_ERROR_BAD_RESUMPTION_TOKEN_ERROR);
ssl_FreeSID(ss->sec.ci.sid);
ss->sec.ci.sid = NULL;
goto done;
goto loser;
}
// Make sure that the token is valid.
if (!ssl_IsResumptionTokenValid(ss)) {
ssl_FreeSID(ss->sec.ci.sid);
ss->sec.ci.sid = NULL;
// Make sure that the token is currently usable.
if (!ssl_IsResumptionTokenUsable(ss, sid)) {
PORT_SetError(SSL_ERROR_BAD_RESUMPTION_TOKEN_ERROR);
goto done;
goto loser;
}
// Generate a new random session ID for this ticket.
rv = PK11_GenerateRandom(sid->u.ssl3.sessionID, SSL3_SESSIONID_BYTES);
if (rv != SECSuccess) {
goto loser; // Code set by PK11_GenerateRandom.
}
sid->u.ssl3.sessionIDLength = SSL3_SESSIONID_BYTES;
/* Use the sid->cached as marker that this is from an external cache and
* we don't have to look up anything in the NSS internal cache. */
ss->sec.ci.sid->cached = in_external_cache;
// This has to be 2 to not free this in sendClientHello.
ss->sec.ci.sid->references = 2;
ss->sec.ci.sid->lastAccessTime = ssl_TimeSec();
sid->cached = in_external_cache;
sid->lastAccessTime = ssl_TimeSec();
ss->sec.ci.sid = sid;
ssl_ReleaseSSL3HandshakeLock(ss);
ssl_Release1stHandshakeLock(ss);
return SECSuccess;
done:
loser:
ssl_FreeSID(sid);
ssl_ReleaseSSL3HandshakeLock(ss);
ssl_Release1stHandshakeLock(ss);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment