Remove SSL_OP_TLS_D5_BUG.

This dates to SSLeay 0.9.0. The Internet seems to have completely
forgotten what "D5" is. (I can't find reference to it beyond
documentation of this quirk.) The use counter we added sees virtually no
hits.

Change-Id: I9781d401acb98ce3790b1b165fc257a6f5e9b155
Reviewed-on: https://boringssl-review.googlesource.com/6557
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/ssl.h

@@ -550,10 +550,6 @@ OPENSSL_EXPORT int SSL_version(const SSL *ssl);
  * bytes above the maximum record size. */
 #define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER 0x00000020L
 
-/* SSL_OP_TLS_D5_BUG accepts an RSAClientKeyExchange in TLS encoded as in SSL3
- * (i.e. without a length prefix). */
-#define SSL_OP_TLS_D5_BUG 0x00000100L
-
 /* SSL_OP_ALL enables the above bug workarounds that are enabled by many
  * consumers.
  * TODO(davidben): Determine which of the remaining may be removed now. */
@@ -2767,13 +2763,6 @@ OPENSSL_EXPORT void SSL_set_max_send_fragment(SSL *ssl,
  * unnecessary. */
 OPENSSL_EXPORT uint64_t OPENSSL_get_big_buffer_use_count(void);
 
-/* OPENSSL_get_d5_bug_use_count returns the total number of invalid RSA
- * ClientKeyExchanges that were accepted because of |SSL_OP_TLS_D5_BUG|.
- *
- * TODO(davidben): Remove this when (hopefully!) the quirk is demonstrated to be
- * unnecessary. */
-OPENSSL_EXPORT uint64_t OPENSSL_get_d5_bug_use_count(void);
-
 /* ssl_early_callback_ctx is passed to certain callbacks that are called very
  * early on during the server handshake. At this point, much of the SSL* hasn't
  * been filled out and only the ClientHello can be depended on. */
@@ -3208,6 +3197,7 @@ DECLARE_STACK_OF(SSL_COMP)
 #define SSL_OP_SSLEAY_080_CLIENT_DH_BUG 0
 #define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG 0
 #define SSL_OP_TLS_BLOCK_PADDING_BUG 0
+#define SSL_OP_TLS_D5_BUG 0
 #define SSL_OP_TLS_ROLLBACK_BUG 0
 #define SSL_VERIFY_CLIENT_ONCE 0
 

ssl/s3_srvr.c

@@ -1587,16 +1587,6 @@ err:
   return -1;
 }
 
-static struct CRYPTO_STATIC_MUTEX g_d5_bug_lock = CRYPTO_STATIC_MUTEX_INIT;
-static uint64_t g_d5_bug_use_count = 0;
-
-uint64_t OPENSSL_get_d5_bug_use_count(void) {
-  CRYPTO_STATIC_MUTEX_lock_read(&g_d5_bug_lock);
-  uint64_t ret = g_d5_bug_use_count;
-  CRYPTO_STATIC_MUTEX_unlock(&g_d5_bug_lock);
-  return ret;
-}
-
 int ssl3_get_client_key_exchange(SSL *s) {
   int al;
   CBS client_key_exchange;
@@ -1702,22 +1692,13 @@ int ssl3_get_client_key_exchange(SSL *s) {
       }
       /* TLS and [incidentally] DTLS{0xFEFF} */
       if (s->version > SSL3_VERSION) {
-        CBS copy = client_key_exchange;
         if (!CBS_get_u16_length_prefixed(&client_key_exchange,
                                          &encrypted_premaster_secret) ||
             CBS_len(&client_key_exchange) != 0) {
-          if (!(s->options & SSL_OP_TLS_D5_BUG)) {
-            al = SSL_AD_DECODE_ERROR;
-            OPENSSL_PUT_ERROR(SSL,
-                              SSL_R_TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG);
-            goto f_err;
-          } else {
-            CRYPTO_STATIC_MUTEX_lock_write(&g_d5_bug_lock);
-            g_d5_bug_use_count++;
-            CRYPTO_STATIC_MUTEX_unlock(&g_d5_bug_lock);
-
-            encrypted_premaster_secret = copy;
-          }
+          al = SSL_AD_DECODE_ERROR;
+          OPENSSL_PUT_ERROR(SSL,
+                            SSL_R_TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG);
+          goto f_err;
         }
       } else {
         encrypted_premaster_secret = client_key_exchange;

ssl/test/bssl_shim.cc

@@ -1161,9 +1161,6 @@ static bool DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx,
   if (config->no_ssl3) {
     SSL_set_options(ssl.get(), SSL_OP_NO_SSLv3);
   }
-  if (config->tls_d5_bug) {
-    SSL_set_options(ssl.get(), SSL_OP_TLS_D5_BUG);
-  }
   if (config->microsoft_big_sslv3_buffer) {
     SSL_set_options(ssl.get(), SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER);
   }

ssl/test/runner/common.go

@@ -535,11 +535,6 @@ type ProtocolBugs struct {
 	// closed the connection) before or after sending app data.
 	AlertBeforeFalseStartTest alert
 
-	// SSL3RSAKeyExchange causes the client to always send an RSA
-	// ClientKeyExchange message without the two-byte length
-	// prefix, as if it were SSL3.
-	SSL3RSAKeyExchange bool
-
 	// SkipCipherVersionCheck causes the server to negotiate
 	// TLS 1.2 ciphers in earlier versions of TLS.
 	SkipCipherVersionCheck bool

ssl/test/runner/key_agreement.go

@@ -156,7 +156,7 @@ func (ka *rsaKeyAgreement) generateClientKeyExchange(config *Config, clientHello
 		return nil, nil, err
 	}
 	ckx := new(clientKeyExchangeMsg)
-	if clientHello.vers != VersionSSL30 && !config.Bugs.SSL3RSAKeyExchange {
+	if clientHello.vers != VersionSSL30 {
 		ckx.ciphertext = make([]byte, len(encrypted)+2)
 		ckx.ciphertext[0] = byte(len(encrypted) >> 8)
 		ckx.ciphertext[1] = byte(len(encrypted))

ssl/test/runner/runner.go

@@ -3208,40 +3208,6 @@ func addMinimumVersionTests() {
 	}
 }
 
-func addD5BugTests() {
-	testCases = append(testCases, testCase{
-		testType: serverTest,
-		name:     "D5Bug-NoQuirk-Reject",
-		config: Config{
-			CipherSuites: []uint16{TLS_RSA_WITH_AES_128_GCM_SHA256},
-			Bugs: ProtocolBugs{
-				SSL3RSAKeyExchange: true,
-			},
-		},
-		shouldFail:    true,
-		expectedError: ":TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG:",
-	})
-	testCases = append(testCases, testCase{
-		testType: serverTest,
-		name:     "D5Bug-Quirk-Normal",
-		config: Config{
-			CipherSuites: []uint16{TLS_RSA_WITH_AES_128_GCM_SHA256},
-		},
-		flags: []string{"-tls-d5-bug"},
-	})
-	testCases = append(testCases, testCase{
-		testType: serverTest,
-		name:     "D5Bug-Quirk-Bug",
-		config: Config{
-			CipherSuites: []uint16{TLS_RSA_WITH_AES_128_GCM_SHA256},
-			Bugs: ProtocolBugs{
-				SSL3RSAKeyExchange: true,
-			},
-		},
-		flags: []string{"-tls-d5-bug"},
-	})
-}
-
 func addExtensionTests() {
 	testCases = append(testCases, testCase{
 		testType: clientTest,
@@ -4644,7 +4610,6 @@ func main() {
 	addDDoSCallbackTests()
 	addVersionNegotiationTests()
 	addMinimumVersionTests()
-	addD5BugTests()
 	addExtensionTests()
 	addResumptionVersionTests()
 	addExtendedMasterSecretTests()

ssl/test/test_config.cc

@@ -61,7 +61,6 @@ const Flag<bool> kBoolFlags[] = {
   { "-no-tls1", &TestConfig::no_tls1 },
   { "-no-ssl3", &TestConfig::no_ssl3 },
   { "-shim-writes-first", &TestConfig::shim_writes_first },
-  { "-tls-d5-bug", &TestConfig::tls_d5_bug },
   { "-expect-session-miss", &TestConfig::expect_session_miss },
   { "-expect-extended-master-secret",
     &TestConfig::expect_extended_master_secret },

ssl/test/test_config.h

@@ -45,7 +45,6 @@ struct TestConfig {
   std::string expected_channel_id;
   std::string send_channel_id;
   bool shim_writes_first = false;
-  bool tls_d5_bug = false;
   std::string host_name;
   std::string advertise_alpn;
   std::string expected_alpn;