Skip to content

Commit

Permalink
Bug 1418862 - Make HelloRetryRequest look like ServerHello, r=ekr
Browse files Browse the repository at this point in the history 
Update TLS 1.3 implementation for draft-22.

This makes the changes from Bug 1411475 the default mode of operation.  A new
option, SSL_ENABLE_TLS13_COMPAT_MODE, is added to control whether a client
attempts to force the server into compatibility mode.  When enabled, clients
will send a fake session_id in the ClientHello and send a ChangeCipherSpec
message before sending any encrypted records.

This patch also includes changes to make a HelloRetryRequest look like a
ServerHello.

This includes the version number change to draft-22.

--HG--
branch : NSS_TLS13_DRAFT19_BRANCH
extra : rebase_source : 0a2868314e7fed0930be029352a0824ec1eb4b46
extra : amend_source : 8a39378587292bd9acaed5de1da105806f3d0522
extra : histedit_source : b2a5f8f4439eba4c41347fea267af6a320c16e52%2C2b1453083a95b9dcfa95f54e70a197f3d870d885
  • Loading branch information
@martinthomson
martinthomson committed Nov 22, 2017
1 parent 8561929 commit 875149a
Show file tree
Hide file tree
Showing 32 changed files with 815 additions and 687 deletions.
2 changes: 1 addition & 1 deletion gtests/ssl_gtest/manifest.mn
View file
Expand Up @@ -15,7 +15,6 @@ CPPSRCS = \
bloomfilter_unittest.cc \
ssl_0rtt_unittest.cc \
ssl_agent_unittest.cc \
ssl_alths_unittest.cc \
ssl_auth_unittest.cc \
ssl_cert_ext_unittest.cc \
ssl_ciphersuite_unittest.cc \
Expand All @@ -40,6 +39,7 @@ CPPSRCS = \
ssl_renegotiation_unittest.cc \
ssl_skip_unittest.cc \
ssl_staticrsa_unittest.cc \
ssl_tls13compat_unittest.cc \
ssl_v2_client_hello_unittest.cc \
ssl_version_unittest.cc \
ssl_versionpolicy_unittest.cc \
Expand Down
15 changes: 8 additions & 7 deletions gtests/ssl_gtest/ssl_agent_unittest.cc
View file
Expand Up @@ -44,13 +44,14 @@ const static uint8_t kCannedTls13ClientHello[] = {
0x02, 0x05, 0x02, 0x06, 0x02, 0x02, 0x02};

const static uint8_t kCannedTls13ServerHello[] = {
0x7f, kD13, 0x9c, 0xbc, 0x14, 0x9b, 0x0e, 0x2e, 0xfa, 0x0d, 0xf3, 0xf0,
0x5c, 0x70, 0x7a, 0xe0, 0xd1, 0x9b, 0x3e, 0x5a, 0x44, 0x6b, 0xdf, 0xe5,
0xc2, 0x28, 0x64, 0xf7, 0x00, 0xc1, 0x9c, 0x08, 0x76, 0x08, 0x13, 0x01,
0x00, 0x28, 0x00, 0x28, 0x00, 0x24, 0x00, 0x1d, 0x00, 0x20, 0xc2, 0xcf,
0x23, 0x17, 0x64, 0x23, 0x03, 0xf0, 0xfb, 0x45, 0x98, 0x26, 0xd1, 0x65,
0x24, 0xa1, 0x6c, 0xa9, 0x80, 0x8f, 0x2c, 0xac, 0x0a, 0xea, 0x53, 0x3a,
0xcb, 0xe3, 0x08, 0x84, 0xae, 0x19};
0x03, 0x03, 0x9c, 0xbc, 0x14, 0x9b, 0x0e, 0x2e, 0xfa, 0x0d, 0xf3,
0xf0, 0x5c, 0x70, 0x7a, 0xe0, 0xd1, 0x9b, 0x3e, 0x5a, 0x44, 0x6b,
0xdf, 0xe5, 0xc2, 0x28, 0x64, 0xf7, 0x00, 0xc1, 0x9c, 0x08, 0x76,
0x08, 0x00, 0x13, 0x01, 0x00, 0x00, 0x2e, 0x00, 0x28, 0x00, 0x24,
0x00, 0x1d, 0x00, 0x20, 0xc2, 0xcf, 0x23, 0x17, 0x64, 0x23, 0x03,
0xf0, 0xfb, 0x45, 0x98, 0x26, 0xd1, 0x65, 0x24, 0xa1, 0x6c, 0xa9,
0x80, 0x8f, 0x2c, 0xac, 0x0a, 0xea, 0x53, 0x3a, 0xcb, 0xe3, 0x08,
0x84, 0xae, 0x19, 0x00, 0x2b, 0x00, 0x02, 0x7f, kD13};
static const char *k0RttData = "ABCDEF";

TEST_P(TlsAgentTest, EarlyFinished) {
Expand Down
188 changes: 0 additions & 188 deletions gtests/ssl_gtest/ssl_alths_unittest.cc
View file

This file was deleted.

6 changes: 2 additions & 4 deletions gtests/ssl_gtest/ssl_auth_unittest.cc
View file
Expand Up @@ -159,13 +159,11 @@ TEST_P(TlsConnectTls12, ClientAuthBigRsaCheckSigAlg) {

class TlsZeroCertificateRequestSigAlgsFilter : public TlsHandshakeFilter {
public:
TlsZeroCertificateRequestSigAlgsFilter()
: TlsHandshakeFilter({kTlsHandshakeCertificateRequest}) {}
virtual PacketFilter::Action FilterHandshake(
const TlsHandshakeFilter::HandshakeHeader& header,
const DataBuffer& input, DataBuffer* output) {
if (header.handshake_type() != kTlsHandshakeCertificateRequest) {
return KEEP;
}

TlsParser parser(input);
std::cerr << "Zeroing CertReq.supported_signature_algorithms" << std::endl;

Expand Down
44 changes: 15 additions & 29 deletions gtests/ssl_gtest/ssl_dhe_unittest.cc
View file
Expand Up @@ -103,14 +103,11 @@ TEST_P(TlsConnectGenericPre13, ConnectFfdheServer) {

class TlsDheServerKeyExchangeDamager : public TlsHandshakeFilter {
public:
TlsDheServerKeyExchangeDamager() {}
TlsDheServerKeyExchangeDamager()
: TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}) {}
virtual PacketFilter::Action FilterHandshake(
const TlsHandshakeFilter::HandshakeHeader& header,
const DataBuffer& input, DataBuffer* output) {
if (header.handshake_type() != kTlsHandshakeServerKeyExchange) {
return KEEP;
}

// Damage the first octet of dh_p. Anything other than the known prime will
// be rejected as "weak" when we have SSL_REQUIRE_DH_NAMED_GROUPS enabled.
*output = input;
Expand Down Expand Up @@ -144,7 +141,8 @@ class TlsDheSkeChangeY : public TlsHandshakeFilter {
kYZeroPad
};

TlsDheSkeChangeY(ChangeYTo change) : change_Y_(change) {}
TlsDheSkeChangeY(uint8_t handshake_type, ChangeYTo change)
: TlsHandshakeFilter({handshake_type}), change_Y_(change) {}

protected:
void ChangeY(const DataBuffer& input, DataBuffer* output, size_t offset,
Expand Down Expand Up @@ -210,18 +208,16 @@ class TlsDheSkeChangeY : public TlsHandshakeFilter {
class TlsDheSkeChangeYServer : public TlsDheSkeChangeY {
public:
TlsDheSkeChangeYServer(ChangeYTo change, bool modify)
: TlsDheSkeChangeY(change), modify_(modify), p_() {}
: TlsDheSkeChangeY(kTlsHandshakeServerKeyExchange, change),
modify_(modify),
p_() {}

const DataBuffer& prime() const { return p_; }

protected:
virtual PacketFilter::Action FilterHandshake(
const TlsHandshakeFilter::HandshakeHeader& header,
const DataBuffer& input, DataBuffer* output) override {
if (header.handshake_type() != kTlsHandshakeServerKeyExchange) {
return KEEP;
}

size_t offset = 2;
// Read dh_p
uint32_t dh_len = 0;
Expand Down Expand Up @@ -251,16 +247,13 @@ class TlsDheSkeChangeYClient : public TlsDheSkeChangeY {
TlsDheSkeChangeYClient(
ChangeYTo change,
std::shared_ptr<const TlsDheSkeChangeYServer> server_filter)
: TlsDheSkeChangeY(change), server_filter_(server_filter) {}
: TlsDheSkeChangeY(kTlsHandshakeClientKeyExchange, change),
server_filter_(server_filter) {}

protected:
virtual PacketFilter::Action FilterHandshake(
const TlsHandshakeFilter::HandshakeHeader& header,
const DataBuffer& input, DataBuffer* output) override {
if (header.handshake_type() != kTlsHandshakeClientKeyExchange) {
return KEEP;
}

ChangeY(input, output, 0, server_filter_->prime());
return CHANGE;
}
Expand Down Expand Up @@ -365,13 +358,10 @@ INSTANTIATE_TEST_CASE_P(

class TlsDheSkeMakePEven : public TlsHandshakeFilter {
public:
TlsDheSkeMakePEven() : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}) {}
virtual PacketFilter::Action FilterHandshake(
const TlsHandshakeFilter::HandshakeHeader& header,
const DataBuffer& input, DataBuffer* output) {
if (header.handshake_type() != kTlsHandshakeServerKeyExchange) {
return KEEP;
}

// Find the end of dh_p
uint32_t dh_len = 0;
EXPECT_TRUE(input.Read(0, 2, &dh_len));
Expand Down Expand Up @@ -399,13 +389,10 @@ TEST_P(TlsConnectGenericPre13, MakeDhePEven) {

class TlsDheSkeZeroPadP : public TlsHandshakeFilter {
public:
TlsDheSkeZeroPadP() : TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}) {}
virtual PacketFilter::Action FilterHandshake(
const TlsHandshakeFilter::HandshakeHeader& header,
const DataBuffer& input, DataBuffer* output) {
if (header.handshake_type() != kTlsHandshakeServerKeyExchange) {
return KEEP;
}

*output = input;
uint32_t dh_len = 0;
EXPECT_TRUE(input.Read(0, 2, &dh_len));
Expand Down Expand Up @@ -559,16 +546,15 @@ TEST_P(TlsConnectTls13, ResumeFfdhe) {
class TlsDheSkeChangeSignature : public TlsHandshakeFilter {
public:
TlsDheSkeChangeSignature(uint16_t version, const uint8_t* data, size_t len)
: version_(version), data_(data), len_(len) {}
: TlsHandshakeFilter({kTlsHandshakeServerKeyExchange}),
version_(version),
data_(data),
len_(len) {}

protected:
virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
const DataBuffer& input,
DataBuffer* output) {
if (header.handshake_type() != kTlsHandshakeServerKeyExchange) {
return KEEP;
}

TlsParser parser(input);
EXPECT_TRUE(parser.SkipVariable(2)); // dh_p
EXPECT_TRUE(parser.SkipVariable(2)); // dh_g
Expand Down
14 changes: 7 additions & 7 deletions gtests/ssl_gtest/ssl_drop_unittest.cc
View file
Expand Up @@ -105,7 +105,7 @@ class TlsDropDatagram13 : public TlsConnectDatagram13 {
server_->ssl_fd(), 1,
[](PRFileDesc* fd, SSLHandshakeType message, PRUint8* data,
unsigned int* len, unsigned int maxLen, void* arg) -> PRBool {
SSLInt_SetMTU(fd, 400); // Splits the certificate.
SSLInt_SetMTU(fd, 500); // Splits the certificate.
return PR_FALSE;
},
nullptr,
Expand Down Expand Up @@ -654,9 +654,9 @@ TEST_F(TlsDropDatagram13, SendOutOfOrderAppWithHandshakeKey) {
auto spec = capturer.spec(0);
ASSERT_NE(nullptr, spec.get());
ASSERT_EQ(2, spec->epoch());
ASSERT_TRUE(client_->SendEncryptedRecord(spec, 0xfeff, 0x0002000000000002,
kTlsApplicationDataType,
DataBuffer(buf, sizeof(buf))));
ASSERT_TRUE(client_->SendEncryptedRecord(
spec, SSL_LIBRARY_VERSION_DTLS_1_2_WIRE, 0x0002000000000002,
kTlsApplicationDataType, DataBuffer(buf, sizeof(buf))));

// Now have the server consume the bogus message.
server_->ExpectSendAlert(illegal_parameter, kTlsAlertFatal);
Expand All @@ -680,9 +680,9 @@ TEST_F(TlsDropDatagram13, SendOutOfOrderHsNonsenseWithHandshakeKey) {
auto spec = capturer.spec(0);
ASSERT_NE(nullptr, spec.get());
ASSERT_EQ(2, spec->epoch());
ASSERT_TRUE(client_->SendEncryptedRecord(spec, 0xfeff, 0x0002000000000002,
kTlsHandshakeType,
DataBuffer(buf, sizeof(buf))));
ASSERT_TRUE(client_->SendEncryptedRecord(
spec, SSL_LIBRARY_VERSION_DTLS_1_2_WIRE, 0x0002000000000002,
kTlsHandshakeType, DataBuffer(buf, sizeof(buf))));
server_->Handshake();
EXPECT_EQ(2UL, server_filters_.ack_->count());
CheckAcks(server_filters_, 0, {0x0002000000000000ULL});
Expand Down

0 comments on commit 875149a

Please sign in to comment.