From b8590b2f365a963965d799c438c5c92659c2fcae Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Tue, 21 Nov 2023 20:42:12 +0100 Subject: [PATCH] Add option `SSL_OP_PREFER_NO_DHE_KEX`, allowing the server to prefer non-dhe psk key exchange over psk with dhe (config file option `PreferNoDHEKEX`, server option `prefer_no_dhe_kex`). Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/22794) --- apps/include/opt.h | 6 +++- include/openssl/ssl.h.in | 2 ++ ssl/ssl_conf.c | 4 +++ ssl/statem/extensions.c | 17 +++++++--- ssl/statem/extensions_srvr.c | 21 +++++++++--- test/recipes/70-test_tls13kexmodes.t | 50 ++++++++++++++++++++++------ 6 files changed, 81 insertions(+), 19 deletions(-) diff --git a/apps/include/opt.h b/apps/include/opt.h index 5a2faa150b..2bd2fb2484 100644 --- a/apps/include/opt.h +++ b/apps/include/opt.h @@ -157,7 +157,8 @@ OPT_S_NOTLS1_3, OPT_S_BUGS, OPT_S_NO_COMP, OPT_S_NOTICKET, \ OPT_S_SERVERPREF, OPT_S_LEGACYRENEG, OPT_S_CLIENTRENEG, \ OPT_S_LEGACYCONN, \ - OPT_S_ONRESUMP, OPT_S_NOLEGACYCONN, OPT_S_ALLOW_NO_DHE_KEX, \ + OPT_S_ONRESUMP, OPT_S_NOLEGACYCONN, \ + OPT_S_ALLOW_NO_DHE_KEX, OPT_S_PREFER_NO_DHE_KEX, \ OPT_S_PRIORITIZE_CHACHA, \ OPT_S_STRICT, OPT_S_SIGALGS, OPT_S_CLIENTSIGALGS, OPT_S_GROUPS, \ OPT_S_CURVES, OPT_S_NAMEDCURVE, OPT_S_CIPHER, OPT_S_CIPHERSUITES, \ @@ -198,6 +199,8 @@ "Disallow initial connection to servers that don't support RI"}, \ {"allow_no_dhe_kex", OPT_S_ALLOW_NO_DHE_KEX, '-', \ "In TLSv1.3 allow non-(ec)dhe based key exchange on resumption"}, \ + {"prefer_no_dhe_kex", OPT_S_PREFER_NO_DHE_KEX, '-', \ + "In TLSv1.3 prefer non-(ec)dhe over (ec)dhe-based key exchange on resumption"}, \ {"prioritize_chacha", OPT_S_PRIORITIZE_CHACHA, '-', \ "Prioritize ChaCha ciphers when preferred by clients"}, \ {"strict", OPT_S_STRICT, '-', \ @@ -248,6 +251,7 @@ case OPT_S_ONRESUMP: \ case OPT_S_NOLEGACYCONN: \ case OPT_S_ALLOW_NO_DHE_KEX: \ + case OPT_S_PREFER_NO_DHE_KEX: \ case OPT_S_PRIORITIZE_CHACHA: \ case OPT_S_STRICT: \ case OPT_S_SIGALGS: \ diff --git a/include/openssl/ssl.h.in b/include/openssl/ssl.h.in index 9f91039f8a..b22522d006 100644 --- a/include/openssl/ssl.h.in +++ b/include/openssl/ssl.h.in @@ -426,6 +426,8 @@ typedef int (*SSL_async_callback_fn)(SSL *s, void *arg); /* Enable KTLS TX zerocopy on Linux */ # define SSL_OP_ENABLE_KTLS_TX_ZEROCOPY_SENDFILE SSL_OP_BIT(34) +#define SSL_OP_PREFER_NO_DHE_KEX SSL_OP_BIT(35) + /* * Option "collections." */ diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c index 3142370016..b361719a5a 100644 --- a/ssl/ssl_conf.c +++ b/ssl/ssl_conf.c @@ -391,6 +391,7 @@ static int cmd_Options(SSL_CONF_CTX *cctx, const char *value) SSL_FLAG_TBL_INV("EncryptThenMac", SSL_OP_NO_ENCRYPT_THEN_MAC), SSL_FLAG_TBL("NoRenegotiation", SSL_OP_NO_RENEGOTIATION), SSL_FLAG_TBL("AllowNoDHEKEX", SSL_OP_ALLOW_NO_DHE_KEX), + SSL_FLAG_TBL("PreferNoDHEKEX", SSL_OP_PREFER_NO_DHE_KEX), SSL_FLAG_TBL("PrioritizeChaCha", SSL_OP_PRIORITIZE_CHACHA), SSL_FLAG_TBL("MiddleboxCompat", SSL_OP_ENABLE_MIDDLEBOX_COMPAT), SSL_FLAG_TBL_INV("AntiReplay", SSL_OP_NO_ANTI_REPLAY), @@ -725,6 +726,7 @@ static const ssl_conf_cmd_tbl ssl_conf_cmds[] = { SSL_CONF_CMD_SWITCH("no_resumption_on_reneg", SSL_CONF_FLAG_SERVER), SSL_CONF_CMD_SWITCH("no_legacy_server_connect", SSL_CONF_FLAG_CLIENT), SSL_CONF_CMD_SWITCH("allow_no_dhe_kex", 0), + SSL_CONF_CMD_SWITCH("prefer_no_dhe_kex", 0), SSL_CONF_CMD_SWITCH("prioritize_chacha", SSL_CONF_FLAG_SERVER), SSL_CONF_CMD_SWITCH("strict", 0), SSL_CONF_CMD_SWITCH("no_middlebox", 0), @@ -816,6 +818,8 @@ static const ssl_switch_tbl ssl_cmd_switches[] = { {SSL_OP_LEGACY_SERVER_CONNECT, SSL_TFLAG_INV}, /* allow_no_dhe_kex */ {SSL_OP_ALLOW_NO_DHE_KEX, 0}, + /* prefer_no_dhe_kex */ + {SSL_OP_PREFER_NO_DHE_KEX, 0}, /* chacha reprioritization */ {SSL_OP_PRIORITIZE_CHACHA, 0}, {SSL_CERT_FLAG_TLS_STRICT, SSL_TFLAG_CERT}, /* strict */ diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 0a64ca2246..e77ab2f3e5 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1393,7 +1393,8 @@ 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) + * OR (the kex_mode allows key_share resumes + * AND (kex_mode doesn't allow non-dh resumes OR non-dh is not preferred))) * AND * a shared group exists * THEN @@ -1428,10 +1429,18 @@ 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 - || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) - != 0)) { + && (!s->hit || dhe_psk)) { 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 64ccb3ed6d..2d28ea3b3d 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -1645,12 +1645,25 @@ EXT_RETURN tls_construct_stoc_key_share(SSL_CONNECTION *s, WPACKET *pkt, } return EXT_RETURN_NOT_SENT; } - if (s->hit && (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) == 0) { + if (s->hit) { + /* PSK ('hit') */ + /* - * PSK ('hit') and explicitly not doing DHE (if the client sent the - * DHE option we always take it); don't send key share. + * If we're doing PSK ('hit') but the client doesn't support psk-dhe, + * we don't need to send a key share. */ - return EXT_RETURN_NOT_SENT; + 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; + } } if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share) diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t index c4711e442b..1f45edc7b7 100644 --- a/test/recipes/70-test_tls13kexmodes.t +++ b/test/recipes/70-test_tls13kexmodes.t @@ -191,7 +191,7 @@ $proxy->clientflags("-no_rx_cert_comp -sess_out ".$session); $proxy->serverflags("-no_rx_cert_comp -servername localhost"); $proxy->sessionfile($session); $proxy->start() or plan skip_all => "Unable to start up Proxy for tests"; -plan tests => 11; +plan tests => 13; ok(TLSProxy::Message->success(), "Initial connection"); #Test 2: Attempt a resume with no kex modes extension. Should fail (server @@ -251,10 +251,11 @@ checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE, | checkhandshake::PSK_CLI_EXTENSION, "Resume with unrecognized kex mode"); -#Test 7: Attempt a resume with both non-dhe and dhe kex mode. Should resume with -# a key_share +#Test 7: Attempt a resume with both, non-dhe and dhe kex mode. Should resume with +# a key_share, even though non-dhe is allowed, but not explicitly preferred. $proxy->clear(); -$proxy->clientflags("-no_rx_cert_comp -sess_in ".$session); +$proxy->clientflags("-no_rx_cert_comp -allow_no_dhe_kex -sess_in ".$session); +$proxy->serverflags("-allow_no_dhe_kex"); $testtype = BOTH_KEX_MODES; $proxy->start(); checkhandshake($proxy, checkhandshake::RESUME_HANDSHAKE, @@ -263,9 +264,38 @@ checkhandshake($proxy, checkhandshake::RESUME_HANDSHAKE, | checkhandshake::KEY_SHARE_SRV_EXTENSION | checkhandshake::PSK_CLI_EXTENSION | checkhandshake::PSK_SRV_EXTENSION, - "Resume with non-dhe kex mode"); + "Resume with both kex modes"); -#Test 8: Attempt a resume with both non-dhe and dhe kex mode, but unacceptable +#Test 8: Attempt a resume with both, non-dhe and dhe kex mode, but with server-side +# preference for non-dhe. Should resume without a key_share. +$proxy->clear(); +$proxy->clientflags("-no_rx_cert_comp -allow_no_dhe_kex -sess_in ".$session); +$proxy->serverflags("-allow_no_dhe_kex -prefer_no_dhe_kex"); +$testtype = BOTH_KEX_MODES; +$proxy->start(); +checkhandshake($proxy, checkhandshake::RESUME_HANDSHAKE, + checkhandshake::DEFAULT_EXTENSIONS + | checkhandshake::PSK_KEX_MODES_EXTENSION + | checkhandshake::PSK_CLI_EXTENSION + | checkhandshake::PSK_SRV_EXTENSION, + "Resume with both kex modes, preference for non-dhe"); + +#Test 9: Attempt a resume with both, non-dhe and dhe kex mode, with server-side +# preference for non-dhe, but non-dhe not allowed. Should resume with a key_share. +$proxy->clear(); +$proxy->clientflags("-no_rx_cert_comp -allow_no_dhe_kex -sess_in ".$session); +$proxy->serverflags("-prefer_no_dhe_kex"); +$testtype = BOTH_KEX_MODES; +$proxy->start(); +checkhandshake($proxy, checkhandshake::RESUME_HANDSHAKE, + checkhandshake::DEFAULT_EXTENSIONS + | checkhandshake::PSK_KEX_MODES_EXTENSION + | checkhandshake::KEY_SHARE_SRV_EXTENSION + | checkhandshake::PSK_CLI_EXTENSION + | checkhandshake::PSK_SRV_EXTENSION, + "Resume with both kex modes, preference for but disabled non-dhe"); + +#Test 10: Attempt a resume with both non-dhe and dhe kex mode, but unacceptable # initial key_share. Should resume with a key_share following an HRR $proxy->clear(); $proxy->clientflags("-no_rx_cert_comp -sess_in ".$session); @@ -281,7 +311,7 @@ checkhandshake($proxy, checkhandshake::HRR_RESUME_HANDSHAKE, | checkhandshake::PSK_SRV_EXTENSION, "Resume with both kex modes and HRR"); -#Test 9: Attempt a resume with dhe kex mode only and an unacceptable initial +#Test 11: Attempt a resume with dhe kex mode only and an unacceptable initial # key_share. Should resume with a key_share following an HRR $proxy->clear(); $proxy->clientflags("-no_rx_cert_comp -sess_in ".$session); @@ -297,7 +327,7 @@ checkhandshake($proxy, checkhandshake::HRR_RESUME_HANDSHAKE, | checkhandshake::PSK_SRV_EXTENSION, "Resume with dhe kex mode and HRR"); -#Test 10: Attempt a resume with both non-dhe and dhe kex mode, unacceptable +#Test 12: Attempt a resume with both non-dhe and dhe kex mode, unacceptable # initial key_share and no overlapping groups. Should resume without a # key_share $proxy->clear(); @@ -312,7 +342,7 @@ checkhandshake($proxy, checkhandshake::RESUME_HANDSHAKE, | checkhandshake::PSK_SRV_EXTENSION, "Resume with both kex modes, no overlapping groups"); -#Test 11: Attempt a resume with dhe kex mode only, unacceptable +#Test 13: Attempt a resume with dhe kex mode only, unacceptable # initial key_share and no overlapping groups. Should fail $proxy->clear(); $proxy->clientflags("-no_rx_cert_comp -curves P-384 -sess_in ".$session); @@ -351,7 +381,7 @@ sub modify_kex_modes_filter 0xfe, #unknown 0xff; #unknown } elsif ($testtype == BOTH_KEX_MODES) { - #We deliberately list psk_ke first...should still use psk_dhe_ke + #We deliberately list psk_ke first...should still use psk_dhe_ke, except if the server is configured otherwise. $ext = pack "C3", 0x02, #List length 0x00, #psk_ke