Negotiate ciphers before resumption.
This changes our resumption strategy. Before, we would negotiate ciphers
only on fresh handshakes. On resumption, we would blindly use whatever
was in the session.
Instead, evaluate cipher suite preferences on every handshake.
Resumption requires that the saved cipher suite match the one that would
have been negotiated anyway. If client or server preferences changed
sufficiently, we decline the session.
This is much easier to reason about (we always pick the best cipher
suite), simpler, and avoids getting stuck under old preferences if
tickets are continuously renewed. Notably, although TLS 1.2 ticket
renewal does not work in practice, TLS 1.3 will renew tickets like
there's no tomorrow.
It also means we don't need dedicated code to avoid resuming a cipher
which has since been disabled. (That dedicated code was a little odd
anyway since the mask_k, etc., checks didn't occur. When cert_cb was
skipped on resumption, one could resume without ever configuring a
certificate! So we couldn't know whether to mask off RSA or ECDSA cipher
suites.)
Add tests which assert on this new arrangement.
BUG=116
Change-Id: Id40d851ccd87e06c46c6ec272527fd8ece8abfc6
Reviewed-on: https://boringssl-review.googlesource.com/11847
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 0a8b97b..83ef679 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -138,6 +138,7 @@
client_hello->cipher_suites_len);
const int aes_is_fine = EVP_has_aes_hardware();
+ const uint16_t version = ssl3_protocol_version(ssl);
const SSL_CIPHER *best = NULL;
while (CBS_len(&cipher_suites) > 0) {
@@ -146,8 +147,11 @@
return NULL;
}
+ /* Limit to TLS 1.3 ciphers we know about. */
const SSL_CIPHER *candidate = SSL_get_cipher_by_value(cipher_suite);
- if (candidate == NULL || !ssl_is_valid_cipher(ssl, candidate)) {
+ if (candidate == NULL ||
+ SSL_CIPHER_get_min_version(candidate) > version ||
+ SSL_CIPHER_get_max_version(candidate) < version) {
continue;
}
@@ -192,6 +196,15 @@
return ssl_hs_error;
}
+ /* Negotiate the cipher suite. */
+ ssl->s3->tmp.new_cipher = choose_tls13_cipher(ssl, &client_hello);
+ if (ssl->s3->tmp.new_cipher == NULL) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
+ return ssl_hs_error;
+ }
+
+ /* Decode the ticket if we agree on a PSK key exchange mode. */
uint8_t alert = SSL_AD_DECODE_ERROR;
SSL_SESSION *session = NULL;
CBS pre_shared_key, binders;
@@ -220,11 +233,24 @@
session = NULL;
}
+ /* Set up the new session, either using the original one as a template or
+ * creating a fresh one. */
if (session == NULL) {
if (!ssl_get_new_session(ssl, 1 /* server */)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return ssl_hs_error;
}
+
+ ssl->s3->new_session->cipher = ssl->s3->tmp.new_cipher;
+
+ /* On new sessions, stash the SNI value in the session. */
+ if (ssl->s3->hs->hostname != NULL) {
+ ssl->s3->new_session->tlsext_hostname = BUF_strdup(ssl->s3->hs->hostname);
+ if (ssl->s3->new_session->tlsext_hostname == NULL) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+ return ssl_hs_error;
+ }
+ }
} else {
/* Check the PSK binder. */
if (!tls13_verify_psk_binder(ssl, session, &binders)) {
@@ -251,40 +277,8 @@
return ssl_hs_error;
}
- if (ssl->s3->session_reused) {
- /* Clients may not offer sessions containing unsupported ciphers. */
- if (!ssl_client_cipher_list_contains_cipher(
- &client_hello,
- (uint16_t)SSL_CIPHER_get_id(ssl->s3->new_session->cipher))) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_REQUIRED_CIPHER_MISSING);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
- } else {
- const SSL_CIPHER *cipher = choose_tls13_cipher(ssl, &client_hello);
- if (cipher == NULL) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
- return ssl_hs_error;
- }
-
- ssl->s3->new_session->cipher = cipher;
-
- /* On new sessions, stash the SNI value in the session. */
- if (ssl->s3->hs->hostname != NULL) {
- ssl->s3->new_session->tlsext_hostname = BUF_strdup(ssl->s3->hs->hostname);
- if (ssl->s3->new_session->tlsext_hostname == NULL) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
- return ssl_hs_error;
- }
- }
- }
-
- ssl->s3->tmp.new_cipher = ssl->s3->new_session->cipher;
- ssl->method->received_flight(ssl);
-
- /* Resolve ALPN after the cipher suite is selected. HTTP/2 negotiation depends
- * on the cipher suite. */
+ /* HTTP/2 negotiation depends on the cipher suite, so ALPN negotiation was
+ * deferred. Complete it now. */
if (!ssl_negotiate_alpn(ssl, &alert, &client_hello)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
@@ -310,6 +304,8 @@
return ssl_hs_error;
}
+ ssl->method->received_flight(ssl);
+
/* Resolve ECDHE and incorporate it into the secret. */
int need_retry;
if (!resolve_ecdhe_secret(ssl, &need_retry, &client_hello)) {
