Commit 5d49b9e3 authored by Daiki Ueno's avatar Daiki Ueno

Bug 1481271, resend the same ticket in ClientHello after HRR, r=mt

Summary:
This is an another attempt to fix the issue: store the sent session ticket in `ssl3.hs` until the client receives ServerHello.
Test is not ready as I couldn't find any easy way to establish multiple connections in gtests to reproduce the scenario described in comment 7.

Reviewers: mt

Reviewed By: mt

Subscribers: franziskus, jcj, mt, ekr, ueno, rrelyea, Alex_Gaynor, mccr8, HubertKario

Tags: #secure-revision, PHID-PROJ-ffhf7tdvqze7zrdn6dh3

Bug #: 1481271

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

--HG--
extra : amend_source : e7d34e4b47bc7d495197ef2cdca09876e76676b5
parent be4da8cb
......@@ -718,6 +718,86 @@ TEST_F(TlsConnectStreamTls13, RetryStatelessDamageSecondClientHello) {
client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
}
// Stream because SSL_SendSessionTicket only supports that.
TEST_F(TlsConnectStreamTls13, SecondClientHelloSendSameTicket) {
// This simulates the scenario described at:
// https://bugzilla.mozilla.org/show_bug.cgi?id=1481271#c7
//
// Here two connections are interleaved. Tickets are issued on one
// connection. A HelloRetryRequest is triggered on the second connection,
// meaning that there are two ClientHellos. We need to check that both
// ClientHellos have the same ticket, even if a new ticket is issued on the
// other connection in the meantime.
//
// Connection 1: <handshake>
// Connection 1: S->C: NST=X
// Connection 2: C->S: CH [PSK_ID=X]
// Connection 1: S->C: NST=Y
// Connection 2: S->C: HRR
// Connection 2: C->S: CH [PSK_ID=Y]
// Connection 1, send a ticket after handshake is complete.
ConfigureSessionCache(RESUME_TICKET, RESUME_TICKET);
Connect();
// Set this token so that RetryHelloWithToken() will check that this
// is the token that it receives in the HelloRetryRequest callback.
EXPECT_EQ(SECSuccess,
SSL_SendSessionTicket(server_->ssl_fd(), kApplicationToken,
sizeof(kApplicationToken)));
SendReceive(50);
// Connection 2, trigger HRR.
auto client2 =
std::make_shared<TlsAgent>(client_->name(), TlsAgent::CLIENT, variant_);
auto server2 =
std::make_shared<TlsAgent>(server_->name(), TlsAgent::SERVER, variant_);
client2->SetPeer(server2);
server2->SetPeer(client2);
client_.swap(client2);
server_.swap(server2);
ConfigureSessionCache(RESUME_TICKET, RESUME_TICKET);
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
client_->StartConnect();
server_->StartConnect();
size_t cb_called = 0;
EXPECT_EQ(SECSuccess,
SSL_HelloRetryRequestCallback(server_->ssl_fd(),
RetryHelloWithToken, &cb_called));
client_->Handshake(); // Send ClientHello.
server_->Handshake(); // Process ClientHello, send HelloRetryRequest.
EXPECT_EQ(1U, cb_called) << "callback should be called once here";
// Connection 1, send another ticket.
client_.swap(client2);
server_.swap(server2);
// If the client uses this token, RetryHelloWithToken() will fail the test.
const uint8_t kAnotherApplicationToken[] = {0x92, 0x44, 0x01};
EXPECT_EQ(SECSuccess,
SSL_SendSessionTicket(server_->ssl_fd(), kAnotherApplicationToken,
sizeof(kAnotherApplicationToken)));
SendReceive(60);
// Connection 2, continue the handshake.
// The client should use kApplicationToken, not kAnotherApplicationToken.
client_.swap(client2);
server_.swap(server2);
client_->Handshake();
server_->Handshake();
EXPECT_EQ(2U, cb_called) << "callback should be called twice here";
}
// Read the cipher suite from the HRR and disable it on the identified agent.
static void DisableSuiteFromHrr(
std::shared_ptr<TlsAgent>& agent,
......
......@@ -1272,4 +1272,34 @@ TEST_P(TlsConnectGenericResumption, ConnectResumeClientAuth) {
SendReceive();
}
TEST_F(TlsConnectStreamTls13, ExternalTokenAfterHrr) {
ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
Connect();
SendReceive();
Reset();
ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
ExpectResumption(RESUME_TICKET);
static const std::vector<SSLNamedGroup> groups = {ssl_grp_ec_secp384r1,
ssl_grp_ec_secp521r1};
server_->ConfigNamedGroups(groups);
StartConnect();
ASSERT_TRUE(client_->MaybeSetResumptionToken());
client_->Handshake(); // Send ClientHello.
server_->Handshake(); // Process ClientHello, send HelloRetryRequest.
auto& token = client_->GetResumptionToken();
SECStatus rv =
SSL_SetResumptionToken(client_->ssl_fd(), token.data(), token.size());
ASSERT_EQ(SECFailure, rv);
ASSERT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
Handshake();
CheckConnected();
SendReceive();
}
} // namespace nss_test
......@@ -4774,6 +4774,10 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
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->sec.ci.sid && ss->statelessResume && type == client_hello_retry) {
/* If we are sending a second ClientHello, reuse the same SID
* as the original one. */
sid = ssl_ReferenceSID(ss->sec.ci.sid);
} else if (!ss->opt.noCache) {
/* We ignore ss->sec.ci.sid here, and use ssl_Lookup because Lookup
* handles expired entries and other details.
......
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