The option '--force-dpd' should be followed even if the server specifies a lesser...

The `openconnect_set_dpd()` API function, and the command-line option
`--force-dpd=SEC`, specify an exact DPD (Dead Peer Detection) interval for
the client, which is intended to override the server-specified value.

However, several protocols
would simply *ignore* the specified interval, and accept the
server's interval, if the server's interval was *less* than the one
specified via API or command-line.

- AnyConnect/ocserv has behaved this way since the `--force-dpd` option was
  first added in 2012 (see 54784befa190a670a7c33661e505415eaaab8dd1)
- ESP support, added for oNCP/Pulse, copied this behavior
- Fortinet copied this behavior
- Array copied this behavior

This behavior can cause problems for protocols where DPD, or reconnection in
general, is broken, and where a user wishes to specify a long interval (e.g.
`--force-dpd=99999`) to effectively disable DPD.  Specifically, Fortinet is
affected by this; see
https://gitlab.com/openconnect/openconnect/-/issues/334#note_732774445.

The straightforward solution here is just to make the `--force-dpd` option
do what it says, and set the client's DPD interval unconditionally.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

diff --git a/array.c b/array.c
index aa84653b9861fe569f18886c8917198763714438..96e7c4550f4df3caf33eac7d26323c632453e781 100644 (file)
--- a/array.c
+++ b/array.c
@@ -522,9 +522,9 @@ static int parse_speed_tunnel(struct openconnect_info *vpninfo,
 
        /* We don't support DPD yet...*/
        if (dpd) {
-               if (!vpninfo->ssl_times.dpd || vpninfo->ssl_times.dpd > dpd)
+               if (!vpninfo->ssl_times.dpd)
                        vpninfo->ssl_times.dpd = dpd;
-               if (!vpninfo->dtls_times.dpd || vpninfo->dtls_times.dpd > dpd)
+               if (!vpninfo->dtls_times.dpd)
                        vpninfo->dtls_times.dpd = dpd;
        }
 

diff --git a/cstp.c b/cstp.c
index 2e04f9066f28c50f7937b42ccb90f185ac928e34..0b7a09301a4a667fd547be99a0ff8e5406ff3725 100644 (file)
--- a/cstp.c
+++ b/cstp.c
@@ -486,7 +486,7 @@ static int start_cstp_connection(struct openconnect_info *vpninfo)
                                vpninfo->dtls_times.keepalive = atol(colon);
                        } else if (!strcmp(buf + i, "DPD")) {
                                int j = atol(colon);
-                               if (j && (!vpninfo->dtls_times.dpd || j < vpninfo->dtls_times.dpd))
+                               if (j && !vpninfo->dtls_times.dpd)
                                        vpninfo->dtls_times.dpd = j;
                        } else if (!strcmp(buf + i, "Rekey-Method")) {
                                if (!strcmp(colon, "new-tunnel"))
@@ -524,7 +524,7 @@ static int start_cstp_connection(struct openconnect_info *vpninfo)
                        vpninfo->idle_timeout = atol(colon);
                } else if (!strcmp(buf + 7, "DPD")) {
                        int j = atol(colon);
-                       if (j && (!vpninfo->ssl_times.dpd || j < vpninfo->ssl_times.dpd))
+                       if (j && !vpninfo->ssl_times.dpd)
                                vpninfo->ssl_times.dpd = j;
                } else if (!strcmp(buf + 7, "Rekey-Time")) {
                        vpninfo->ssl_times.rekey = atol(colon);

diff --git a/esp.c b/esp.c
index 3f15e21412a6915e2c6c2b6611f3a7ec2c6a6cdc..d607a4e0df8bc4c707baee13dd0cea3c1abc930f 100644 (file)
--- a/esp.c
+++ b/esp.c
@@ -82,10 +82,13 @@ int esp_setup(struct openconnect_info *vpninfo)
            vpninfo->dtls_state == DTLS_NOSECRET)
                return -EINVAL;
 
-       if (vpninfo->esp_ssl_fallback)
-               vpninfo->dtls_times.dpd = vpninfo->esp_ssl_fallback;
-       else
-               vpninfo->dtls_times.dpd = vpninfo->dtls_attempt_period;
+       /* XX: set ESP DPD interval if not already set */
+       if (!vpninfo->dtls_times.dpd) {
+               if (vpninfo->esp_ssl_fallback)
+                       vpninfo->dtls_times.dpd = vpninfo->esp_ssl_fallback;
+               else
+                       vpninfo->dtls_times.dpd = vpninfo->dtls_attempt_period;
+       }
 
        print_esp_keys(vpninfo, _("incoming"), &vpninfo->esp_in[vpninfo->current_esp_in]);
        print_esp_keys(vpninfo, _("outgoing"), &vpninfo->esp_out);

diff --git a/fortinet.c b/fortinet.c
index 7380269043339ddd5c42714b9e5a68b52d1bb58f..28abb90dced6ab8bc8c554fc251c1f54cb8b31b0 100644 (file)
--- a/fortinet.c
+++ b/fortinet.c
@@ -378,7 +378,7 @@ static int parse_fortinet_xml_config(struct openconnect_info *vpninfo, char *buf
                        vpn_progress(vpninfo, PRG_INFO, _("Idle timeout is %d minutes.\n"), sec/60);
                } else if (xmlnode_is_named(xml_node, "dtls-config") && !xmlnode_get_prop(xml_node, "heartbeat-interval", &s)) {
                        int sec = atoi(s);
-                       if (sec && (!vpninfo->dtls_times.dpd || sec < vpninfo->dtls_times.dpd))
+                       if (sec && !vpninfo->dtls_times.dpd)
                                vpninfo->dtls_times.dpd = vpninfo->ssl_times.dpd = sec;
                } else if (xmlnode_is_named(xml_node, "auth-ses")) {
                        /* These settings were apparently added in v6.2.1 of the Fortigate server,