Skip to content

Commit

Permalink
Bug 1386096 - Stateless HelloRetryRequest for server, r=ekr
Browse files Browse the repository at this point in the history 
Differential Revision: https://nss-review.dev.mozaws.net/D396

--HG--
branch : NSS_TLS13_DRAFT19_BRANCH
extra : rebase_source : d14b3aa5144a153c192b35b70d4bcad1596db1ad
extra : amend_source : 7efffaa368abf0457450c4b01a97c4788691642b
  • Loading branch information
@martinthomson
martinthomson committed Aug 7, 2017
1 parent 1d1717d commit 0b67022
Show file tree
Hide file tree
Showing 14 changed files with 333 additions and 87 deletions.
42 changes: 0 additions & 42 deletions gtests/ssl_gtest/ssl_extension_unittest.cc
View file
Expand Up @@ -61,48 +61,6 @@ class TlsExtensionDamager : public TlsExtensionFilter {
size_t index_;
};

class TlsExtensionInjector : public TlsHandshakeFilter {
public:
TlsExtensionInjector(uint16_t ext, DataBuffer& data)
: extension_(ext), data_(data) {}

virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
const DataBuffer& input,
DataBuffer* output) {
TlsParser parser(input);
if (!TlsExtensionFilter::FindExtensions(&parser, header)) {
return KEEP;
}
size_t offset = parser.consumed();

*output = input;

// Increase the size of the extensions.
uint16_t ext_len;
memcpy(&ext_len, output->data() + offset, sizeof(ext_len));
ext_len = htons(ntohs(ext_len) + data_.len() + 4);
memcpy(output->data() + offset, &ext_len, sizeof(ext_len));

// Insert the extension type and length.
DataBuffer type_length;
type_length.Allocate(4);
type_length.Write(0, extension_, 2);
type_length.Write(2, data_.len(), 2);
output->Splice(type_length, offset + 2);

// Insert the payload.
if (data_.len() > 0) {
output->Splice(data_, offset + 6);
}

return CHANGE;
}

private:
const uint16_t extension_;
const DataBuffer data_;
};

class TlsExtensionAppender : public TlsHandshakeFilter {
public:
TlsExtensionAppender(uint8_t handshake_type, uint16_t ext, DataBuffer& data)
Expand Down
186 changes: 186 additions & 0 deletions gtests/ssl_gtest/ssl_hrr_unittest.cc
View file
Expand Up @@ -488,6 +488,192 @@ TEST_F(TlsConnectStreamTls13, RetryCallbackWithSessionTicketToken) {
EXPECT_TRUE(cb_run);
}

std::shared_ptr<TlsAgent> MakeNewServer(std::shared_ptr<TlsAgent>& client) {
auto server = std::make_shared<TlsAgent>(TlsAgent::kServerRsa,
TlsAgent::SERVER, client->variant());
server->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
SSL_LIBRARY_VERSION_TLS_1_3);
client->SetPeer(server);
server->SetPeer(client);
server->StartConnect();
return server;
}

void TriggerHelloRetryRequest(std::shared_ptr<TlsAgent>& client,
std::shared_ptr<TlsAgent>& server) {
size_t cb_called = 0;
EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server->ssl_fd(),
RetryHello, &cb_called));

// Start the handshake.
client->StartConnect();
server->StartConnect();
client->Handshake();
server->Handshake();
EXPECT_EQ(1U, cb_called);
}

TEST_P(TlsConnectTls13, RetryStateless) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

TriggerHelloRetryRequest(client_, server_);
server_ = MakeNewServer(client_);

Handshake();
SendReceive();
}

// Stream only because DTLS drops bad packets.
TEST_F(TlsConnectStreamTls13, RetryStatelessDamageFirstClientHello) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

auto damage_ch = std::make_shared<TlsExtensionInjector>(0xfff3, DataBuffer());
client_->SetPacketFilter(damage_ch);

TriggerHelloRetryRequest(client_, server_);
server_ = MakeNewServer(client_);

// Key exchange fails when the handshake continues because client and server
// disagree about the transcript.
client_->ExpectSendAlert(kTlsAlertBadRecordMac);
server_->ExpectSendAlert(kTlsAlertBadRecordMac);
Handshake();
server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
}

TEST_F(TlsConnectStreamTls13, RetryStatelessDamageSecondClientHello) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

TriggerHelloRetryRequest(client_, server_);
server_ = MakeNewServer(client_);

auto damage_ch = std::make_shared<TlsExtensionInjector>(0xfff3, DataBuffer());
client_->SetPacketFilter(damage_ch);

// Key exchange fails when the handshake continues because client and server
// disagree about the transcript.
client_->ExpectSendAlert(kTlsAlertBadRecordMac);
server_->ExpectSendAlert(kTlsAlertBadRecordMac);
Handshake();
server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
}

TEST_P(TlsConnectTls13, RetryStatelessDisableSuiteClient) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

auto capture_hrr = std::make_shared<TlsInspectorRecordHandshakeMessage>(
ssl_hs_hello_retry_request);
server_->SetPacketFilter(capture_hrr);

TriggerHelloRetryRequest(client_, server_);
server_ = MakeNewServer(client_);

// Read the cipher suite from the HRR and disable it on the client.
uint32_t suite;
ASSERT_TRUE(capture_hrr->buffer().Read(2, 2, &suite));
EXPECT_EQ(SECSuccess,
SSL_CipherPrefSet(client_->ssl_fd(), static_cast<uint16_t>(suite),
PR_FALSE));

// The client thinks that the HelloRetryRequest is bad, even though its
// because it changed its mind about the cipher suite.
ExpectAlert(client_, kTlsAlertIllegalParameter);
Handshake();
client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_HELLO_RETRY_REQUEST);
server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
}

TEST_P(TlsConnectTls13, RetryStatelessDisableSuiteServer) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

auto capture_hrr = std::make_shared<TlsInspectorRecordHandshakeMessage>(
ssl_hs_hello_retry_request);
server_->SetPacketFilter(capture_hrr);

TriggerHelloRetryRequest(client_, server_);
server_ = MakeNewServer(client_);

// Read the cipher suite from the HRR and disable it on the server.
uint32_t suite;
ASSERT_TRUE(capture_hrr->buffer().Read(2, 2, &suite));
EXPECT_EQ(SECSuccess,
SSL_CipherPrefSet(server_->ssl_fd(), static_cast<uint16_t>(suite),
PR_FALSE));

ExpectAlert(server_, kTlsAlertIllegalParameter);
Handshake();
server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO);
client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
}

TEST_P(TlsConnectTls13, RetryStatelessDisableGroupClient) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

TriggerHelloRetryRequest(client_, server_);
server_ = MakeNewServer(client_);

static const std::vector<SSLNamedGroup> groups = {ssl_grp_ec_secp384r1};
client_->ConfigNamedGroups(groups);

// We're into undefined behavior on the client side, but - at the point this
// test was written - the client here doesn't amend its key shares because the
// server doesn't ask it to. The server notices that the key share (x25519)
// doesn't match the negotiated group (P-384) and objects.
ExpectAlert(server_, kTlsAlertIllegalParameter);
Handshake();
server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO);
client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
}

TEST_P(TlsConnectTls13, RetryStatelessDisableGroupServer) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

TriggerHelloRetryRequest(client_, server_);
server_ = MakeNewServer(client_);

static const std::vector<SSLNamedGroup> groups = {ssl_grp_ec_secp384r1};
server_->ConfigNamedGroups(groups);

ExpectAlert(server_, kTlsAlertIllegalParameter);
Handshake();
server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO);
client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
}

TEST_P(TlsConnectTls13, RetryStatelessBadCookie) {
ConfigureSelfEncrypt();
EnsureTlsSetup();

TriggerHelloRetryRequest(client_, server_);

// Now replace the self-encrypt MAC key with a garbage key.
static const uint8_t bad_hmac_key[32] = {0};
SECItem key_item = {siBuffer, const_cast<uint8_t*>(bad_hmac_key),
sizeof(bad_hmac_key)};
ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
PK11SymKey* hmac_key =
PK11_ImportSymKey(slot.get(), CKM_SHA256_HMAC, PK11_OriginUnwrap,
CKA_SIGN, &key_item, nullptr);
ASSERT_NE(nullptr, hmac_key);
SSLInt_SetSelfEncryptMacKey(hmac_key); // Passes ownership.

server_ = MakeNewServer(client_);

ExpectAlert(server_, kTlsAlertIllegalParameter);
Handshake();
server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO);
client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
}

// Stream because the server doesn't consume the alert and terminate.
TEST_F(TlsConnectStreamTls13, RetryWithDifferentCipherSuite) {
EnsureTlsSetup();
Expand Down
2 changes: 2 additions & 0 deletions gtests/ssl_gtest/tls_agent.h
View file
Expand Up @@ -171,6 +171,8 @@ class TlsAgent : public PollTarget {
Role role() const { return role_; }
std::string role_str() const { return role_ == SERVER ? "server" : "client"; }

SSLProtocolVariant variant() const { return variant_; }

State state() const { return state_; }

const CERTCertificate* peer_cert() const {
Expand Down
24 changes: 14 additions & 10 deletions gtests/ssl_gtest/tls_connect.cc
View file
Expand Up @@ -459,21 +459,25 @@ void TlsConnectTestBase::EnableSomeEcdhCiphers() {
}
}

void TlsConnectTestBase::ConfigureSelfEncrypt() {
ScopedCERTCertificate cert;
ScopedSECKEYPrivateKey privKey;
ASSERT_TRUE(
TlsAgent::LoadCertificate(TlsAgent::kServerRsaDecrypt, &cert, &privKey));

ScopedSECKEYPublicKey pubKey(CERT_ExtractPublicKey(cert.get()));
ASSERT_TRUE(pubKey);

EXPECT_EQ(SECSuccess,
SSL_SetSessionTicketKeyPair(pubKey.get(), privKey.get()));
}

void TlsConnectTestBase::ConfigureSessionCache(SessionResumptionMode client,
SessionResumptionMode server) {
client_->ConfigureSessionCache(client);
server_->ConfigureSessionCache(server);
if ((server & RESUME_TICKET) != 0) {
ScopedCERTCertificate cert;
ScopedSECKEYPrivateKey privKey;
ASSERT_TRUE(TlsAgent::LoadCertificate(TlsAgent::kServerRsaDecrypt, &cert,
&privKey));

ScopedSECKEYPublicKey pubKey(CERT_ExtractPublicKey(cert.get()));
ASSERT_TRUE(pubKey);

EXPECT_EQ(SECSuccess,
SSL_SetSessionTicketKeyPair(pubKey.get(), privKey.get()));
ConfigureSelfEncrypt();
}
}

Expand Down
1 change: 1 addition & 0 deletions gtests/ssl_gtest/tls_connect.h
View file
Expand Up @@ -95,6 +95,7 @@ class TlsConnectTestBase : public ::testing::Test {
void EnableOnlyDheCiphers();
void EnableSomeEcdhCiphers();
void EnableExtendedMasterSecret();
void ConfigureSelfEncrypt();
void ConfigureSessionCache(SessionResumptionMode client,
SessionResumptionMode server);
void EnableAlpn();
Expand Down
32 changes: 32 additions & 0 deletions gtests/ssl_gtest/tls_filter.cc
View file
Expand Up @@ -665,6 +665,38 @@ PacketFilter::Action TlsExtensionDropper::FilterExtension(
return KEEP;
}

PacketFilter::Action TlsExtensionInjector::FilterHandshake(
const HandshakeHeader& header, const DataBuffer& input,
DataBuffer* output) {
TlsParser parser(input);
if (!TlsExtensionFilter::FindExtensions(&parser, header)) {
return KEEP;
}
size_t offset = parser.consumed();

*output = input;

// Increase the size of the extensions.
uint16_t ext_len;
memcpy(&ext_len, output->data() + offset, sizeof(ext_len));
ext_len = htons(ntohs(ext_len) + data_.len() + 4);
memcpy(output->data() + offset, &ext_len, sizeof(ext_len));

// Insert the extension type and length.
DataBuffer type_length;
type_length.Allocate(4);
type_length.Write(0, extension_, 2);
type_length.Write(2, data_.len(), 2);
output->Splice(type_length, offset + 2);

// Insert the payload.
if (data_.len() > 0) {
output->Splice(data_, offset + 6);
}

return CHANGE;
}

PacketFilter::Action AfterRecordN::FilterRecord(const TlsRecordHeader& header,
const DataBuffer& body,
DataBuffer* out) {
Expand Down
15 changes: 15 additions & 0 deletions gtests/ssl_gtest/tls_filter.h
View file
Expand Up @@ -358,6 +358,21 @@ class TlsExtensionDropper : public TlsExtensionFilter {
uint16_t extension_;
};

class TlsExtensionInjector : public TlsHandshakeFilter {
public:
TlsExtensionInjector(uint16_t ext, const DataBuffer& data)
: extension_(ext), data_(data) {}

protected:
PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
const DataBuffer& input,
DataBuffer* output) override;

private:
const uint16_t extension_;
const DataBuffer data_;
};

class TlsAgent;
typedef std::function<void(void)> VoidFunction;

Expand Down
14 changes: 14 additions & 0 deletions lib/ssl/dtlscon.c
View file
Expand Up @@ -333,6 +333,20 @@ dtls_HandleHandshake(sslSocket *ss, sslBuffer *origBuf)
break;
}

/* If we're a server and we receive what appears to be a retried
* ClientHello, and we are expecting a ClientHello, move the receive
* sequence number forward. This allows for a retried ClientHello if we
* send a stateless HelloRetryRequest. */
if (message_seq > ss->ssl3.hs.recvMessageSeq &&
message_seq == 1 &&
fragment_offset == 0 &&
ss->ssl3.hs.ws == wait_client_hello &&
(SSLHandshakeType)type == ssl_hs_client_hello) {
SSL_TRC(5, ("%d: DTLS[%d]: Received apparent 2nd ClientHello",
SSL_GETPID(), ss->fd));
ss->ssl3.hs.recvMessageSeq = 1;
}

/* There are three ways we could not be ready for this packet.
*
* 1. It's a partial next message.
Expand Down
3 changes: 2 additions & 1 deletion lib/ssl/ssl3con.c
View file
Expand Up @@ -4123,7 +4123,7 @@ ssl3_UpdateHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l
return sslBuffer_Append(&ss->ssl3.hs.messages, b, l);
}

PRINT_BUF(90, (NULL, "handshake hash input:", b, l));
PRINT_BUF(90, (ss, "handshake hash input:", b, l));

if (ss->ssl3.hs.hashType == handshake_hash_single) {
PORT_Assert(ss->version >= SSL_LIBRARY_VERSION_TLS_1_3);
Expand Down Expand Up @@ -8467,6 +8467,7 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
}
#endif

/* If there is a cookie, then this is a second ClientHello (TLS 1.3). */
if (ssl3_FindExtension(ss, ssl_tls13_cookie_xtn)) {
ss->ssl3.hs.helloRetry = PR_TRUE;
}
Expand Down

0 comments on commit 0b67022

Please sign in to comment.