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 <todd.short@me.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22844)

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index e77ab2f3e50efd934e1557a8bb86c95604f1a59b..0a64ca2246987e24f5d8d0d848551417be02f72d 100644 (file)
--- 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 2d28ea3b3d70974af977da293cbc0377fb8e7b64..c4287bd853c1e776d73f029ded79f7572382ac4c 100644 (file)
--- 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)