Add ssl_renegotiate_ignore.

This option causes clients to ignore HelloRequest messages completely.
This can be suitable in cases where a server tries to perform concurrent
application data and handshake flow, e.g. because they are trying to
“renew” symmetric keys.

Change-Id: I2779f7eff30d82163f2c34a625ec91dc34fab548
Reviewed-on: https://boringssl-review.googlesource.com/6431
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>

8 files changed, 47 insertions, 2 deletions

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index f9e0d85b..79d7205f 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2663,6 +2663,7 @@ enum ssl_renegotiate_mode_t {
   ssl_renegotiate_never = 0,
   ssl_renegotiate_once,
   ssl_renegotiate_freely,
+  ssl_renegotiate_ignore,
 };
 
 /* SSL_set_renegotiate_mode configures how |ssl|, a client, reacts to
@@ -2671,8 +2672,10 @@ enum ssl_renegotiate_mode_t {
  *
  * 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.
+ * allow one renegotiation, |ssl_renegotiate_freely| to allow all
+ * renegotiations or |ssl_renegotiate_ignore| to ignore HelloRequest messages.
+ * Note that ignoring HelloRequest messages may cause the connection to stall
+ * if the server waits for the renegotiation to complete.
  *
  * There is no support in BoringSSL for initiating renegotiations as a client
  * or server. */
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index c50b315d..7416d0e6 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -346,6 +346,8 @@ static int ssl3_can_renegotiate(SSL *ssl) {
       return ssl->s3->total_renegotiations == 0;
     case ssl_renegotiate_freely:
       return 1;
+    case ssl_renegotiate_ignore:
+      return 1;
   }
 
   assert(0);
@@ -567,6 +569,10 @@ start:
       goto err;
     }
 
+    if (s->renegotiate_mode == ssl_renegotiate_ignore) {
+      goto start;
+    }
+
     /* Renegotiation is only supported at quiescent points in the application
      * protocol, namely in HTTPS, just before reading the HTTP response. Require
      * the record-layer be idle and avoid complexities of sending a handshake
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 32c572e2..fe3dd6f3 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1205,6 +1205,9 @@ static bool DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx,
   if (config->renegotiate_freely) {
     SSL_set_renegotiate_mode(ssl.get(), ssl_renegotiate_freely);
   }
+  if (config->renegotiate_ignore) {
+    SSL_set_renegotiate_mode(ssl.get(), ssl_renegotiate_ignore);
+  }
   if (!config->check_close_notify) {
     SSL_set_quiet_shutdown(ssl.get(), 1);
   }
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 7defec10..078c227e 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -782,6 +782,11 @@ type ProtocolBugs struct {
 	// connections where the client offers a non-empty session ID or session
 	// ticket.
 	FailIfSessionOffered bool
+
+	// SendHelloRequestBeforeEveryAppDataRecord, if true, causes a
+	// HelloRequest handshake message to be sent before each application
+	// data record. This only makes sense for a server.
+	SendHelloRequestBeforeEveryAppDataRecord bool
 }
 
 func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index c911ad02..ab9e2330 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -1152,6 +1152,10 @@ func (c *Conn) Write(b []byte) (int, error) {
 		c.sendAlertLocked(alertLevelError, c.config.Bugs.SendSpuriousAlert)
 	}
 
+	if c.config.Bugs.SendHelloRequestBeforeEveryAppDataRecord {
+		c.writeRecord(recordTypeHandshake, []byte{typeHelloRequest, 0, 0, 0})
+	}
+
 	// SSL 3.0 and TLS 1.0 are susceptible to a chosen-plaintext
 	// attack when using block mode ciphers due to predictable IVs.
 	// This can be prevented by splitting each Application Data
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 4f0f8a3c..ad8e12a8 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -3920,6 +3920,28 @@ func addRenegotiationTests() {
 			"-expect-total-renegotiations", "2",
 		},
 	})
+	testCases = append(testCases, testCase{
+		name: "Renegotiate-Client-NoIgnore",
+		config: Config{
+			Bugs: ProtocolBugs{
+				SendHelloRequestBeforeEveryAppDataRecord: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":NO_RENEGOTIATION:",
+	})
+	testCases = append(testCases, testCase{
+		name: "Renegotiate-Client-Ignore",
+		config: Config{
+			Bugs: ProtocolBugs{
+				SendHelloRequestBeforeEveryAppDataRecord: true,
+			},
+		},
+		flags: []string{
+			"-renegotiate-ignore",
+			"-expect-total-renegotiations", "0",
+		},
+	})
 }
 
 func addDTLSReplayTests() {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index abec0e13..8b540c3a 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -96,6 +96,7 @@ const Flag<bool> kBoolFlags[] = {
   { "-expect-verify-result", &TestConfig::expect_verify_result },
   { "-renegotiate-once", &TestConfig::renegotiate_once },
   { "-renegotiate-freely", &TestConfig::renegotiate_freely },
+  { "-renegotiate-ignore", &TestConfig::renegotiate_ignore },
   { "-disable-npn", &TestConfig::disable_npn },
 };
 
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 23fa1f11..a72d66b7 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -98,6 +98,7 @@ struct TestConfig {
   int expect_total_renegotiations = 0;
   bool renegotiate_once = false;
   bool renegotiate_freely = false;
+  bool renegotiate_ignore = false;
   bool disable_npn = false;
 };