Skip to content

Commit

Permalink
wireguard: netlink: send staged packets when setting initial private key
Browse files Browse the repository at this point in the history
commit f58d0a9b4c6a7a5199c3af967e43cc8b654604d4 upstream.

Packets bound for peers can queue up prior to the device private key
being set. For example, if persistent keepalive is set, a packet is
queued up to be sent as soon as the device comes up. However, if the
private key hasn't been set yet, the handshake message never sends, and
no timer is armed to retry, since that would be pointless.

But, if a user later sets a private key, the expectation is that those
queued packets, such as a persistent keepalive, are actually sent. So
adjust the configuration logic to account for this edge case, and add a
test case to make sure this works.

Maxim noticed this with a wg-quick(8) config to the tune of:

    [Interface]
    PostUp = wg set %i private-key somefile

    [Peer]
    PublicKey = ...
    Endpoint = ...
    PersistentKeepalive = 25

Here, the private key gets set after the device comes up using a PostUp
script, triggering the bug.

Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Cc: [email protected]
Reported-by: Maxim Cournoyer <[email protected]>
Tested-by: Maxim Cournoyer <[email protected]>
Link: https://lore.kernel.org/wireguard/[email protected]/
Signed-off-by: Jason A. Donenfeld <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: engstk <[email protected]>
  • Loading branch information
zx2c4 authored and engstk committed Dec 12, 2023
1 parent a218816 commit 4c1ba7c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
14 changes: 9 additions & 5 deletions drivers/net/wireguard/netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
u8 *private_key = nla_data(info->attrs[WGDEVICE_A_PRIVATE_KEY]);
u8 public_key[NOISE_PUBLIC_KEY_LEN];
struct wg_peer *peer, *temp;
bool send_staged_packets;

if (!crypto_memneq(wg->static_identity.static_private,
private_key, NOISE_PUBLIC_KEY_LEN))
Expand All @@ -564,14 +565,17 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
}

down_write(&wg->static_identity.lock);
wg_noise_set_static_identity_private_key(&wg->static_identity,
private_key);
list_for_each_entry_safe(peer, temp, &wg->peer_list,
peer_list) {
send_staged_packets = !wg->static_identity.has_identity && netif_running(wg->dev);
wg_noise_set_static_identity_private_key(&wg->static_identity, private_key);
send_staged_packets = send_staged_packets && wg->static_identity.has_identity;

wg_cookie_checker_precompute_device_keys(&wg->cookie_checker);
list_for_each_entry_safe(peer, temp, &wg->peer_list, peer_list) {
wg_noise_precompute_static_static(peer);
wg_noise_expire_current_peer_keypairs(peer);
if (send_staged_packets)
wg_packet_send_staged_packets(peer);
}
wg_cookie_checker_precompute_device_keys(&wg->cookie_checker);
up_write(&wg->static_identity.lock);
}
skip_set_private_key:
Expand Down
30 changes: 26 additions & 4 deletions tools/testing/selftests/wireguard/netns.sh
Original file line number Diff line number Diff line change
Expand Up @@ -502,10 +502,32 @@ n2 bash -c 'printf 0 > /proc/sys/net/ipv4/conf/all/rp_filter'
n1 ping -W 1 -c 1 192.168.241.2
[[ $(n2 wg show wg0 endpoints) == "$pub1 10.0.0.3:1" ]]

ip1 link del veth1
ip1 link del veth3
ip1 link del wg0
ip2 link del wg0
ip1 link del dev veth3
ip1 link del dev wg0
ip2 link del dev wg0

# Make sure persistent keep alives are sent when an adapter comes up
ip1 link add dev wg0 type wireguard
n1 wg set wg0 private-key <(echo "$key1") peer "$pub2" endpoint 10.0.0.1:1 persistent-keepalive 1
read _ _ tx_bytes < <(n1 wg show wg0 transfer)
[[ $tx_bytes -eq 0 ]]
ip1 link set dev wg0 up
read _ _ tx_bytes < <(n1 wg show wg0 transfer)
[[ $tx_bytes -gt 0 ]]
ip1 link del dev wg0
# This should also happen even if the private key is set later
ip1 link add dev wg0 type wireguard
n1 wg set wg0 peer "$pub2" endpoint 10.0.0.1:1 persistent-keepalive 1
read _ _ tx_bytes < <(n1 wg show wg0 transfer)
[[ $tx_bytes -eq 0 ]]
ip1 link set dev wg0 up
read _ _ tx_bytes < <(n1 wg show wg0 transfer)
[[ $tx_bytes -eq 0 ]]
n1 wg set wg0 private-key <(echo "$key1")
read _ _ tx_bytes < <(n1 wg show wg0 transfer)
[[ $tx_bytes -gt 0 ]]
ip1 link del dev veth1
ip1 link del dev wg0

# We test that Netlink/IPC is working properly by doing things that usually cause split responses
ip0 link add dev wg0 type wireguard
Expand Down

0 comments on commit 4c1ba7c

Please sign in to comment.