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>
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 4b8fe45..4bd31b9 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c
@@ -505,6 +505,11 @@ 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 8f204a1..2e2f580 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc
@@ -1078,6 +1078,16 @@ 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 5e58733..ae1d0b6 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -7022,6 +7022,7 @@ useExportContext: true, }) } + testCases = append(testCases, testCase{ name: "ExportKeyingMaterial-SSL3", config: Config{ @@ -7034,6 +7035,47 @@ 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 70758df..112d642 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -106,6 +106,7 @@ { "-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 0116f14..d91a74b 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -116,6 +116,7 @@ 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);