Skip to content

Commit

Permalink
Fix implementation of PreferNoDHEKEX option.
Browse files Browse the repository at this point in the history
`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 <todd.short@me.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #22844)
  • Loading branch information
minichma authored and t8m committed Nov 30, 2023
1 parent 58d9262 commit f290663
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 29 deletions.
17 changes: 4 additions & 13 deletions ssl/statem/extensions.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
37 changes: 21 additions & 16 deletions ssl/statem/extensions_srvr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit f290663

Please sign in to comment.