Skip to content

Commit

Permalink
Add option SSL_OP_PREFER_NO_DHE_KEX, allowing the server to prefer …
Browse files Browse the repository at this point in the history
…non-dhe psk key exchange over psk with dhe (config file option `PreferNoDHEKEX`, server option `prefer_no_dhe_kex`).

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#22794)
  • Loading branch information
minichma authored and wbeck10p committed Jan 8, 2024
1 parent 89d4728 commit 770f5e1
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 19 deletions.
6 changes: 5 additions & 1 deletion apps/include/opt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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, \
Expand Down Expand Up @@ -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, '-', \
Expand Down Expand Up @@ -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: \
Expand Down
2 changes: 2 additions & 0 deletions include/openssl/ssl.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -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."
*/
Expand Down
4 changes: 4 additions & 0 deletions ssl/ssl_conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -731,6 +732,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),
Expand Down Expand Up @@ -822,6 +824,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 */
Expand Down
17 changes: 13 additions & 4 deletions ssl/statem/extensions.c
Original file line number Diff line number Diff line change
Expand Up @@ -1396,7 +1396,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
Expand Down Expand Up @@ -1431,10 +1432,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;
Expand Down
21 changes: 17 additions & 4 deletions ssl/statem/extensions_srvr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1662,12 +1662,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)
Expand Down
50 changes: 40 additions & 10 deletions test/recipes/70-test_tls13kexmodes.t
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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 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 8: Attempt a resume with both non-dhe and dhe kex mode, but unacceptable
#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);
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 770f5e1

Please sign in to comment.