From 630369d9ce4f6bf67c4a055790362cd27b32229e Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 1 Aug 2017 15:45:29 +0100 Subject: [PATCH] Add server side sanity checks of SNI/ALPN for use with early_data Reviewed-by: Ben Kaduk (Merged from https://github.com/openssl/openssl/pull/3926) --- ssl/statem/extensions.c | 26 ++++++++++++++++++++------ ssl/statem/extensions_srvr.c | 10 ++++++++-- ssl/statem/statem_locl.h | 2 ++ ssl/statem/statem_srvr.c | 29 +++++++++++++++++++++++------ 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 3d830a7b76..9d32366e12 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -29,6 +29,7 @@ static int init_status_request(SSL *s, unsigned int context); static int init_npn(SSL *s, unsigned int context); #endif static int init_alpn(SSL *s, unsigned int context); +static int final_alpn(SSL *s, unsigned int context, int sent, int *al); static int init_sig_algs(SSL *s, unsigned int context); static int init_certificate_authorities(SSL *s, unsigned int context); static EXT_RETURN tls_construct_certificate_authorities(SSL *s, WPACKET *pkt, @@ -203,7 +204,7 @@ static const EXTENSION_DEFINITION ext_defs[] = { SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO | SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS, init_alpn, tls_parse_ctos_alpn, tls_parse_stoc_alpn, - tls_construct_stoc_alpn, tls_construct_ctos_alpn, NULL + tls_construct_stoc_alpn, tls_construct_ctos_alpn, final_alpn }, #ifndef OPENSSL_NO_SRTP { @@ -843,6 +844,8 @@ static int final_server_name(SSL *s, unsigned int context, int sent, case SSL_TLSEXT_ERR_NOACK: s->servername_done = 0; + if (s->session->ext.hostname != NULL) + s->ext.early_data_ok = 0; return 1; default: @@ -940,6 +943,21 @@ static int init_alpn(SSL *s, unsigned int context) return 1; } +static int final_alpn(SSL *s, unsigned int context, int sent, int *al) +{ + if (!s->server || !SSL_IS_TLS13(s)) + return 1; + + /* + * Call alpn_select callback if needed. Has to be done after SNI and + * cipher negotiation (HTTP/2 restricts permitted ciphers). In TLSv1.3 + * we also have to do this before we decide whether to accept early_data. + * In TLSv1.3 we've already negotiated our cipher so we do this call now. + * For < TLSv1.3 we defer it until after cipher negotiation. + */ + return tls_handle_alpn(s, al); +} + static int init_sig_algs(SSL *s, unsigned int context) { /* Clear any signature algorithms extension received */ @@ -1377,11 +1395,7 @@ static int final_early_data(SSL *s, unsigned int context, int sent, int *al) || s->session->ext.tick_identity != 0 || s->early_data_state != SSL_EARLY_DATA_ACCEPTING || !s->ext.early_data_ok - || s->hello_retry_request - || s->s3->alpn_selected_len != s->session->ext.alpn_selected_len - || (s->s3->alpn_selected_len > 0 - && memcmp(s->s3->alpn_selected, s->session->ext.alpn_selected, - s->s3->alpn_selected_len) != 0)) { + || s->hello_retry_request) { s->ext.early_data = SSL_EARLY_DATA_REJECTED; } else { s->ext.early_data = SSL_EARLY_DATA_ACCEPTED; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 2363c426e0..0dbec91303 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -131,6 +131,9 @@ int tls_parse_ctos_server_name(SSL *s, PACKET *pkt, unsigned int context, s->servername_done = s->session->ext.hostname && PACKET_equal(&hostname, s->session->ext.hostname, strlen(s->session->ext.hostname)); + + if (!s->servername_done && s->session->ext.hostname != NULL) + s->ext.early_data_ok = 0; } return 1; @@ -745,7 +748,8 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, memcpy(sess->sid_ctx, s->sid_ctx, s->sid_ctx_length); sess->sid_ctx_length = s->sid_ctx_length; ext = 1; - s->ext.early_data_ok = 1; + if (id == 0) + s->ext.early_data_ok = 1; } else { uint32_t ticket_age = 0, now, agesec, agems; int ret = tls_decrypt_ticket(s, PACKET_data(&identity), @@ -774,7 +778,8 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, * Therefore we add 1000ms to our age calculation to adjust for * rounding errors. */ - if (sess->timeout >= (long)agesec + if (id == 0 + && sess->timeout >= (long)agesec && agems / (uint32_t)1000 == agesec && ticket_age <= agems + 1000 && ticket_age + TICKET_AGE_ALLOWANCE >= agems + 1000) { @@ -791,6 +796,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, /* The ciphersuite is not compatible with this session. */ SSL_SESSION_free(sess); sess = NULL; + s->ext.early_data_ok = 0; continue; } break; diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index 1f8c22dd23..ae33fe5e5c 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -390,3 +390,5 @@ int tls_parse_stoc_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al); int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al); + +int tls_handle_alpn(SSL *s, int *al); diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 8c5f77bce5..8c2a4e97ac 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1938,7 +1938,7 @@ static int tls_handle_status_request(SSL *s, int *al) * Call the alpn_select callback if needed. Upon success, returns 1. * Upon failure, returns 0 and sets |*al| to the appropriate fatal alert. */ -static int tls_handle_alpn(SSL *s, int *al) +int tls_handle_alpn(SSL *s, int *al) { const unsigned char *selected = NULL; unsigned char selected_len = 0; @@ -1961,15 +1961,30 @@ static int tls_handle_alpn(SSL *s, int *al) /* ALPN takes precedence over NPN. */ s->s3->npn_seen = 0; #endif - } else if (r == SSL_TLSEXT_ERR_NOACK) { - /* Behave as if no callback was present. */ + + /* Check ALPN is consistent with early_data */ + if (s->ext.early_data_ok + && (s->session->ext.alpn_selected == NULL + || selected_len != s->session->ext.alpn_selected_len + || memcmp(selected, s->session->ext.alpn_selected, + selected_len) != 0)) + s->ext.early_data_ok = 0; + return 1; - } else { + } else if (r != SSL_TLSEXT_ERR_NOACK) { *al = SSL_AD_NO_APPLICATION_PROTOCOL; return 0; } + /* + * If r == SSL_TLSEXT_ERR_NOACK then behave as if no callback was + * present. + */ } + /* Check ALPN is consistent with early_data */ + if (s->ext.early_data_ok && s->session->ext.alpn_selected != NULL) + s->ext.early_data_ok = 0; + return 1; } @@ -2059,9 +2074,11 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst) } /* * Call alpn_select callback if needed. Has to be done after SNI and - * cipher negotiation (HTTP/2 restricts permitted ciphers). + * cipher negotiation (HTTP/2 restricts permitted ciphers). In TLSv1.3 + * we already did this because cipher negotiation happens earlier, and + * we must handle ALPN before we decide whether to accept early_data. */ - if (!tls_handle_alpn(s, &al)) { + if (!SSL_IS_TLS13(s) && !tls_handle_alpn(s, &al)) { SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT); goto f_err; -- 2.25.1