diff options
-rw-r--r-- | ssl/ssl_locl.h | 11 | ||||
-rw-r--r-- | ssl/statem/extensions.c | 2 | ||||
-rw-r--r-- | ssl/statem/extensions_clnt.c | 36 | ||||
-rw-r--r-- | ssl/statem/extensions_srvr.c | 4 | ||||
-rw-r--r-- | ssl/statem/statem_clnt.c | 5 |
5 files changed, 33 insertions, 25 deletions
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index f326399e03..33db1460ab 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -574,7 +574,6 @@ struct ssl_session_st { /* Session lifetime hint in seconds */ unsigned long tick_lifetime_hint; uint32_t tick_age_add; - int tick_identity; /* Max number of bytes that can be sent as early data */ uint32_t max_early_data; /* The ALPN protocol selected for this session */ @@ -1356,6 +1355,13 @@ struct ssl_st { * as this extension is optional on server side. */ uint8_t max_fragment_len_mode; + + /* + * On the client side the number of ticket identities we sent in the + * ClientHello. On the server side the identity of the ticket we + * selected. + */ + int tick_identity; } ext; /* @@ -2052,9 +2058,6 @@ typedef enum downgrade_en { #define TLSEXT_KEX_MODE_FLAG_KE 1 #define TLSEXT_KEX_MODE_FLAG_KE_DHE 2 -/* An invalid index into the TLSv1.3 PSK identities */ -#define TLSEXT_PSK_BAD_IDENTITY -1 - #define SSL_USE_PSS(s) (s->s3->tmp.peer_sigalg != NULL && \ s->s3->tmp.peer_sigalg->sig == EVP_PKEY_RSA_PSS) diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index c3d3441a1c..b27608cbb1 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -989,7 +989,6 @@ static int final_server_name(SSL *s, unsigned int context, int sent) ss->ext.ticklen = 0; ss->ext.tick_lifetime_hint = 0; ss->ext.tick_age_add = 0; - ss->ext.tick_identity = 0; if (!ssl_generate_session_id(s, ss)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_FINAL_SERVER_NAME, ERR_R_INTERNAL_ERROR); @@ -1646,7 +1645,6 @@ static int final_early_data(SSL *s, unsigned int context, int sent) if (s->max_early_data == 0 || !s->hit - || s->session->ext.tick_identity != 0 || s->early_data_state != SSL_EARLY_DATA_ACCEPTING || !s->ext.early_data_ok || s->hello_retry_request != SSL_HRR_NONE diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index ab4dbf6713..012e68701c 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -993,7 +993,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context, const EVP_MD *handmd = NULL, *mdres = NULL, *mdpsk = NULL; int dores = 0; - s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY; + s->ext.tick_identity = 0; /* * Note: At this stage of the code we only support adding a single @@ -1083,6 +1083,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context, agems += s->session->ext.tick_age_add; reshashsize = EVP_MD_size(mdres); + s->ext.tick_identity++; dores = 1; } @@ -1142,6 +1143,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } + s->ext.tick_identity++; } if (!WPACKET_close(pkt) @@ -1180,11 +1182,6 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context, return EXT_RETURN_FAIL; } - if (dores) - s->session->ext.tick_identity = 0; - if (s->psksession != NULL) - s->psksession->ext.tick_identity = (dores ? 1 : 0); - return EXT_RETURN_SENT; #else return EXT_RETURN_NOT_SENT; @@ -1927,8 +1924,7 @@ int tls_parse_stoc_early_data(SSL *s, PACKET *pkt, unsigned int context, } if (!s->ext.early_data_ok - || !s->hit - || s->session->ext.tick_identity != 0) { + || !s->hit) { /* * If we get here then we didn't send early data, or we didn't resume * using the first identity, or the SNI/ALPN is not consistent so the @@ -1956,17 +1952,28 @@ int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, return 0; } - if (s->session->ext.tick_identity == (int)identity) { + if (identity >= (unsigned int)s->ext.tick_identity) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_PSK, + SSL_R_BAD_PSK_IDENTITY); + return 0; + } + + /* + * Session resumption tickets are always sent before PSK tickets. If the + * ticket index is 0 then it must be for a session resumption ticket if we + * sent two tickets, or if we didn't send a PSK ticket. + */ + if (identity == 0 && (s->psksession == NULL || s->ext.tick_identity == 2)) { s->hit = 1; SSL_SESSION_free(s->psksession); s->psksession = NULL; return 1; } - if (s->psksession == NULL - || s->psksession->ext.tick_identity != (int)identity) { - SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_PSK, - SSL_R_BAD_PSK_IDENTITY); + if (s->psksession == NULL) { + /* Should never happen */ + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_PSK, + ERR_R_INTERNAL_ERROR); return 0; } @@ -1985,6 +1992,9 @@ int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, s->session = s->psksession; s->psksession = NULL; s->hit = 1; + /* Early data is only allowed if we used the first ticket */ + if (identity != 0) + s->ext.early_data_ok = 0; #endif return 1; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 0f2b22392b..c5ced14f4d 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -1274,7 +1274,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, goto err; } - sess->ext.tick_identity = id; + s->ext.tick_identity = id; SSL_SESSION_free(s->session); s->session = sess; @@ -1948,7 +1948,7 @@ EXT_RETURN tls_construct_stoc_psk(SSL *s, WPACKET *pkt, unsigned int context, if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_psk) || !WPACKET_start_sub_packet_u16(pkt) - || !WPACKET_put_bytes_u16(pkt, s->session->ext.tick_identity) + || !WPACKET_put_bytes_u16(pkt, s->ext.tick_identity) || !WPACKET_close(pkt)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_STOC_PSK, ERR_R_INTERNAL_ERROR); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index e56d24dfff..87800cd835 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1613,10 +1613,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) * so the PAC-based session secret is always preserved. It'll be * overwritten if the server refuses resumption. */ - if (s->session->session_id_length > 0 - || (SSL_IS_TLS13(s) - && s->session->ext.tick_identity - != TLSEXT_PSK_BAD_IDENTITY)) { + if (s->session->session_id_length > 0) { tsan_counter(&s->session_ctx->stats.sess_miss); if (!ssl_get_new_session(s, 0)) { /* SSLfatal() already called */ |