Add SSL_set_renegotiate_mode.

Add a slightly richer API. Notably, one can configure ssl_renegotiate_once to
only accept the first renego.

Also, this API doesn't repeat the mistake I made with
SSL_set_reject_peer_renegotiations which is super-confusing with the negation.

Change-Id: I7eb5d534e3e6c553b641793f4677fe5a56451c71
Reviewed-on: https://boringssl-review.googlesource.com/6221
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 01bd7cc..dc258ef 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2368,11 +2368,25 @@
  * https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format. */
 OPENSSL_EXPORT void SSL_CTX_set_keylog_bio(SSL_CTX *ctx, BIO *keylog_bio);
 
-/* 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 control future renegotiations programmatically. By default, renegotiations
- * are rejected. (Renegotiations requested by a client are always rejected.) */
-OPENSSL_EXPORT void SSL_set_reject_peer_renegotiations(SSL *ssl, int reject);
+enum ssl_renegotiate_mode_t {
+  ssl_renegotiate_never = 0,
+  ssl_renegotiate_once,
+  ssl_renegotiate_freely,
+};
+
+/* SSL_set_renegotiate_mode configures how |ssl|, a client, reacts to
+ * renegotiation attempts by a server. If |ssl| is a server, peer-initiated
+ * renegotiations are *always* rejected and this function does nothing.
+ *
+ * The renegotiation mode defaults to |ssl_renegotiate_never|, but may be set
+ * at any point in a connection's lifetime. Set it to |ssl_renegotiate_once| to
+ * allow one renegotiation and |ssl_renegotiate_freely| to allow all
+ * renegotiations.
+ *
+ * There is no support in BoringSSL for initiating renegotiations as a client
+ * or server. */
+OPENSSL_EXPORT void SSL_set_renegotiate_mode(SSL *ssl,
+                                             enum ssl_renegotiate_mode_t mode);
 
 
 /* Underdocumented functions.
@@ -2720,6 +2734,11 @@
 
 /* Deprecated functions. */
 
+/* SSL_set_reject_peer_renegotiations calls |SSL_set_renegotiate_mode| with
+ * |ssl_never_renegotiate| if |reject| is one and |ssl_renegotiate_freely| if
+ * zero. */
+OPENSSL_EXPORT void SSL_set_reject_peer_renegotiations(SSL *ssl, int reject);
+
 /* SSL_CIPHER_description writes a description of |cipher| into |buf| and
  * returns |buf|. If |buf| is NULL, it returns a newly allocated string, to be
  * freed with |OPENSSL_free|, or NULL on error.
@@ -3567,9 +3586,8 @@
   uint8_t *alpn_client_proto_list;
   unsigned alpn_client_proto_list_len;
 
-  /* accept_peer_renegotiations, if one, accepts renegotiation attempts from the
-   * peer. Otherwise, they will be rejected with a fatal error. */
-  char accept_peer_renegotiations;
+  /* renegotiate_mode controls how peer renegotiation attempts are handled. */
+  enum ssl_renegotiate_mode_t renegotiate_mode;
 
   /* 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
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index db49601..f2a26a9 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -337,6 +337,20 @@
   ssl3_read_bytes(ssl, 0, NULL, 0, 0);
 }
 
+static int ssl3_can_renegotiate(SSL *ssl) {
+  switch (ssl->renegotiate_mode) {
+    case ssl_renegotiate_never:
+      return 0;
+    case ssl_renegotiate_once:
+      return ssl->s3->total_renegotiations == 0;
+    case ssl_renegotiate_freely:
+      return 1;
+  }
+
+  assert(0);
+  return 0;
+}
+
 /* Return up to 'len' payload bytes received in 'type' records.
  * 'type' is one of the following:
  *
@@ -509,7 +523,7 @@
   if (rr->type == SSL3_RT_HANDSHAKE) {
     /* If peer renegotiations are disabled, all out-of-order handshake records
      * are fatal. Renegotiations as a server are never supported. */
-    if (!s->accept_peer_renegotiations || s->server) {
+    if (s->server || !ssl3_can_renegotiate(s)) {
       al = SSL_AD_NO_RENEGOTIATION;
       OPENSSL_PUT_ERROR(SSL, SSL_R_NO_RENEGOTIATION);
       goto f_err;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 66867ba..355195e 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2699,8 +2699,13 @@
   ctx->dos_protection_cb = cb;
 }
 
-void SSL_set_reject_peer_renegotiations(SSL *s, int reject) {
-  s->accept_peer_renegotiations = !reject;
+void SSL_set_renegotiate_mode(SSL *ssl, enum ssl_renegotiate_mode_t mode) {
+  ssl->renegotiate_mode = mode;
+}
+
+void SSL_set_reject_peer_renegotiations(SSL *ssl, int reject) {
+  SSL_set_renegotiate_mode(
+      ssl, reject ? ssl_renegotiate_never : ssl_renegotiate_freely);
 }
 
 int SSL_get_rc4_state(const SSL *ssl, const RC4_KEY **read_key,
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 237ebf1..8d1f37b 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1114,9 +1114,11 @@
   if (config->install_ddos_callback) {
     SSL_CTX_set_dos_protection_cb(ssl_ctx, DDoSCallback);
   }
-  if (!config->reject_peer_renegotiations) {
-    /* Renegotiations are disabled by default. */
-    SSL_set_reject_peer_renegotiations(ssl.get(), 0);
+  if (config->renegotiate_once) {
+    SSL_set_renegotiate_mode(ssl.get(), ssl_renegotiate_once);
+  }
+  if (config->renegotiate_freely) {
+    SSL_set_renegotiate_mode(ssl.get(), ssl_renegotiate_freely);
   }
   if (!config->check_close_notify) {
     SSL_set_quiet_shutdown(ssl.get(), 1);
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ed37b77..189854e 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -206,9 +206,9 @@
 	// connection immediately after the handshake rather than echoing
 	// messages from the runner.
 	shimShutsDown bool
-	// renegotiate indicates the the connection should be renegotiated
-	// during the exchange.
-	renegotiate bool
+	// renegotiate indicates the number of times the connection should be
+	// renegotiated during the exchange.
+	renegotiate int
 	// renegotiateCiphers is a list of ciphersuite ids that will be
 	// switched in just before renegotiation.
 	renegotiateCiphers []uint16
@@ -403,12 +403,14 @@
 		tlsConn.SendAlert(alertLevelWarning, alertUnexpectedMessage)
 	}
 
-	if test.renegotiate {
+	if test.renegotiate > 0 {
 		if test.renegotiateCiphers != nil {
 			config.CipherSuites = test.renegotiateCiphers
 		}
-		if err := tlsConn.Renegotiate(); err != nil {
-			return err
+		for i := 0; i < test.renegotiate; i++ {
+			if err := tlsConn.Renegotiate(); err != nil {
+				return err
+			}
 		}
 	} else if test.renegotiateCiphers != nil {
 		panic("renegotiateCiphers without renegotiate")
@@ -2710,8 +2712,11 @@
 	if protocol == tls {
 		tests = append(tests, testCase{
 			name:        "Renegotiate-Client",
-			renegotiate: true,
-			flags:       []string{"-expect-total-renegotiations", "1"},
+			renegotiate: 1,
+			flags: []string{
+				"-renegotiate-freely",
+				"-expect-total-renegotiations", "1",
+			},
 		})
 		// NPN on client and server; results in post-handshake message.
 		tests = append(tests, testCase{
@@ -3675,8 +3680,7 @@
 	testCases = append(testCases, testCase{
 		testType:           serverTest,
 		name:               "Renegotiate-Server-Forbidden",
-		renegotiate:        true,
-		flags:              []string{"-reject-peer-renegotiations"},
+		renegotiate:        1,
 		shouldFail:         true,
 		expectedError:      ":NO_RENEGOTIATION:",
 		expectedLocalError: "remote error: no renegotiation",
@@ -3715,28 +3719,33 @@
 				FailIfResumeOnRenego: true,
 			},
 		},
-		renegotiate: true,
-		flags:       []string{"-expect-total-renegotiations", "1"},
+		renegotiate: 1,
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
 	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-Client-EmptyExt",
-		renegotiate: true,
+		renegotiate: 1,
 		config: Config{
 			Bugs: ProtocolBugs{
 				EmptyRenegotiationInfo: true,
 			},
 		},
+		flags:         []string{"-renegotiate-freely"},
 		shouldFail:    true,
 		expectedError: ":RENEGOTIATION_MISMATCH:",
 	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-Client-BadExt",
-		renegotiate: true,
+		renegotiate: 1,
 		config: Config{
 			Bugs: ProtocolBugs{
 				BadRenegotiationInfo: true,
 			},
 		},
+		flags:         []string{"-renegotiate-freely"},
 		shouldFail:    true,
 		expectedError: ":RENEGOTIATION_MISMATCH:",
 	})
@@ -3753,54 +3762,58 @@
 	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-Client-NoExt-Allowed",
-		renegotiate: true,
+		renegotiate: 1,
 		config: Config{
 			Bugs: ProtocolBugs{
 				NoRenegotiationInfo: true,
 			},
 		},
-		flags: []string{"-expect-total-renegotiations", "1"},
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
 	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-Client-SwitchCiphers",
-		renegotiate: true,
+		renegotiate: 1,
 		config: Config{
 			CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
 		},
 		renegotiateCiphers: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
-		flags:              []string{"-expect-total-renegotiations", "1"},
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
 	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-Client-SwitchCiphers2",
-		renegotiate: true,
+		renegotiate: 1,
 		config: Config{
 			CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
 		},
 		renegotiateCiphers: []uint16{TLS_RSA_WITH_RC4_128_SHA},
-		flags:              []string{"-expect-total-renegotiations", "1"},
-	})
-	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",
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
 	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-SameClientVersion",
-		renegotiate: true,
+		renegotiate: 1,
 		config: Config{
 			MaxVersion: VersionTLS10,
 			Bugs: ProtocolBugs{
 				RequireSameRenegoClientVersion: true,
 			},
 		},
-		flags: []string{"-expect-total-renegotiations", "1"},
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
 	})
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-FalseStart",
-		renegotiate: true,
+		renegotiate: 1,
 		config: Config{
 			CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
 			NextProtos:   []string{"foo"},
@@ -3808,10 +3821,52 @@
 		flags: []string{
 			"-false-start",
 			"-select-next-proto", "foo",
+			"-renegotiate-freely",
 			"-expect-total-renegotiations", "1",
 		},
 		shimWritesFirst: true,
 	})
+
+	// Client-side renegotiation controls.
+	testCases = append(testCases, testCase{
+		name:               "Renegotiate-Client-Forbidden-1",
+		renegotiate:        1,
+		shouldFail:         true,
+		expectedError:      ":NO_RENEGOTIATION:",
+		expectedLocalError: "remote error: no renegotiation",
+	})
+	testCases = append(testCases, testCase{
+		name:        "Renegotiate-Client-Once-1",
+		renegotiate: 1,
+		flags: []string{
+			"-renegotiate-once",
+			"-expect-total-renegotiations", "1",
+		},
+	})
+	testCases = append(testCases, testCase{
+		name:        "Renegotiate-Client-Freely-1",
+		renegotiate: 1,
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
+	})
+	testCases = append(testCases, testCase{
+		name:               "Renegotiate-Client-Once-2",
+		renegotiate:        2,
+		flags:              []string{"-renegotiate-once"},
+		shouldFail:         true,
+		expectedError:      ":NO_RENEGOTIATION:",
+		expectedLocalError: "remote error: no renegotiation",
+	})
+	testCases = append(testCases, testCase{
+		name:        "Renegotiate-Client-Freely-2",
+		renegotiate: 2,
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "2",
+		},
+	})
 }
 
 func addDTLSReplayTests() {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 6503494..4e19be6 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -76,7 +76,6 @@
   { "-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 },
   { "-no-legacy-server-connect", &TestConfig::no_legacy_server_connect },
   { "-tls-unique", &TestConfig::tls_unique },
   { "-use-async-private-key", &TestConfig::use_async_private_key },
@@ -95,7 +94,9 @@
   { "-microsoft-big-sslv3-buffer", &TestConfig::microsoft_big_sslv3_buffer },
   { "-verify-fail", &TestConfig::verify_fail },
   { "-verify-peer", &TestConfig::verify_peer },
-  { "-expect-verify-result", &TestConfig::expect_verify_result }
+  { "-expect-verify-result", &TestConfig::expect_verify_result },
+  { "-renegotiate-once", &TestConfig::renegotiate_once },
+  { "-renegotiate-freely", &TestConfig::renegotiate_freely },
 };
 
 const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index ea05271..4601851 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -77,7 +77,6 @@
   std::string export_label;
   std::string export_context;
   bool use_export_context = false;
-  bool reject_peer_renegotiations = false;
   bool no_legacy_server_connect = false;
   bool tls_unique = false;
   bool use_async_private_key = false;
@@ -98,6 +97,8 @@
   bool expect_verify_result = false;
   std::string signed_cert_timestamps;
   int expect_total_renegotiations = 0;
+  bool renegotiate_once = false;
+  bool renegotiate_freely = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_config);