From b6b6ff3bef673afaa4d05400564f8f4f4d31ad9c Mon Sep 17 00:00:00 2001 From: Steven Valdez Date: Wed, 26 Oct 2016 11:56:35 -0400 Subject: [PATCH] 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 Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/handshake_server.c | 5 ++++- ssl/internal.h | 5 +++++ ssl/s3_lib.c | 11 +++++++++++ ssl/test/bssl_shim.cc | 5 +++++ ssl/test/runner/runner.go | 25 +++++++++++++++++++++++++ ssl/test/test_config.cc | 1 + ssl/test/test_config.h | 1 + ssl/tls13_server.c | 3 ++- 8 files changed, 54 insertions(+), 2 deletions(-) diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index ad3ede37..8db9bee3 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -771,7 +771,10 @@ static int ssl3_get_client_hello(SSL *ssl) { 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 2772e1bd..114c810f 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1695,6 +1695,11 @@ int ssl3_read_handshake_bytes(SSL *ssl, uint8_t *buf, int len); 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 69d3a9dc..ad7a5443 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -240,6 +240,17 @@ struct ssl_cipher_preference_list_st *ssl_get_cipher_preferences(SSL *ssl) { 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 76caa4e6..2eee8885 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -1456,6 +1456,11 @@ static bool DoExchange(bssl::UniquePtr *out_session, 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 ab5fdee1..03a0ef90 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -5367,6 +5367,31 @@ func addResumptionVersionTests() { } } + // 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 425664d4..43b28a27 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -128,6 +128,7 @@ const Flag kStringFlags[] = { { "-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 9f742975..8aa6785b 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -76,6 +76,7 @@ struct TestConfig { 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 fac43646..2280a2be 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c @@ -123,7 +123,8 @@ static enum ssl_hs_wait_t do_process_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) { /* 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; }