# HG changeset patch # User Taylor R Campbell # Date 1782154294 0 # Mon Jun 22 18:51:34 2026 +0000 # Branch trunk # Node ID f3ac31d605e78161ff48c8da8e617b4898645011 # Parent babdf5c7e2cbe4d552248d2f38ead15e92b0b64f # EXP-Topic riastradh-pr60106-wgdhcheck wg(4): Add test case for bad peer public keys. wg(4) should not crash on an assertion if they are used -- it should just gracefully accept them, with degraded security, since a peer that maliciously provides an invalid public key is no worse than a peer that voluntarily exposes all its plaintext anyway. PR kern/60106: wg(4) should properly handle invalid or insecure ephemeral Curve25119 public keys diff -r babdf5c7e2cb -r f3ac31d605e7 tests/net/if_wg/t_basic.sh --- a/tests/net/if_wg/t_basic.sh Tue Jun 16 23:37:48 2026 +0000 +++ b/tests/net/if_wg/t_basic.sh Mon Jun 22 18:51:34 2026 +0000 @@ -63,6 +63,23 @@ check_badudp() fi } +check_badpeerkey() +{ + local proto=$1 + local ip=$2 + local ping= size= + + atf_expect_fail "PR kern/6016: wg(4) should properly handle invalid or insecure ephemeral Curve25119 public keys" + + if [ $proto = inet ]; then + ping="atf_check -s not-exit:0 -o ignore rump.ping -n -c 1 -w 1" + else + ping="atf_check -s not-exit:0 -o ignore rump.ping6 -n -c 1 -X 1" + fi + + $ping $ip +} + test_common() { local type=$1 @@ -104,6 +121,12 @@ test_common() # It sets key_priv_local key_pub_local key_priv_peer key_pub_peer generate_keys + case $type in + badpeerkey) + key_pub_peer=$4 + ;; + esac + export RUMP_SERVER=$SOCK_LOCAL setup_common shmif0 $outer_proto $ip_local $outer_prefix setup_wg_common wg0 $inner_proto $ip_wg_local $inner_prefix $port "$key_priv_local" @@ -125,6 +148,9 @@ test_common() elif [ $type = badudp ]; then export RUMP_SERVER=$SOCK_LOCAL check_badudp $outer_proto $ip_peer + elif [ $type = badpeerkey ]; then + export RUMP_SERVER=$SOCK_LOCAL + check_badpeerkey $outer_proto $ip_wg_peer fi destroy_wg_interfaces @@ -330,6 +356,38 @@ add_badudp_test() atf_add_test_case ${name} } +add_badpeerkey_test() +{ + local inner=$1 + local outer=$2 + local testno=$3 + local pubkey=$4 + local ipv4=inet + local ipv6=inet6 + + name="wg_badpeerkey_${inner}_over_${outer}_test_${testno}" + fulldesc="Test wg(4) with ${inner} over ${outer} with bad peer key" + + eval inner=\$$inner + eval outer=\$$outer + + atf_test_case ${name} cleanup + eval " + ${name}_head() { + atf_set descr \"${fulldesc}\" + atf_set require.progs rump_server wgconfig wg-keygen nc + } + ${name}_body() { + test_common badpeerkey $outer $inner $pubkey + rump_server_destroy_ifaces + } + ${name}_cleanup() { + \$DEBUG && dump + cleanup + }" + atf_add_test_case ${name} +} + atf_test_case wg_multiple_interfaces cleanup wg_multiple_interfaces_head() { @@ -506,6 +564,7 @@ wg_multiple_peers_cleanup() atf_init_test_cases() { + local testno badkey add_badudp_test ipv4 ipv4 add_badudp_test ipv4 ipv6 @@ -527,4 +586,25 @@ atf_init_test_cases() atf_add_test_case wg_create_destroy_peers_ipv6 atf_add_test_case wg_multiple_interfaces atf_add_test_case wg_multiple_peers + + # These are all possible little-endian x coordinates of points + # of order <=8 on Curve25519. See + # + # for details. + while read testno badkey; do + add_badpeerkey_test ipv4 ipv4 "$testno" "$badkey" + done < # Date 1782158445 0 # Mon Jun 22 20:00:45 2026 +0000 # Branch trunk # Node ID 0a29eb3534bd190188fbde0fc561bbf4689cd1b7 # Parent f3ac31d605e78161ff48c8da8e617b4898645011 # EXP-Topic riastradh-pr60106-wgdhcheck wg(4): Add test case for bad ephemeral handshake public keys. wg(4) should not crash on an assertion if they appear on the network; it should just gracefully drop them as forgeries, if a MITM attempted to send them without knowledge of a peer's public key, or accept them, if a peer legitimately sent them, since that peer could just as well simply forward the plaintext of the session on to the NSA. PR kern/60106: wg(4) should properly handle invalid or insecure ephemeral Curve25119 public keys diff -r f3ac31d605e7 -r 0a29eb3534bd tests/net/if_wg/common.sh --- a/tests/net/if_wg/common.sh Mon Jun 22 18:51:34 2026 +0000 +++ b/tests/net/if_wg/common.sh Mon Jun 22 20:00:45 2026 +0000 @@ -188,6 +188,12 @@ delete_peer() $HIJACKING wgconfig $ifname } +# generate_keys +# +# Generate two key pairs, $key_priv_local/$key_pub_local and +# $key_priv_peer/$key_pub_peer, and export them in the +# environment. +# generate_keys() { @@ -198,3 +204,32 @@ generate_keys() export key_priv_local key_pub_local key_priv_peer key_pub_peer } + +# generate_fixed_test_keys +# +# Set two key pairs, $key_priv_local/$key_pub_local and +# $key_priv_peer/$key_pub_peer, to be fixed keys for testing +# purposes. There is nothing special about these keys. We will +# use them to generate potentially troublesome protocol messages, +# which are inconvenient to construct dynamically -- so I +# generated a couple key pairs at random with wg-keygen(8) and +# used a modified if_wg.c to construct the troublesome protocol +# messages with them. +# +generate_fixed_test_keys() +{ + + # b865d7c2687b673e905b787a6b16441ac9959abc0ec9f07d552b429e97b08074 + key_priv_local=uGXXwmh7Zz6QW3h6axZEGsmVmrwOyfB9VStCnpewgHQ= + + # 7dd90f9855877e923045abd12d6bb2ce604e3ab2b14fa28a2e0a956f58392e16 + key_pub_local=fdkPmFWHfpIwRavRLWuyzmBOOrKxT6KKLgqVb1g5LhY= + + # 681d8b8ebbd8e009e299c051da62c255e787c776d47d7edd09628e9121d4ee51 + key_priv_peer=aB2LjrvY4AnimcBR2mLCVeeHx3bUfX7dCWKOkSHU7lE= + + # d2aa91ed9f4ae52001b8435a49decc2016fecba05178c25413d2f6a81cf6ad69 + key_pub_peer=0qqR7Z9K5SABuENaSd7MIBb+y6BReMJUE9L2qBz2rWk= + + export key_priv_local key_pub_local key_priv_peer key_pub_peer +} diff -r f3ac31d605e7 -r 0a29eb3534bd tests/net/if_wg/t_basic.sh --- a/tests/net/if_wg/t_basic.sh Mon Jun 22 18:51:34 2026 +0000 +++ b/tests/net/if_wg/t_basic.sh Mon Jun 22 20:00:45 2026 +0000 @@ -80,6 +80,131 @@ check_badpeerkey() $ping $ip } +check_badhandshakekey() +{ + local proto=$1 + local wg_ip=$2 + local ip=$3 + local pubkey=$4 + local port=51820 # XXX parametrize more clearly + + atf_expect_fail "PR kern/6016: wg(4) should properly handle invalid or insecure ephemeral Curve25119 public keys" + + # For each invalid public key (representing the 32-byte + # little-endian encoding of an x coordinate of a point of order + # <=8 on Curve25519), we have a preassembled init message that + # uses that public key as its ephemeral public key and + # otherwise has all the hashes computed correctly -- generated + # by tweaking if_wg.c to hard-code each possible invalid key, + # and printing the resulting init message. + # + # (Fortunately, the set of points of order <=8 on Curve25519 + # does not change very often, so we won't have to generate this + # list until we move on to some post-quantum key agreement that + # has its own weird inputs!) + # + case $pubkey in + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=") + openssl base64 -d <initmsg +AQAAAFUFlIIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK/jDw2scvj0v9G2UVt/+EyV +y/n+15Jq6+h2ttmPvmDSlNE9Ye6POFzitBVW30Q6jVJXmU8LmQS7c1heUmIFpA57sILsldvY9Zck +u+fbNoiZqTOtLfg/jEscBkKIAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "AQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=") + openssl base64 -d <initmsg +AQAAAFUFlIIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAOSNjvF3Hl3dJtvCp1H6ZMS +mG4Aw86kG4/yIYjTvUsLjirhal8l7Ed/Q/Ne2naAQFz7YGufHsxUA/2JIE0mny+wq888qObeK7gz +S9e2dYZmvn15f06+aso5vV1yAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "4Ot6fDtBuK4WVuP68Z/EatoJjeucMrH9hmIFFl9JuAA=") + openssl base64 -d <initmsg +AQAAAFUFlILg63p8O0G4rhZW4/rxn8Rq2gmN65wysf2GYgUWX0m4AONODwHdcJwRxKmw2oW4prDK +AKpg9WPA3PBgs+SYYi58hyueAzHa4Nl8wQ6qIV87jBz2nqwiqwRRzvSYCZDvB0W1obVEeJTjS71C +RT/VRq0J9xYL8UXM/69C2Pt8AAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "X5yVvKNQjCSx0LFVnIPvWwREXMRYHI6G2CJO3dCfEVc=") + openssl base64 -d <initmsg +AQAAAFUFlIJfnJW8o1CMJLHQsVWcg+9bBERcxFgcjobYIk7d0J8RV+Lelfe8koazxp5GiU6YDujl +Teck8Vcf+2Ja8YFDr5AJDmC+LSZwdO0E1U6pJO4//zuzKYZ8l/tqP5uEn+6fpIVTRUFdFh0DOMHk +78XiRuLMeo9uFYGXM7GFUQsRAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "7P///////////////////////////////////////38=") + openssl base64 -d <initmsg +AQAAAFUFlILs////////////////////////////////////////f2V4uZtAkzNU3dDcnwOD+8Qu +KIe6aFkMRYFl8KdCYAtmuwd7WzzSrM/l4YeS7VbkJAV+F6/zFykwijFporJebtDfirc5JTA055Bd +CQ4HFV7de3UCXQI4jLyzz0TwAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "7f///////////////////////////////////////38=") + openssl base64 -d <initmsg +AQAAAFUFlILt////////////////////////////////////////f1a5i10Q+gmID8QIAcxwe5xm +OcuWZosr2bjCCP/0kNeE++Gw6AynD+OAf8uW/rWSUXE59JmiDDrP6jc4mBk2ax3vy9x5+YXAuGK5 +iBXvgYPqEdlc8Fxa4jtDEXDRAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "7v///////////////////////////////////////38=") + openssl base64 -d <initmsg +AQAAAFUFlILu////////////////////////////////////////fziZHxCJC02xxcsZh2Fm2XaG +RgfOO/oDgEgMhfZF81n/kZgmLP2rUtIWYOpNvfvOlISlJp/8Mc0OBxnO4XVpl6Ux047I/WSDZaT/ +2DyOI7NnCF4UwFeH8DoL+3IPAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "zet6fDtBuK4WVuP68Z/EatoJjeucMrH9hmIFFl9JuIA=") + openssl base64 -d <initmsg +AQAAAFUFlILN63p8O0G4rhZW4/rxn8Rq2gmN65wysf2GYgUWX0m4gDabPNBQhXIf883p655csReY +y9xTkkkDOW8fwEfFtVeALp0D0yu3C1HyEUifwnd+o2Aa8YYaeT8vkcL1wTiS3Ap1iQO1J/OTmWek +SOOopCkEWxOxF2+D9PNF27nGAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "TJyVvKNQjCSx0LFVnIPvWwREXMRYHI6G2CJO3dCfEdc=") + openssl base64 -d <initmsg +AQAAAFUFlIJMnJW8o1CMJLHQsVWcg+9bBERcxFgcjobYIk7d0J8R1w9SPoGi2YVq3znle0dn0/5R +W4opQdY+1jkSbHuTwhYBiqgaBIbeGrNz7d88dF2lt/vkMlYfH6TGBEIHw+iIwjT/eQcjTy7sqV5h +edWqvJ00Bi97u95JKm27ogsUAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "2f////////////////////////////////////////8=") + openssl base64 -d <initmsg +AQAAAFUFlILZ/////////////////////////////////////////wAJbt3qhxKn6Cu3UyTrkt0I +rSEtWlwOf3J7aGUVpIDdK+L68oMmM+GoCe40JJdsmdFAnKq0kcicOlPiuB+Dg+OABRsLQy/WNsEh +9UNuoKUU8GzWVjslXfJKqpSjAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "2v////////////////////////////////////////8=") + openssl base64 -d <initmsg +AQAAAFUFlILa/////////////////////////////////////////9VCEJnMrtIplbej+1z/eoLI +3/YfsJo81t1kaJk/iTmhHvMCUxW0jFOD3DLTF6bGe9ZxqNczcRRPeAIZJnT0107QhlAjtS/EtzO8 +6EnJrm75o9KcN6J5dIox963MAAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + "2/////////////////////////////////////////8=") + openssl base64 -d <initmsg +AQAAAFUFlILb//////////////////////////////////////////H9a+a+mgjUU5agdYuUlcIC +MM72yinuThd0eNVAJ5HzvxFZiVvycJuWGB8GsY5uAWWHmZydf19tH8OKWr4ZAn5cE0uJYJibUUlf +zzfeXl7dhyg51yP62ZIFMgw3AAAAAAAAAAAAAAAAAAAAAA== +EOF + ;; + *) atf_fail "They're the wrong trousers, and they've gone wrong!" + ;; + esac + + if [ $proto = inet ]; then + atf_check -o ignore -e ignore \ + $HIJACKING nc -4u -w0 $ip $port # Date 1782136276 0 # Mon Jun 22 13:51:16 2026 +0000 # Branch trunk # Node ID bfdd14260e30d4ea148e199a9ff0a61679afaad6 # Parent 0a29eb3534bd190188fbde0fc561bbf4689cd1b7 # EXP-Topic riastradh-pr60106-wgdhcheck wg(4): Drop KASSERT on result of crypto_scalarmult. The result of this check is not relevant to security of the protocol, either for static peer identity keys or for ephemeral handshake keys. See comments for details. We can't simply write (void)crypto_scalarmult(...); because the function was tagged with warn_unused_result. And apparently libsodium may leave the output uninitialized if the check fails. So just yield zero instead of stack garbage / UB -- stack garbage is probably actually fine since it is immediately hashed into something that won't match anything so downstream logic will just drop it, but UB might invite nefarious compilers to cause trouble. PR security/60106: wg(4) should properly handle invalid or insecure ephemeral Curve25119 public keys diff -r 0a29eb3534bd -r bfdd14260e30 sys/net/if_wg.c --- a/sys/net/if_wg.c Mon Jun 22 20:00:45 2026 +0000 +++ b/sys/net/if_wg.c Mon Jun 22 13:51:16 2026 +0000 @@ -1094,8 +1094,71 @@ wg_algo_dh(uint8_t out[static WG_DH_OUTP CTASSERT(WG_STATIC_KEY_LEN == crypto_scalarmult_curve25519_BYTES); - int ret __diagused = crypto_scalarmult(out, privkey, pubkey); - KASSERT(ret == 0); + /* + * libsodium crypto_scalarmult may fail early (return -1) if + * pubkey is a point of order <=8 -- and thus if the output + * _would_ be all-zero -- in order to mitigate _potential_ + * timing side channel attacks prompted by: + * + * Daniel Genkin, Luke Valenta, and Yuval Yarom, `May the + * Fourth Be With You: A Microarchitectural Side Channel + * Attack on Several Real-World Applications of + * Curve25519', ACM CCS 2017 + * https://dl.acm.org/doi/10.1145/3133956.3134029 + * + * (The paper is actually about exploiting variable-time logic + * in erstwhile versions of libgcrypt; the risk libsodium + * mitigates is only the potential of compiler optimizations + * that convert branchless arithmetic circuits written in C + * into variable-time machine code. Of course, this early + * abort itself introduces timing variation! But that timing + * variation only reveals the distinction between a point of + * order <=8 (not possible for legitimate keys) and a point of + * order >8.) + * + * The X25519 function was explicitly designed from the + * beginning to be safe without point validation in DH key + * agreements: + * + * https://web.archive.org/web/20260618014320/https://cr.yp.to/ecdh/curve25519-20060209.pdf + * https://web.archive.org/web/20260613191208/https://cr.yp.to/ecdh.html#validate + * + * Consistent with the `MAY' in the RFC 7748 procedure for + * X25519 DH key agreements, we deliberately ignore the result + * of this check -- except to memset the output to zero -- + * because: + * + * - If a malicious peer provides a static public key of low + * order as its identity, that malicious peer could also just + * maliciously forward traffic to the NSA anyway. + * + * - If a MITM on the network provides an ephemeral public key + * in a key agreement, we will reject it as a forgery anyway + * using the static public key of the peer's identity. + * + * So there is no value in using the result of the check -- and + * if we did use it, it would introduce unnecessary code + * complexity downstream, raising the cost of auditing. + * + * Note that not all of libsodium's implementations of + * crypto_scalarmult_curve25519 even do the check! At time of + * writing (both in the version of libsodium in NetBSD, 1.0.16, + * and the latest libsodium, 1.0.22), the ref10 implementation + * may return -1, while the sandy2x implementation never does. + * The libsodium documentation doesn't even mention what the + * return value means, even though the function is annotated + * with __attribute__((warn_unused_result)): + * + * https://web.archive.org/web/20260521174050/https://libsodium.gitbook.io/doc/advanced/scalar_multiplication + * + * Further reading on the check, its value, and its + * limitations: + * + * https://web.archive.org/web/20260404134530/https://moderncrypto.org/mail-archive/curves/2017/000896.html + * https://web.archive.org/web/20210506235924/https://crypto.stackexchange.com/questions/55632/libsodium-x25519-and-ed25519-small-order-check/55643#55643 + */ + if (crypto_scalarmult(out, privkey, pubkey)) + memset(out, 0, WG_DH_OUTPUT_LEN); } static void diff -r 0a29eb3534bd -r bfdd14260e30 tests/net/if_wg/t_basic.sh --- a/tests/net/if_wg/t_basic.sh Mon Jun 22 20:00:45 2026 +0000 +++ b/tests/net/if_wg/t_basic.sh Mon Jun 22 13:51:16 2026 +0000 @@ -69,8 +69,6 @@ check_badpeerkey() local ip=$2 local ping= size= - atf_expect_fail "PR kern/6016: wg(4) should properly handle invalid or insecure ephemeral Curve25119 public keys" - if [ $proto = inet ]; then ping="atf_check -s not-exit:0 -o ignore rump.ping -n -c 1 -w 1" else @@ -88,8 +86,6 @@ check_badhandshakekey() local pubkey=$4 local port=51820 # XXX parametrize more clearly - atf_expect_fail "PR kern/6016: wg(4) should properly handle invalid or insecure ephemeral Curve25119 public keys" - # For each invalid public key (representing the 32-byte # little-endian encoding of an x coordinate of a point of order # <=8 on Curve25519), we have a preassembled init message that