Remove SSL_OP_LEGACY_SERVER_CONNECT.
I don't think we're ever going to manage to enforce this, and it doesn't
seem worth the trouble. We don't support application protocols which use
renegotiation outside of the HTTP/1.1 mid-stream client auth hack.
There, it's on the server to reject legacy renegotiations.
This removes the last of SSL_OP_ALL.
Change-Id: I996fdeaabf175b6facb4f687436549c0d3bb0042
Reviewed-on: https://boringssl-review.googlesource.com/6580
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index aaebf32..839cf94 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -285,10 +285,6 @@
ret->options |= SSL_OP_NO_TICKET;
}
- /* Default is to connect to non-RI servers. When RI is more widely deployed
- * might change this. */
- ret->options |= SSL_OP_LEGACY_SERVER_CONNECT;
-
/* Lock the SSL_CTX to the specified version, for compatibility with legacy
* uses of SSL_METHOD. */
if (method->version != 0) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 69d411b..d312a52 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -939,22 +939,14 @@
}
if (contents == NULL) {
- /* No renegotiation extension received.
- *
- * Strictly speaking if we want to avoid an attack we should *always* see
+ /* Strictly speaking, if we want to avoid an attack we should *always* see
* RI even on initial ServerHello because the client doesn't see any
* renegotiation during an attack. However this would mean we could not
* connect to any server which doesn't support RI.
*
- * A lack of the extension is allowed if SSL_OP_LEGACY_SERVER_CONNECT is
- * defined. */
- if (ssl->options & SSL_OP_LEGACY_SERVER_CONNECT) {
- return 1;
- }
-
- *out_alert = SSL_AD_HANDSHAKE_FAILURE;
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
- return 0;
+ * OpenSSL has |SSL_OP_LEGACY_SERVER_CONNECT| to control this, but in
+ * practical terms every client sets it so it's just assumed here. */
+ return 1;
}
const size_t expected_len = ssl->s3->previous_client_finished_len +
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 644e89d..6c5c31b 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1161,9 +1161,6 @@
if (config->no_ssl3) {
SSL_set_options(ssl.get(), SSL_OP_NO_SSLv3);
}
- if (config->no_legacy_server_connect) {
- SSL_clear_options(ssl.get(), SSL_OP_LEGACY_SERVER_CONNECT);
- }
if (!config->expected_channel_id.empty()) {
SSL_enable_tls_channel_id(ssl.get());
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 3c4e2ea..ba0eeeb 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -3793,17 +3793,6 @@
expectedError: ":RENEGOTIATION_MISMATCH:",
})
testCases = append(testCases, testCase{
- name: "Renegotiate-Client-NoExt",
- config: Config{
- Bugs: ProtocolBugs{
- NoRenegotiationInfo: true,
- },
- },
- shouldFail: true,
- expectedError: ":UNSAFE_LEGACY_RENEGOTIATION_DISABLED:",
- flags: []string{"-no-legacy-server-connect"},
- })
- testCases = append(testCases, testCase{
name: "Renegotiate-Client-Downgrade",
renegotiate: 1,
config: Config{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 4393c02..d2c66f2 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -75,7 +75,6 @@
{ "-fail-second-ddos-callback", &TestConfig::fail_second_ddos_callback },
{ "-handshake-never-done", &TestConfig::handshake_never_done },
{ "-use-export-context", &TestConfig::use_export_context },
- { "-no-legacy-server-connect", &TestConfig::no_legacy_server_connect },
{ "-tls-unique", &TestConfig::tls_unique },
{ "-expect-ticket-renewal", &TestConfig::expect_ticket_renewal },
{ "-expect-no-session", &TestConfig::expect_no_session },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index fb1ff3c..76ac375 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -76,7 +76,6 @@
std::string export_label;
std::string export_context;
bool use_export_context = false;
- bool no_legacy_server_connect = false;
bool tls_unique = false;
bool expect_ticket_renewal = false;
bool expect_no_session = false;