diff --git a/security/nss/TAG-INFO b/security/nss/TAG-INFO index 5e95d43bbaf0..85b08364b5eb 100644 --- a/security/nss/TAG-INFO +++ b/security/nss/TAG-INFO @@ -1 +1 @@ -NSS_3_18_RTM +NSS_3_18_1_BETA1 diff --git a/security/nss/cmd/platlibs.mk b/security/nss/cmd/platlibs.mk index 833952a5ad43..812a27fdc5c1 100644 --- a/security/nss/cmd/platlibs.mk +++ b/security/nss/cmd/platlibs.mk @@ -87,8 +87,8 @@ EXTRA_LIBS += \ $(DIST)/lib/$(LIB_PREFIX)nssb.$(LIB_SUFFIX) \ $(PKIXLIB) \ $(DBMLIB) \ - $(DIST)/lib/$(LIB_PREFIX)$(SQLITE_LIB_NAME).$(LIB_SUFFIX) \ - $(DIST)/lib/$(LIB_PREFIX)nssutil3.$(LIB_SUFFIX) \ + $(SQLITE_LIB_DIR)/$(LIB_PREFIX)$(SQLITE_LIB_NAME).$(LIB_SUFFIX) \ + $(NSSUTIL_LIB_DIR)/$(LIB_PREFIX)nssutil3.$(LIB_SUFFIX) \ $(NSPR_LIB_DIR)/$(NSPR31_LIB_PREFIX)plc4.$(LIB_SUFFIX) \ $(NSPR_LIB_DIR)/$(NSPR31_LIB_PREFIX)plds4.$(LIB_SUFFIX) \ $(NSPR_LIB_DIR)/$(NSPR31_LIB_PREFIX)nspr4.$(LIB_SUFFIX) \ @@ -135,7 +135,7 @@ endif # $(PROGRAM) has NO explicit dependencies on $(EXTRA_SHARED_LIBS) # $(EXTRA_SHARED_LIBS) come before $(OS_LIBS), except on AIX. EXTRA_SHARED_LIBS += \ - -L$(DIST)/lib \ + -L$(SQLITE_LIB_DIR) \ -l$(SQLITE_LIB_NAME) \ -L$(NSSUTIL_LIB_DIR) \ -lnssutil3 \ @@ -153,7 +153,7 @@ ifeq ($(OS_ARCH), WINNT) # $(PROGRAM) has explicit dependencies on $(EXTRA_LIBS) EXTRA_LIBS += \ $(DIST)/lib/$(LIB_PREFIX)sectool.$(LIB_SUFFIX) \ - $(DIST)/lib/$(IMPORT_LIB_PREFIX)nssutil3$(IMPORT_LIB_SUFFIX) \ + $(NSSUTIL_LIB_DIR)/$(IMPORT_LIB_PREFIX)nssutil3$(IMPORT_LIB_SUFFIX) \ $(DIST)/lib/$(IMPORT_LIB_PREFIX)smime3$(IMPORT_LIB_SUFFIX) \ $(DIST)/lib/$(IMPORT_LIB_PREFIX)ssl3$(IMPORT_LIB_SUFFIX) \ $(DIST)/lib/$(IMPORT_LIB_PREFIX)nss3$(IMPORT_LIB_SUFFIX) \ diff --git a/security/nss/coreconf/coreconf.dep b/security/nss/coreconf/coreconf.dep index 5182f75552c8..590d1bfaeee3 100644 --- a/security/nss/coreconf/coreconf.dep +++ b/security/nss/coreconf/coreconf.dep @@ -10,3 +10,4 @@ */ #error "Do not include this header file." + diff --git a/security/nss/coreconf/location.mk b/security/nss/coreconf/location.mk index 0eb9d91412e6..b11558a4586a 100644 --- a/security/nss/coreconf/location.mk +++ b/security/nss/coreconf/location.mk @@ -67,6 +67,10 @@ ifndef SOFTOKEN_LIB_DIR SOFTOKEN_LIB_DIR = $(DIST)/lib endif +ifndef SQLITE_LIB_DIR + SQLITE_LIB_DIR = $(DIST)/lib +endif + ifndef SQLITE_LIB_NAME SQLITE_LIB_NAME = sqlite3 endif diff --git a/security/nss/external_tests/ssl_gtest/manifest.mn b/security/nss/external_tests/ssl_gtest/manifest.mn index ee883e9ac15e..9b8669b3eebb 100644 --- a/security/nss/external_tests/ssl_gtest/manifest.mn +++ b/security/nss/external_tests/ssl_gtest/manifest.mn @@ -8,6 +8,7 @@ MODULE = nss CPPSRCS = \ ssl_loopback_unittest.cc \ + ssl_extension_unittest.cc \ ssl_gtest.cc \ test_io.cc \ tls_agent.cc \ diff --git a/security/nss/external_tests/ssl_gtest/ssl_extension_unittest.cc b/security/nss/external_tests/ssl_gtest/ssl_extension_unittest.cc new file mode 100644 index 000000000000..a11f905436a4 --- /dev/null +++ b/security/nss/external_tests/ssl_gtest/ssl_extension_unittest.cc @@ -0,0 +1,578 @@ +/* -*- 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 "ssl.h" +#include "sslproto.h" + +#include + +#include "tls_parser.h" +#include "tls_filter.h" +#include "tls_connect.h" + +namespace nss_test { + +class TlsExtensionFilter : public TlsHandshakeFilter { + protected: + virtual bool FilterHandshake(uint16_t version, uint8_t handshake_type, + const DataBuffer& input, DataBuffer* output) { + if (handshake_type == kTlsHandshakeClientHello) { + TlsParser parser(input); + if (!FindClientHelloExtensions(parser, version)) { + return false; + } + return FilterExtensions(parser, input, output); + } + if (handshake_type == kTlsHandshakeServerHello) { + TlsParser parser(input); + if (!FindServerHelloExtensions(parser, version)) { + return false; + } + return FilterExtensions(parser, input, output); + } + return false; + } + + virtual bool FilterExtension(uint16_t extension_type, + const DataBuffer& input, DataBuffer* output) = 0; + + public: + static bool FindClientHelloExtensions(TlsParser& parser, uint16_t version) { + if (!parser.Skip(2 + 32)) { // version + random + return false; + } + if (!parser.SkipVariable(1)) { // session ID + return false; + } + if (IsDtls(version) && !parser.SkipVariable(1)) { // DTLS cookie + return false; + } + if (!parser.SkipVariable(2)) { // cipher suites + return false; + } + if (!parser.SkipVariable(1)) { // compression methods + return false; + } + return true; + } + + static bool FindServerHelloExtensions(TlsParser& parser, uint16_t version) { + if (!parser.Skip(2 + 32)) { // version + random + return false; + } + if (!parser.SkipVariable(1)) { // session ID + return false; + } + if (!parser.Skip(2)) { // cipher suite + return false; + } + if (NormalizeTlsVersion(version) <= SSL_LIBRARY_VERSION_TLS_1_2) { + if (!parser.Skip(1)) { // compression method + return false; + } + } + return true; + } + + private: + bool FilterExtensions(TlsParser& parser, + const DataBuffer& input, DataBuffer* output) { + size_t length_offset = parser.consumed(); + uint32_t all_extensions; + if (!parser.Read(&all_extensions, 2)) { + return false; // no extensions, odd but OK + } + if (all_extensions != parser.remaining()) { + return false; // malformed + } + + bool changed = false; + + // Write out the start of the message. + output->Allocate(input.len()); + output->Write(0, input.data(), parser.consumed()); + size_t output_offset = parser.consumed(); + + while (parser.remaining()) { + uint32_t extension_type; + if (!parser.Read(&extension_type, 2)) { + return false; // malformed + } + + // Copy extension type. + output->Write(output_offset, extension_type, 2); + + DataBuffer extension; + if (!parser.ReadVariable(&extension, 2)) { + return false; // malformed + } + output_offset = ApplyFilter(static_cast(extension_type), extension, + output, output_offset + 2, &changed); + } + output->Truncate(output_offset); + + if (changed) { + size_t newlen = output->len() - length_offset - 2; + if (newlen >= 0x10000) { + return false; // bad: size increased too much + } + output->Write(length_offset, newlen, 2); + } + return changed; + } + + size_t ApplyFilter(uint16_t extension_type, const DataBuffer& extension, + DataBuffer* output, size_t offset, bool* changed) { + const DataBuffer* source = &extension; + DataBuffer filtered; + if (FilterExtension(extension_type, extension, &filtered) && + filtered.len() < 0x10000) { + *changed = true; + std::cerr << "extension old: " << extension << std::endl; + std::cerr << "extension new: " << filtered << std::endl; + source = &filtered; + } + + output->Write(offset, source->len(), 2); + output->Write(offset + 2, *source); + return offset + 2 + source->len(); + } +}; + +class TlsExtensionTruncator : public TlsExtensionFilter { + public: + TlsExtensionTruncator(uint16_t extension, size_t length) + : extension_(extension), length_(length) {} + virtual bool FilterExtension(uint16_t extension_type, + const DataBuffer& input, DataBuffer* output) { + if (extension_type != extension_) { + return false; + } + if (input.len() <= length_) { + return false; + } + + output->Assign(input.data(), length_); + return true; + } + private: + uint16_t extension_; + size_t length_; +}; + +class TlsExtensionDamager : public TlsExtensionFilter { + public: + TlsExtensionDamager(uint16_t extension, size_t index) + : extension_(extension), index_(index) {} + virtual bool FilterExtension(uint16_t extension_type, + const DataBuffer& input, DataBuffer* output) { + if (extension_type != extension_) { + return false; + } + + *output = input; + output->data()[index_] += 73; // Increment selected for maximum damage + return true; + } + private: + uint16_t extension_; + size_t index_; +}; + +class TlsExtensionReplacer : public TlsExtensionFilter { + public: + TlsExtensionReplacer(uint16_t extension, const DataBuffer& data) + : extension_(extension), data_(data) {} + virtual bool FilterExtension(uint16_t extension_type, + const DataBuffer& input, DataBuffer* output) { + if (extension_type != extension_) { + return false; + } + + *output = data_; + return true; + } + private: + uint16_t extension_; + DataBuffer data_; +}; + +class TlsExtensionInjector : public TlsHandshakeFilter { + public: + TlsExtensionInjector(uint16_t ext, DataBuffer& data) + : extension_(ext), data_(data) {} + + virtual bool FilterHandshake(uint16_t version, uint8_t handshake_type, + const DataBuffer& input, DataBuffer* output) { + size_t offset; + if (handshake_type == kTlsHandshakeClientHello) { + TlsParser parser(input); + if (!TlsExtensionFilter::FindClientHelloExtensions(parser, version)) { + return false; + } + offset = parser.consumed(); + } else if (handshake_type == kTlsHandshakeServerHello) { + TlsParser parser(input); + if (!TlsExtensionFilter::FindServerHelloExtensions(parser, version)) { + return false; + } + offset = parser.consumed(); + } else { + return false; + } + + *output = input; + + std::cerr << "Pre:" << input << std::endl; + std::cerr << "Lof:" << offset << std::endl; + + // Increase the size of the extensions. + uint16_t* len_addr = reinterpret_cast(output->data() + offset); + std::cerr << "L-p:" << ntohs(*len_addr) << std::endl; + *len_addr = htons(ntohs(*len_addr) + data_.len() + 4); + std::cerr << "L-i:" << ntohs(*len_addr) << std::endl; + + + // 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. + output->Splice(data_, offset + 6); + + std::cerr << "Aft:" << *output << std::endl; + return true; + } + + private: + uint16_t extension_; + DataBuffer data_; +}; + +class TlsExtensionTestBase : public TlsConnectTestBase { + protected: + TlsExtensionTestBase(Mode mode, uint16_t version) + : TlsConnectTestBase(mode, version) {} + + void ClientHelloErrorTest(PacketFilter* filter, + uint8_t alert = kTlsAlertDecodeError) { + auto alert_recorder = new TlsAlertRecorder(); + server_->SetPacketFilter(alert_recorder); + if (filter) { + client_->SetPacketFilter(filter); + } + ConnectExpectFail(); + ASSERT_EQ(kTlsAlertFatal, alert_recorder->level()); + ASSERT_EQ(alert, alert_recorder->description()); + } + + void ServerHelloErrorTest(PacketFilter* filter, + uint8_t alert = kTlsAlertDecodeError) { + auto alert_recorder = new TlsAlertRecorder(); + client_->SetPacketFilter(alert_recorder); + if (filter) { + server_->SetPacketFilter(filter); + } + ConnectExpectFail(); + ASSERT_EQ(kTlsAlertFatal, alert_recorder->level()); + ASSERT_EQ(alert, alert_recorder->description()); + } + + static void InitSimpleSni(DataBuffer* extension) { + const char* name = "host.name"; + const size_t namelen = PL_strlen(name); + extension->Allocate(namelen + 5); + extension->Write(0, namelen + 3, 2); + extension->Write(2, static_cast(0), 1); // 0 == hostname + extension->Write(3, namelen, 2); + extension->Write(5, reinterpret_cast(name), namelen); + } +}; + +class TlsExtensionTestDtls + : public TlsExtensionTestBase, + public ::testing::WithParamInterface { + public: + TlsExtensionTestDtls() : TlsExtensionTestBase(DGRAM, GetParam()) {} +}; + +class TlsExtensionTest12Plus + : public TlsExtensionTestBase, + public ::testing::WithParamInterface { + public: + TlsExtensionTest12Plus() + : TlsExtensionTestBase(TlsConnectTestBase::ToMode(GetParam()), + SSL_LIBRARY_VERSION_TLS_1_2) {} +}; + +class TlsExtensionTestGeneric + : public TlsExtensionTestBase, + public ::testing::WithParamInterface> { + public: + TlsExtensionTestGeneric() + : TlsExtensionTestBase(TlsConnectTestBase::ToMode((std::get<0>(GetParam()))), + std::get<1>(GetParam())) {} +}; + +TEST_P(TlsExtensionTestGeneric, DamageSniLength) { + ClientHelloErrorTest(new TlsExtensionDamager(ssl_server_name_xtn, 1)); +} + +TEST_P(TlsExtensionTestGeneric, DamageSniHostLength) { + ClientHelloErrorTest(new TlsExtensionDamager(ssl_server_name_xtn, 4)); +} + +TEST_P(TlsExtensionTestGeneric, TruncateSni) { + ClientHelloErrorTest(new TlsExtensionTruncator(ssl_server_name_xtn, 7)); +} + +// A valid extension that appears twice will be reported as unsupported. +TEST_P(TlsExtensionTestGeneric, RepeatSni) { + DataBuffer extension; + InitSimpleSni(&extension); + ClientHelloErrorTest(new TlsExtensionInjector(ssl_server_name_xtn, extension), + kTlsAlertIllegalParameter); +} + +// An SNI entry with zero length is considered invalid (strangely, not if it is +// the last entry, which is probably a bug). +TEST_P(TlsExtensionTestGeneric, BadSni) { + DataBuffer simple; + InitSimpleSni(&simple); + DataBuffer extension; + extension.Allocate(simple.len() + 3); + extension.Write(0, static_cast(0), 3); + extension.Write(3, simple); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_server_name_xtn, extension)); +} + +TEST_P(TlsExtensionTestGeneric, EmptyAlpnExtension) { + EnableAlpn(); + DataBuffer extension; + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension), + kTlsAlertIllegalParameter); +} + +// An empty ALPN isn't considered bad, though it does lead to there being no +// protocol for the server to select. +TEST_P(TlsExtensionTestGeneric, EmptyAlpnList) { + EnableAlpn(); + const uint8_t val[] = { 0x00, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension), + kTlsAlertNoApplicationProtocol); +} + +TEST_P(TlsExtensionTestGeneric, OneByteAlpn) { + EnableAlpn(); + ClientHelloErrorTest(new TlsExtensionTruncator(ssl_app_layer_protocol_xtn, 1)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnMissingValue) { + EnableAlpn(); + // This will leave the length of the second entry, but no value. + ClientHelloErrorTest(new TlsExtensionTruncator(ssl_app_layer_protocol_xtn, 5)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnZeroLength) { + EnableAlpn(); + const uint8_t val[] = { 0x01, 0x61, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnMismatch) { + const uint8_t client_alpn[] = { 0x01, 0x61 }; + client_->EnableAlpn(client_alpn, sizeof(client_alpn)); + const uint8_t server_alpn[] = { 0x02, 0x61, 0x62 }; + server_->EnableAlpn(server_alpn, sizeof(server_alpn)); + + ClientHelloErrorTest(nullptr, kTlsAlertNoApplicationProtocol); +} + +TEST_P(TlsExtensionTestGeneric, AlpnReturnedEmptyList) { + EnableAlpn(); + const uint8_t val[] = { 0x00, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ServerHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnReturnedEmptyName) { + EnableAlpn(); + const uint8_t val[] = { 0x00, 0x01, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ServerHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnReturnedListTrailingData) { + EnableAlpn(); + const uint8_t val[] = { 0x00, 0x02, 0x01, 0x61, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ServerHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnReturnedExtraEntry) { + EnableAlpn(); + const uint8_t val[] = { 0x00, 0x04, 0x01, 0x61, 0x01, 0x62 }; + DataBuffer extension(val, sizeof(val)); + ServerHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnReturnedBadListLength) { + EnableAlpn(); + const uint8_t val[] = { 0x00, 0x99, 0x01, 0x61, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ServerHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension)); +} + +TEST_P(TlsExtensionTestGeneric, AlpnReturnedBadNameLength) { + EnableAlpn(); + const uint8_t val[] = { 0x00, 0x02, 0x99, 0x61 }; + DataBuffer extension(val, sizeof(val)); + ServerHelloErrorTest(new TlsExtensionReplacer(ssl_app_layer_protocol_xtn, extension)); +} + +TEST_P(TlsExtensionTestDtls, SrtpShort) { + EnableSrtp(); + ClientHelloErrorTest(new TlsExtensionTruncator(ssl_use_srtp_xtn, 3)); +} + +TEST_P(TlsExtensionTestDtls, SrtpOdd) { + EnableSrtp(); + const uint8_t val[] = { 0x00, 0x01, 0xff, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_use_srtp_xtn, extension)); +} + +TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsBadLength) { + const uint8_t val[] = { 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_signature_algorithms_xtn, + extension)); +} + +TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsTrailingData) { + const uint8_t val[] = { 0x00, 0x02, 0x04, 0x01, 0x00 }; // sha-256, rsa + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_signature_algorithms_xtn, + extension)); +} + +TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsEmpty) { + const uint8_t val[] = { 0x00, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_signature_algorithms_xtn, + extension)); +} + +TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsOddLength) { + const uint8_t val[] = { 0x00, 0x01, 0x04 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_signature_algorithms_xtn, + extension)); +} + +// The extension handling ignores unsupported hashes, so breaking that has no +// effect on success rates. However, ssl3_SendServerKeyExchange catches an +// unsupported signature algorithm. + +// This actually fails with a decryption error (fatal alert 51). That's a bad +// to fail, since any tampering with the handshake will trigger that alert when +// verifying the Finished message. Thus, this test is disabled until this error +// is turned into an alert. +TEST_P(TlsExtensionTest12Plus, DISABLED_SignatureAlgorithmsSigUnsupported) { + const uint8_t val[] = { 0x00, 0x02, 0x04, 0x99 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_signature_algorithms_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, SupportedCurvesShort) { + EnableSomeECDHECiphers(); + const uint8_t val[] = { 0x00, 0x01, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_elliptic_curves_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, SupportedCurvesBadLength) { + EnableSomeECDHECiphers(); + const uint8_t val[] = { 0x09, 0x99, 0x00, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_elliptic_curves_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, SupportedCurvesTrailingData) { + EnableSomeECDHECiphers(); + const uint8_t val[] = { 0x00, 0x02, 0x00, 0x00, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_elliptic_curves_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, SupportedPointsEmpty) { + EnableSomeECDHECiphers(); + const uint8_t val[] = { 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_ec_point_formats_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, SupportedPointsBadLength) { + EnableSomeECDHECiphers(); + const uint8_t val[] = { 0x99, 0x00, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_ec_point_formats_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, SupportedPointsTrailingData) { + EnableSomeECDHECiphers(); + const uint8_t val[] = { 0x01, 0x00, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_ec_point_formats_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, RenegotiationInfoBadLength) { + const uint8_t val[] = { 0x99 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_renegotiation_info_xtn, + extension)); +} + +TEST_P(TlsExtensionTestGeneric, RenegotiationInfoMismatch) { + const uint8_t val[] = { 0x01, 0x00 }; + DataBuffer extension(val, sizeof(val)); + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_renegotiation_info_xtn, + extension)); +} + +// The extension has to contain a length. +TEST_P(TlsExtensionTestGeneric, RenegotiationInfoExtensionEmpty) { + DataBuffer extension; + ClientHelloErrorTest(new TlsExtensionReplacer(ssl_renegotiation_info_xtn, + extension)); +} + +INSTANTIATE_TEST_CASE_P(ExtensionTls10, TlsExtensionTestGeneric, + ::testing::Combine( + TlsConnectTestBase::kTlsModesStream, + TlsConnectTestBase::kTlsV10)); +INSTANTIATE_TEST_CASE_P(ExtensionVariants, TlsExtensionTestGeneric, + ::testing::Combine( + TlsConnectTestBase::kTlsModesAll, + TlsConnectTestBase::kTlsV11V12)); +INSTANTIATE_TEST_CASE_P(ExtensionTls12Plus, TlsExtensionTest12Plus, + TlsConnectTestBase::kTlsModesAll); +INSTANTIATE_TEST_CASE_P(ExtensionDgram, TlsExtensionTestDtls, + TlsConnectTestBase::kTlsV11V12); + +} // namespace nspr_test diff --git a/security/nss/external_tests/ssl_gtest/ssl_loopback_unittest.cc b/security/nss/external_tests/ssl_gtest/ssl_loopback_unittest.cc index d70e2ceebb4b..b372412f8520 100644 --- a/security/nss/external_tests/ssl_gtest/ssl_loopback_unittest.cc +++ b/security/nss/external_tests/ssl_gtest/ssl_loopback_unittest.cc @@ -44,13 +44,7 @@ TEST_P(TlsConnectGeneric, SetupOnly) {} TEST_P(TlsConnectGeneric, Connect) { Connect(); - - // Check that we negotiated the expected version. - if (mode_ == STREAM) { - client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_0); - } else { - client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_1); - } + client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_2); } TEST_P(TlsConnectGeneric, ConnectResumed) { @@ -152,29 +146,6 @@ TEST_P(TlsConnectGeneric, ConnectClientNoneServerBoth) { CheckResumption(RESUME_NONE); } -TEST_P(TlsConnectGeneric, ConnectTLS_1_1_Only) { - EnsureTlsSetup(); - client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1, - SSL_LIBRARY_VERSION_TLS_1_1); - - server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1, - SSL_LIBRARY_VERSION_TLS_1_1); - - Connect(); - - client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_1); -} - -TEST_P(TlsConnectGeneric, ConnectTLS_1_2_Only) { - EnsureTlsSetup(); - client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, - SSL_LIBRARY_VERSION_TLS_1_2); - server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, - SSL_LIBRARY_VERSION_TLS_1_2); - Connect(); - client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_2); -} - TEST_P(TlsConnectGeneric, ConnectAlpn) { EnableAlpn(); Connect(); @@ -182,19 +153,19 @@ TEST_P(TlsConnectGeneric, ConnectAlpn) { server_->CheckAlpn(SSL_NEXT_PROTO_NEGOTIATED, "a"); } -TEST_F(DtlsConnectTest, ConnectSrtp) { +TEST_P(TlsConnectDatagram, ConnectSrtp) { EnableSrtp(); Connect(); CheckSrtp(); } -TEST_F(TlsConnectTest, ConnectECDHE) { +TEST_P(TlsConnectStream, ConnectECDHE) { EnableSomeECDHECiphers(); Connect(); client_->CheckKEAType(ssl_kea_ecdh); } -TEST_F(TlsConnectTest, ConnectECDHETwiceReuseKey) { +TEST_P(TlsConnectStream, ConnectECDHETwiceReuseKey) { EnableSomeECDHECiphers(); TlsInspectorRecordHandshakeMessage* i1 = new TlsInspectorRecordHandshakeMessage(kTlsHandshakeServerKeyExchange); @@ -223,7 +194,7 @@ TEST_F(TlsConnectTest, ConnectECDHETwiceReuseKey) { dhe1.public_key_.len())); } -TEST_F(TlsConnectTest, ConnectECDHETwiceNewKey) { +TEST_P(TlsConnectStream, ConnectECDHETwiceNewKey) { EnableSomeECDHECiphers(); SECStatus rv = SSL_OptionSet(server_->ssl_fd(), SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE); @@ -257,7 +228,17 @@ TEST_F(TlsConnectTest, ConnectECDHETwiceNewKey) { dhe1.public_key_.len()))); } -INSTANTIATE_TEST_CASE_P(Variants, TlsConnectGeneric, - ::testing::Values("TLS", "DTLS")); +INSTANTIATE_TEST_CASE_P(VariantsStream10, TlsConnectGeneric, + ::testing::Combine( + TlsConnectTestBase::kTlsModesStream, + TlsConnectTestBase::kTlsV10)); +INSTANTIATE_TEST_CASE_P(VariantsAll, TlsConnectGeneric, + ::testing::Combine( + TlsConnectTestBase::kTlsModesAll, + TlsConnectTestBase::kTlsV11V12)); +INSTANTIATE_TEST_CASE_P(VersionsDatagram, TlsConnectDatagram, + TlsConnectTestBase::kTlsV11V12); +INSTANTIATE_TEST_CASE_P(VersionsDatagram, TlsConnectStream, + TlsConnectTestBase::kTlsV11V12); } // namespace nspr_test diff --git a/security/nss/external_tests/ssl_gtest/test_io.cc b/security/nss/external_tests/ssl_gtest/test_io.cc index 2bfd091789ce..70b2b1e1b0a5 100644 --- a/security/nss/external_tests/ssl_gtest/test_io.cc +++ b/security/nss/external_tests/ssl_gtest/test_io.cc @@ -328,7 +328,7 @@ int32_t DummyPrSocket::Recv(void *buf, int32_t buflen) { } Packet *front = input_.front(); - if (buflen < front->len()) { + if (static_cast(buflen) < front->len()) { PR_ASSERT(false); PR_SetError(PR_BUFFER_OVERFLOW_ERROR, 0); return -1; @@ -344,33 +344,23 @@ int32_t DummyPrSocket::Recv(void *buf, int32_t buflen) { } int32_t DummyPrSocket::Write(const void *buf, int32_t length) { - DataBuffer packet(static_cast(buf), - static_cast(length)); - if (filter_) { - DataBuffer filtered; - if (filter_->Filter(packet, &filtered)) { - if (WriteDirect(filtered) != filtered.len()) { - PR_SetError(PR_IO_ERROR, 0); - return -1; - } - LOG("Wrote: " << packet); - // libssl can't handle if this reports something other than the length of - // what was passed in (or less, but we're not doing partial writes). - return packet.len(); - } - } - - return WriteDirect(packet); -} - -int32_t DummyPrSocket::WriteDirect(const DataBuffer& packet) { if (!peer_) { PR_SetError(PR_IO_ERROR, 0); return -1; } - peer_->PacketReceived(packet); - return static_cast(packet.len()); // ignore truncation + DataBuffer packet(static_cast(buf), + static_cast(length)); + DataBuffer filtered; + if (filter_ && filter_->Filter(packet, &filtered)) { + LOG("Filtered packet: " << filtered); + peer_->PacketReceived(filtered); + } else { + peer_->PacketReceived(packet); + } + // libssl can't handle it if this reports something other than the length + // of what was passed in (or less, but we're not doing partial writes). + return static_cast(packet.len()); } Poller *Poller::instance; diff --git a/security/nss/external_tests/ssl_gtest/test_io.h b/security/nss/external_tests/ssl_gtest/test_io.h index d2424c60ce25..d55d3e4d8ef4 100644 --- a/security/nss/external_tests/ssl_gtest/test_io.h +++ b/security/nss/external_tests/ssl_gtest/test_io.h @@ -12,6 +12,7 @@ #include #include #include +#include #include "prio.h" @@ -36,6 +37,10 @@ class PacketFilter { enum Mode { STREAM, DGRAM }; +inline std::ostream& operator<<(std::ostream& os, Mode m) { + return os << ((m == STREAM) ? "TLS" : "DTLS"); +} + class DummyPrSocket { public: ~DummyPrSocket(); @@ -52,7 +57,6 @@ class DummyPrSocket { int32_t Read(void* data, int32_t len); int32_t Recv(void* buf, int32_t buflen); int32_t Write(const void* buf, int32_t length); - int32_t WriteDirect(const DataBuffer& data); Mode mode() const { return mode_; } bool readable() const { return !input_.empty(); } diff --git a/security/nss/external_tests/ssl_gtest/tls_agent.cc b/security/nss/external_tests/ssl_gtest/tls_agent.cc index 6eeb651f5248..76e924574db3 100644 --- a/security/nss/external_tests/ssl_gtest/tls_agent.cc +++ b/security/nss/external_tests/ssl_gtest/tls_agent.cc @@ -58,8 +58,12 @@ bool TlsAgent::EnsureTlsSetup() { if (rv != SECSuccess) return false; } - SECStatus rv = SSL_AuthCertificateHook(ssl_fd_, AuthCertificateHook, - reinterpret_cast(this)); + SECStatus rv = SSL_VersionRangeSet(ssl_fd_, &vrange_); + EXPECT_EQ(SECSuccess, rv); + if (rv != SECSuccess) return false; + + rv = SSL_AuthCertificateHook(ssl_fd_, AuthCertificateHook, + reinterpret_cast(this)); EXPECT_EQ(SECSuccess, rv); if (rv != SECSuccess) return false; @@ -104,8 +108,13 @@ void TlsAgent::SetSessionCacheEnabled(bool en) { } void TlsAgent::SetVersionRange(uint16_t minver, uint16_t maxver) { - SSLVersionRange range = {minver, maxver}; - ASSERT_EQ(SECSuccess, SSL_VersionRangeSet(ssl_fd_, &range)); + vrange_.min = minver; + vrange_.max = maxver; + + if (ssl_fd_) { + SECStatus rv = SSL_VersionRangeSet(ssl_fd_, &vrange_); + ASSERT_EQ(SECSuccess, rv); + } } void TlsAgent::CheckKEAType(SSLKEAType type) const { diff --git a/security/nss/external_tests/ssl_gtest/tls_agent.h b/security/nss/external_tests/ssl_gtest/tls_agent.h index aee835ea7334..dd2a5e01b4d2 100644 --- a/security/nss/external_tests/ssl_gtest/tls_agent.h +++ b/security/nss/external_tests/ssl_gtest/tls_agent.h @@ -14,6 +14,9 @@ #include "test_io.h" +#define GTEST_HAS_RTTI 0 +#include "gtest/gtest.h" + namespace nss_test { #define LOG(msg) std::cerr << name_ << ": " << msg << std::endl @@ -40,6 +43,10 @@ class TlsAgent : public PollTarget { state_(INIT) { memset(&info_, 0, sizeof(info_)); memset(&csinfo_, 0, sizeof(csinfo_)); + SECStatus rv = SSL_VersionRangeGetDefault(mode_ == STREAM ? + ssl_variant_stream : ssl_variant_datagram, + &vrange_); + EXPECT_EQ(SECSuccess, rv); } ~TlsAgent() { @@ -95,6 +102,9 @@ class TlsAgent : public PollTarget { PRFileDesc* ssl_fd() { return ssl_fd_; } + uint16_t min_version() const { return vrange_.min; } + uint16_t max_version() const { return vrange_.max; } + bool version(uint16_t* version) const { if (state_ != CONNECTED) return false; @@ -103,6 +113,12 @@ class TlsAgent : public PollTarget { return true; } + uint16_t version() const { + EXPECT_EQ(CONNECTED, state_); + + return info_.protocolVersion; + } + bool cipher_suite(int16_t* cipher_suite) const { if (state_ != CONNECTED) return false; @@ -163,6 +179,7 @@ class TlsAgent : public PollTarget { State state_; SSLChannelInfo info_; SSLCipherSuiteInfo csinfo_; + SSLVersionRange vrange_; }; } // namespace nss_test diff --git a/security/nss/external_tests/ssl_gtest/tls_connect.cc b/security/nss/external_tests/ssl_gtest/tls_connect.cc index 6c6fd1a53d02..6101b271a796 100644 --- a/security/nss/external_tests/ssl_gtest/tls_connect.cc +++ b/security/nss/external_tests/ssl_gtest/tls_connect.cc @@ -8,16 +8,56 @@ #include +#include "sslproto.h" #include "gtest_utils.h" extern std::string g_working_dir_path; namespace nss_test { -TlsConnectTestBase::TlsConnectTestBase(Mode mode) +static const std::string kTlsModesStreamArr[] = {"TLS"}; +::testing::internal::ParamGenerator + TlsConnectTestBase::kTlsModesStream = ::testing::ValuesIn(kTlsModesStreamArr); +static const std::string kTlsModesAllArr[] = {"TLS", "DTLS"}; +::testing::internal::ParamGenerator + TlsConnectTestBase::kTlsModesAll = ::testing::ValuesIn(kTlsModesAllArr); +static const uint16_t kTlsV10Arr[] = {SSL_LIBRARY_VERSION_TLS_1_0}; +::testing::internal::ParamGenerator + TlsConnectTestBase::kTlsV10 = ::testing::ValuesIn(kTlsV10Arr); +static const uint16_t kTlsV11V12Arr[] = {SSL_LIBRARY_VERSION_TLS_1_1, + SSL_LIBRARY_VERSION_TLS_1_2}; +::testing::internal::ParamGenerator + TlsConnectTestBase::kTlsV11V12 = ::testing::ValuesIn(kTlsV11V12Arr); +// TODO: add TLS 1.3 +static const uint16_t kTlsV12PlusArr[] = {SSL_LIBRARY_VERSION_TLS_1_2}; +::testing::internal::ParamGenerator + TlsConnectTestBase::kTlsV12Plus = ::testing::ValuesIn(kTlsV12PlusArr); + +static std::string VersionString(uint16_t version) { + switch(version) { + case 0: + return "(no version)"; + case SSL_LIBRARY_VERSION_TLS_1_0: + return "1.0"; + case SSL_LIBRARY_VERSION_TLS_1_1: + return "1.1"; + case SSL_LIBRARY_VERSION_TLS_1_2: + return "1.2"; + default: + std::cerr << "Invalid version: " << version << std::endl; + EXPECT_TRUE(false); + return ""; + } +} + +TlsConnectTestBase::TlsConnectTestBase(Mode mode, uint16_t version) : mode_(mode), client_(new TlsAgent("client", TlsAgent::CLIENT, mode_)), - server_(new TlsAgent("server", TlsAgent::SERVER, mode_)) {} + server_(new TlsAgent("server", TlsAgent::SERVER, mode_)), + version_(version), + session_ids_() { + std::cerr << "Version: " << mode_ << " " << VersionString(version_) << std::endl; +} TlsConnectTestBase::~TlsConnectTestBase() { delete client_; @@ -49,6 +89,11 @@ void TlsConnectTestBase::Init() { client_->SetPeer(server_); server_->SetPeer(client_); + + if (version_) { + client_->SetVersionRange(version_, version_); + server_->SetVersionRange(version_, version_); + } } void TlsConnectTestBase::Reset() { @@ -72,14 +117,21 @@ void TlsConnectTestBase::Handshake() { client_->Handshake(); server_->Handshake(); - ASSERT_TRUE_WAIT(client_->state() != TlsAgent::CONNECTING && - server_->state() != TlsAgent::CONNECTING, + ASSERT_TRUE_WAIT((client_->state() != TlsAgent::CONNECTING) && + (server_->state() != TlsAgent::CONNECTING), 5000); + } void TlsConnectTestBase::Connect() { Handshake(); + // Check the version is as expected + ASSERT_EQ(client_->version(), server_->version()); + ASSERT_EQ(std::min(client_->max_version(), + server_->max_version()), + client_->version()); + ASSERT_EQ(TlsAgent::CONNECTED, client_->state()); ASSERT_EQ(TlsAgent::CONNECTED, server_->state()); @@ -90,14 +142,15 @@ void TlsConnectTestBase::Connect() { ASSERT_TRUE(ret); ASSERT_EQ(cipher_suite1, cipher_suite2); - std::cerr << "Connected with cipher suite " << client_->cipher_suite_name() + std::cerr << "Connected with version " << client_->version() + << " cipher suite " << client_->cipher_suite_name() << std::endl; // Check and store session ids. std::vector sid_c1 = client_->session_id(); - ASSERT_EQ(32, sid_c1.size()); + ASSERT_EQ(32U, sid_c1.size()); std::vector sid_s1 = server_->session_id(); - ASSERT_EQ(32, sid_s1.size()); + ASSERT_EQ(32U, sid_s1.size()); ASSERT_EQ(sid_c1, sid_s1); session_ids_.push_back(sid_c1); } @@ -136,7 +189,7 @@ void TlsConnectTestBase::CheckResumption(SessionResumptionMode expected) { if (resume_ct) { // Check that the last two session ids match. - ASSERT_GE(2, session_ids_.size()); + ASSERT_GE(2U, session_ids_.size()); ASSERT_EQ(session_ids_[session_ids_.size()-1], session_ids_[session_ids_.size()-2]); } @@ -163,8 +216,7 @@ void TlsConnectTestBase::CheckSrtp() { } TlsConnectGeneric::TlsConnectGeneric() - : TlsConnectTestBase((GetParam() == "TLS") ? STREAM : DGRAM) { - std::cerr << "Variant: " << GetParam() << std::endl; -} + : TlsConnectTestBase(TlsConnectTestBase::ToMode(std::get<0>(GetParam())), + std::get<1>(GetParam())) {} } // namespace nss_test diff --git a/security/nss/external_tests/ssl_gtest/tls_connect.h b/security/nss/external_tests/ssl_gtest/tls_connect.h index c263fe83f01e..a981399f683b 100644 --- a/security/nss/external_tests/ssl_gtest/tls_connect.h +++ b/security/nss/external_tests/ssl_gtest/tls_connect.h @@ -7,6 +7,8 @@ #ifndef tls_connect_h_ #define tls_connect_h_ +#include + #include "sslt.h" #include "tls_agent.h" @@ -19,7 +21,17 @@ namespace nss_test { // A generic TLS connection test base. class TlsConnectTestBase : public ::testing::Test { public: - TlsConnectTestBase(Mode mode); + static ::testing::internal::ParamGenerator kTlsModesStream; + static ::testing::internal::ParamGenerator kTlsModesAll; + static ::testing::internal::ParamGenerator kTlsV10; + static ::testing::internal::ParamGenerator kTlsV11V12; + static ::testing::internal::ParamGenerator kTlsV12Plus; + + static inline Mode ToMode(const std::string& str) { + return str == "TLS" ? STREAM : DGRAM; + } + + TlsConnectTestBase(Mode mode, uint16_t version); virtual ~TlsConnectTestBase(); void SetUp(); @@ -46,30 +58,35 @@ class TlsConnectTestBase : public ::testing::Test { void EnableAlpn(); void EnableSrtp(); void CheckSrtp(); - protected: + Mode mode_; TlsAgent* client_; TlsAgent* server_; + uint16_t version_; std::vector> session_ids_; }; // A TLS-only test base. -class TlsConnectTest : public TlsConnectTestBase { +class TlsConnectStream : public TlsConnectTestBase, + public ::testing::WithParamInterface { public: - TlsConnectTest() : TlsConnectTestBase(STREAM) {} + TlsConnectStream() : TlsConnectTestBase(STREAM, GetParam()) {} }; // A DTLS-only test base. -class DtlsConnectTest : public TlsConnectTestBase { +class TlsConnectDatagram : public TlsConnectTestBase, + public ::testing::WithParamInterface { public: - DtlsConnectTest() : TlsConnectTestBase(DGRAM) {} + TlsConnectDatagram() : TlsConnectTestBase(DGRAM, GetParam()) {} }; -// A generic test class that can be either STREAM or DGRAM. This is configured -// in ssl_loopback_unittest.cc. All uses of this should use TEST_P(). -class TlsConnectGeneric : public TlsConnectTestBase, - public ::testing::WithParamInterface { +// A generic test class that can be either STREAM or DGRAM and a single version +// of TLS. This is configured in ssl_loopback_unittest.cc. All uses of this +// should use TEST_P(). +class TlsConnectGeneric + : public TlsConnectTestBase, + public ::testing::WithParamInterface> { public: TlsConnectGeneric(); }; diff --git a/security/nss/external_tests/ssl_gtest/tls_filter.cc b/security/nss/external_tests/ssl_gtest/tls_filter.cc index 3cbe9e5ac0e8..4ed74e4aa564 100644 --- a/security/nss/external_tests/ssl_gtest/tls_filter.cc +++ b/security/nss/external_tests/ssl_gtest/tls_filter.cc @@ -192,6 +192,8 @@ bool TlsAlertRecorder::FilterRecord(uint8_t content_type, uint16_t version, return false; } + std::cerr << "Alert: " << input << std::endl; + TlsParser parser(input); uint8_t lvl; if (!parser.Read(&lvl)) { diff --git a/security/nss/lib/nss/nss.h b/security/nss/lib/nss/nss.h index e596f00b797b..bb4d208154f2 100644 --- a/security/nss/lib/nss/nss.h +++ b/security/nss/lib/nss/nss.h @@ -33,12 +33,12 @@ * The format of the version string should be * ".[.[.]][ ][ ]" */ -#define NSS_VERSION "3.18" _NSS_ECC_STRING _NSS_CUSTOMIZED +#define NSS_VERSION "3.18.1" _NSS_ECC_STRING _NSS_CUSTOMIZED " Beta" #define NSS_VMAJOR 3 #define NSS_VMINOR 18 -#define NSS_VPATCH 0 -#define NSS_VBUILD 2 -#define NSS_BETA PR_FALSE +#define NSS_VPATCH 1 +#define NSS_VBUILD 0 +#define NSS_BETA PR_TRUE #ifndef RC_INVOKED diff --git a/security/nss/lib/softoken/config.mk b/security/nss/lib/softoken/config.mk index 6058e71a761c..2924b24f86de 100644 --- a/security/nss/lib/softoken/config.mk +++ b/security/nss/lib/softoken/config.mk @@ -22,7 +22,7 @@ RESNAME = $(LIBRARY_NAME).rc ifdef NS_USE_GCC EXTRA_SHARED_LIBS += \ - -L$(DIST)/lib \ + -L$(SQLITE_LIB_DIR) \ -l$(SQLITE_LIB_NAME) \ -L$(NSSUTIL_LIB_DIR) \ -lnssutil3 \ @@ -34,7 +34,7 @@ EXTRA_SHARED_LIBS += \ else # ! NS_USE_GCC EXTRA_SHARED_LIBS += \ - $(DIST)/lib/$(SQLITE_LIB_NAME).lib \ + $(SQLITE_LIB_DIR)/$(SQLITE_LIB_NAME).lib \ $(NSSUTIL_LIB_DIR)/nssutil3.lib \ $(NSPR_LIB_DIR)/$(NSPR31_LIB_PREFIX)plc4.lib \ $(NSPR_LIB_DIR)/$(NSPR31_LIB_PREFIX)plds4.lib \ @@ -47,7 +47,7 @@ else # $(PROGRAM) has NO explicit dependencies on $(EXTRA_SHARED_LIBS) # $(EXTRA_SHARED_LIBS) come before $(OS_LIBS), except on AIX. EXTRA_SHARED_LIBS += \ - -L$(DIST)/lib \ + -L$(SQLITE_LIB_DIR) \ -l$(SQLITE_LIB_NAME) \ -L$(NSSUTIL_LIB_DIR) \ -lnssutil3 \ diff --git a/security/nss/lib/softoken/softkver.h b/security/nss/lib/softoken/softkver.h index d3de246f8b08..67075e613970 100644 --- a/security/nss/lib/softoken/softkver.h +++ b/security/nss/lib/softoken/softkver.h @@ -25,11 +25,11 @@ * The format of the version string should be * ".[.[.]][ ][ ]" */ -#define SOFTOKEN_VERSION "3.18" SOFTOKEN_ECC_STRING +#define SOFTOKEN_VERSION "3.18.1" SOFTOKEN_ECC_STRING " Beta" #define SOFTOKEN_VMAJOR 3 #define SOFTOKEN_VMINOR 18 -#define SOFTOKEN_VPATCH 0 -#define SOFTOKEN_VBUILD 2 -#define SOFTOKEN_BETA PR_FALSE +#define SOFTOKEN_VPATCH 1 +#define SOFTOKEN_VBUILD 0 +#define SOFTOKEN_BETA PR_TRUE #endif /* _SOFTKVER_H_ */ diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c index 529eb42b504b..492b321fe252 100644 --- a/security/nss/lib/ssl/ssl3con.c +++ b/security/nss/lib/ssl/ssl3con.c @@ -2788,6 +2788,12 @@ ssl3_SendRecord( sslSocket * ss, PORT_Assert( ss->opt.noLocks || ssl_HaveXmitBufLock(ss) ); + if (ss->ssl3.fatalAlertSent) { + SSL_TRC(3, ("%d: SSL3[%d] Suppress write, fatal alert already sent", + SSL_GETPID(), ss->fd)); + return SECFailure; + } + capRecordVersion = ((flags & ssl_SEND_FLAG_CAP_RECORD_VERSION) != 0); if (capRecordVersion) { @@ -3233,6 +3239,9 @@ SSL3_SendAlert(sslSocket *ss, SSL3AlertLevel level, SSL3AlertDescription desc) ? ssl_SEND_FLAG_FORCE_INTO_BUFFER : 0); rv = (sent >= 0) ? SECSuccess : (SECStatus)sent; } + if (level == alert_fatal) { + ss->ssl3.fatalAlertSent = PR_TRUE; + } ssl_ReleaseXmitBufLock(ss); ssl_ReleaseSSL3HandshakeLock(ss); return rv; /* error set by ssl3_FlushHandshake or ssl3_SendRecord */ diff --git a/security/nss/lib/ssl/ssl3ecc.c b/security/nss/lib/ssl/ssl3ecc.c index 555c89dca5fe..aca2b74d46ae 100644 --- a/security/nss/lib/ssl/ssl3ecc.c +++ b/security/nss/lib/ssl/ssl3ecc.c @@ -1,3 +1,4 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ /* * SSL3 Protocol * @@ -1184,8 +1185,7 @@ ssl3_HandleSupportedPointFormatsXtn(sslSocket *ss, PRUint16 ex_type, if (data->len < 2 || data->len > 255 || !data->data || data->len != (unsigned int)data->data[0] + 1) { - /* malformed */ - goto loser; + return ssl3_DecodeError(ss); } for (i = data->len; --i > 0; ) { if (data->data[i] == 0) { @@ -1196,10 +1196,10 @@ ssl3_HandleSupportedPointFormatsXtn(sslSocket *ss, PRUint16 ex_type, return rv; } } -loser: + /* evil client doesn't support uncompressed */ ssl3_DisableECCSuites(ss, ecSuites); - return SECFailure; + return SECSuccess; } @@ -1220,7 +1220,7 @@ ECName ssl3_GetSvrCertCurveName(sslSocket *ss) return ec_curve; } -/* Ensure that the curve in our server cert is one of the ones suppored +/* Ensure that the curve in our server cert is one of the ones supported * by the remote client, and disable all ECC cipher suites if not. */ SECStatus @@ -1231,26 +1231,34 @@ ssl3_HandleSupportedCurvesXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) PRUint32 mutualCurves = 0; PRUint16 svrCertCurveName; - if (!data->data || data->len < 4 || data->len > 65535) - goto loser; + if (!data->data || data->len < 4) { + (void)ssl3_DecodeError(ss); + return SECFailure; + } + /* get the length of elliptic_curve_list */ list_len = ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); if (list_len < 0 || data->len != list_len || (data->len % 2) != 0) { - /* malformed */ - goto loser; + (void)ssl3_DecodeError(ss); + return SECFailure; } /* build bit vector of peer's supported curve names */ while (data->len) { - PRInt32 curve_name = - ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); + PRInt32 curve_name = + ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); + if (curve_name < 0) { + return SECFailure; /* fatal alert already sent */ + } if (curve_name > ec_noName && curve_name < ec_pastLastName) { peerCurves |= (1U << curve_name); } } /* What curves do we support in common? */ mutualCurves = ss->ssl3.hs.negotiatedECCurves &= peerCurves; - if (!mutualCurves) { /* no mutually supported EC Curves */ - goto loser; + if (!mutualCurves) { + /* no mutually supported EC Curves, disable ECC */ + ssl3_DisableECCSuites(ss, ecSuites); + return SECSuccess; } /* if our ECC cert doesn't use one of these supported curves, @@ -1266,12 +1274,7 @@ ssl3_HandleSupportedCurvesXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) */ ssl3_DisableECCSuites(ss, ecdh_ecdsa_suites); ssl3_DisableECCSuites(ss, ecdhe_ecdsa_suites); - return SECFailure; - -loser: - /* no common curve supported */ - ssl3_DisableECCSuites(ss, ecSuites); - return SECFailure; + return SECSuccess; } #endif /* NSS_DISABLE_ECC */ diff --git a/security/nss/lib/ssl/ssl3ext.c b/security/nss/lib/ssl/ssl3ext.c index 36608669d9b2..6965a6df4ec7 100644 --- a/security/nss/lib/ssl/ssl3ext.c +++ b/security/nss/lib/ssl/ssl3ext.c @@ -1,3 +1,4 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ /* * SSL3 Protocol * @@ -64,10 +65,14 @@ static PRInt32 ssl3_ClientSendAppProtoXtn(sslSocket *ss, PRBool append, PRUint32 maxBytes); static PRInt32 ssl3_ServerSendAppProtoXtn(sslSocket *ss, PRBool append, PRUint32 maxBytes); -static PRInt32 ssl3_SendUseSRTPXtn(sslSocket *ss, PRBool append, - PRUint32 maxBytes); -static SECStatus ssl3_HandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, - SECItem *data); +static PRInt32 ssl3_ClientSendUseSRTPXtn(sslSocket *ss, PRBool append, + PRUint32 maxBytes); +static PRInt32 ssl3_ServerSendUseSRTPXtn(sslSocket *ss, PRBool append, + PRUint32 maxBytes); +static SECStatus ssl3_ClientHandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, + SECItem *data); +static SECStatus ssl3_ServerHandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, + SECItem *data); static PRInt32 ssl3_ServerSendStatusRequestXtn(sslSocket * ss, PRBool append, PRUint32 maxBytes); static SECStatus ssl3_ServerHandleStatusRequestXtn(sslSocket *ss, @@ -247,7 +252,7 @@ static const ssl3HelloExtensionHandler clientHelloHandlers[] = { { ssl_renegotiation_info_xtn, &ssl3_HandleRenegotiationInfoXtn }, { ssl_next_proto_nego_xtn, &ssl3_ServerHandleNextProtoNegoXtn }, { ssl_app_layer_protocol_xtn, &ssl3_ServerHandleAppProtoXtn }, - { ssl_use_srtp_xtn, &ssl3_HandleUseSRTPXtn }, + { ssl_use_srtp_xtn, &ssl3_ServerHandleUseSRTPXtn }, { ssl_cert_status_xtn, &ssl3_ServerHandleStatusRequestXtn }, { ssl_signature_algorithms_xtn, &ssl3_ServerHandleSigAlgsXtn }, { ssl_tls13_draft_version_xtn, &ssl3_ServerHandleDraftVersionXtn }, @@ -263,7 +268,7 @@ static const ssl3HelloExtensionHandler serverHelloHandlersTLS[] = { { ssl_renegotiation_info_xtn, &ssl3_HandleRenegotiationInfoXtn }, { ssl_next_proto_nego_xtn, &ssl3_ClientHandleNextProtoNegoXtn }, { ssl_app_layer_protocol_xtn, &ssl3_ClientHandleAppProtoXtn }, - { ssl_use_srtp_xtn, &ssl3_HandleUseSRTPXtn }, + { ssl_use_srtp_xtn, &ssl3_ClientHandleUseSRTPXtn }, { ssl_cert_status_xtn, &ssl3_ClientHandleStatusRequestXtn }, { -1, NULL } }; @@ -290,7 +295,7 @@ ssl3HelloExtensionSender clientHelloSendersTLS[SSL_MAX_EXTENSIONS] = { { ssl_session_ticket_xtn, &ssl3_SendSessionTicketXtn }, { ssl_next_proto_nego_xtn, &ssl3_ClientSendNextProtoNegoXtn }, { ssl_app_layer_protocol_xtn, &ssl3_ClientSendAppProtoXtn }, - { ssl_use_srtp_xtn, &ssl3_SendUseSRTPXtn }, + { ssl_use_srtp_xtn, &ssl3_ClientSendUseSRTPXtn }, { ssl_cert_status_xtn, &ssl3_ClientSendStatusRequestXtn }, { ssl_signature_algorithms_xtn, &ssl3_ClientSendSigAlgsXtn }, { ssl_tls13_draft_version_xtn, &ssl3_ClientSendDraftVersionXtn }, @@ -398,13 +403,7 @@ ssl3_HandleServerNameXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) PRInt32 listLenBytes = 0; if (!ss->sec.isServer) { - /* Verify extension_data is empty. */ - if (data->data || data->len || - !ssl3_ExtensionNegotiated(ss, ssl_server_name_xtn)) { - /* malformed or was not initiated by the client.*/ - return SECFailure; - } - return SECSuccess; + return SECSuccess; /* ignore extension */ } /* Server side - consume client data and register server sender. */ @@ -414,33 +413,38 @@ ssl3_HandleServerNameXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) } /* length of server_name_list */ listLenBytes = ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); - if (listLenBytes == 0 || listLenBytes != data->len) { + if (listLenBytes < 0 || listLenBytes != data->len) { + (void)ssl3_DecodeError(ss); return SECFailure; } + if (listLenBytes == 0) { + return SECSuccess; /* ignore an empty extension */ + } ldata = *data; /* Calculate the size of the array.*/ while (listLenBytes > 0) { SECItem litem; SECStatus rv; - PRInt32 type; - /* Name Type (sni_host_name) */ + PRInt32 type; + /* Skip Name Type (sni_host_name); checks are on the second pass */ type = ssl3_ConsumeHandshakeNumber(ss, 1, &ldata.data, &ldata.len); - if (!ldata.len) { + if (type < 0) { /* i.e., SECFailure cast to PRint32 */ return SECFailure; } rv = ssl3_ConsumeHandshakeVariable(ss, &litem, 2, &ldata.data, &ldata.len); if (rv != SECSuccess) { - return SECFailure; + return rv; } - /* Adjust total length for cunsumed item, item len and type.*/ + /* Adjust total length for consumed item, item len and type.*/ listLenBytes -= litem.len + 3; if (listLenBytes > 0 && !ldata.len) { + (void)ssl3_DecodeError(ss); return SECFailure; } listCount += 1; } if (!listCount) { - return SECFailure; + return SECFailure; /* nothing we can act on */ } names = PORT_ZNewArray(SECItem, listCount); if (!names) { @@ -455,6 +459,7 @@ ssl3_HandleServerNameXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) type = ssl3_ConsumeHandshakeNumber(ss, 1, &data->data, &data->len); /* Check if we have such type in the list */ for (j = 0;j < listCount && names[j].data;j++) { + /* TODO bug 998524: .type is not assigned a value */ if (names[j].type == type) { nametypePresent = PR_TRUE; break; @@ -464,7 +469,10 @@ ssl3_HandleServerNameXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) rv = ssl3_ConsumeHandshakeVariable(ss, &names[namesPos], 2, &data->data, &data->len); if (rv != SECSuccess) { - goto loser; + PORT_Assert(0); + PORT_Free(names); + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + return rv; } if (nametypePresent == PR_FALSE) { namesPos += 1; @@ -479,10 +487,6 @@ ssl3_HandleServerNameXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) xtnData->negotiated[xtnData->numNegotiated++] = ssl_server_name_xtn; return SECSuccess; - -loser: - PORT_Free(names); - return SECFailure; } /* Called by both clients and servers. @@ -603,17 +607,11 @@ ssl3_ValidateNextProtoNego(const unsigned char* data, unsigned int length) * store protocol identifiers in null-terminated strings. */ if (newOffset > length || data[offset] == 0) { - PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); return SECFailure; } offset = newOffset; } - if (offset > length) { - PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); - return SECFailure; - } - return SECSuccess; } @@ -626,34 +624,41 @@ ssl3_SelectAppProtocol(sslSocket *ss, PRUint16 ex_type, SECItem *data) SECItem result = { siBuffer, resultBuffer, 0 }; rv = ssl3_ValidateNextProtoNego(data->data, data->len); - if (rv != SECSuccess) + if (rv != SECSuccess) { + PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); return rv; + } PORT_Assert(ss->nextProtoCallback); rv = ss->nextProtoCallback(ss->nextProtoArg, ss->fd, data->data, data->len, - result.data, &result.len, sizeof resultBuffer); - if (rv != SECSuccess) - return rv; - /* If the callback wrote more than allowed to |result| it has corrupted our - * stack. */ - if (result.len > sizeof resultBuffer) { - PORT_SetError(SEC_ERROR_OUTPUT_LEN); + result.data, &result.len, sizeof(resultBuffer)); + if (rv != SECSuccess) { + /* Expect callback to call PORT_SetError() */ + (void)SSL3_SendAlert(ss, alert_fatal, internal_error); return SECFailure; } + /* If the callback wrote more than allowed to |result| it has corrupted our + * stack. */ + if (result.len > sizeof(resultBuffer)) { + PORT_SetError(SEC_ERROR_OUTPUT_LEN); + /* TODO: crash */ + return SECFailure; + } + + SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); + if (ex_type == ssl_app_layer_protocol_xtn && ss->ssl3.nextProtoState != SSL_NEXT_PROTO_NEGOTIATED) { - /* The callback might say OK, but then it's picked a default. - * That's OK for NPN, but not ALPN. */ - SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); + /* The callback might say OK, but then it picks a default value - one + * that was not listed. That's OK for NPN, but not ALPN. */ PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_NO_PROTOCOL); (void)SSL3_SendAlert(ss, alert_fatal, no_application_protocol); return SECFailure; } ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type; - - SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); return SECITEM_CopyItem(NULL, &ss->ssl3.nextProto, &result); } @@ -669,17 +674,16 @@ ssl3_ServerHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) if (ss->firstHsDone || data->len == 0) { /* Clients MUST send a non-empty ALPN extension. */ PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter); return SECFailure; } - /* unlike NPN, ALPN has extra redundant length information so that - * the extension is the same in both ClientHello and ServerHello */ + /* Unlike NPN, ALPN has extra redundant length information so that + * the extension is the same in both ClientHello and ServerHello. */ count = ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); - if (count < 0) { - return SECFailure; /* fatal alert was sent */ - } if (count != data->len) { - return ssl3_DecodeError(ss); + (void)ssl3_DecodeError(ss); + return SECFailure; } if (!ss->nextProtoCallback) { @@ -694,8 +698,13 @@ ssl3_ServerHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) /* prepare to send back a response, if we negotiated */ if (ss->ssl3.nextProtoState == SSL_NEXT_PROTO_NEGOTIATED) { - return ssl3_RegisterServerHelloExtensionSender( + rv = ssl3_RegisterServerHelloExtensionSender( ss, ex_type, ssl3_ServerSendAppProtoXtn); + if (rv != SECSuccess) { + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + (void)SSL3_SendAlert(ss, alert_fatal, internal_error); + return rv; + } } return SECSuccess; } @@ -713,7 +722,8 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type, * we've negotiated NPN then we're required to send the NPN handshake * message. Thus, these two extensions cannot both be negotiated on the * same connection. */ - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + PORT_SetError(SSL_ERROR_BAD_SERVER); + (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter); return SECFailure; } @@ -722,7 +732,9 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type, * that an application erroneously cleared the callback between the time * we sent the ClientHello and now. */ if (!ss->nextProtoCallback) { + PORT_Assert(0); PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_NO_CALLBACK); + (void)SSL3_SendAlert(ss, alert_fatal, internal_error); return SECFailure; } @@ -732,8 +744,8 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type, static SECStatus ssl3_ClientHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) { - const unsigned char* d = data->data; - PRUint16 name_list_len; + SECStatus rv; + PRInt32 list_len; SECItem protocol_name; if (ssl3_ExtensionNegotiated(ss, ssl_next_proto_nego_xtn)) { @@ -743,22 +755,30 @@ ssl3_ClientHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) /* The extension data from the server has the following format: * uint16 name_list_len; - * uint8 len; + * uint8 len; // where len >= 1 * uint8 protocol_name[len]; */ if (data->len < 4 || data->len > 2 + 1 + 255) { PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); return SECFailure; } - name_list_len = ((PRUint16) d[0]) << 8 | - ((PRUint16) d[1]); - if (name_list_len != data->len - 2 || d[2] != data->len - 3) { + list_len = ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); + /* The list has to be the entire extension. */ + if (list_len != data->len) { PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); return SECFailure; } - protocol_name.data = data->data + 3; - protocol_name.len = data->len - 3; + rv = ssl3_ConsumeHandshakeVariable(ss, &protocol_name, 1, + &data->data, &data->len); + /* The list must have exactly one value. */ + if (rv != SECSuccess || data->len != 0) { + PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); + return SECFailure; + } SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); ss->ssl3.nextProtoState = SSL_NEXT_PROTO_SELECTED; @@ -1386,8 +1406,9 @@ ssl3_ServerHandleSessionTicketXtn(sslSocket *ss, PRUint16 ex_type, SSL3Statistics *ssl3stats; /* Ignore the SessionTicket extension if processing is disabled. */ - if (!ss->opt.enableSessionTickets) + if (!ss->opt.enableSessionTickets) { return SECSuccess; + } /* Keep track of negotiated extensions. */ ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type; @@ -1445,8 +1466,9 @@ ssl3_ServerHandleSessionTicketXtn(sslSocket *ss, PRUint16 ex_type, extension_data.len = data->len; if (ssl3_ParseEncryptedSessionTicket(ss, data, &enc_session_ticket) - != SECSuccess) - return SECFailure; + != SECSuccess) { + return SECSuccess; /* Pretend it isn't there */ + } /* Get session ticket keys. */ #ifndef NO_PKCS11_BYPASS @@ -1874,18 +1896,22 @@ ssl3_HandleHelloExtensions(sslSocket *ss, SSL3Opaque **b, PRUint32 *length) /* get the data for this extension, so we can pass it or skip it. */ rv = ssl3_ConsumeHandshakeVariable(ss, &extension_data, 2, b, length); if (rv != SECSuccess) - return rv; + return rv; /* alert already sent */ /* Check whether the server sent an extension which was not advertised * in the ClientHello. */ if (!ss->sec.isServer && - !ssl3_ClientExtensionAdvertised(ss, extension_type)) - return SECFailure; /* TODO: send unsupported_extension alert */ + !ssl3_ClientExtensionAdvertised(ss, extension_type)) { + (void)SSL3_SendAlert(ss, alert_fatal, unsupported_extension); + return SECFailure; + } /* Check whether an extension has been sent multiple times. */ - if (ssl3_ExtensionNegotiated(ss, extension_type)) + if (ssl3_ExtensionNegotiated(ss, extension_type)) { + (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter); return SECFailure; + } /* find extension_type in table of Hello Extension Handlers */ for (handler = handlers; handler->ex_type >= 0; handler++) { @@ -1893,9 +1919,13 @@ ssl3_HandleHelloExtensions(sslSocket *ss, SSL3Opaque **b, PRUint32 *length) if (handler->ex_type == extension_type) { rv = (*handler->ex_handler)(ss, (PRUint16)extension_type, &extension_data); - /* Ignore this result */ - /* Treat all bad extensions as unrecognized types. */ - break; + if (rv != SECSuccess) { + if (!ss->ssl3.fatalAlertSent) { + /* send a generic alert if the handler didn't already */ + (void)SSL3_SendAlert(ss, alert_fatal, handshake_failure); + } + return SECFailure; + } } } } @@ -2027,13 +2057,14 @@ ssl3_HandleRenegotiationInfoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) len = ss->sec.isServer ? ss->ssl3.hs.finishedBytes : ss->ssl3.hs.finishedBytes * 2; } - if (data->len != 1 + len || - data->data[0] != len || (len && - NSS_SecureMemcmp(ss->ssl3.hs.finishedMsgs.data, - data->data + 1, len))) { - /* Can we do this here? Or, must we arrange for the caller to do it? */ - (void)SSL3_SendAlert(ss, alert_fatal, handshake_failure); + if (data->len != 1 + len || data->data[0] != len ) { + (void)ssl3_DecodeError(ss); + return SECFailure; + } + if (len && NSS_SecureMemcmp(ss->ssl3.hs.finishedMsgs.data, + data->data + 1, len)) { PORT_SetError(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE); + (void)SSL3_SendAlert(ss, alert_fatal, handshake_failure); return SECFailure; } /* remember that we got this extension and it was correct. */ @@ -2042,13 +2073,13 @@ ssl3_HandleRenegotiationInfoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) if (ss->sec.isServer) { /* prepare to send back the appropriate response */ rv = ssl3_RegisterServerHelloExtensionSender(ss, ex_type, - ssl3_SendRenegotiationInfoXtn); + ssl3_SendRenegotiationInfoXtn); } return rv; } static PRInt32 -ssl3_SendUseSRTPXtn(sslSocket *ss, PRBool append, PRUint32 maxBytes) +ssl3_ClientSendUseSRTPXtn(sslSocket *ss, PRBool append, PRUint32 maxBytes) { PRUint32 ext_data_len; PRInt16 i; @@ -2057,65 +2088,139 @@ ssl3_SendUseSRTPXtn(sslSocket *ss, PRBool append, PRUint32 maxBytes) if (!ss) return 0; - if (!ss->sec.isServer) { - /* Client side */ + if (!IS_DTLS(ss) || !ss->ssl3.dtlsSRTPCipherCount) + return 0; /* Not relevant */ - if (!IS_DTLS(ss) || !ss->ssl3.dtlsSRTPCipherCount) - return 0; /* Not relevant */ + ext_data_len = 2 + 2 * ss->ssl3.dtlsSRTPCipherCount + 1; - ext_data_len = 2 + 2 * ss->ssl3.dtlsSRTPCipherCount + 1; - - if (append && maxBytes >= 4 + ext_data_len) { - /* Extension type */ - rv = ssl3_AppendHandshakeNumber(ss, ssl_use_srtp_xtn, 2); - if (rv != SECSuccess) return -1; - /* Length of extension data */ - rv = ssl3_AppendHandshakeNumber(ss, ext_data_len, 2); - if (rv != SECSuccess) return -1; - /* Length of the SRTP cipher list */ - rv = ssl3_AppendHandshakeNumber(ss, - 2 * ss->ssl3.dtlsSRTPCipherCount, - 2); - if (rv != SECSuccess) return -1; - /* The SRTP ciphers */ - for (i = 0; i < ss->ssl3.dtlsSRTPCipherCount; i++) { - rv = ssl3_AppendHandshakeNumber(ss, - ss->ssl3.dtlsSRTPCiphers[i], - 2); - } - /* Empty MKI value */ - ssl3_AppendHandshakeVariable(ss, NULL, 0, 1); - - ss->xtnData.advertised[ss->xtnData.numAdvertised++] = - ssl_use_srtp_xtn; - } - - return 4 + ext_data_len; - } - - /* Server side */ - if (append && maxBytes >= 9) { + if (append && maxBytes >= 4 + ext_data_len) { /* Extension type */ rv = ssl3_AppendHandshakeNumber(ss, ssl_use_srtp_xtn, 2); if (rv != SECSuccess) return -1; /* Length of extension data */ - rv = ssl3_AppendHandshakeNumber(ss, 5, 2); + rv = ssl3_AppendHandshakeNumber(ss, ext_data_len, 2); if (rv != SECSuccess) return -1; /* Length of the SRTP cipher list */ - rv = ssl3_AppendHandshakeNumber(ss, 2, 2); - if (rv != SECSuccess) return -1; - /* The selected cipher */ - rv = ssl3_AppendHandshakeNumber(ss, ss->ssl3.dtlsSRTPCipherSuite, 2); + rv = ssl3_AppendHandshakeNumber(ss, + 2 * ss->ssl3.dtlsSRTPCipherCount, + 2); if (rv != SECSuccess) return -1; + /* The SRTP ciphers */ + for (i = 0; i < ss->ssl3.dtlsSRTPCipherCount; i++) { + rv = ssl3_AppendHandshakeNumber(ss, + ss->ssl3.dtlsSRTPCiphers[i], + 2); + } /* Empty MKI value */ ssl3_AppendHandshakeVariable(ss, NULL, 0, 1); + + ss->xtnData.advertised[ss->xtnData.numAdvertised++] = + ssl_use_srtp_xtn; } + return 4 + ext_data_len; +} + +static PRInt32 +ssl3_ServerSendUseSRTPXtn(sslSocket *ss, PRBool append, PRUint32 maxBytes) +{ + SECStatus rv; + + /* Server side */ + if (!append || maxBytes < 9) { + return 9; + } + + /* Extension type */ + rv = ssl3_AppendHandshakeNumber(ss, ssl_use_srtp_xtn, 2); + if (rv != SECSuccess) return -1; + /* Length of extension data */ + rv = ssl3_AppendHandshakeNumber(ss, 5, 2); + if (rv != SECSuccess) return -1; + /* Length of the SRTP cipher list */ + rv = ssl3_AppendHandshakeNumber(ss, 2, 2); + if (rv != SECSuccess) return -1; + /* The selected cipher */ + rv = ssl3_AppendHandshakeNumber(ss, ss->ssl3.dtlsSRTPCipherSuite, 2); + if (rv != SECSuccess) return -1; + /* Empty MKI value */ + ssl3_AppendHandshakeVariable(ss, NULL, 0, 1); + return 9; } static SECStatus -ssl3_HandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) +ssl3_ClientHandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) +{ + SECStatus rv; + SECItem ciphers = {siBuffer, NULL, 0}; + PRUint16 i; + PRUint16 cipher = 0; + PRBool found = PR_FALSE; + SECItem litem; + + if (!data->data || !data->len) { + (void)ssl3_DecodeError(ss); + return SECFailure; + } + + /* Get the cipher list */ + rv = ssl3_ConsumeHandshakeVariable(ss, &ciphers, 2, + &data->data, &data->len); + if (rv != SECSuccess) { + return SECFailure; /* fatal alert already sent */ + } + /* Now check that the server has picked just 1 (i.e., len = 2) */ + if (ciphers.len != 2) { + (void)ssl3_DecodeError(ss); + return SECFailure; + } + + /* Get the selected cipher */ + cipher = (ciphers.data[0] << 8) | ciphers.data[1]; + + /* Now check that this is one of the ciphers we offered */ + for (i = 0; i < ss->ssl3.dtlsSRTPCipherCount; i++) { + if (cipher == ss->ssl3.dtlsSRTPCiphers[i]) { + found = PR_TRUE; + break; + } + } + + if (!found) { + PORT_SetError(SSL_ERROR_RX_MALFORMED_SERVER_HELLO); + (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter); + return SECFailure; + } + + /* Get the srtp_mki value */ + rv = ssl3_ConsumeHandshakeVariable(ss, &litem, 1, + &data->data, &data->len); + if (rv != SECSuccess) { + return SECFailure; /* alert already sent */ + } + + /* We didn't offer an MKI, so this must be 0 length */ + if (litem.len != 0) { + PORT_SetError(SSL_ERROR_RX_MALFORMED_SERVER_HELLO); + (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter); + return SECFailure; + } + + /* extra trailing bytes */ + if (data->len != 0) { + (void)ssl3_DecodeError(ss); + return SECFailure; + } + + /* OK, this looks fine. */ + ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ssl_use_srtp_xtn; + ss->ssl3.dtlsSRTPCipherSuite = cipher; + return SECSuccess; +} + +static SECStatus +ssl3_ServerHandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) { SECStatus rv; SECItem ciphers = {siBuffer, NULL, 0}; @@ -2125,74 +2230,6 @@ ssl3_HandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) PRBool found = PR_FALSE; SECItem litem; - if (!ss->sec.isServer) { - /* Client side */ - if (!data->data || !data->len) { - /* malformed */ - return SECFailure; - } - - /* Get the cipher list */ - rv = ssl3_ConsumeHandshakeVariable(ss, &ciphers, 2, - &data->data, &data->len); - if (rv != SECSuccess) { - return SECFailure; - } - /* Now check that the number of ciphers listed is 1 (len = 2) */ - if (ciphers.len != 2) { - return SECFailure; - } - - /* Get the selected cipher */ - cipher = (ciphers.data[0] << 8) | ciphers.data[1]; - - /* Now check that this is one of the ciphers we offered */ - for (i = 0; i < ss->ssl3.dtlsSRTPCipherCount; i++) { - if (cipher == ss->ssl3.dtlsSRTPCiphers[i]) { - found = PR_TRUE; - break; - } - } - - if (!found) { - return SECFailure; - } - - /* Get the srtp_mki value */ - rv = ssl3_ConsumeHandshakeVariable(ss, &litem, 1, - &data->data, &data->len); - if (rv != SECSuccess) { - return SECFailure; - } - - /* We didn't offer an MKI, so this must be 0 length */ - /* XXX RFC 5764 Section 4.1.3 says: - * If the client detects a nonzero-length MKI in the server's - * response that is different than the one the client offered, - * then the client MUST abort the handshake and SHOULD send an - * invalid_parameter alert. - * - * Due to a limitation of the ssl3_HandleHelloExtensions function, - * returning SECFailure here won't abort the handshake. It will - * merely cause the use_srtp extension to be not negotiated. We - * should fix this. See NSS bug 753136. - */ - if (litem.len != 0) { - return SECFailure; - } - - if (data->len != 0) { - /* malformed */ - return SECFailure; - } - - /* OK, this looks fine. */ - ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ssl_use_srtp_xtn; - ss->ssl3.dtlsSRTPCipherSuite = cipher; - return SECSuccess; - } - - /* Server side */ if (!IS_DTLS(ss) || !ss->ssl3.dtlsSRTPCipherCount) { /* Ignore the extension if we aren't doing DTLS or no DTLS-SRTP * preferences have been set. */ @@ -2200,7 +2237,7 @@ ssl3_HandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) } if (!data->data || data->len < 5) { - /* malformed */ + (void)ssl3_DecodeError(ss); return SECFailure; } @@ -2208,10 +2245,11 @@ ssl3_HandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) rv = ssl3_ConsumeHandshakeVariable(ss, &ciphers, 2, &data->data, &data->len); if (rv != SECSuccess) { - return SECFailure; + return SECFailure; /* alert already sent */ } /* Check that the list is even length */ if (ciphers.len % 2) { + (void)ssl3_DecodeError(ss); return SECFailure; } @@ -2234,12 +2272,13 @@ ssl3_HandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) } if (data->len != 0) { - return SECFailure; /* Malformed */ + (void)ssl3_DecodeError(ss); /* trailing bytes */ + return SECFailure; } /* Now figure out what to do */ if (!found) { - /* No matching ciphers */ + /* No matching ciphers, pretend we don't support use_srtp */ return SECSuccess; } @@ -2248,7 +2287,7 @@ ssl3_HandleUseSRTPXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ssl_use_srtp_xtn; return ssl3_RegisterServerHelloExtensionSender(ss, ssl_use_srtp_xtn, - ssl3_SendUseSRTPXtn); + ssl3_ServerSendUseSRTPXtn); } /* ssl3_ServerHandleSigAlgsXtn handles the signature_algorithms extension @@ -2267,9 +2306,6 @@ ssl3_ServerHandleSigAlgsXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) return SECSuccess; } - /* Keep track of negotiated extensions. */ - ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type; - rv = ssl3_ConsumeHandshakeVariable(ss, &algorithms, 2, &data->data, &data->len); if (rv != SECSuccess) { @@ -2278,6 +2314,7 @@ ssl3_ServerHandleSigAlgsXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) /* Trailing data, empty value, or odd-length value is invalid. */ if (data->len != 0 || algorithms.len == 0 || (algorithms.len & 1) != 0) { PORT_SetError(SSL_ERROR_RX_MALFORMED_CLIENT_HELLO); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); return SECFailure; } @@ -2291,6 +2328,8 @@ ssl3_ServerHandleSigAlgsXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) ss->ssl3.hs.clientSigAndHash = PORT_NewArray(SSL3SignatureAndHashAlgorithm, numAlgorithms); if (!ss->ssl3.hs.clientSigAndHash) { + PORT_SetError(SSL_ERROR_RX_MALFORMED_CLIENT_HELLO); + (void)SSL3_SendAlert(ss, alert_fatal, internal_error); return SECFailure; } ss->ssl3.hs.numClientSigAndHash = 0; @@ -2320,6 +2359,8 @@ ssl3_ServerHandleSigAlgsXtn(sslSocket * ss, PRUint16 ex_type, SECItem *data) ss->ssl3.hs.clientSigAndHash = NULL; } + /* Keep track of negotiated extensions. */ + ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type; return SECSuccess; } @@ -2483,41 +2524,32 @@ ssl3_ServerHandleDraftVersionXtn(sslSocket * ss, PRUint16 ex_type, return SECSuccess; } - if (data->len != 2) - goto loser; + if (data->len != 2) { + (void)ssl3_DecodeError(ss); + return SECFailure; + } /* Get the draft version out of the handshake */ draft_version = ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); if (draft_version < 0) { - goto loser; + return SECFailure; } /* Keep track of negotiated extensions. */ ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type; - /* Compare the version */ if (draft_version != TLS_1_3_DRAFT_VERSION) { + /* + * Incompatible/broken TLS 1.3 implementation. Fall back to TLS 1.2. + * TODO(ekr@rtfm.com): It's not entirely clear it's safe to roll back + * here. Need to double-check. + */ SSL_TRC(30, ("%d: SSL3[%d]: Incompatible version of TLS 1.3 (%d), " "expected %d", SSL_GETPID(), ss->fd, draft_version, TLS_1_3_DRAFT_VERSION)); - goto loser; + ss->version = SSL_LIBRARY_VERSION_TLS_1_2; } - return SECSuccess; - -loser: - /* - * Incompatible/broken TLS 1.3 implementation. Fall back to TLS 1.2. - * TODO(ekr@rtfm.com): It's not entirely clear it's safe to roll back - * here. Need to double-check. - * TODO(ekr@rtfm.com): Currently we fall back even on broken extensions. - * because SECFailure does not cause handshake failures. See bug - * 753136. - */ - SSL_TRC(30, ("%d: SSL3[%d]: Rolling back to TLS 1.2", SSL_GETPID(), ss->fd)); - ss->version = SSL_LIBRARY_VERSION_TLS_1_2; - return SECSuccess; } - diff --git a/security/nss/lib/ssl/sslimpl.h b/security/nss/lib/ssl/sslimpl.h index 858ae0cc90f6..896d05a183f7 100644 --- a/security/nss/lib/ssl/sslimpl.h +++ b/security/nss/lib/ssl/sslimpl.h @@ -981,6 +981,7 @@ struct ssl3StateStr { PRUint16 dtlsSRTPCiphers[MAX_DTLS_SRTP_CIPHER_SUITES]; PRUint16 dtlsSRTPCipherCount; PRUint16 dtlsSRTPCipherSuite; /* 0 if not selected */ + PRBool fatalAlertSent; }; #define DTLS_MAX_MTU 1500 /* Ethernet MTU but without subtracting the diff --git a/security/nss/lib/util/nssutil.h b/security/nss/lib/util/nssutil.h index 6daa12455e30..05c2805d0591 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.18" +#define NSSUTIL_VERSION "3.18.1 Beta" #define NSSUTIL_VMAJOR 3 #define NSSUTIL_VMINOR 18 -#define NSSUTIL_VPATCH 0 -#define NSSUTIL_VBUILD 2 -#define NSSUTIL_BETA PR_FALSE +#define NSSUTIL_VPATCH 1 +#define NSSUTIL_VBUILD 0 +#define NSSUTIL_BETA PR_TRUE SEC_BEGIN_PROTOS