Skip to content

Commit

Permalink
Bug 1375837 - Allow TLS 1.3 when renegotiation isn't enabled, r=ekr
Browse files Browse the repository at this point in the history
--HG--
extra : rebase_source : 7ba119b3f792ff3100e367888c3b97a8c9c859aa
  • Loading branch information
martinthomson committed Sep 4, 2017
1 parent 0336159 commit 5eacfc3
Show file tree
Hide file tree
Showing 14 changed files with 216 additions and 266 deletions.
1 change: 1 addition & 0 deletions gtests/ssl_gtest/manifest.mn
Expand Up @@ -33,6 +33,7 @@ CPPSRCS = \
ssl_misc_unittest.cc \
ssl_record_unittest.cc \
ssl_resumption_unittest.cc \
ssl_renegotiation_unittest.cc \
ssl_skip_unittest.cc \
ssl_staticrsa_unittest.cc \
ssl_v2_client_hello_unittest.cc \
Expand Down
25 changes: 8 additions & 17 deletions gtests/ssl_gtest/ssl_cert_ext_unittest.cc
Expand Up @@ -82,9 +82,8 @@ TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsLegacy) {
ssl_kea_rsa));
EXPECT_EQ(SECSuccess, SSL_SetSignedCertTimestamps(server_->ssl_fd(),
&kSctItem, ssl_kea_rsa));
EXPECT_EQ(SECSuccess,
SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
PR_TRUE));

client_->SetOption(SSL_ENABLE_SIGNED_CERT_TIMESTAMPS, PR_TRUE);
SignedCertificateTimestampsExtractor timestamps_extractor(client_);

Connect();
Expand All @@ -96,9 +95,7 @@ TEST_P(TlsConnectGeneric, SignedCertificateTimestampsSuccess) {
EnsureTlsSetup();
EXPECT_TRUE(
server_->ConfigServerCert(TlsAgent::kServerRsa, true, &kExtraSctData));
EXPECT_EQ(SECSuccess,
SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
PR_TRUE));
client_->SetOption(SSL_ENABLE_SIGNED_CERT_TIMESTAMPS, PR_TRUE);
SignedCertificateTimestampsExtractor timestamps_extractor(client_);

Connect();
Expand All @@ -120,9 +117,7 @@ TEST_P(TlsConnectGeneric, SignedCertificateTimestampsInactiveClient) {

TEST_P(TlsConnectGeneric, SignedCertificateTimestampsInactiveServer) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess,
SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
PR_TRUE));
client_->SetOption(SSL_ENABLE_SIGNED_CERT_TIMESTAMPS, PR_TRUE);
SignedCertificateTimestampsExtractor timestamps_extractor(client_);

Connect();
Expand Down Expand Up @@ -173,16 +168,14 @@ TEST_P(TlsConnectGeneric, OcspNotRequested) {
// Even if the client asks, the server has nothing unless it is configured.
TEST_P(TlsConnectGeneric, OcspNotProvided) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
client_->SetAuthCertificateCallback(CheckNoOCSP);
Connect();
}

TEST_P(TlsConnectGenericPre13, OcspMangled) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
EXPECT_TRUE(
server_->ConfigServerCert(TlsAgent::kServerRsa, true, &kOcspExtraData));

Expand All @@ -197,8 +190,7 @@ TEST_P(TlsConnectGenericPre13, OcspMangled) {

TEST_P(TlsConnectGeneric, OcspSuccess) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
auto capture_ocsp =
std::make_shared<TlsExtensionCapture>(ssl_cert_status_xtn);
server_->SetPacketFilter(capture_ocsp);
Expand All @@ -225,8 +217,7 @@ TEST_P(TlsConnectGeneric, OcspSuccess) {

TEST_P(TlsConnectGeneric, OcspHugeSuccess) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);

uint8_t hugeOcspValue[16385];
memset(hugeOcspValue, 0xa1, sizeof(hugeOcspValue));
Expand Down
24 changes: 8 additions & 16 deletions gtests/ssl_gtest/ssl_dhe_unittest.cc
Expand Up @@ -59,8 +59,7 @@ TEST_P(TlsConnectTls13, SharesForBothEcdheAndDhe) {

TEST_P(TlsConnectGeneric, ConnectFfdheClient) {
EnableOnlyDheCiphers();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
auto groups_capture =
std::make_shared<TlsExtensionCapture>(ssl_supported_groups_xtn);
auto shares_capture =
Expand Down Expand Up @@ -90,8 +89,7 @@ TEST_P(TlsConnectGeneric, ConnectFfdheClient) {
// because the client automatically sends the supported groups extension.
TEST_P(TlsConnectGenericPre13, ConnectFfdheServer) {
EnableOnlyDheCiphers();
EXPECT_EQ(SECSuccess, SSL_OptionSet(server_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
server_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);

if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
Connect();
Expand Down Expand Up @@ -126,8 +124,7 @@ class TlsDheServerKeyExchangeDamager : public TlsHandshakeFilter {
// the signature until everything else has been checked.
TEST_P(TlsConnectGenericPre13, DamageServerKeyShare) {
EnableOnlyDheCiphers();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
server_->SetPacketFilter(std::make_shared<TlsDheServerKeyExchangeDamager>());

ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
Expand Down Expand Up @@ -289,8 +286,7 @@ class TlsDamageDHYTest
TEST_P(TlsDamageDHYTest, DamageServerY) {
EnableOnlyDheCiphers();
if (std::get<3>(GetParam())) {
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
}
TlsDheSkeChangeY::ChangeYTo change = std::get<2>(GetParam());
server_->SetPacketFilter(
Expand Down Expand Up @@ -320,8 +316,7 @@ TEST_P(TlsDamageDHYTest, DamageServerY) {
TEST_P(TlsDamageDHYTest, DamageClientY) {
EnableOnlyDheCiphers();
if (std::get<3>(GetParam())) {
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
}
// The filter on the server is required to capture the prime.
auto server_filter =
Expand Down Expand Up @@ -445,8 +440,7 @@ TEST_P(TlsConnectGenericPre13, PadDheP) {
// Note: This test case can take ages to generate the weak DH key.
TEST_P(TlsConnectGenericPre13, WeakDHGroup) {
EnableOnlyDheCiphers();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
EXPECT_EQ(SECSuccess,
SSL_EnableWeakDHEPrimeGroup(server_->ssl_fd(), PR_TRUE));

Expand Down Expand Up @@ -496,8 +490,7 @@ TEST_P(TlsConnectTls13, NamedGroupMismatch13) {
// custom group in contrast to the previous test.
TEST_P(TlsConnectGenericPre13, RequireNamedGroupsMismatchPre13) {
EnableOnlyDheCiphers();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
static const std::vector<SSLNamedGroup> server_groups = {ssl_grp_ffdhe_3072};
static const std::vector<SSLNamedGroup> client_groups = {ssl_grp_ec_secp256r1,
ssl_grp_ffdhe_2048};
Expand Down Expand Up @@ -525,8 +518,7 @@ TEST_P(TlsConnectGenericPre13, PreferredFfdhe) {

TEST_P(TlsConnectGenericPre13, MismatchDHE) {
EnableOnlyDheCiphers();
EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
static const SSLDHEGroupType serverGroups[] = {ssl_ff_dhe_3072_group};
EXPECT_EQ(SECSuccess, SSL_DHEGroupPrefSet(server_->ssl_fd(), serverGroups,
PR_ARRAY_SIZE(serverGroups)));
Expand Down
1 change: 1 addition & 0 deletions gtests/ssl_gtest/ssl_gtest.gyp
Expand Up @@ -34,6 +34,7 @@
'ssl_misc_unittest.cc',
'ssl_record_unittest.cc',
'ssl_resumption_unittest.cc',
'ssl_renegotiation_unittest.cc',
'ssl_skip_unittest.cc',
'ssl_staticrsa_unittest.cc',
'ssl_v2_client_hello_unittest.cc',
Expand Down
22 changes: 2 additions & 20 deletions gtests/ssl_gtest/ssl_loopback_unittest.cc
Expand Up @@ -168,24 +168,6 @@ TEST_P(TlsConnectDatagram, ConnectSrtp) {
SendReceive();
}

// 1.3 is disabled in the next few tests because we don't
// presently support resumption in 1.3.
TEST_P(TlsConnectStreamPre13, ConnectAndClientRenegotiate) {
Connect();
server_->PrepareForRenegotiate();
client_->StartRenegotiate();
Handshake();
CheckConnected();
}

TEST_P(TlsConnectStreamPre13, ConnectAndServerRenegotiate) {
Connect();
client_->PrepareForRenegotiate();
server_->StartRenegotiate();
Handshake();
CheckConnected();
}

TEST_P(TlsConnectGeneric, ConnectSendReceive) {
Connect();
SendReceive();
Expand Down Expand Up @@ -230,8 +212,8 @@ TEST_P(TlsConnectStream, ShortRead) {
// so we should never get it.
TEST_P(TlsConnectGeneric, ConnectWithCompressionEnabled) {
EnsureTlsSetup();
client_->EnableCompression();
server_->EnableCompression();
client_->SetOption(SSL_ENABLE_DEFLATE, PR_TRUE);
server_->SetOption(SSL_ENABLE_DEFLATE, PR_TRUE);
Connect();
EXPECT_FALSE(client_->is_compressed());
SendReceive();
Expand Down
144 changes: 144 additions & 0 deletions gtests/ssl_gtest/ssl_renegotiation_unittest.cc
@@ -0,0 +1,144 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include <functional>
#include <memory>
#include "secerr.h"
#include "ssl.h"
#include "sslerr.h"
#include "sslproto.h"

#include "gtest_utils.h"
#include "tls_connect.h"

namespace nss_test {

// 1.3 is disabled in the next few tests because we don't
// presently support resumption in 1.3.
TEST_P(TlsConnectStreamPre13, RenegotiateClient) {
Connect();
server_->PrepareForRenegotiate();
client_->StartRenegotiate();
Handshake();
CheckConnected();
}

TEST_P(TlsConnectStreamPre13, RenegotiateServer) {
Connect();
client_->PrepareForRenegotiate();
server_->StartRenegotiate();
Handshake();
CheckConnected();
}

// The renegotiation options shouldn't cause an error if TLS 1.3 is chosen.
TEST_F(TlsConnectTest, RenegotiationConfigTls13) {
EnsureTlsSetup();
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
server_->SetOption(SSL_ENABLE_RENEGOTIATION, SSL_RENEGOTIATE_UNRESTRICTED);
server_->SetOption(SSL_REQUIRE_SAFE_NEGOTIATION, PR_TRUE);
Connect();
SendReceive();
CheckKeys();
}

TEST_P(TlsConnectStream, ConnectTls10AndServerRenegotiateHigher) {
if (version_ == SSL_LIBRARY_VERSION_TLS_1_0) {
return;
}
// Set the client so it will accept any version from 1.0
// to |version_|.
client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, version_);
server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
SSL_LIBRARY_VERSION_TLS_1_0);
// Reset version so that the checks succeed.
uint16_t test_version = version_;
version_ = SSL_LIBRARY_VERSION_TLS_1_0;
Connect();

// Now renegotiate, with the server being set to do
// |version_|.
client_->PrepareForRenegotiate();
server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, test_version);
// Reset version and cipher suite so that the preinfo callback
// doesn't fail.
server_->ResetPreliminaryInfo();
server_->StartRenegotiate();

if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
ExpectAlert(server_, kTlsAlertUnexpectedMessage);
} else {
ExpectAlert(client_, kTlsAlertIllegalParameter);
}

Handshake();
if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
// In TLS 1.3, the server detects this problem.
client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
server_->CheckErrorCode(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED);
} else {
client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
}
}

TEST_P(TlsConnectStream, ConnectTls10AndClientRenegotiateHigher) {
if (version_ == SSL_LIBRARY_VERSION_TLS_1_0) {
return;
}
// Set the client so it will accept any version from 1.0
// to |version_|.
client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, version_);
server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
SSL_LIBRARY_VERSION_TLS_1_0);
// Reset version so that the checks succeed.
uint16_t test_version = version_;
version_ = SSL_LIBRARY_VERSION_TLS_1_0;
Connect();

// Now renegotiate, with the server being set to do
// |version_|.
server_->PrepareForRenegotiate();
server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, test_version);
// Reset version and cipher suite so that the preinfo callback
// doesn't fail.
server_->ResetPreliminaryInfo();
client_->StartRenegotiate();
if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
ExpectAlert(server_, kTlsAlertUnexpectedMessage);
} else {
ExpectAlert(client_, kTlsAlertIllegalParameter);
}
Handshake();
if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
// In TLS 1.3, the server detects this problem.
client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
server_->CheckErrorCode(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED);
} else {
client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
}
}

TEST_F(TlsConnectTest, Tls13RejectsRehandshakeClient) {
EnsureTlsSetup();
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
Connect();
SECStatus rv = SSL_ReHandshake(client_->ssl_fd(), PR_TRUE);
EXPECT_EQ(SECFailure, rv);
EXPECT_EQ(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, PORT_GetError());
}

TEST_F(TlsConnectTest, Tls13RejectsRehandshakeServer) {
EnsureTlsSetup();
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
Connect();
SECStatus rv = SSL_ReHandshake(server_->ssl_fd(), PR_TRUE);
EXPECT_EQ(SECFailure, rv);
EXPECT_EQ(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, PORT_GetError());
}

} // namespace nss_test
9 changes: 2 additions & 7 deletions gtests/ssl_gtest/ssl_resumption_unittest.cc
Expand Up @@ -355,10 +355,7 @@ TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceReuseKey) {

// This test parses the ServerKeyExchange, which isn't in 1.3
TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceNewKey) {
server_->EnsureTlsSetup();
SECStatus rv =
SSL_OptionSet(server_->ssl_fd(), SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
EXPECT_EQ(SECSuccess, rv);
server_->SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
auto i1 = std::make_shared<TlsInspectorRecordHandshakeMessage>(
kTlsHandshakeServerKeyExchange);
server_->SetPacketFilter(i1);
Expand All @@ -369,9 +366,7 @@ TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceNewKey) {

// Restart
Reset();
server_->EnsureTlsSetup();
rv = SSL_OptionSet(server_->ssl_fd(), SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
EXPECT_EQ(SECSuccess, rv);
server_->SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
auto i2 = std::make_shared<TlsInspectorRecordHandshakeMessage>(
kTlsHandshakeServerKeyExchange);
server_->SetPacketFilter(i2);
Expand Down
4 changes: 2 additions & 2 deletions gtests/ssl_gtest/ssl_staticrsa_unittest.cc
Expand Up @@ -71,7 +71,7 @@ TEST_P(TlsConnectGenericPre13, ConnectStaticRSABogusPMSVersionIgnore) {
EnableOnlyStaticRsaCiphers();
client_->SetPacketFilter(
std::make_shared<TlsInspectorClientHelloVersionChanger>(server_));
server_->DisableRollbackDetection();
server_->SetOption(SSL_ROLLBACK_DETECTION, PR_FALSE);
Connect();
}

Expand Down Expand Up @@ -102,7 +102,7 @@ TEST_P(TlsConnectStreamPre13,
EnableExtendedMasterSecret();
client_->SetPacketFilter(
std::make_shared<TlsInspectorClientHelloVersionChanger>(server_));
server_->DisableRollbackDetection();
server_->SetOption(SSL_ROLLBACK_DETECTION, PR_FALSE);
Connect();
}

Expand Down

0 comments on commit 5eacfc3

Please sign in to comment.