From f290663148ddddaffc0dc8737b08a244b49a76ba Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Mon, 27 Nov 2023 18:26:51 +0100 Subject: Fix implementation of `PreferNoDHEKEX` option. `tls_parse_ctos_key_share()` didn't properly handle the option. Avoid the need to deal with the option in multiple places by properly handling it in `tls_parse_ctos_psk_kex_modes()`. Reviewed-by: Todd Short Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22844) --- ssl/statem/extensions.c | 17 ++++------------- ssl/statem/extensions_srvr.c | 37 +++++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 29 deletions(-) (limited to 'ssl') diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index e77ab2f3e5..0a64ca2246 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1393,8 +1393,7 @@ static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent) * the client sent a key_share extension * AND * (we are not resuming - * OR (the kex_mode allows key_share resumes - * AND (kex_mode doesn't allow non-dh resumes OR non-dh is not preferred))) + * OR the kex_mode allows key_share resumes) * AND * a shared group exists * THEN @@ -1429,18 +1428,10 @@ static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent) } } else { /* No suitable key_share */ - - /* Do DHE PSK? */ - int dhe_psk = - /* kex_mode allows key_share resume */ - (((s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) != 0) - - /* and psk-only is not available or not explicitly preferred */ - && ((((s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0) - || (s->options & SSL_OP_PREFER_NO_DHE_KEX) == 0))); - if (s->hello_retry_request == SSL_HRR_NONE && sent - && (!s->hit || dhe_psk)) { + && (!s->hit + || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) + != 0)) { const uint16_t *pgroups, *clntgroups; size_t num_groups, clnt_num_groups, i; unsigned int group_id = 0; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 2d28ea3b3d..c4287bd853 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -573,6 +573,21 @@ int tls_parse_ctos_psk_kex_modes(SSL_CONNECTION *s, PACKET *pkt, && (s->options & SSL_OP_ALLOW_NO_DHE_KEX) != 0) s->ext.psk_kex_mode |= TLSEXT_KEX_MODE_FLAG_KE; } + + if (((s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) != 0) + && (s->options & SSL_OP_PREFER_NO_DHE_KEX) != 0) { + + /* + * If NO_DHE is supported and preferred, then we only remember this + * mode. DHE PSK will not be used for sure, because in any case where + * it would be supported (i.e. if a key share is present), NO_DHE would + * be supported as well. As the latter is preferred it would be + * choosen. By removing DHE PSK here, we don't have to deal with the + * SSL_OP_PREFER_NO_DHE_KEX option in any other place. + */ + s->ext.psk_kex_mode = TLSEXT_KEX_MODE_FLAG_KE; + } + #endif return 1; @@ -1645,25 +1660,15 @@ EXT_RETURN tls_construct_stoc_key_share(SSL_CONNECTION *s, WPACKET *pkt, } return EXT_RETURN_NOT_SENT; } - if (s->hit) { - /* PSK ('hit') */ + if (s->hit && (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) == 0) { /* - * If we're doing PSK ('hit') but the client doesn't support psk-dhe, - * we don't need to send a key share. + * PSK ('hit') and explicitly not doing DHE. If the client sent the + * DHE option, we take it by default, except if non-DHE would be + * preferred by config, but this case would have been handled in + * tls_parse_ctos_psk_kex_modes(). */ - if ((s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) == 0) - return EXT_RETURN_NOT_SENT; - - /* - * If both, psk_ke and psk_dh_ke are available, we do psk_dh_ke and - * send a key share by default, but not if the server is explicitly - * configured to prefer psk_ke. - */ - if (((s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) != 0) - && ((s->options & SSL_OP_PREFER_NO_DHE_KEX) != 0)) { - return EXT_RETURN_NOT_SENT; - } + return EXT_RETURN_NOT_SENT; } if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share) -- cgit v1.2.3