From 869e6b5b907c68e4839189fff88328a992b866cc Mon Sep 17 00:00:00 2001 From: Cole Christensen Date: Fri, 5 Jun 2026 23:38:56 -0500 Subject: [PATCH 1/3] ssh: fix mlkem768x25519 hybrid shared secret encoding The classical X25519 part of the mlkem768x25519-sha256 hybrid key exchange shared secret was reconstructed by taking the last 32 bytes of the mpint encoding of the ECDH secret. mpint is a minimal-length big-endian representation, so when the shared secret's most significant byte is 0x00 and the following byte is < 0x80 the payload is only 31 bytes and "the last 32 bytes" pulls in a byte of the 4-byte length prefix (0x1f) in place of the genuine leading 0x00. hybrid_common/4 then hashes a different octet string than the peer (e.g. OpenSSH), producing a different exchange hash H. The server signs an H the client never computed, and the client aborts the connection with "incorrect signature". This affects roughly 1 in 512 handshakes (P[msb=0x00] * P[next<0x80] = 1/256 * 1/2). Encode the ECDH shared secret as a fixed-width big-endian octet string so a genuine leading 0x00 byte is preserved, matching the peer. Add a regression test (ssh_algorithms_SUITE:mlkem768x25519_hybrid_secret_encoding) that searches for an X25519 key pair whose ECDH secret triggers the condition and asserts hybrid_common/4 hashes it as a fixed-width 32-byte octet string. The test fails against the old code and passes with the fix. Introduced in 95d0848c60 (ssh: MLKEM kex ssh); present since OTP-28.4. --- lib/ssh/src/ssh_transport.erl | 8 ++--- lib/ssh/test/ssh_algorithms_SUITE.erl | 46 +++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl index 0b72ec260ad5..f5e3cdae954c 100644 --- a/lib/ssh/src/ssh_transport.erl +++ b/lib/ssh/src/ssh_transport.erl @@ -67,7 +67,7 @@ -define(MIN_DH_KEY_SIZE, 400). %%% For test suites --export([pack/3, adjust_algs_for_peer_version/2]). +-export([pack/3, adjust_algs_for_peer_version/2, hybrid_common/4]). %%%---------------------------------------------------------------------------- %%% @@ -2395,10 +2395,8 @@ compute_key(Algorithm, PeerPublic, MyPrivate, Args) -> hybrid_common(K_pq_secret, Curve, PeerPublic, MyPrivate) -> K_cl_secret = compute_key(ecdh, PeerPublic, MyPrivate, Curve), - K_cl_secret_mpint = <>, - K_cl_secret_mpint_trim = - binary:part(K_cl_secret_mpint, byte_size(K_cl_secret_mpint), -?X25519_PUBLICKEY_SIZE), - crypto:hash(sha(Curve), <>). + K_cl_secret_fixed = <>, + crypto:hash(sha(Curve), <>). dh_bits(#alg{encrypt = Encrypt, send_mac = SendMac}) -> diff --git a/lib/ssh/test/ssh_algorithms_SUITE.erl b/lib/ssh/test/ssh_algorithms_SUITE.erl index 2489d939caf6..6d99d8966ca5 100644 --- a/lib/ssh/test/ssh_algorithms_SUITE.erl +++ b/lib/ssh/test/ssh_algorithms_SUITE.erl @@ -42,6 +42,7 @@ -export([ interpolate/1, + mlkem768x25519_hybrid_secret_encoding/1, simple_connect/1, simple_exec/1, simple_exec_groups/0, @@ -61,9 +62,9 @@ suite() -> [{ct_hooks,[ts_install_cth]}, {timetrap,{seconds,120}}]. -all() -> +all() -> %% [{group,kex},{group,cipher}... etc - [{group,C} || C <- tags()]. + [mlkem768x25519_hybrid_secret_encoding | [{group,C} || C <- tags()]]. groups() -> @@ -311,6 +312,47 @@ end_per_testcase(_TC, Config) -> %%-------------------------------------------------------------------- %% Test Cases -------------------------------------------------------- +%%-------------------------------------------------------------------- +%% Regression test for the mlkem768x25519-sha256 hybrid key exchange: the +%% classical (X25519) shared secret must be hashed as a fixed-width 32-byte +%% octet string. Encoding it as a trimmed mpint dropped a genuine leading 0x00 +%% byte ~1/512 of the time (when the secret's most significant byte is 0x00 and +%% the next byte is < 0x80), so the two peers derived different exchange hashes +%% and the handshake failed with "incorrect signature". +mlkem768x25519_hybrid_secret_encoding(_Config) -> + %% Find an X25519 key pair whose ECDH shared secret triggers the bug. + {PeerPublic, MyPrivate, Raw} = find_case_c_x25519(1000000), + 32 = byte_size(Raw), + K_pq = crypto:strong_rand_bytes(32), + %% A spec-conformant peer (e.g. OpenSSH) hashes K_pq concatenated with the + %% X25519 secret as a fixed-width 32-byte string, preserving the leading 0x00. + Expected = crypto:hash(sha256, <>), + Got = ssh_transport:hybrid_common(K_pq, x25519, PeerPublic, MyPrivate), + case Got of + Expected -> + ok; + _ -> + ct:fail({hybrid_secret_encoding_mismatch, + {x25519_secret, Raw}, + {expected, Expected}, + {got, Got}}) + end. + +%% Search for an X25519 key pair whose ECDH shared secret has most significant +%% byte 0x00 and next byte < 0x80 -- the value whose minimal mpint encoding is +%% only 31 bytes, which the old code mis-trimmed. Expected after ~512 tries. +find_case_c_x25519(0) -> + ct:fail("no case-C X25519 shared secret found"); +find_case_c_x25519(N) -> + {_MyPublic, MyPrivate} = crypto:generate_key(ecdh, x25519), + {PeerPublic, _PeerPrivate} = crypto:generate_key(ecdh, x25519), + case crypto:compute_key(ecdh, PeerPublic, MyPrivate, x25519) of + <<0, B1, _/binary>> = Raw when B1 < 16#80 -> + {PeerPublic, MyPrivate, Raw}; + _ -> + find_case_c_x25519(N - 1) + end. + %%-------------------------------------------------------------------- %% A simple sftp transfer simple_sftp(Config) -> From c2f2e8b7b251b1765809efe54e3994b883a26788 Mon Sep 17 00:00:00 2001 From: Jakub Witczak Date: Fri, 12 Jun 2026 16:54:27 +0200 Subject: [PATCH 2/3] ssh: Skip mlkem768x25519 test on unsupported platforms Add init_per_testcase guard for mlkem768x25519_hybrid_secret_encoding that checks crypto:supports(curves) for x25519 and crypto:supports(kems) for mlkem768 before running the test. Without this, the test crashes with {badmatch,notsup} on platforms where the crypto backend lacks X25519 or ML-KEM768 support. --- lib/ssh/test/ssh_algorithms_SUITE.erl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/ssh/test/ssh_algorithms_SUITE.erl b/lib/ssh/test/ssh_algorithms_SUITE.erl index 6d99d8966ca5..fed70c3c8312 100644 --- a/lib/ssh/test/ssh_algorithms_SUITE.erl +++ b/lib/ssh/test/ssh_algorithms_SUITE.erl @@ -291,6 +291,14 @@ init_per_testcase(sshc_simple_exec_os_cmd, _, Config) -> start_pubkey_daemon([proplists:get_value(pref_algs,Config)], [{extra_daemon,true}|Config]); +init_per_testcase(mlkem768x25519_hybrid_secret_encoding, _, Config) -> + case lists:member(x25519, crypto:supports(curves)) + andalso lists:member(mlkem768, crypto:supports(kems)) + of + false -> {skip, "X25519 or ML-KEM768 not supported"}; + true -> Config + end; + init_per_testcase(_, _, Config) -> Config. From 20ce3498431d92e7e258c4a2e63b9e55afe9e4c1 Mon Sep 17 00:00:00 2001 From: Jakub Witczak Date: Mon, 15 Jun 2026 15:35:53 +0200 Subject: [PATCH 3/3] ssh: use static key in test suite --- lib/ssh/test/ssh_algorithms_SUITE.erl | 46 ++++++++++----------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/lib/ssh/test/ssh_algorithms_SUITE.erl b/lib/ssh/test/ssh_algorithms_SUITE.erl index fed70c3c8312..6098fa3cc832 100644 --- a/lib/ssh/test/ssh_algorithms_SUITE.erl +++ b/lib/ssh/test/ssh_algorithms_SUITE.erl @@ -327,39 +327,27 @@ end_per_testcase(_TC, Config) -> %% byte ~1/512 of the time (when the secret's most significant byte is 0x00 and %% the next byte is < 0x80), so the two peers derived different exchange hashes %% and the handshake failed with "incorrect signature". +%% +%% The keys below produce a shared secret starting with <<0x00, 0x42, ...>> +%% which triggers the edge case (0x42 < 0x80). mlkem768x25519_hybrid_secret_encoding(_Config) -> - %% Find an X25519 key pair whose ECDH shared secret triggers the bug. - {PeerPublic, MyPrivate, Raw} = find_case_c_x25519(1000000), - 32 = byte_size(Raw), - K_pq = crypto:strong_rand_bytes(32), + PeerPublic = <<16#94,16#5E,16#6F,16#5A,16#CA,16#B1,16#AD,16#A6, + 16#31,16#CF,16#12,16#F5,16#47,16#A4,16#25,16#6B, + 16#6A,16#E5,16#3A,16#A2,16#48,16#6A,16#0D,16#08, + 16#C6,16#D6,16#73,16#2C,16#B3,16#E2,16#0E,16#5B>>, + MyPrivate = <<16#58,16#EF,16#AB,16#DA,16#4C,16#C5,16#6B,16#8E, + 16#B2,16#43,16#8A,16#86,16#92,16#2C,16#5D,16#73, + 16#83,16#98,16#B4,16#38,16#0E,16#A3,16#91,16#37, + 16#F9,16#38,16#5B,16#E5,16#BA,16#B5,16#94,16#6D>>, + %% Verify the shared secret triggers the edge case + <<0, B1, _/binary>> = crypto:compute_key(ecdh, PeerPublic, MyPrivate, x25519), + true = B1 < 16#80, %% A spec-conformant peer (e.g. OpenSSH) hashes K_pq concatenated with the %% X25519 secret as a fixed-width 32-byte string, preserving the leading 0x00. + K_pq = crypto:strong_rand_bytes(32), + Raw = crypto:compute_key(ecdh, PeerPublic, MyPrivate, x25519), Expected = crypto:hash(sha256, <>), - Got = ssh_transport:hybrid_common(K_pq, x25519, PeerPublic, MyPrivate), - case Got of - Expected -> - ok; - _ -> - ct:fail({hybrid_secret_encoding_mismatch, - {x25519_secret, Raw}, - {expected, Expected}, - {got, Got}}) - end. - -%% Search for an X25519 key pair whose ECDH shared secret has most significant -%% byte 0x00 and next byte < 0x80 -- the value whose minimal mpint encoding is -%% only 31 bytes, which the old code mis-trimmed. Expected after ~512 tries. -find_case_c_x25519(0) -> - ct:fail("no case-C X25519 shared secret found"); -find_case_c_x25519(N) -> - {_MyPublic, MyPrivate} = crypto:generate_key(ecdh, x25519), - {PeerPublic, _PeerPrivate} = crypto:generate_key(ecdh, x25519), - case crypto:compute_key(ecdh, PeerPublic, MyPrivate, x25519) of - <<0, B1, _/binary>> = Raw when B1 < 16#80 -> - {PeerPublic, MyPrivate, Raw}; - _ -> - find_case_c_x25519(N - 1) - end. + Expected = ssh_transport:hybrid_common(K_pq, x25519, PeerPublic, MyPrivate). %%-------------------------------------------------------------------- %% A simple sftp transfer