Skip to content

Commit

Permalink
Bug 1307650 - Various tests to improve coverage. r=mt
Browse files Browse the repository at this point in the history 
Badly formed supported_versions extension

Curve non-overlap

Unknown server key shares

Badly formed supported_versions extension

Set 0-RTT option late on both sides

Reviewers: mt

Differential Revision: https://nss-dev.phacility.com/D54
  • Loading branch information
@ekr
ekr committed Oct 5, 2016
1 parent 3379c87 commit 15d8067
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 11 deletions.
36 changes: 28 additions & 8 deletions external_tests/ssl_gtest/ssl_0rtt_unittest.cc
View file
Expand Up @@ -27,7 +27,7 @@ TEST_P(TlsConnectTls13, ZeroRtt) {
client_->Set0RttEnabled(true);
server_->Set0RttEnabled(true);
ExpectResumption(RESUME_TICKET);
ZeroRttSendReceive(true);
ZeroRttSendReceive(true, true);
Handshake();
ExpectEarlyDataAccepted(true);
CheckConnected();
Expand All @@ -38,7 +38,27 @@ TEST_P(TlsConnectTls13, ZeroRttServerRejectByOption) {
SetupForZeroRtt();
client_->Set0RttEnabled(true);
ExpectResumption(RESUME_TICKET);
ZeroRttSendReceive(false);
ZeroRttSendReceive(true, false);
Handshake();
CheckConnected();
SendReceive();
}

// Test that we don't try to send 0-RTT data when the server sent
// us a ticket without the 0-RTT flags.
TEST_P(TlsConnectTls13, ZeroRttOptionsSetLate) {
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
Connect();
SendReceive(); // Need to read so that we absorb the session ticket.
CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign);
Reset();
server_->StartConnect();
client_->StartConnect();
// Now turn on 0-RTT but too late for the ticket.
client_->Set0RttEnabled(true);
server_->Set0RttEnabled(true);
ExpectResumption(RESUME_TICKET);
ZeroRttSendReceive(false, false);
Handshake();
CheckConnected();
SendReceive();
Expand All @@ -51,7 +71,7 @@ TEST_P(TlsConnectTls13, ZeroRttServerForgetTicket) {
ClearServerCache();
ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
ExpectResumption(RESUME_NONE);
ZeroRttSendReceive(false);
ZeroRttSendReceive(true, false);
Handshake();
CheckConnected();
SendReceive();
Expand Down Expand Up @@ -87,7 +107,7 @@ TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpn) {
server_->Set0RttEnabled(true);
ExpectResumption(RESUME_TICKET);
ExpectEarlyDataAccepted(true);
ZeroRttSendReceive(true, [this]() {
ZeroRttSendReceive(true, true, [this]() {
client_->CheckAlpn(SSL_NEXT_PROTO_EARLY_VALUE, "a");
return true;
});
Expand All @@ -109,7 +129,7 @@ TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpnChangeServer) {
client_->Set0RttEnabled(true);
server_->Set0RttEnabled(true);
ExpectResumption(RESUME_TICKET);
ZeroRttSendReceive(false, [this]() {
ZeroRttSendReceive(true, false, [this]() {
client_->CheckAlpn(SSL_NEXT_PROTO_EARLY_VALUE, "a");
return true;
});
Expand All @@ -130,7 +150,7 @@ TEST_P(TlsConnectTls13, TestTls13ZeroRttNoAlpnServer) {
server_->Set0RttEnabled(true);
EnableAlpn();
ExpectResumption(RESUME_TICKET);
ZeroRttSendReceive(true, [this]() {
ZeroRttSendReceive(true, true, [this]() {
PRUint8 b[] = {'b'};
client_->CheckAlpn(SSL_NEXT_PROTO_EARLY_VALUE, "a");
EXPECT_EQ(SECSuccess, SSLInt_Set0RttAlpn(client_->ssl_fd(), b, sizeof(b)));
Expand All @@ -150,7 +170,7 @@ TEST_P(TlsConnectTls13, TestTls13ZeroRttNoAlpnClient) {
client_->Set0RttEnabled(true);
server_->Set0RttEnabled(true);
ExpectResumption(RESUME_TICKET);
ZeroRttSendReceive(true, [this]() {
ZeroRttSendReceive(true, true, [this]() {
PRUint8 b[] = {'b'};
EXPECT_EQ(SECSuccess, SSLInt_Set0RttAlpn(client_->ssl_fd(), b, 1));
client_->CheckAlpn(SSL_NEXT_PROTO_EARLY_VALUE, "b");
Expand All @@ -170,7 +190,7 @@ TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpnChangeBoth) {
client_->Set0RttEnabled(true);
server_->Set0RttEnabled(true);
ExpectResumption(RESUME_TICKET);
ZeroRttSendReceive(false, [this]() {
ZeroRttSendReceive(true, false, [this]() {
client_->CheckAlpn(SSL_NEXT_PROTO_NO_SUPPORT);
return false;
});
Expand Down
18 changes: 18 additions & 0 deletions external_tests/ssl_gtest/ssl_ecdh_unittest.cc
View file
Expand Up @@ -260,6 +260,24 @@ TEST_P(TlsConnectGeneric, P256andCurve25519OnlyServer) {
CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign, 255);
}

TEST_P(TlsConnectGeneric, P256ClientAndCurve25519Server) {
EnsureTlsSetup();
client_->DisableAllCiphers();
client_->EnableCiphersByKeyExchange(ssl_kea_ecdh);

// the client sends a P256 key share while the server prefers 25519.
const std::vector<SSLNamedGroup> server_groups = {ssl_grp_ec_curve25519};
const std::vector<SSLNamedGroup> client_groups = {ssl_grp_ec_secp256r1};

client_->ConfigNamedGroups(client_groups);
server_->ConfigNamedGroups(server_groups);

ConnectExpectFail();
client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
}


// Replace the point in the client key exchange message with an empty one
class ECCClientKEXFilter : public TlsHandshakeFilter {
public:
Expand Down
53 changes: 53 additions & 0 deletions external_tests/ssl_gtest/ssl_extension_unittest.cc
View file
Expand Up @@ -187,6 +187,15 @@ class TlsExtensionTest13 : public TlsExtensionTestBase,
: TlsExtensionTestBase(TlsConnectTestBase::ToMode(GetParam()),
SSL_LIBRARY_VERSION_TLS_1_3) {}

void ConnectWithBogusVersionList(const uint8_t* buf, size_t len) {
DataBuffer versions_buf(buf, len);
client_->SetPacketFilter(new TlsExtensionReplacer(
ssl_tls13_supported_versions_xtn, versions_buf));
ConnectExpectFail();
client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
server_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_CLIENT_HELLO);
}

void ConnectWithReplacementVersionList(uint16_t version) {
DataBuffer versions_buf;

Expand Down Expand Up @@ -518,6 +527,40 @@ TEST_F(TlsExtensionTest13Stream, DropServerKeyShare) {
EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
}

TEST_F(TlsExtensionTest13Stream, WrongServerKeyShare) {
const uint16_t wrong_group = ssl_grp_ec_secp384r1;

static const uint8_t key_share[] = {
wrong_group >> 8, wrong_group & 0xff, // Group we didn't offer.
0x00, 0x02, // length = 2
0x01, 0x02 };
DataBuffer buf(key_share, sizeof(key_share));
EnsureTlsSetup();
server_->SetPacketFilter(new TlsExtensionReplacer(ssl_tls13_key_share_xtn,
buf));
ConnectExpectFail();
EXPECT_EQ(SSL_ERROR_RX_MALFORMED_KEY_SHARE, client_->error_code());
EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
}

// TODO(ekr@rtfm.com): This is the wrong error code. See bug 1307269.
TEST_F(TlsExtensionTest13Stream, UnknownServerKeyShare) {
const uint16_t wrong_group = 0xffff;

static const uint8_t key_share[] = {
wrong_group >> 8, wrong_group & 0xff, // Group we didn't offer.
0x00, 0x02, // length = 2
0x01, 0x02 };
DataBuffer buf(key_share, sizeof(key_share));
EnsureTlsSetup();
server_->SetPacketFilter(new TlsExtensionReplacer(ssl_tls13_key_share_xtn,
buf));
ConnectExpectFail();
EXPECT_EQ(SSL_ERROR_MISSING_KEY_SHARE, client_->error_code());
EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
}


TEST_F(TlsExtensionTest13Stream, DropServerSignatureAlgorithms) {
EnsureTlsSetup();
server_->SetPacketFilter(
Expand Down Expand Up @@ -728,6 +771,16 @@ TEST_P(TlsExtensionTest13, RemoveTls13FromVersionListBothV12) {
#endif
}

TEST_P(TlsExtensionTest13, EmptyVersionList) {
static const uint8_t ext[] = {0x00, 0x00};
ConnectWithBogusVersionList(ext, sizeof(ext));
}

TEST_P(TlsExtensionTest13, OddVersionList) {
static const uint8_t ext[] = {0x00, 0x01, 0x00};
ConnectWithBogusVersionList(ext, sizeof(ext));
}

INSTANTIATE_TEST_CASE_P(ExtensionStream, TlsExtensionTestGeneric,
::testing::Combine(TlsConnectTestBase::kTlsModesStream,
TlsConnectTestBase::kTlsVAll));
Expand Down
14 changes: 14 additions & 0 deletions external_tests/ssl_gtest/ssl_loopback_unittest.cc
View file
Expand Up @@ -39,6 +39,20 @@ TEST_P(TlsConnectGeneric, ConnectEcdsa) {
CheckKeys(ssl_kea_ecdh, ssl_auth_ecdsa);
}

TEST_P(TlsConnectGenericPre13, CipherSuiteMismatch) {
EnsureTlsSetup();
if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
client_->EnableSingleCipher(TLS_AES_128_GCM_SHA256);
server_->EnableSingleCipher(TLS_AES_256_GCM_SHA384);
} else {
client_->EnableSingleCipher(TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA);
server_->EnableSingleCipher(TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA);
}
ConnectExpectFail();
client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
}

TEST_P(TlsConnectGenericPre13, ConnectFalseStart) {
client_->EnableFalseStart();
Connect();
Expand Down
7 changes: 6 additions & 1 deletion external_tests/ssl_gtest/tls_connect.cc
View file
Expand Up @@ -486,6 +486,7 @@ void TlsConnectTestBase::SetupForResume() {
}

void TlsConnectTestBase::ZeroRttSendReceive(
bool expect_writable,
bool expect_readable, std::function<bool()> post_clienthello_check) {
const char* k0RttData = "ABCDEF";
const PRInt32 k0RttDataLen = static_cast<PRInt32>(strlen(k0RttData));
Expand All @@ -496,7 +497,11 @@ void TlsConnectTestBase::ZeroRttSendReceive(
}
PRInt32 rv =
PR_Write(client_->ssl_fd(), k0RttData, k0RttDataLen); // 0-RTT write.
EXPECT_EQ(k0RttDataLen, rv);
if (expect_writable) {
EXPECT_EQ(k0RttDataLen, rv);
} else {
EXPECT_EQ(SECFailure, rv);
}
server_->Handshake(); // Consume ClientHello, EE, Finished.

std::vector<uint8_t> buf(k0RttDataLen);
Expand Down
1 change: 1 addition & 0 deletions external_tests/ssl_gtest/tls_connect.h
View file
Expand Up @@ -99,6 +99,7 @@ class TlsConnectTestBase : public ::testing::Test {
void SetupForZeroRtt();
void SetupForResume();
void ZeroRttSendReceive(
bool expect_writable,
bool expect_readable,
std::function<bool()> post_clienthello_check = nullptr);
void Receive(size_t amount);
Expand Down
5 changes: 3 additions & 2 deletions lib/ssl/ssl3con.c
View file
Expand Up @@ -8253,8 +8253,9 @@ ssl3_HandleClientHello(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
if (versionExtension) {
rv = tls13_NegotiateVersion(ss, versionExtension);
if (rv != SECSuccess) {
desc = protocol_version;
errCode = SSL_ERROR_UNSUPPORTED_VERSION;
errCode = PORT_GetError();
desc = (errCode == SSL_ERROR_UNSUPPORTED_VERSION) ?
protocol_version : illegal_parameter;
goto alert_loser;
}
} else {
Expand Down

0 comments on commit 15d8067

Please sign in to comment.