]> www.infradead.org Git - users/dwmw2/openconnect.git/commitdiff
Fix a really subtle bug causing 100% CPU utilization after ESP tunnel failure, and...
authorDaniel Lenski <dlenski@gmail.com>
Tue, 9 Jan 2018 08:01:21 +0000 (00:01 -0800)
committerDavid Woodhouse <dwmw2@infradead.org>
Tue, 27 Feb 2018 15:28:32 +0000 (16:28 +0100)
(This was reported at https://github.com/dlenski/openconnect/issues/76.)

Here's what was happening:

1. GlobalProtect connect, start ESP

   -> dtls_state = DTLS_CONNECTED, dtls_fd is read-monitored

2. ESP tunnel fails and GP switches to HTTPS (due to network outage, dead peer?),

   -> dtls_state = DTLS_NOSECRET, dtls_fd is still read-monitored (!!!)

3. Tunnel restarts (due to rekey or pause-and-reconnect signal, USR2) and
   /ssl-vpn/getconfig.esp is repulled, including new ESP keys.

   -> dtls_state = DTLS_SECRET, dtls_fd is still read-monitored (!!!)

4. ESP probes are sent out *once* in esp_setup(), but dtls_fd != -1, so the
   dtls_state is *not* upgraded to DTLS_SLEEPING.

   -> dtls_state = DTLS_SECRET, dtls_fd is still read-monitored (!!!)

As a result of the probes being sent out, ESP packets will subsequently arrive
and select() call in openconnect_mainloop() will wake up… but
udp_mainloop() will never be called to service it because…

    if (vpninfo->dtls_state > DTLS_DISABLED) {
     ...
        ret = vpninfo->proto->udp_mainloop(vpninfo, &timeout);
    }

This patch fixes that by not just setting dtls_state = DTLS_SECRET when the
HTTPS tunnel connects, but actually calling esp_close_secret (which closes
dtls_fd, unmonitors it, and sets it to -1).

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
esp.c
gpst.c

diff --git a/esp.c b/esp.c
index 8cf3fc560786b44d6321393817adadac78e35e48..7a74b1fcdaadbf7032d235f5e64941fc92192efe 100644 (file)
--- a/esp.c
+++ b/esp.c
@@ -305,7 +305,8 @@ void esp_close(struct openconnect_info *vpninfo)
 void esp_close_secret(struct openconnect_info *vpninfo)
 {
        esp_close(vpninfo);
-       vpninfo->dtls_state = DTLS_NOSECRET;
+       if (vpninfo->dtls_state > DTLS_DISABLED)
+               vpninfo->dtls_state = DTLS_NOSECRET;
 }
 
 void esp_shutdown(struct openconnect_info *vpninfo)
diff --git a/gpst.c b/gpst.c
index b0ab76447d31d003218fa6c26a3d114e28ca4686..23e76fb4d800284883b6150ef78282b97e3220a5 100644 (file)
--- a/gpst.c
+++ b/gpst.c
@@ -655,8 +655,7 @@ static int gpst_connect(struct openconnect_info *vpninfo)
                monitor_read_fd(vpninfo, ssl);
                monitor_except_fd(vpninfo, ssl);
                vpninfo->ssl_times.last_rx = vpninfo->ssl_times.last_tx = time(NULL);
-               if (vpninfo->dtls_state != DTLS_DISABLED)
-                       vpninfo->dtls_state = DTLS_NOSECRET;
+               esp_close_secret(vpninfo);
        }
 
        return ret;