Bug 1242308 - Handle bogus DTLS version numbers better, r=mt
authorEKR <ekr@rtfm.com>
Thu, 04 Feb 2016 05:59:50 +1100
changeset 11849 fa49b4c300422a316cce1ea7b4c256f9ea7b5775
parent 11848 7d389601644f8dbcfea3212de3ab8ca9c9fb54b1
child 11850 2388858af96dcb99b4845674ff90292e6baa08e1
push id960
push usermartin.thomson@gmail.com
push dateWed, 03 Feb 2016 19:15:17 +0000
reviewersmt
bugs1242308
Bug 1242308 - Handle bogus DTLS version numbers better, r=mt
external_tests/ssl_gtest/ssl_loopback_unittest.cc
external_tests/ssl_gtest/tls_connect.h
lib/ssl/dtlscon.c
--- a/external_tests/ssl_gtest/ssl_loopback_unittest.cc
+++ b/external_tests/ssl_gtest/ssl_loopback_unittest.cc
@@ -757,16 +757,27 @@ TEST_P(TlsConnectStream, ServerNegotiate
 // SSL_SetDowngradeCheckVersion() API.
 TEST_F(TlsConnectTest, TestDowngradeDetectionToTls11) {
   client_->SetPacketFilter(new TlsInspectorClientHelloVersionSetter
                            (SSL_LIBRARY_VERSION_TLS_1_1));
   ConnectExpectFail();
   ASSERT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code());
 }
 
+/* Attempt to negotiate the bogus DTLS 1.1 version. */
+TEST_F(DtlsConnectTest, TestDtlsVersion11) {
+  client_->SetPacketFilter(new TlsInspectorClientHelloVersionSetter(
+      ((~0x0101) & 0xffff)));
+  ConnectExpectFail();
+  // It's kind of surprising that SSL_ERROR_NO_CYPHER_OVERLAP is
+  // what is returned here, but this is deliberate in ssl3_HandleAlert().
+  EXPECT_EQ(SSL_ERROR_NO_CYPHER_OVERLAP, client_->error_code());
+  EXPECT_EQ(SSL_ERROR_UNSUPPORTED_VERSION, server_->error_code());
+}
+
 #ifdef NSS_ENABLE_TLS_1_3
 TEST_F(TlsConnectTest, TestDowngradeDetectionToTls12) {
   EnsureTlsSetup();
   client_->SetPacketFilter(new TlsInspectorClientHelloVersionSetter
                            (SSL_LIBRARY_VERSION_TLS_1_2));
   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_3);
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
--- a/external_tests/ssl_gtest/tls_connect.h
+++ b/external_tests/ssl_gtest/tls_connect.h
@@ -92,16 +92,22 @@ class TlsConnectTestBase : public ::test
 };
 
 // A non-parametrized TLS test base.
 class TlsConnectTest : public TlsConnectTestBase {
  public:
  TlsConnectTest() : TlsConnectTestBase(STREAM, 0) {}
 };
 
+// A non-parametrized DTLS-only test base.
+class DtlsConnectTest : public TlsConnectTestBase {
+ public:
+  DtlsConnectTest() : TlsConnectTestBase(DGRAM, 0) {}
+};
+
 // A TLS-only test base.
 class TlsConnectStream : public TlsConnectTestBase,
                          public ::testing::WithParamInterface<uint16_t> {
  public:
   TlsConnectStream() : TlsConnectTestBase(STREAM, GetParam()) {}
 };
 
 // A TLS-only test base for tests before 1.3
--- a/lib/ssl/dtlscon.c
+++ b/lib/ssl/dtlscon.c
@@ -81,25 +81,30 @@ dtls_DTLSVersionToTLSVersion(SSL3Protoco
 {
     if (MSB(dtlsv) == 0xff) {
         return 0;
     }
 
     if (dtlsv == SSL_LIBRARY_VERSION_DTLS_1_0_WIRE) {
         return SSL_LIBRARY_VERSION_TLS_1_1;
     }
+    /* Handle the skipped version of DTLS 1.1 by returning
+     * an error. */
+    if (dtlsv == ((~0x0101) & 0xffff)) {
+        return 0;
+    }
     if (dtlsv == SSL_LIBRARY_VERSION_DTLS_1_2_WIRE) {
         return SSL_LIBRARY_VERSION_TLS_1_2;
     }
     if (dtlsv == SSL_LIBRARY_VERSION_DTLS_1_3_WIRE) {
         return SSL_LIBRARY_VERSION_TLS_1_3;
     }
 
     /* Return a fictional higher version than we know of */
-    return SSL_LIBRARY_VERSION_TLS_1_2 + 1;
+    return SSL_LIBRARY_VERSION_MAX_SUPPORTED + 1;
 }
 
 /* On this socket, Disable non-DTLS cipher suites in the argument's list */
 SECStatus
 ssl3_DisableNonDTLSSuites(sslSocket * ss)
 {
     const ssl3CipherSuite * suite;