Skip to content

Commit

Permalink
Bug 1558681 - Anti-replay contexts, r=jcj,kjacobs
Browse files Browse the repository at this point in the history
Stop using a global anti-replay context and enable creating a context directly.
This increases the overhead of managing anti-replay for applications marginally,
but allows much greater flexibility in use of anti-replay mechanisms.  In
particular, it enables the testing of 0-RTT in a threaded environment.

The comments in sslexp should be clear enough in explaining how this works.
Basically, this is a new reference-counted object that can be created and
tracked by applications.

The only thing that I can see might be a problem with the API is that I haven't
exposed a function to add a reference for use by applications.  My thinking is
that reference counting is an internal thing; it seems like applications won't
need to worry about that.

selfserv is updated to create a context and attach it to sockets.  This shows
that the management overhead is minor.

The gtests have been tweaked to create a context during setup. The context is
owned by the overall test framework and is passed to server instances after the
sockets are initialized.

Bonus changes:

* ESNI keys are copied from the model socket when calling SSL_ReConfigFD().
* Some better tracing in the anti-replay functions.

Neither of these seemed worth the overhead of a bug to fix.

Differential Revision: https://phabricator.services.mozilla.com/D34660

--HG--
extra : rebase_source : ded5e9a70c76a6c4178d374aa8bcbce158abc505
extra : absorb_source : 54a054bc14ab32fd1b671f53381cdeaa48a001ef
  • Loading branch information
martinthomson committed Jun 25, 2019
1 parent 546c8de commit 601f3e1
Show file tree
Hide file tree
Showing 12 changed files with 264 additions and 109 deletions.
12 changes: 11 additions & 1 deletion cmd/selfserv/selfserv.c
Expand Up @@ -805,6 +805,7 @@ PRBool enableSessionTickets = PR_FALSE;
PRBool failedToNegotiateName = PR_FALSE;
PRBool enableExtendedMasterSecret = PR_FALSE;
PRBool zeroRTT = PR_FALSE;
SSLAntiReplayContext *antiReplay = NULL;
PRBool enableALPN = PR_FALSE;
PRBool enablePostHandshakeAuth = PR_FALSE;
SSLNamedGroup *enabledGroups = NULL;
Expand Down Expand Up @@ -1954,7 +1955,7 @@ server_main(
if (enabledVersions.max < SSL_LIBRARY_VERSION_TLS_1_3) {
errExit("You tried enabling 0RTT without enabling TLS 1.3!");
}
rv = SSL_InitAntiReplay(PR_Now(), 10L * PR_USEC_PER_SEC, 7, 14);
rv = SSL_SetAntiReplayContext(model_sock, antiReplay);
if (rv != SECSuccess) {
errExit("error configuring anti-replay ");
}
Expand Down Expand Up @@ -2469,6 +2470,12 @@ main(int argc, char **argv)

case 'Z':
zeroRTT = PR_TRUE;
rv = SSL_CreateAntiReplayContext(PR_Now(), 10L * PR_USEC_PER_SEC, 7, 14, &antiReplay);
if (rv != SECSuccess) {
PL_DestroyOptState(optstate);
fprintf(stderr, "Unable to create anti-replay context for 0-RTT.\n");
exit(1);
}
break;

case 'Q':
Expand Down Expand Up @@ -2798,6 +2805,9 @@ main(int argc, char **argv)
if (enabledGroups) {
PORT_Free(enabledGroups);
}
if (antiReplay) {
SSL_ReleaseAntiReplayContext(antiReplay);
}
if (NSS_Shutdown() != SECSuccess) {
SECU_PrintError(progName, "NSS_Shutdown");
if (loggerThread) {
Expand Down
8 changes: 6 additions & 2 deletions cpputil/scoped_ptrs_ssl.h
Expand Up @@ -11,10 +11,13 @@
#include "sslexp.h"

struct ScopedDeleteSSL {
void operator()(SSLAeadContext* ctx) { SSL_DestroyAead(ctx); }
void operator()(SSLAntiReplayContext* ctx) {
SSL_ReleaseAntiReplayContext(ctx);
}
void operator()(SSLResumptionTokenInfo* token) {
SSL_DestroyResumptionTokenInfo(token);
}
void operator()(SSLAeadContext* ctx) { SSL_DestroyAead(ctx); }
};

template <class T>
Expand All @@ -29,8 +32,9 @@ struct ScopedMaybeDeleteSSL {

#define SCOPED(x) typedef std::unique_ptr<x, ScopedMaybeDeleteSSL<x> > Scoped##x

SCOPED(SSLResumptionTokenInfo);
SCOPED(SSLAeadContext);
SCOPED(SSLAntiReplayContext);
SCOPED(SSLResumptionTokenInfo);

#undef SCOPED

Expand Down
61 changes: 46 additions & 15 deletions gtests/ssl_gtest/ssl_0rtt_unittest.cc
Expand Up @@ -74,10 +74,11 @@ TEST_P(TlsConnectTls13, ZeroRttApplicationReject) {
}

TEST_P(TlsConnectTls13, ZeroRttApparentReplayAfterRestart) {
// The test fixtures call SSL_InitAntiReplay() in SetUp(). This results in
// 0-RTT being rejected until at least one window passes. SetupFor0Rtt()
// forces a rollover of the anti-replay filters, which clears this state.
// Here, we do the setup manually here without that forced rollover.
// The test fixtures enable anti-replay in SetUp(). This results in 0-RTT
// being rejected until at least one window passes. SetupFor0Rtt() forces a
// rollover of the anti-replay filters, which clears that state and allows
// 0-RTT to work. Make the first connection manually to avoid that rollover
// and cause 0-RTT to be rejected.

ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
Expand Down Expand Up @@ -217,7 +218,7 @@ TEST_P(TlsConnectTls13, ZeroRttServerOnly) {
// delay exceeds half the anti-replay window.
TEST_P(TlsConnectTls13, ZeroRttRejectOldTicket) {
static const PRTime kWindow = 10 * PR_USEC_PER_SEC;
EXPECT_EQ(SECSuccess, SSL_InitAntiReplay(now(), kWindow, 1, 3));
ResetAntiReplay(kWindow);
SetupForZeroRtt();

Reset();
Expand All @@ -242,7 +243,7 @@ TEST_P(TlsConnectTls13, ZeroRttRejectOldTicket) {
// arrive prematurely, causing the server to reject early data.
TEST_P(TlsConnectTls13, ZeroRttRejectPrematureTicket) {
static const PRTime kWindow = 10 * PR_USEC_PER_SEC;
EXPECT_EQ(SECSuccess, SSL_InitAntiReplay(now(), kWindow, 1, 3));
ResetAntiReplay(kWindow);
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
server_->Set0RttEnabled(true);
Expand Down Expand Up @@ -904,20 +905,21 @@ TEST_F(TlsConnectDatagram13, ZeroRttShortReadDtls) {
// that a small sleep results in rejection of early data. 0-RTT has a
// configurable timer, which makes it ideal for this.
TEST_F(TlsConnectStreamTls13, TimePassesByDefault) {
// Set a tiny anti-replay window. This has to be at least 2 milliseconds to
// have any chance of being relevant as that is the smallest window that we
// can detect. Anything smaller rounds to zero.
static const unsigned int kTinyWindowMs = 5;
EXPECT_EQ(SECSuccess, SSL_InitAntiReplay(
PR_Now(), kTinyWindowMs * PR_USEC_PER_MSEC, 1, 5));

// Calling EnsureTlsSetup() replaces the time function on client and server,
// which we don't want, so initialize each directly.
// and sets up anti-replay, which we don't want, so initialize each directly.
client_->EnsureTlsSetup();
server_->EnsureTlsSetup();
client_->StartConnect(); // Also avoid StartConnect().
// StartConnect() calls EnsureTlsSetup(), so avoid that too.
client_->StartConnect();
server_->StartConnect();

// Set a tiny anti-replay window. This has to be at least 2 milliseconds to
// have any chance of being relevant as that is the smallest window that we
// can detect. Anything smaller rounds to zero.
static const unsigned int kTinyWindowMs = 5;
ResetAntiReplay(static_cast<PRTime>(kTinyWindowMs * PR_USEC_PER_MSEC));
server_->SetAntiReplayContext(anti_replay_);

ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
server_->Set0RttEnabled(true);
Expand Down Expand Up @@ -951,6 +953,35 @@ TEST_F(TlsConnectStreamTls13, TimePassesByDefault) {
CheckConnected();
}

// Test that SSL_CreateAntiReplayContext doesn't pass bad inputs.
TEST_F(TlsConnectStreamTls13, BadAntiReplayArgs) {
SSLAntiReplayContext* p;
// Zero or negative window.
EXPECT_EQ(SECFailure, SSL_CreateAntiReplayContext(0, -1, 1, 1, &p));
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
EXPECT_EQ(SECFailure, SSL_CreateAntiReplayContext(0, 0, 1, 1, &p));
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
// Zero k.
EXPECT_EQ(SECFailure, SSL_CreateAntiReplayContext(0, 1, 0, 1, &p));
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
// Zero bits.
EXPECT_EQ(SECFailure, SSL_CreateAntiReplayContext(0, 1, 1, 0, &p));
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
EXPECT_EQ(SECFailure, SSL_CreateAntiReplayContext(0, 1, 1, 1, nullptr));
EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());

// Prove that these parameters do work, even if they are useless..
EXPECT_EQ(SECSuccess, SSL_CreateAntiReplayContext(0, 1, 1, 1, &p));
ASSERT_NE(nullptr, p);
ScopedSSLAntiReplayContext ctx(p);

// The socket isn't a client or server until later, so configuring a client
// should work OK.
client_->EnsureTlsSetup();
EXPECT_EQ(SECSuccess, SSL_SetAntiReplayContext(client_->ssl_fd(), ctx.get()));
EXPECT_EQ(SECSuccess, SSL_SetAntiReplayContext(client_->ssl_fd(), nullptr));
}

#ifndef NSS_DISABLE_TLS_1_3
INSTANTIATE_TEST_CASE_P(Tls13ZeroRttReplayTest, TlsZeroRttReplayTest,
TlsConnectTestBase::kTlsVariantsAll);
Expand Down
4 changes: 4 additions & 0 deletions gtests/ssl_gtest/tls_agent.cc
Expand Up @@ -251,6 +251,10 @@ bool TlsAgent::MaybeSetResumptionToken() {
return true;
}

void TlsAgent::SetAntiReplayContext(ScopedSSLAntiReplayContext& ctx) {
EXPECT_EQ(SECSuccess, SSL_SetAntiReplayContext(ssl_fd_.get(), ctx.get()));
}

void TlsAgent::SetupClientAuth() {
EXPECT_TRUE(EnsureTlsSetup());
ASSERT_EQ(CLIENT, role_);
Expand Down
2 changes: 2 additions & 0 deletions gtests/ssl_gtest/tls_agent.h
Expand Up @@ -108,6 +108,8 @@ class TlsAgent : public PollTarget {
void PrepareForRenegotiate();
// Prepares for renegotiation, then actually triggers it.
void StartRenegotiate();
void SetAntiReplayContext(ScopedSSLAntiReplayContext& ctx);

static bool LoadCertificate(const std::string& name,
ScopedCERTCertificate* cert,
ScopedSECKEYPrivateKey* priv);
Expand Down
14 changes: 12 additions & 2 deletions gtests/ssl_gtest/tls_connect.cc
Expand Up @@ -215,7 +215,7 @@ void TlsConnectTestBase::SetUp() {
SSL_ConfigServerSessionIDCache(1024, 0, 0, g_working_dir_path.c_str());
SSLInt_ClearSelfEncryptKey();
now_ = PR_Now();
SSL_InitAntiReplay(now_, kAntiReplayWindow, 1, 3);
ResetAntiReplay(kAntiReplayWindow);
ClearStats();
SaveAlgorithmPolicy();
Init();
Expand All @@ -240,6 +240,14 @@ void TlsConnectTestBase::Init() {
}
}

void TlsConnectTestBase::ResetAntiReplay(PRTime window) {
SSLAntiReplayContext* p_anti_replay;
EXPECT_EQ(SECSuccess,
SSL_CreateAntiReplayContext(now_, window, 1, 3, &p_anti_replay));
EXPECT_NE(nullptr, p_anti_replay);
anti_replay_.reset(p_anti_replay);
}

void TlsConnectTestBase::Reset() {
// Take a copy of the names because they are about to disappear.
std::string server_name = server_->name();
Expand All @@ -258,7 +266,8 @@ void TlsConnectTestBase::Reset(const std::string& server_name,
server_->SkipVersionChecks();
}

std::cerr << "Reset" << std::endl;
std::cerr << "Reset server:" << server_name << ", client:" << client_name
<< std::endl;
Init();
}

Expand Down Expand Up @@ -290,6 +299,7 @@ void TlsConnectTestBase::EnsureTlsSetup() {
: nullptr));
EXPECT_TRUE(client_->EnsureTlsSetup(client_model_ ? client_model_->ssl_fd()
: nullptr));
server_->SetAntiReplayContext(anti_replay_);
EXPECT_EQ(SECSuccess, SSL_SetTimeFunc(client_->ssl_fd(),
TlsConnectTestBase::TimeFunc, &now_));
EXPECT_EQ(SECSuccess, SSL_SetTimeFunc(server_->ssl_fd(),
Expand Down
3 changes: 3 additions & 0 deletions gtests/ssl_gtest/tls_connect.h
Expand Up @@ -134,6 +134,8 @@ class TlsConnectTestBase : public ::testing::Test {
// Move the DTLS timers for both endpoints to pop the next timer.
void ShiftDtlsTimers();
void AdvanceTime(PRTime time_shift);

void ResetAntiReplay(PRTime window);
void RolloverAntiReplay();

void SaveAlgorithmPolicy();
Expand All @@ -149,6 +151,7 @@ class TlsConnectTestBase : public ::testing::Test {
SessionResumptionMode expected_resumption_mode_;
uint8_t expected_resumptions_;
std::vector<std::vector<uint8_t>> session_ids_;
ScopedSSLAntiReplayContext anti_replay_;

// A simple value of "a", "b". Note that the preferred value of "a" is placed
// at the end, because the NSS API follows the now defunct NPN specification,
Expand Down
36 changes: 27 additions & 9 deletions lib/ssl/sslexp.h
Expand Up @@ -159,11 +159,16 @@ typedef SECStatus(PR_CALLBACK *SSLExtensionHandler)(
handler, handlerArg))

/*
* Initialize the anti-replay buffer for supporting 0-RTT in TLS 1.3 on servers.
* Create an anti-replay context for supporting 0-RTT in TLS 1.3 on servers.
*
* To use 0-RTT on a server, you must call this function. Failing to call this
* function will result in all 0-RTT being rejected. Connections will complete,
* but early data will be rejected.
* To use 0-RTT on a server, you must create an anti-replay context using
* SSL_CreateAntiReplayContext and set that on the socket with
* SSL_SetAntiReplayContext. Failing to set a context on the server will result
* in all 0-RTT being rejected. Connections will complete, but early data will
* be rejected.
*
* Anti-replay contexts are reference counted and are released with
* SSL_ReleaseAntiReplayContext.
*
* NSS uses a Bloom filter to track the ClientHello messages that it receives
* (specifically, it uses the PSK binder). This function initializes a pair of
Expand Down Expand Up @@ -219,11 +224,23 @@ typedef SECStatus(PR_CALLBACK *SSLExtensionHandler)(
* Early data can be replayed at least once with every server instance that will
* accept tickets that are encrypted with the same key.
*/
#define SSL_InitAntiReplay(now, window, k, bits) \
SSL_EXPERIMENTAL_API("SSL_InitAntiReplay", \
(PRTime _now, PRTime _window, \
unsigned int _k, unsigned int _bits), \
(now, window, k, bits))
typedef struct SSLAntiReplayContextStr SSLAntiReplayContext;
#define SSL_CreateAntiReplayContext(now, window, k, bits, ctx) \
SSL_EXPERIMENTAL_API("SSL_CreateAntiReplayContext", \
(PRTime _now, PRTime _window, \
unsigned int _k, unsigned int _bits, \
SSLAntiReplayContext **_ctx), \
(now, window, k, bits, ctx))

#define SSL_SetAntiReplayContext(fd, ctx) \
SSL_EXPERIMENTAL_API("SSL_SetAntiReplayContext", \
(PRFileDesc * _fd, SSLAntiReplayContext * _ctx), \
(fd, ctx))

#define SSL_ReleaseAntiReplayContext(ctx) \
SSL_EXPERIMENTAL_API("SSL_ReleaseAntiReplayContext", \
(SSLAntiReplayContext * _ctx), \
(ctx))

/*
* This function allows a server application to generate a session ticket that
Expand Down Expand Up @@ -743,6 +760,7 @@ typedef PRTime(PR_CALLBACK *SSLTimeFunc)(void *arg);
/* Deprecated experimental APIs */
#define SSL_UseAltServerHelloType(fd, enable) SSL_DEPRECATED_EXPERIMENTAL_API
#define SSL_SetupAntiReplay(a, b, c) SSL_DEPRECATED_EXPERIMENTAL_API
#define SSL_InitAntiReplay(a, b, c) SSL_DEPRECATED_EXPERIMENTAL_API

SEC_END_PROTOS

Expand Down
3 changes: 3 additions & 0 deletions lib/ssl/sslimpl.h
Expand Up @@ -1094,6 +1094,9 @@ struct sslSocketStr {
/* The information from the ESNI keys record
* (also the private key for the server). */
sslEsniKeys *esniKeys;

/* Anti-replay for TLS 1.3 0-RTT. */
SSLAntiReplayContext *antiReplay;
};

struct sslSelfEncryptKeysStr {
Expand Down

0 comments on commit 601f3e1

Please sign in to comment.