From f4bbb37c4c95ea8cdb4b3470098a1b5d7d1977ed Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 18 Jan 2017 11:31:37 +0000 Subject: [PATCH] Provide a key_share extension finaliser This mops up various edge cases with key_shares and makes sure we still generate the handshake secret if we haven't been provided with one but we have a PSK. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2259) --- include/openssl/ssl.h | 1 + ssl/ssl_err.c | 1 + ssl/statem/extensions.c | 43 +++++++++++++++++++++++++++++++++++- ssl/statem/extensions_srvr.c | 2 +- ssl/statem/statem_srvr.c | 9 -------- 5 files changed, 45 insertions(+), 11 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 52be4064fa..cee7549d9b 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2097,6 +2097,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_DTLS_PROCESS_HELLO_VERIFY 386 # define SSL_F_FINAL_EC_PT_FORMATS 485 # define SSL_F_FINAL_EMS 486 +# define SSL_F_FINAL_KEY_SHARE 503 # define SSL_F_FINAL_RENEGOTIATE 483 # define SSL_F_FINAL_SIG_ALGS 497 # define SSL_F_NSS_KEYLOG_INT 500 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 9b02c58737..9edee93727 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -51,6 +51,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_DTLS_PROCESS_HELLO_VERIFY), "dtls_process_hello_verify"}, {ERR_FUNC(SSL_F_FINAL_EC_PT_FORMATS), "final_ec_pt_formats"}, {ERR_FUNC(SSL_F_FINAL_EMS), "final_ems"}, + {ERR_FUNC(SSL_F_FINAL_KEY_SHARE), "final_key_share"}, {ERR_FUNC(SSL_F_FINAL_RENEGOTIATE), "final_renegotiate"}, {ERR_FUNC(SSL_F_FINAL_SIG_ALGS), "final_sig_algs"}, {ERR_FUNC(SSL_F_NSS_KEYLOG_INT), "nss_keylog_int"}, diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 4c66b3362f..f1a1675d66 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -36,6 +36,7 @@ static int init_etm(SSL *s, unsigned int context); static int init_ems(SSL *s, unsigned int context); static int final_ems(SSL *s, unsigned int context, int sent, int *al); static int init_psk_kex_modes(SSL *s, unsigned int context); +static int final_key_share(SSL *s, unsigned int context, int sent, int *al); #ifndef OPENSSL_NO_SRTP static int init_srtp(SSL *s, unsigned int context); #endif @@ -252,7 +253,8 @@ static const EXTENSION_DEFINITION ext_defs[] = { | EXT_TLS1_3_HELLO_RETRY_REQUEST | EXT_TLS_IMPLEMENTATION_ONLY | EXT_TLS1_3_ONLY, NULL, tls_parse_ctos_key_share, tls_parse_stoc_key_share, - tls_construct_stoc_key_share, tls_construct_ctos_key_share, NULL + tls_construct_stoc_key_share, tls_construct_ctos_key_share, + final_key_share }, { /* @@ -955,6 +957,45 @@ static int final_sig_algs(SSL *s, unsigned int context, int sent, int *al) return 1; } + +static int final_key_share(SSL *s, unsigned int context, int sent, int *al) +{ + if (!SSL_IS_TLS13(s)) + return 1; + + /* + * If + * we have no key_share + * AND + * (we are not resuming + * OR the kex_mode doesn't allow non key_share resumes) + * THEN + * fail + */ + if (((s->server && s->s3->peer_tmp == NULL) || (!s->server && !sent)) + && (!s->hit + || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0)) { + /* No suitable share */ + /* TODO(TLS1.3): Send a HelloRetryRequest */ + *al = SSL_AD_HANDSHAKE_FAILURE; + SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE); + return 0; + } + + /* + * For a client side resumption with no key_share we need to generate + * the handshake secret (otherwise this is done during key_share + * processing). + */ + if (!sent && !s->server && !tls13_generate_handshake_secret(s, NULL, 0)) { + *al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR); + return 0; + } + + return 1; +} + static int init_psk_kex_modes(SSL *s, unsigned int context) { s->ext.psk_kex_mode = TLSEXT_KEX_MODE_FLAG_NONE; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 8ee292818e..1e10a10c47 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -523,7 +523,7 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int group_nid, found = 0; unsigned int curve_flags; - if (s->hit) + if (s->hit && (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) == 0) return 1; /* Sanity check */ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 98171b948c..f9659e2d9d 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1557,15 +1557,6 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) goto f_err; } - /* Check we've got a key_share for TLSv1.3 */ - if (SSL_IS_TLS13(s) && s->s3->peer_tmp == NULL && !s->hit) { - /* No suitable share */ - /* TODO(TLS1.3): Send a HelloRetryRequest */ - al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_NO_SUITABLE_KEY_SHARE); - goto f_err; - } - /* * Check if we want to use external pre-shared secret for this handshake * for not reused session only. We need to generate server_random before -- 2.25.1