From b6fdc12d94d7527c3018492ab838cbcda81ba346 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 11 May 2017 10:55:54 +0100 Subject: [PATCH] Send a missing_extension alert if key_share/supported groups not present Only applies if we're not doing psk. Reviewed-by: Tim Hudson (Merged from https://github.com/openssl/openssl/pull/3436) --- include/openssl/ssl.h | 1 + ssl/ssl_err.c | 2 ++ ssl/statem/extensions.c | 5 ++++- ssl/statem/extensions_srvr.c | 17 ++++++++++++----- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 23dde11808..4558b17c3c 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2675,6 +2675,7 @@ int ERR_load_SSL_strings(void); # define SSL_R_MISSING_RSA_SIGNING_CERT 170 # define SSL_R_MISSING_SIGALGS_EXTENSION 112 # define SSL_R_MISSING_SRP_PARAM 358 +# define SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION 209 # define SSL_R_MISSING_TMP_DH_KEY 171 # define SSL_R_MISSING_TMP_ECDH_KEY 311 # define SSL_R_NOT_ON_RECORD_BOUNDARY 182 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 42bd6aa678..62d7d76835 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -641,6 +641,8 @@ static ERR_STRING_DATA SSL_str_reasons[] = { {ERR_REASON(SSL_R_MISSING_SIGALGS_EXTENSION), "missing sigalgs extension"}, {ERR_REASON(SSL_R_MISSING_SRP_PARAM), "can't find SRP server param"}, + {ERR_REASON(SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION), + "missing supported groups extension"}, {ERR_REASON(SSL_R_MISSING_TMP_DH_KEY), "missing tmp dh key"}, {ERR_REASON(SSL_R_MISSING_TMP_ECDH_KEY), "missing tmp ecdh key"}, {ERR_REASON(SSL_R_NOT_ON_RECORD_BOUNDARY), "not on record boundary"}, diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 9b16014f7b..578ca13a74 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1151,7 +1151,10 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al) if (!s->hit || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0) { /* Nothing left we can do - just fail */ - *al = SSL_AD_HANDSHAKE_FAILURE; + if (!sent) + *al = SSL_AD_MISSING_EXTENSION; + else + *al = SSL_AD_HANDSHAKE_FAILURE; SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE); return 0; } diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 0daf7d4b01..f85477c8f4 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -524,16 +524,23 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, return 0; } - /* - * Get the clients list of supported curves. - * TODO(TLS1.3): We should validate that we actually received - * supported_groups! - */ + /* Get the clients list of supported curves. */ if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) { *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); return 0; } + if (clnt_num_curves == 0) { + /* + * This can only happen if the supported_groups extension was not sent, + * because we verify that the length is non-zero when we process that + * extension. + */ + *al = SSL_AD_MISSING_EXTENSION; + SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, + SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION); + return 0; + } while (PACKET_remaining(&key_share_list) > 0) { if (!PACKET_get_net_2(&key_share_list, &group_id) -- 2.40.0