Add SSL_set_reject_peer_renegotiations.

This causes any unexpected handshake records to be met with a fatal
no_renegotiation alert.

In addition, restore the redundant version sanity-checks in the handshake state
machines. Some code would zero the version field as a hacky way to break the
handshake on renego. Those will be removed when switching to this API.

The spec allows for a non-fatal no_renegotiation alert, but ssl3_read_bytes
makes it difficult to find the end of a ClientHello and skip it entirely. Given
that OpenSSL goes out of its way to map non-fatal no_renegotiation alerts to
fatal ones, this seems probably fine. This avoids needing to account for
another source of the library consuming an unbounded number of bytes without
returning data up.

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

include/openssl/ssl.h

@@ -1135,6 +1135,11 @@ OPENSSL_EXPORT void SSL_get0_alpn_selected(const SSL *ssl, const uint8_t **data,
  * causes 3G radios to switch to DCH mode (high data rate). */
 OPENSSL_EXPORT void SSL_enable_fastradio_padding(SSL *ssl, char on_off);
 
+/* SSL_set_reject_peer_renegotiations controls whether renegotiation attempts by
+ * the peer are rejected. It may be set at any point in a connection's lifetime
+ * to disallow future renegotiations programmatically. */
+OPENSSL_EXPORT void SSL_set_reject_peer_renegotiations(SSL *ssl, int reject);
+
 /* the maximum length of the buffer given to callbacks containing the resulting
  * identity/psk */
 #define PSK_MAX_IDENTITY_LEN 128
@@ -1395,6 +1400,10 @@ struct ssl_st {
    * data rate) state in 3G networks. */
   char fastradio_padding;
 
+  /* reject_peer_renegotiations, if one, causes causes renegotiation attempts
+   * from the peer to be rejected with a fatal error. */
+  char reject_peer_renegotiations;
+
   /* These fields are always NULL and exist only to keep wpa_supplicant happy
    * about the change to EVP_AEAD. They are only needed for EAP-FAST, which we
    * don't support. */

ssl/s3_clnt.c

@@ -203,6 +203,15 @@ int ssl3_connect(SSL *s) {
           cb(s, SSL_CB_HANDSHAKE_START, 1);
         }
 
+        if ((s->version >> 8) != 3) {
+          /* TODO(davidben): Some consumers clear |s->version| to break the
+           * handshake in a callback. Remove this when they're using proper
+           * APIs. */
+          OPENSSL_PUT_ERROR(SSL, ssl3_connect, ERR_R_INTERNAL_ERROR);
+          ret = -1;
+          goto end;
+        }
+
         if (s->init_buf == NULL) {
           buf = BUF_MEM_new();
           if (buf == NULL ||

ssl/s3_pkt.c

@@ -885,6 +885,14 @@ start:
    * that we can process the data at a fixed place. */
 
   if (rr->type == SSL3_RT_HANDSHAKE) {
+    /* If peer renegotiations are disabled, all out-of-order handshake records
+     * are fatal. */
+    if (s->reject_peer_renegotiations) {
+      al = SSL_AD_NO_RENEGOTIATION;
+      OPENSSL_PUT_ERROR(SSL, ssl3_read_bytes, SSL_R_NO_RENEGOTIATION);
+      goto f_err;
+    }
+
     const size_t size = sizeof(s->s3->handshake_fragment);
     const size_t avail = size - s->s3->handshake_fragment_len;
     const size_t todo = (rr->length < avail) ? rr->length : avail;

ssl/s3_srvr.c

@@ -276,6 +276,15 @@ int ssl3_accept(SSL *s) {
           cb(s, SSL_CB_HANDSHAKE_START, 1);
         }
 
+        if ((s->version >> 8) != 3) {
+          /* TODO(davidben): Some consumers clear |s->version| to break the
+           * handshake in a callback. Remove this when they're using proper
+           * APIs. */
+          OPENSSL_PUT_ERROR(SSL, ssl3_accept, ERR_R_INTERNAL_ERROR);
+          ret = -1;
+          goto end;
+        }
+
         if (s->init_buf == NULL) {
           buf = BUF_MEM_new();
           if (!buf || !BUF_MEM_grow(buf, SSL3_RT_MAX_PLAIN_LENGTH)) {

ssl/ssl_lib.c

@@ -3105,6 +3105,10 @@ void SSL_enable_fastradio_padding(SSL *s, char on_off) {
   s->fastradio_padding = on_off;
 }
 
+void SSL_set_reject_peer_renegotiations(SSL *s, int reject) {
+  s->reject_peer_renegotiations = !!reject;
+}
+
 const SSL_CIPHER *SSL_get_cipher_by_value(uint16_t value) {
   return ssl3_get_cipher_by_value(value);
 }

ssl/test/bssl_shim.cc

@@ -663,6 +663,9 @@ static bool DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx,
       !SSL_set_cipher_list(ssl.get(), config->cipher.c_str())) {
     return false;
   }
+  if (config->reject_peer_renegotiations) {
+    SSL_set_reject_peer_renegotiations(ssl.get(), 1);
+  }
 
   int sock = Connect(config->port);
   if (sock == -1) {

ssl/test/runner/runner.go

@@ -2873,6 +2873,15 @@ func addRenegotiationTests() {
 		},
 		flags: []string{"-allow-unsafe-legacy-renegotiation"},
 	})
+	testCases = append(testCases, testCase{
+		testType:           serverTest,
+		name:               "Renegotiate-Server-ClientInitiated-Forbidden",
+		renegotiate:        true,
+		flags:              []string{"-reject-peer-renegotiations"},
+		shouldFail:         true,
+		expectedError:      ":NO_RENEGOTIATION:",
+		expectedLocalError: "remote error: no renegotiation",
+	})
 	// Regression test for CVE-2015-0291.
 	testCases = append(testCases, testCase{
 		testType: serverTest,
@@ -2938,6 +2947,14 @@ func addRenegotiationTests() {
 		},
 		renegotiateCiphers: []uint16{TLS_RSA_WITH_RC4_128_SHA},
 	})
+	testCases = append(testCases, testCase{
+		name:               "Renegotiate-Client-Forbidden",
+		renegotiate:        true,
+		flags:              []string{"-reject-peer-renegotiations"},
+		shouldFail:         true,
+		expectedError:      ":NO_RENEGOTIATION:",
+		expectedLocalError: "remote error: no renegotiation",
+	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-SameClientVersion",
 		renegotiate: true,

ssl/test/test_config.cc

@@ -80,6 +80,7 @@ const Flag<bool> kBoolFlags[] = {
   { "-fail-second-ddos-callback", &TestConfig::fail_second_ddos_callback },
   { "-handshake-never-done", &TestConfig::handshake_never_done },
   { "-use-export-context", &TestConfig::use_export_context },
+  { "-reject-peer-renegotiations", &TestConfig::reject_peer_renegotiations },
 };
 
 const Flag<std::string> kStringFlags[] = {

ssl/test/test_config.h

@@ -77,6 +77,7 @@ struct TestConfig {
   std::string export_label;
   std::string export_context;
   bool use_export_context = false;
+  bool reject_peer_renegotiations = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_config);