supported_groups: fix hybrid group selection

There was a logic error that prevents enabling hybrid groups, when
either EC or FFDH group is specified, not both. This fixes this by
extending the condition to cover the case.

Reported and analyzed by Glenn Strauss in
https://gitlab.com/gnutls/gnutls/-/work_items/1828.

Signed-off-by: Daiki Ueno <ueno@gnu.org>

diff --git a/lib/ext/supported_groups.c b/lib/ext/supported_groups.c
index 4c31d2f8f8eba3a213a6d3f42e5d3c37c78b2632..d1182b2bc8b419465045169b4a67d5ce416bd441 100644 (file)
--- a/lib/ext/supported_groups.c
+++ b/lib/ext/supported_groups.c
@@ -247,9 +247,15 @@ static int _gnutls_supported_groups_recv_params(gnutls_session_t session,
                if (serv_hybrid_idx != -1) {
                        if (session->internals.cand_group == NULL ||
                            (session->internals.priorities->server_precedence &&
-                            serv_hybrid_idx < MIN(serv_ec_idx, serv_dh_idx)) ||
+                            (serv_dh_idx == -1 ||
+                             serv_hybrid_idx < serv_dh_idx) &&
+                            (serv_ec_idx == -1 ||
+                             serv_hybrid_idx < serv_ec_idx)) ||
                            (!session->internals.priorities->server_precedence &&
-                            cli_hybrid_pos < MIN(cli_ec_pos, cli_dh_pos))) {
+                            (cli_dh_pos == -1 ||
+                             cli_hybrid_pos < cli_dh_pos) &&
+                            (cli_ec_pos == -1 ||
+                             cli_hybrid_pos < cli_ec_pos))) {
                                session->internals.cand_group =
                                        session->internals.priorities->groups
                                                .entry[serv_hybrid_idx];

diff --git a/tests/pqc-hybrid-kx.sh b/tests/pqc-hybrid-kx.sh
index 37eefc11ccb0a2e509882b4b8a621acf09d116db..cb0a7b1dd31b286f24f7e5ef1c886ea0d73cd238 100644 (file)
--- a/tests/pqc-hybrid-kx.sh
+++ b/tests/pqc-hybrid-kx.sh
@@ -95,16 +95,23 @@ for group in X25519-KYBER768 SECP256R1-MLKEM768 SECP384R1-MLKEM1024 X25519-MLKEM
        esac
     fi
 
-    eval "${GETPORT}"
-    launch_server --echo --priority "NORMAL:-GROUP-ALL:+GROUP-$group" --x509keyfile="$KEY" --x509certfile="$CERT"
-    PID=$!
-    wait_server ${PID}
-
-    ${VALGRIND} "${CLI}" --attime "${ATTIME_VALID}" -p "${PORT}" localhost --priority "NORMAL:-GROUP-ALL:+GROUP-$group" --x509cafile="$CACERT" --logfile="$testdir/cli.log" </dev/null
-    kill ${PID}
-    wait
-
-    grep -- "- Description: (TLS1.3-X.509)-(HYBRID-$group)-(ECDSA-SECP256R1-SHA256)-(AES-256-GCM)" "$testdir/cli.log" || { echo "unexpected handshake description"; cat "$testdir/cli.log"; exit 1; }
+    # Test hybrid alone, hybrid+EC, hybrid+FFDH, and hybrid+EC+FFDH:
+    # https://gitlab.com/gnutls/gnutls/-/work_items/1828
+    for prio in "NORMAL:-GROUP-ALL:+GROUP-$group" \
+                   "NORMAL:-GROUP-ALL:+GROUP-$group:+GROUP-X25519" \
+                   "NORMAL:-GROUP-ALL:+GROUP-$group:+GROUP-FFDHE2048" \
+                   "NORMAL:-GROUP-ALL:+GROUP-$group:+GROUP-X25519:+GROUP-FFDHE2048"; do
+       eval "${GETPORT}"
+       launch_server --echo --priority "$prio" --x509keyfile="$KEY" --x509certfile="$CERT"
+       PID=$!
+       wait_server ${PID}
+
+       ${VALGRIND} "${CLI}" --attime "${ATTIME_VALID}" -p "${PORT}" localhost --priority "$prio" --x509cafile="$CACERT" --logfile="$testdir/cli.log" </dev/null
+       kill ${PID}
+       wait
+
+       grep -- "- Description: (TLS1.3-X.509)-(HYBRID-$group)-(ECDSA-SECP256R1-SHA256)-(AES-256-GCM)" "$testdir/cli.log" || { echo "unexpected handshake description"; cat "$testdir/cli.log"; exit 1; }
+    done
 done
 
 # KEM based groups cannot be used standalone