From 7bb1d292cb90c19385de715266591677ff7d930b Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Tue, 1 Nov 2016 19:45:06 -0400
Subject: [PATCH] Forbid using exporters during a renego.

They will get very confused about which key they're using. Any caller
using exporters must either (a) leave renegotiation off or (b) be very
aware of when renegotiations happen anyway. (You need to somehow
coordinate with the peer about which epoch's exporter to use.)

Change-Id: I921ad01ac9bdc88f3fd0f8283757ce673a47ec75
Reviewed-on: https://boringssl-review.googlesource.com/12003
Reviewed-by: Adam Langley <agl@google.com>
---
 ssl/t1_enc.c              |  5 +++++
 ssl/test/bssl_shim.cc     | 10 ++++++++++
 ssl/test/runner/runner.go | 42 +++++++++++++++++++++++++++++++++++++++
 ssl/test/test_config.cc   |  1 +
 ssl/test/test_config.h    |  1 +
 5 files changed, 59 insertions(+)

diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 4b8fe455b..4bd31b95f 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -505,6 +505,11 @@ int SSL_export_keying_material(SSL *ssl, uint8_t *out, size_t out_len,
     return 0;
   }
 
+  /* Exporters may not be used in the middle of a renegotiation. */
+  if (SSL_in_init(ssl) && !SSL_in_false_start(ssl)) {
+    return 0;
+  }
+
   if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
     return tls13_export_keying_material(ssl, out, out_len, label, label_len,
                                         context, context_len, use_context);
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 8f204a12f..2e2f5802f 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1078,6 +1078,16 @@ static int DoRead(SSL *ssl, uint8_t *out, size_t max_out) {
     if (config->async) {
       AsyncBioEnforceWriteQuota(test_state->async_bio, true);
     }
+
+    // Run the exporter after each read. This is to test that the exporter fails
+    // during a renegotiation.
+    if (config->use_exporter_between_reads) {
+      uint8_t buf;
+      if (!SSL_export_keying_material(ssl, &buf, 1, NULL, 0, NULL, 0, 0)) {
+        fprintf(stderr, "failed to export keying material\n");
+        return -1;
+      }
+    }
   } while (config->async && RetryAsync(ssl, ret));
 
   if (config->peek_then_read && ret > 0) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 5e587336f..ae1d0b659 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -7022,6 +7022,7 @@ func addExportKeyingMaterialTests() {
 			useExportContext:     true,
 		})
 	}
+
 	testCases = append(testCases, testCase{
 		name: "ExportKeyingMaterial-SSL3",
 		config: Config{
@@ -7034,6 +7035,47 @@ func addExportKeyingMaterialTests() {
 		shouldFail:           true,
 		expectedError:        "failed to export keying material",
 	})
+
+	// Exporters work during a False Start.
+	testCases = append(testCases, testCase{
+		name: "ExportKeyingMaterial-FalseStart",
+		config: Config{
+			MaxVersion:   VersionTLS12,
+			CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+			NextProtos:   []string{"foo"},
+			Bugs: ProtocolBugs{
+				ExpectFalseStart: true,
+			},
+		},
+		flags: []string{
+			"-false-start",
+			"-advertise-alpn", "\x03foo",
+		},
+		shimWritesFirst:      true,
+		exportKeyingMaterial: 1024,
+		exportLabel:          "label",
+		exportContext:        "context",
+		useExportContext:     true,
+	})
+
+	// Exporters do not work in the middle of a renegotiation. Test this by
+	// triggering the exporter after every SSL_read call and configuring the
+	// shim to run asynchronously.
+	testCases = append(testCases, testCase{
+		name: "ExportKeyingMaterial-Renegotiate",
+		config: Config{
+			MaxVersion: VersionTLS12,
+		},
+		renegotiate: 1,
+		flags: []string{
+			"-async",
+			"-use-exporter-between-reads",
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
+		shouldFail:    true,
+		expectedError: "failed to export keying material",
+	})
 }
 
 func addTLSUniqueTests() {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 70758dfff..112d642dc 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -106,6 +106,7 @@ const Flag<bool> kBoolFlags[] = {
   { "-send-alert", &TestConfig::send_alert },
   { "-peek-then-read", &TestConfig::peek_then_read },
   { "-enable-grease", &TestConfig::enable_grease },
+  { "-use-exporter-between-reads", &TestConfig::use_exporter_between_reads },
 };
 
 const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 0116f147a..d91a74be5 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -116,6 +116,7 @@ struct TestConfig {
   bool enable_grease = false;
   int max_cert_list = 0;
   std::string ticket_key;
+  bool use_exporter_between_reads = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_config);