Windows: fix instability with Wintun as tunnel device driver

In https://gitlab.com/openconnect/openconnect/-/issues/338, multiple users
reported that connections using Wintun as the tunnel device driver become
non-functional after 20-30 minutes of operation, without
any message from OpenConnect at all.

We analyzed this issue as follows:

1. There was an off-by-one error in the check of outgoing packet size
   against the tunnel device's MTU (`tun_len < pkt->len`). Outgoing packets
   of exactly the MTU size would be considered errors and silently discarded
   by OpenConnect.
2. However, OpenConnect failed to instruct the driver to release these
   discarded packets. They would accumulate in the Wintun driver buffer and
   probably cause an out-of-memory condition, eventually freezing the
   driver.

We fixed the issued as follows:

1. Fix the off-by-one error, changing to `tun_len <= pkt->len`.
2. Always release outgoing packets, even if discarded.
3. Print extended debugging messages when receiving/sending packets. Such
   messages would have helped us diagnose the error much sooner.

Developers and users have confirmed that, with these changes, Wintun
connections run stably for at least 60 minutes.  (See
https://gitlab.com/openconnect/openconnect/-/merge_requests/306#note_745393731).

Also, in
https://gitlab.com/openconnect/openconnect/-/merge_requests/300?commit_id=b5ff6f3fb1b8d06cf56426b13c7af96e25cd922b,
we reverted to TAP-Windows as the default driver on Windows due to the
aforementioned stability issues.  Although Wintun connections now appear to
be stable, we are not quite ready yet to un/re-revert, and make Wintun the
default driver again.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>
Signed-off-by: Daniel Lenski <dlenski@gmail.com>

diff --git a/wintun.c b/wintun.c
index 79411841e1267a6aecb942f45fb1dc890531f903..5c544ab257a20a8969f5bac6697a27d59af085c2 100644 (file)
--- a/wintun.c
+++ b/wintun.c
@@ -166,25 +166,46 @@ int os_read_wintun(struct openconnect_info *vpninfo, struct pkt *pkt)
        DWORD tun_len;
        BYTE *tun_pkt = WintunReceivePacket(vpninfo->wintun_session,
                                            &tun_len);
-       if (tun_pkt && tun_len < pkt->len) {
+       if (!tun_pkt) {
+               DWORD err = GetLastError();
+               if (err != ERROR_NO_MORE_ITEMS) {
+                       const char *errstr = openconnect__win32_strerror(err);
+                       vpn_progress(vpninfo, PRG_ERR, _("Could not retrieve packet from Wintun adapter '%S': %s\n"),
+                                    vpninfo->ifname_w, errstr);
+                       free(errstr);
+               }
+               return -1;
+       }
+
+       int ret = 0;
+       if (tun_len <= pkt->len) {
                memcpy(pkt->data, tun_pkt, tun_len);
                pkt->len = tun_len;
-               WintunReleaseReceivePacket(vpninfo->wintun_session, tun_pkt);
-               return 0;
+       } else {
+               vpn_progress(vpninfo, PRG_ERR, _("Drop oversized packet retrieved from Wintun adapter '%S' (%ld > %d)\n"),
+                            vpninfo->ifname_w, tun_len, pkt->len);
+               ret = -1;
        }
-       return -1;
+       WintunReleaseReceivePacket(vpninfo->wintun_session, tun_pkt);
+       return ret;
 }
 
 int os_write_wintun(struct openconnect_info *vpninfo, struct pkt *pkt)
 {
        BYTE *tun_pkt = WintunAllocateSendPacket(vpninfo->wintun_session,
                                                 pkt->len);
-       if (tun_pkt) {
-               memcpy(tun_pkt, pkt->data, pkt->len);
-               WintunSendPacket(vpninfo->wintun_session, tun_pkt);
-               return 0;
+       if (!tun_pkt) {
+               DWORD err = GetLastError();
+               const char *errstr = openconnect__win32_strerror(err);
+               vpn_progress(vpninfo, PRG_ERR, _("Could not send packet through Wintun adapter '%S': %s\n"),
+                            vpninfo->ifname_w, errstr);
+               free(errstr);
+               return -1;
        }
-       return -1;
+
+       memcpy(tun_pkt, pkt->data, pkt->len);
+       WintunSendPacket(vpninfo->wintun_session, tun_pkt);
+       return 0;
 }
 
 void os_shutdown_wintun(struct openconnect_info *vpninfo)