kern/58508: experimental wg(4) queues LIFO, not FIFO, pending first handshake

[campbell+netbsd%mumble.net@localhost]

>Number:         58508
>Category:       kern
>Synopsis:       experimental wg(4) queues LIFO, not FIFO, pending first handshake
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jul 29 01:10:00 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10
>Organization:
The NetWG Queuedation
>Environment:
>Description:
When there is no established session with a wg(4) peer, and something attempts to send a packet to the peer, wg(4) will initiate a session but the packet can't go out until a handshake completes.

During that time, wg(4) queued packets so that the moment the handshake completes, it can send data promptly to the peer (the responder is forbidden to send data until it has received the initiator's first data packet).

Currently, the queue has a single element, because normally the handshake takes a single round-trip to complete (initiator sends init, responder sends response, initiator can send data after that).  But it is effectively a LIFO queue, because wg(4) drops any additional outgoing packets.

This has the effect that if the handshake is delayed (e.g., because it has to wait for arp resolution), ping returns answers that look like:

64 bytes from 192.168.1.1: icmp_seq=0 ttl=64 time=4567.890 ms
64 bytes from 192.168.1.1: icmp_seq=5 ttl=64 time=7.706690 ms
64 bytes from 192.168.1.1: icmp_seq=6 ttl=64 time=1.490476 ms
...

Queueing older packets and dropping newer packets is a pretty weird thing to do.  It would probably be better to drop older packets and queue newer packets.  (It's not entirely clear to me it's even worthwhile to queue anything at all here -- ozaki-r's original draft would just send an empty keepalive packet once the session is established, and that worked too.  But it does potentially reduce some round-trips for higher-level applications.)

>How-To-Repeat:
ping
>Fix:
Change

        if (atomic_cas_ptr(&wgp->wgp_pending, NULL, m) != NULL)
                m_freem(m);

to

        if ((m = atomic_swap_ptr(&wgp->wgp_pending, m)) != NULL)
                m_freem(m);

more or less, in wg_output.