Bug 1483128 - Option to disable SSLv2-compatible ClientHello, r=ueno
authorMartin Thomson <martin.thomson@gmail.com>
Fri, 17 Aug 2018 15:20:08 +1000
changeset 14457 2ed9f6afd84eb7fd1033ffaf448655e1d5a79bb8
parent 14456 91b6fb509cb01bbf4466f647cccfa2a2e060c504
child 14458 3e7092b5f18d1fec9bcc163091065c77aacfab44
push id3169
push usermartin.thomson@gmail.com
push dateSun, 26 Aug 2018 23:51:29 +0000
reviewersueno
bugs1483128
Bug 1483128 - Option to disable SSLv2-compatible ClientHello, r=ueno
gtests/ssl_gtest/ssl_gather_unittest.cc
gtests/ssl_gtest/ssl_v2_client_hello_unittest.cc
lib/ssl/ssl.h
lib/ssl/ssl3gthr.c
lib/ssl/sslimpl.h
lib/ssl/sslsock.c
--- a/gtests/ssl_gtest/ssl_gather_unittest.cc
+++ b/gtests/ssl_gtest/ssl_gather_unittest.cc
@@ -10,16 +10,17 @@
 namespace nss_test {
 
 class GatherV2ClientHelloTest : public TlsConnectTestBase {
  public:
   GatherV2ClientHelloTest() : TlsConnectTestBase(ssl_variant_stream, 0) {}
 
   void ConnectExpectMalformedClientHello(const DataBuffer &data) {
     EnsureTlsSetup();
+    server_->SetOption(SSL_ENABLE_V2_COMPATIBLE_HELLO, PR_TRUE);
     server_->ExpectSendAlert(kTlsAlertIllegalParameter);
     client_->SendDirect(data);
     server_->StartConnect();
     server_->Handshake();
     ASSERT_TRUE_WAIT(
         (server_->error_code() == SSL_ERROR_RX_MALFORMED_CLIENT_HELLO), 2000);
   }
 };
--- a/gtests/ssl_gtest/ssl_v2_client_hello_unittest.cc
+++ b/gtests/ssl_gtest/ssl_v2_client_hello_unittest.cc
@@ -146,16 +146,17 @@ class SSLv2ClientHelloTestF : public Tls
       : TlsConnectTestBase(ssl_variant_stream, 0), filter_(nullptr) {}
 
   SSLv2ClientHelloTestF(SSLProtocolVariant variant, uint16_t version)
       : TlsConnectTestBase(variant, version), filter_(nullptr) {}
 
   void SetUp() override {
     TlsConnectTestBase::SetUp();
     filter_ = MakeTlsFilter<SSLv2ClientHelloFilter>(client_, version_);
+    server_->SetOption(SSL_ENABLE_V2_COMPATIBLE_HELLO, PR_TRUE);
   }
 
   void SetExpectedVersion(uint16_t version) {
     TlsConnectTestBase::SetExpectedVersion(version);
     filter_->SetVersion(version);
   }
 
   void SetAvailableCipherSuite(uint16_t cipher) {
@@ -192,16 +193,37 @@ class SSLv2ClientHelloTest : public SSLv
 };
 
 // Test negotiating TLS 1.0 - 1.2.
 TEST_P(SSLv2ClientHelloTest, Connect) {
   SetAvailableCipherSuite(TLS_DHE_RSA_WITH_AES_128_CBC_SHA);
   Connect();
 }
 
+TEST_P(SSLv2ClientHelloTest, ConnectDisabled) {
+  server_->SetOption(SSL_ENABLE_V2_COMPATIBLE_HELLO, PR_FALSE);
+  SetAvailableCipherSuite(TLS_DHE_RSA_WITH_AES_128_CBC_SHA);
+
+  StartConnect();
+  client_->Handshake();  // Send the modified ClientHello.
+  server_->Handshake();  // Read some.
+  // The problem here is that the v2 ClientHello puts the version where the v3
+  // ClientHello puts a version number.  So the version number (0x0301+) appears
+  // to be a length and server blocks waiting for that much data.
+  EXPECT_EQ(PR_WOULD_BLOCK_ERROR, PORT_GetError());
+
+  // This is usually what happens with v2-compatible: the server hangs.
+  // But to be certain, feed in more data to see if an error comes out.
+  uint8_t zeros[SSL_LIBRARY_VERSION_TLS_1_2] = {0};
+  client_->SendDirect(DataBuffer(zeros, sizeof(zeros)));
+  ExpectAlert(server_, kTlsAlertIllegalParameter);
+  server_->Handshake();
+  client_->Handshake();
+}
+
 // Sending a v2 ClientHello after a no-op v3 record must fail.
 TEST_P(SSLv2ClientHelloTest, ConnectAfterEmptyV3Record) {
   DataBuffer buffer;
 
   size_t idx = 0;
   idx = buffer.Write(idx, 0x16, 1);    // handshake
   idx = buffer.Write(idx, 0x0301, 2);  // record_version
   (void)buffer.Write(idx, 0U, 2);      // length=0
--- a/lib/ssl/ssl.h
+++ b/lib/ssl/ssl.h
@@ -286,16 +286,24 @@ SSL_IMPORT PRFileDesc *DTLS_ImportFD(PRF
  * Enables the processing of the downgrade sentinel that can be added to the
  * ServerHello.random by a server that supports Section 4.1.3 of TLS 1.3
  * [RFC8446].  This sentinel will always be generated by a server that
  * negotiates a version lower than its maximum, this only controls whether a
  * client will treat receipt of a value that indicates a downgrade as an error.
  */
 #define SSL_ENABLE_HELLO_DOWNGRADE_CHECK 37
 
+/* Enables the SSLv2-compatible ClientHello for servers. NSS does not support
+ * SSLv2 and will never send an SSLv2-compatible ClientHello as a client.  An
+ * NSS server with this option enabled will accept a ClientHello that is
+ * v2-compatible as defined in Appendix E.1 of RFC 6101.
+ *
+ * This is disabled by default and will be removed in a future version. */
+#define SSL_ENABLE_V2_COMPATIBLE_HELLO 38
+
 #ifdef SSL_DEPRECATED_FUNCTION
 /* Old deprecated function names */
 SSL_IMPORT SECStatus SSL_Enable(PRFileDesc *fd, int option, PRIntn on);
 SSL_IMPORT SECStatus SSL_EnableDefault(int option, PRIntn on);
 #endif
 
 /* Set (and get) options for sockets and defaults for newly created sockets.
  *
--- a/lib/ssl/ssl3gthr.c
+++ b/lib/ssl/ssl3gthr.c
@@ -465,32 +465,32 @@ ssl3_GatherCompleteHandshake(sslSocket *
             PORT_Assert(!IS_DTLS(ss));
             rv = ssl3_HandleNonApplicationData(ss, ssl_ct_handshake,
                                                0, 0, &ss->gs.buf);
         } else {
             /* State for SSLv2 client hello support. */
             ssl2Gather ssl2gs = { PR_FALSE, 0 };
             ssl2Gather *ssl2gs_ptr = NULL;
 
-            /* If we're a server and waiting for a client hello, accept v2. */
-            if (ss->sec.isServer && ss->ssl3.hs.ws == wait_client_hello) {
+            if (ss->sec.isServer && ss->opt.enableV2CompatibleHello &&
+                ss->ssl3.hs.ws == wait_client_hello) {
                 ssl2gs_ptr = &ssl2gs;
             }
 
             /* bring in the next sslv3 record. */
             if (ss->recvdCloseNotify) {
                 /* RFC 5246 Section 7.2.1:
                  *   Any data received after a closure alert is ignored.
                  */
                 return 0;
             }
 
             if (!IS_DTLS(ss)) {
-                /* If we're a server waiting for a ClientHello then pass
-                 * ssl2gs to support SSLv2 ClientHello messages. */
+                /* Passing a non-NULL ssl2gs here enables detection of
+                 * SSLv2-compatible ClientHello messages. */
                 rv = ssl3_GatherData(ss, &ss->gs, flags, ssl2gs_ptr);
             } else {
                 rv = dtls_GatherData(ss, &ss->gs, flags);
 
                 /* If we got a would block error, that means that no data was
                  * available, so we check the timer to see if it's time to
                  * retransmit */
                 if (rv == SECFailure &&
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -262,16 +262,17 @@ typedef struct sslOptionsStr {
     unsigned int enableServerDhe : 1;
     unsigned int enableExtendedMS : 1;
     unsigned int enableSignedCertTimestamps : 1;
     unsigned int requireDHENamedGroups : 1;
     unsigned int enable0RttData : 1;
     unsigned int enableTls13CompatMode : 1;
     unsigned int enableDtlsShortHeader : 1;
     unsigned int enableHelloDowngradeCheck : 1;
+    unsigned int enableV2CompatibleHello : 1;
 } sslOptions;
 
 typedef enum { sslHandshakingUndetermined = 0,
                sslHandshakingAsClient,
                sslHandshakingAsServer
 } sslHandshakingType;
 
 #define SSL_LOCK_RANK_SPEC 255
--- a/lib/ssl/sslsock.c
+++ b/lib/ssl/sslsock.c
@@ -78,17 +78,18 @@ static sslOptions ssl_defaults = {
     .enableFallbackSCSV = PR_FALSE,
     .enableServerDhe = PR_TRUE,
     .enableExtendedMS = PR_FALSE,
     .enableSignedCertTimestamps = PR_FALSE,
     .requireDHENamedGroups = PR_FALSE,
     .enable0RttData = PR_FALSE,
     .enableTls13CompatMode = PR_FALSE,
     .enableDtlsShortHeader = PR_FALSE,
-    .enableHelloDowngradeCheck = PR_FALSE
+    .enableHelloDowngradeCheck = PR_FALSE,
+    .enableV2CompatibleHello = PR_FALSE
 };
 
 /*
  * default range of enabled SSL/TLS protocols
  */
 static SSLVersionRange versions_defaults_stream = {
     SSL_LIBRARY_VERSION_TLS_1_0,
     SSL_LIBRARY_VERSION_TLS_1_2
@@ -821,16 +822,20 @@ SSL_OptionSet(PRFileDesc *fd, PRInt32 wh
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             ss->opt.enableDtlsShortHeader = val;
             break;
 
         case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
             ss->opt.enableHelloDowngradeCheck = val;
             break;
 
+        case SSL_ENABLE_V2_COMPATIBLE_HELLO:
+            ss->opt.enableV2CompatibleHello = val;
+            break;
+
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             rv = SECFailure;
     }
 
     /* We can't use the macros for releasing the locks here,
      * because ss->opt.noLocks might have changed just above.
      * We must release these locks (monitors) here, if we aquired them above,
@@ -966,16 +971,19 @@ SSL_OptionGet(PRFileDesc *fd, PRInt32 wh
             val = ss->opt.enableTls13CompatMode;
             break;
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             val = ss->opt.enableDtlsShortHeader;
             break;
         case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
             val = ss->opt.enableHelloDowngradeCheck;
             break;
+        case SSL_ENABLE_V2_COMPATIBLE_HELLO:
+            val = ss->opt.enableV2CompatibleHello;
+            break;
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             rv = SECFailure;
     }
 
     ssl_ReleaseSSL3HandshakeLock(ss);
     ssl_Release1stHandshakeLock(ss);
 
@@ -1095,16 +1103,19 @@ SSL_OptionGetDefault(PRInt32 which, PRIn
             val = ssl_defaults.enableTls13CompatMode;
             break;
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             val = ssl_defaults.enableDtlsShortHeader;
             break;
         case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
             val = ssl_defaults.enableHelloDowngradeCheck;
             break;
+        case SSL_ENABLE_V2_COMPATIBLE_HELLO:
+            val = ssl_defaults.enableV2CompatibleHello;
+            break;
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             rv = SECFailure;
     }
 
     *pVal = val;
     return rv;
 }
@@ -1294,16 +1305,20 @@ SSL_OptionSetDefault(PRInt32 which, PRIn
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             ssl_defaults.enableDtlsShortHeader = val;
             break;
 
         case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
             ssl_defaults.enableHelloDowngradeCheck = val;
             break;
 
+        case SSL_ENABLE_V2_COMPATIBLE_HELLO:
+            ssl_defaults.enableV2CompatibleHello = val;
+            break;
+
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             return SECFailure;
     }
     return SECSuccess;
 }
 
 SECStatus