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>
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 091dc30..fd4bd60 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -663,6 +663,9 @@
!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) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 650c14f..ce0271f 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2873,6 +2873,15 @@
},
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,
@@ -2939,6 +2948,14 @@
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,
config: Config{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 2527837..25906f7 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -80,6 +80,7 @@
{ "-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[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index de1eda6..f107a0f 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -77,6 +77,7 @@
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);
