Forbid caller-initiated renegotiations and all renego as a servers.
The only case where renego is supported is if we are a client and the
server sends a HelloRequest. That is still needed to support the renego
+ client auth hack in Chrome. Beyond that, no other forms of renego will
work.
The messy logic where the handshake loop is repurposed to send
HelloRequest and the extremely confusing tri-state s->renegotiate (which
makes SSL_renegotiate_pending a lie during the initial handshake as a
server) are now gone. The next change will further simplify things by
removing ssl->s3->renegotiate and the renego deferral logic. There's
also some server-only renegotiation checks that can go now.
Also clean up ssl3_read_bytes' HelloRequest handling. The old logic relied on
the handshake state machine to reject bad HelloRequests which... actually that
code probably lets you initiate renego by sending the first four bytes of a
ServerHello and expecting the peer to read it later.
BUG=429450
Change-Id: Ie0f87d0c2b94e13811fe8e22e810ab2ffc8efa6c
Reviewed-on: https://boringssl-review.googlesource.com/4824
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 71c5bf0..40c3e42 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -840,30 +840,6 @@
}
}
- if (config->renegotiate) {
- if (config->async) {
- fprintf(stderr, "-renegotiate is not supported with -async.\n");
- return false;
- }
- if (config->implicit_handshake) {
- fprintf(stderr, "-renegotiate is not supported with -implicit-handshake.\n");
- return false;
- }
-
- SSL_renegotiate(ssl.get());
-
- ret = SSL_do_handshake(ssl.get());
- if (ret != 1) {
- return false;
- }
-
- SSL_set_state(ssl.get(), SSL_ST_ACCEPT);
- ret = SSL_do_handshake(ssl.get());
- if (ret != 1) {
- return false;
- }
- }
-
if (config->export_keying_material > 0) {
std::vector<uint8_t> result(
static_cast<size_t>(config->export_keying_material));
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 57524a7..9e6cce3 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -690,10 +690,6 @@
// client offers a resumption or the server accepts one.
FailIfResumeOnRenego bool
- // NoSignatureAlgorithmsOnRenego, if true, causes renegotiations to omit
- // the signature_algorithms extension.
- NoSignatureAlgorithmsOnRenego bool
-
// IgnorePeerCipherPreferences, if true, causes the peer's cipher
// preferences to be ignored.
IgnorePeerCipherPreferences bool
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 5129b8f..1f9e84f 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -133,7 +133,7 @@
return errors.New("tls: short read from Rand: " + err.Error())
}
- if hello.vers >= VersionTLS12 && !c.config.Bugs.NoSignatureAndHashes && (c.cipherSuite == nil || !c.config.Bugs.NoSignatureAlgorithmsOnRenego) {
+ if hello.vers >= VersionTLS12 && !c.config.Bugs.NoSignatureAndHashes {
hello.signatureAndHashes = c.config.signatureAndHashesForClient()
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index e4a3f9a..6510e33 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2817,92 +2817,16 @@
}
func addRenegotiationTests() {
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Renegotiate-Server",
- config: Config{
- Bugs: ProtocolBugs{
- FailIfResumeOnRenego: true,
- },
- },
- flags: []string{"-renegotiate"},
- shimWritesFirst: true,
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Renegotiate-Server-EmptyExt",
- config: Config{
- Bugs: ProtocolBugs{
- EmptyRenegotiationInfo: true,
- },
- },
- flags: []string{"-renegotiate"},
- shimWritesFirst: true,
- shouldFail: true,
- expectedError: ":RENEGOTIATION_MISMATCH:",
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Renegotiate-Server-BadExt",
- config: Config{
- Bugs: ProtocolBugs{
- BadRenegotiationInfo: true,
- },
- },
- flags: []string{"-renegotiate"},
- shimWritesFirst: true,
- shouldFail: true,
- expectedError: ":RENEGOTIATION_MISMATCH:",
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Renegotiate-Server-ClientInitiated",
- renegotiate: true,
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Renegotiate-Server-ClientInitiated-NoExt",
- renegotiate: true,
- config: Config{
- Bugs: ProtocolBugs{
- NoRenegotiationInfo: true,
- },
- },
- shouldFail: true,
- expectedError: ":UNSAFE_LEGACY_RENEGOTIATION_DISABLED:",
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Renegotiate-Server-ClientInitiated-NoExt-Allowed",
- renegotiate: true,
- config: Config{
- Bugs: ProtocolBugs{
- NoRenegotiationInfo: true,
- },
- },
- flags: []string{"-allow-unsafe-legacy-renegotiation"},
- })
+ // Servers cannot renegotiate.
testCases = append(testCases, testCase{
testType: serverTest,
- name: "Renegotiate-Server-ClientInitiated-Forbidden",
+ name: "Renegotiate-Server-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,
- name: "Renegotiate-Server-NoSignatureAlgorithms",
- config: Config{
- Bugs: ProtocolBugs{
- NoSignatureAlgorithmsOnRenego: true,
- },
- },
- flags: []string{"-renegotiate"},
- shimWritesFirst: true,
- })
// TODO(agl): test the renegotiation info SCSV.
testCases = append(testCases, testCase{
name: "Renegotiate-Client",
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 4b24da6..df8553c 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -65,7 +65,6 @@
{ "-expect-session-miss", &TestConfig::expect_session_miss },
{ "-expect-extended-master-secret",
&TestConfig::expect_extended_master_secret },
- { "-renegotiate", &TestConfig::renegotiate },
{ "-allow-unsafe-legacy-renegotiation",
&TestConfig::allow_unsafe_legacy_renegotiation },
{ "-enable-ocsp-stapling", &TestConfig::enable_ocsp_stapling },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 4bac561..ff801db 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -54,7 +54,6 @@
bool expect_extended_master_secret = false;
std::string psk;
std::string psk_identity;
- bool renegotiate = false;
bool allow_unsafe_legacy_renegotiation = false;
std::string srtp_profiles;
bool enable_ocsp_stapling = false;
