Verifying resumption cipher validity with current configuration.
BUG=chromium:659593
Change-Id: I73a4751609b85df7cd40f0f60dc3f3046a490940
Reviewed-on: https://boringssl-review.googlesource.com/11861
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/ssl/handshake_server.c b/ssl/handshake_server.c
index ad3ede3..8db9bee 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -771,7 +771,10 @@
ssl->version == session->ssl_version &&
/* If the client offers the EMS extension, but the previous session
* didn't use it, then negotiate a new session. */
- have_extended_master_secret == session->extended_master_secret;
+ have_extended_master_secret == session->extended_master_secret &&
+ /* Only resume if the session's cipher is still valid under the
+ * current configuration. */
+ ssl_is_valid_cipher(ssl, session->cipher);
}
if (has_session) {
diff --git a/ssl/internal.h b/ssl/internal.h
index 2772e1b..114c810 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1695,6 +1695,11 @@
int ssl3_write_app_data(SSL *ssl, const void *buf, int len);
int ssl3_write_bytes(SSL *ssl, int type, const void *buf, int len);
int ssl3_output_cert_chain(SSL *ssl);
+
+/* ssl_is_valid_cipher checks that |cipher| is valid according to the current
+ * server configuration in |ssl|. It returns 1 if valid, and 0 otherwise. */
+int ssl_is_valid_cipher(SSL *ssl, const SSL_CIPHER *cipher);
+
const SSL_CIPHER *ssl3_choose_cipher(
SSL *ssl, const struct ssl_early_callback_ctx *client_hello,
const struct ssl_cipher_preference_list_st *srvr);
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 69d3a9d..ad7a544 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -240,6 +240,17 @@
return NULL;
}
+int ssl_is_valid_cipher(SSL *ssl, const SSL_CIPHER *cipher) {
+ /* Check the TLS version. */
+ if (SSL_CIPHER_get_min_version(cipher) > ssl3_protocol_version(ssl) ||
+ SSL_CIPHER_get_max_version(cipher) < ssl3_protocol_version(ssl)) {
+ return 0;
+ }
+
+ return sk_SSL_CIPHER_find(ssl_get_cipher_preferences(ssl)->ciphers,
+ NULL, cipher);
+}
+
const SSL_CIPHER *ssl3_choose_cipher(
SSL *ssl, const struct ssl_early_callback_ctx *client_hello,
const struct ssl_cipher_preference_list_st *server_pref) {
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 76caa4e..2eee888 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1456,6 +1456,11 @@
if (config->max_cert_list > 0) {
SSL_set_max_cert_list(ssl.get(), config->max_cert_list);
}
+ if (is_resume &&
+ !config->resume_cipher.empty() &&
+ !SSL_set_cipher_list(ssl.get(), config->resume_cipher.c_str())) {
+ return false;
+ }
int sock = Connect(config->port);
if (sock == -1) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ab5fdee..03a0ef9 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5367,6 +5367,31 @@
}
}
+ // Sessions with disabled ciphers are not resumed.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "Resume-Server-CipherMismatch",
+ resumeSession: true,
+ config: Config{
+ MaxVersion: VersionTLS12,
+ },
+ flags: []string{"-cipher", "AES128", "-resume-cipher", "AES256"},
+ shouldFail: false,
+ expectResumeRejected: true,
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "Resume-Server-CipherMismatch-TLS13",
+ resumeSession: true,
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ flags: []string{"-cipher", "AES128", "-resume-cipher", "AES256"},
+ shouldFail: false,
+ expectResumeRejected: true,
+ })
+
testCases = append(testCases, testCase{
name: "Resume-Client-CipherMismatch",
resumeSession: true,
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 425664d..43b28a2 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -128,6 +128,7 @@
{ "-cipher", &TestConfig::cipher },
{ "-cipher-tls10", &TestConfig::cipher_tls10 },
{ "-cipher-tls11", &TestConfig::cipher_tls11 },
+ { "-resume-cipher", &TestConfig::resume_cipher },
{ "-export-label", &TestConfig::export_label },
{ "-export-context", &TestConfig::export_context },
};
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 9f74297..8aa6785 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -76,6 +76,7 @@
std::string cipher;
std::string cipher_tls10;
std::string cipher_tls11;
+ std::string resume_cipher;
bool handshake_never_done = false;
int export_keying_material = 0;
std::string export_label;
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index fac4364..2280a2b 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -123,7 +123,8 @@
/* Only resume if the session's version matches. */
(session->ssl_version != ssl->version ||
!ssl_client_cipher_list_contains_cipher(
- &client_hello, (uint16_t)SSL_CIPHER_get_id(session->cipher)))) {
+ &client_hello, (uint16_t)SSL_CIPHER_get_id(session->cipher)) ||
+ !ssl_is_valid_cipher(ssl, session->cipher))) {
SSL_SESSION_free(session);
session = NULL;
}
