From b560bdb144622a15b6c6cbfbed29f379555eadd9 Mon Sep 17 00:00:00 2001 From: "J.C. Jones" Date: Fri, 3 Jan 2020 22:13:28 +0000 Subject: [PATCH] Bug 1602020 - land NSS NSS_3_49_RTM UPGRADE_NSS_RELEASE, r=kjacobs 2020-01-03 J.C. Jones * lib/nss/nss.h, lib/softoken/softkver.h, lib/util/nssutil.h: Set version numbers to 3.49 final [d41f5350554e] [NSS_3_49_RTM] 2020-01-02 Kevin Jacobs * gtests/ssl_gtest/ssl_version_unittest.cc, lib/ssl/ssl3con.c: Bug 1513586 - Set downgrade sentinel for client TLS versions lower than 1.2. r=mt Per-[[ https://tools.ietf.org/html/rfc8446#section-4.1.3 | RFC 8446 ]], the downgrade sentinel must be set by a TLS 1.3 server (and should be set by a TLS 1.2 server) that negotiates TLS 1.0 or 1.1. This patch corrects the behavior and adds a test. [993717228da0] 2020-01-02 J.C. Jones * .hgtags: Added tag NSS_3_49_BETA1 for changeset 9ecd41cd2fa3 [62d36f2ee1cc] Differential Revision: https://phabricator.services.mozilla.com/D58655 --HG-- extra : moz-landing-system : lando --- security/nss/TAG-INFO | 2 +- security/nss/coreconf/coreconf.dep | 1 - .../gtests/ssl_gtest/ssl_version_unittest.cc | 61 +++++++++++++++++++ security/nss/lib/nss/nss.h | 4 +- security/nss/lib/softoken/softkver.h | 4 +- security/nss/lib/ssl/ssl3con.c | 50 ++++++++------- security/nss/lib/util/nssutil.h | 4 +- 7 files changed, 95 insertions(+), 31 deletions(-) diff --git a/security/nss/TAG-INFO b/security/nss/TAG-INFO index 8e9bbfaa081e..2b7aed8721c3 100644 --- a/security/nss/TAG-INFO +++ b/security/nss/TAG-INFO @@ -1 +1 @@ -NSS_3_49_BETA1 \ No newline at end of file +NSS_3_49_RTM \ No newline at end of file diff --git a/security/nss/coreconf/coreconf.dep b/security/nss/coreconf/coreconf.dep index 590d1bfaeee3..5182f75552c8 100644 --- a/security/nss/coreconf/coreconf.dep +++ b/security/nss/coreconf/coreconf.dep @@ -10,4 +10,3 @@ */ #error "Do not include this header file." - diff --git a/security/nss/gtests/ssl_gtest/ssl_version_unittest.cc b/security/nss/gtests/ssl_gtest/ssl_version_unittest.cc index 3255bd512cc8..699edc64f568 100644 --- a/security/nss/gtests/ssl_gtest/ssl_version_unittest.cc +++ b/security/nss/gtests/ssl_gtest/ssl_version_unittest.cc @@ -102,6 +102,61 @@ TEST_F(TlsConnectTest, TestDisableDowngradeDetection) { server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE); } +typedef std::tuple // server version + TlsDowngradeProfile; + +class TlsDowngradeTest + : public TlsConnectTestBase, + public ::testing::WithParamInterface { + public: + TlsDowngradeTest() + : TlsConnectTestBase(std::get<0>(GetParam()), std::get<1>(GetParam())), + c_ver(std::get<1>(GetParam())), + s_ver(std::get<2>(GetParam())) {} + + protected: + const uint16_t c_ver; + const uint16_t s_ver; +}; + +TEST_P(TlsDowngradeTest, TlsDowngradeSentinelTest) { + static const uint8_t tls12_downgrade_random[] = {0x44, 0x4F, 0x57, 0x4E, + 0x47, 0x52, 0x44, 0x01}; + static const uint8_t tls1_downgrade_random[] = {0x44, 0x4F, 0x57, 0x4E, + 0x47, 0x52, 0x44, 0x00}; + static const size_t kRandomLen = 32; + + if (c_ver > s_ver) { + return; + } + + client_->SetVersionRange(c_ver, c_ver); + server_->SetVersionRange(c_ver, s_ver); + + auto sh = MakeTlsFilter(server_, ssl_hs_server_hello); + Connect(); + ASSERT_TRUE(sh->buffer().len() > (kRandomLen + 2)); + + const uint8_t* downgrade_sentinel = + sh->buffer().data() + 2 + kRandomLen - sizeof(tls1_downgrade_random); + if (c_ver < s_ver) { + if (c_ver == SSL_LIBRARY_VERSION_TLS_1_2) { + EXPECT_EQ(0, memcmp(downgrade_sentinel, tls12_downgrade_random, + sizeof(tls12_downgrade_random))); + } else { + EXPECT_EQ(0, memcmp(downgrade_sentinel, tls1_downgrade_random, + sizeof(tls1_downgrade_random))); + } + } else { + EXPECT_NE(0, memcmp(downgrade_sentinel, tls12_downgrade_random, + sizeof(tls12_downgrade_random))); + EXPECT_NE(0, memcmp(downgrade_sentinel, tls1_downgrade_random, + sizeof(tls1_downgrade_random))); + } +} + // TLS 1.1 clients do not check the random values, so we should // instead get a handshake failure alert from the server. TEST_F(TlsConnectTest, TestDowngradeDetectionToTls10) { @@ -280,4 +335,10 @@ TEST_F(TlsConnectStreamTls13, Ssl30ClientHelloWithSupportedVersions) { ConnectExpectAlert(server_, kTlsAlertProtocolVersion); } +INSTANTIATE_TEST_CASE_P( + TlsDowngradeSentinelTest, TlsDowngradeTest, + ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream, + TlsConnectTestBase::kTlsVAll, + TlsConnectTestBase::kTlsV12Plus)); + } // namespace nss_test diff --git a/security/nss/lib/nss/nss.h b/security/nss/lib/nss/nss.h index 292868f05f45..e370db09992b 100644 --- a/security/nss/lib/nss/nss.h +++ b/security/nss/lib/nss/nss.h @@ -22,12 +22,12 @@ * The format of the version string should be * ".[.[.]][ ][ ]" */ -#define NSS_VERSION "3.49" _NSS_CUSTOMIZED " Beta" +#define NSS_VERSION "3.49" _NSS_CUSTOMIZED #define NSS_VMAJOR 3 #define NSS_VMINOR 49 #define NSS_VPATCH 0 #define NSS_VBUILD 0 -#define NSS_BETA PR_TRUE +#define NSS_BETA PR_FALSE #ifndef RC_INVOKED diff --git a/security/nss/lib/softoken/softkver.h b/security/nss/lib/softoken/softkver.h index b73c18f5e958..08539eaede57 100644 --- a/security/nss/lib/softoken/softkver.h +++ b/security/nss/lib/softoken/softkver.h @@ -17,11 +17,11 @@ * The format of the version string should be * ".[.[.]][ ][ ]" */ -#define SOFTOKEN_VERSION "3.49" SOFTOKEN_ECC_STRING " Beta" +#define SOFTOKEN_VERSION "3.49" SOFTOKEN_ECC_STRING #define SOFTOKEN_VMAJOR 3 #define SOFTOKEN_VMINOR 49 #define SOFTOKEN_VPATCH 0 #define SOFTOKEN_VBUILD 0 -#define SOFTOKEN_BETA PR_TRUE +#define SOFTOKEN_BETA PR_FALSE #endif /* _SOFTKVER_H_ */ diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c index f3c723bbc2ed..60b247fd7cfb 100644 --- a/security/nss/lib/ssl/ssl3con.c +++ b/security/nss/lib/ssl/ssl3con.c @@ -394,12 +394,12 @@ static const SSLCipher2Mech alg2Mech[] = { { ssl_calg_chacha20, CKM_NSS_CHACHA20_POLY1305 }, }; -const PRUint8 tls13_downgrade_random[] = { 0x44, 0x4F, 0x57, 0x4E, - 0x47, 0x52, 0x44, 0x01 }; const PRUint8 tls12_downgrade_random[] = { 0x44, 0x4F, 0x57, 0x4E, - 0x47, 0x52, 0x44, 0x00 }; -PR_STATIC_ASSERT(sizeof(tls13_downgrade_random) == - sizeof(tls13_downgrade_random)); + 0x47, 0x52, 0x44, 0x01 }; +const PRUint8 tls1_downgrade_random[] = { 0x44, 0x4F, 0x57, 0x4E, + 0x47, 0x52, 0x44, 0x00 }; +PR_STATIC_ASSERT(sizeof(tls12_downgrade_random) == + sizeof(tls1_downgrade_random)); /* The ECCWrappedKeyInfo structure defines how various pieces of * information are laid out within wrappedSymmetricWrappingkey @@ -6713,13 +6713,13 @@ ssl_CheckServerRandom(sslSocket *ss) /* Both sections use the same sentinel region. */ PRUint8 *downgrade_sentinel = ss->ssl3.hs.server_random + - SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random); + SSL3_RANDOM_LENGTH - sizeof(tls12_downgrade_random); if (!PORT_Memcmp(downgrade_sentinel, - tls13_downgrade_random, - sizeof(tls13_downgrade_random)) || - !PORT_Memcmp(downgrade_sentinel, tls12_downgrade_random, - sizeof(tls12_downgrade_random))) { + sizeof(tls12_downgrade_random)) || + !PORT_Memcmp(downgrade_sentinel, + tls1_downgrade_random, + sizeof(tls1_downgrade_random))) { return SECFailure; } } @@ -8491,20 +8491,24 @@ ssl_GenerateServerRandom(sslSocket *ss) */ PRUint8 *downgradeSentinel = ss->ssl3.hs.server_random + - SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random); + SSL3_RANDOM_LENGTH - sizeof(tls12_downgrade_random); - switch (ss->vrange.max) { - case SSL_LIBRARY_VERSION_TLS_1_3: - PORT_Memcpy(downgradeSentinel, - tls13_downgrade_random, sizeof(tls13_downgrade_random)); - break; - case SSL_LIBRARY_VERSION_TLS_1_2: - PORT_Memcpy(downgradeSentinel, - tls12_downgrade_random, sizeof(tls12_downgrade_random)); - break; - default: - /* Do not change random. */ - break; + if (ss->vrange.max >= SSL_LIBRARY_VERSION_TLS_1_2) { + switch (ss->version) { + case SSL_LIBRARY_VERSION_TLS_1_2: + /* vrange.max > 1.2, since we didn't early exit above. */ + PORT_Memcpy(downgradeSentinel, + tls12_downgrade_random, sizeof(tls12_downgrade_random)); + break; + case SSL_LIBRARY_VERSION_TLS_1_1: + case SSL_LIBRARY_VERSION_TLS_1_0: + PORT_Memcpy(downgradeSentinel, + tls1_downgrade_random, sizeof(tls1_downgrade_random)); + break; + default: + /* Do not change random. */ + break; + } } return SECSuccess; diff --git a/security/nss/lib/util/nssutil.h b/security/nss/lib/util/nssutil.h index 2312e8c1e810..3c56bbe9b55f 100644 --- a/security/nss/lib/util/nssutil.h +++ b/security/nss/lib/util/nssutil.h @@ -19,12 +19,12 @@ * The format of the version string should be * ".[.[.]][ ]" */ -#define NSSUTIL_VERSION "3.49 Beta" +#define NSSUTIL_VERSION "3.49" #define NSSUTIL_VMAJOR 3 #define NSSUTIL_VMINOR 49 #define NSSUTIL_VPATCH 0 #define NSSUTIL_VBUILD 0 -#define NSSUTIL_BETA PR_TRUE +#define NSSUTIL_BETA PR_FALSE SEC_BEGIN_PROTOS