Report TLS 1.3 as supporting secure renegotiation.
TLS 1.3 doesn't support renegotiation in the first place, but so callers
don't report TLS 1.3 servers as missing it, always report it as
(vacuously) protected against this bug.
BUG=chromium:680281
Change-Id: Ibfec03102b2aec7eaa773c331d6844292e7bb685
Reviewed-on: https://boringssl-review.googlesource.com/13046
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index c230f8c..a9f87ed 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1428,8 +1428,8 @@
OPENSSL_EXPORT int SSL_get_tls_unique(const SSL *ssl, uint8_t *out,
size_t *out_len, size_t max_out);
-/* SSL_get_extms_support returns one if the Extended Master Secret
- * extension was negotiated. Otherwise, it returns zero. */
+/* SSL_get_extms_support returns one if the Extended Master Secret extension or
+ * TLS 1.3 was negotiated. Otherwise, it returns zero. */
OPENSSL_EXPORT int SSL_get_extms_support(const SSL *ssl);
/* SSL_get_current_cipher returns the cipher used in the current outgoing
@@ -1444,7 +1444,7 @@
OPENSSL_EXPORT int SSL_session_reused(const SSL *ssl);
/* SSL_get_secure_renegotiation_support returns one if the peer supports secure
- * renegotiation (RFC 5746) and zero otherwise. */
+ * renegotiation (RFC 5746) or TLS 1.3. Otherwise, it returns zero. */
OPENSSL_EXPORT int SSL_get_secure_renegotiation_support(const SSL *ssl);
/* SSL_export_keying_material exports a value derived from the master secret, as
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index a60bf81..e0ab803 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1506,7 +1506,11 @@
}
int SSL_get_secure_renegotiation_support(const SSL *ssl) {
- return ssl->s3->send_connection_binding;
+ if (!ssl->s3->have_version) {
+ return 0;
+ }
+ return ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
+ ssl->s3->send_connection_binding;
}
LHASH_OF(SSL_SESSION) *SSL_CTX_sessions(SSL_CTX *ctx) { return ctx->sessions; }
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index a98ff43..900e4f6 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1305,11 +1305,22 @@
}
}
- if (config->expect_extended_master_secret) {
- if (!SSL_get_extms_support(ssl)) {
- fprintf(stderr, "No EMS for connection when expected");
- return false;
- }
+ if (config->expect_extended_master_secret && !SSL_get_extms_support(ssl)) {
+ fprintf(stderr, "No EMS for connection when expected\n");
+ return false;
+ }
+
+ if (config->expect_secure_renegotiation &&
+ !SSL_get_secure_renegotiation_support(ssl)) {
+ fprintf(stderr, "No secure renegotiation for connection when expected\n");
+ return false;
+ }
+
+ if (config->expect_no_secure_renegotiation &&
+ SSL_get_secure_renegotiation_support(ssl)) {
+ fprintf(stderr,
+ "Secure renegotiation unexpectedly negotiated for connection\n");
+ return false;
}
if (!config->expected_ocsp_response.empty()) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ba78fce..dee1bb9 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5246,6 +5246,7 @@
RequireRenegotiationInfo: true,
},
},
+ flags: []string{"-expect-secure-renegotiation"},
})
testCases = append(testCases, testCase{
testType: serverTest,
@@ -5258,6 +5259,7 @@
RequireRenegotiationInfo: true,
},
},
+ flags: []string{"-expect-secure-renegotiation"},
})
// Test that illegal extensions in TLS 1.3 are rejected by the client if
@@ -6015,6 +6017,7 @@
flags: []string{
"-renegotiate-freely",
"-expect-total-renegotiations", "1",
+ "-expect-secure-renegotiation",
},
})
testCases = append(testCases, testCase{
@@ -6081,6 +6084,7 @@
flags: []string{
"-renegotiate-freely",
"-expect-total-renegotiations", "1",
+ "-expect-no-secure-renegotiation",
},
})
@@ -6347,6 +6351,22 @@
shouldFail: true,
expectedError: ":UNEXPECTED_MESSAGE:",
})
+
+ // The renegotiation_info extension is not sent in TLS 1.3, but TLS 1.3
+ // always reads as supporting it, regardless of whether it was
+ // negotiated.
+ testCases = append(testCases, testCase{
+ name: "AlwaysReportRenegotiationInfo-TLS13",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ NoRenegotiationInfo: true,
+ },
+ },
+ flags: []string{
+ "-expect-secure-renegotiation",
+ },
+ })
}
func addDTLSReplayTests() {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index a06b5e5..10a86db 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -119,6 +119,10 @@
&TestConfig::expect_sha256_client_cert_resume },
{ "-enable-short-header", &TestConfig::enable_short_header },
{ "-read-with-unfinished-write", &TestConfig::read_with_unfinished_write },
+ { "-expect-secure-renegotiation",
+ &TestConfig::expect_secure_renegotiation },
+ { "-expect-no-secure-renegotiation",
+ &TestConfig::expect_no_secure_renegotiation },
};
const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 1307d56..ff91cef 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -127,6 +127,8 @@
bool expect_sha256_client_cert_resume = false;
bool enable_short_header = false;
bool read_with_unfinished_write = false;
+ bool expect_secure_renegotiation = false;
+ bool expect_no_secure_renegotiation = false;
};
bool ParseConfig(int argc, char **argv, TestConfig *out_config);
