periodic HIP fix: ping /ssl-vpn/hipreportcheck.esp at specified interval no matter...

This is a fix for the very subtle regression between v8.05 and v8.08 described in this thread: https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005609.html

- Server never asks for HIP report submission, and no HIP script specified
  with `--csd-wrapper`
- v8.05 successfully rekeys 1 minute before server-specified rekey interval,
  and continues successfully
- v8.08 appears to successfully rekey, but then gets forced off one minute
  later
- Only apparent difference between the two is that v8.05 re-pings
  /ssl-vpn/hipreportcheck.esp every time it gets the config
  (/ssl-vpn/getconfig.esp), whereas v8.08 only pings it exactly once.

The bottom line is that _even if_ we have no mechanism to actually submit a
HIP report (no `--csd-wrapper`) and _even if_ we think the server will say
"no HIP report needed" every time we check, in order to keep the server from
kicking the client off, we should still…

* ping /ssl-vpn/hipreportcheck.esp at the interval (specified by the portal or `--force-trojan` or 1 hour default)
* ping /ssl-vpn/hipreportcheck.esp every time we re-fetch the config (/ssl-vpn/getconfig.esp) in order to rekey or reconnect

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

diff --git a/auth-globalprotect.c b/auth-globalprotect.c
index 16a3050e1d67fe9ca8a689dd94653700affc0103..73910760891158cfdedb78f7d2e2db25e041b2ca 100644 (file)
--- a/auth-globalprotect.c
+++ b/auth-globalprotect.c
@@ -404,10 +404,7 @@ static int parse_portal_xml(struct openconnect_info *vpninfo, xmlNode *xml_node,
                                for (x2 = x->children; x2; x2 = x2->next) {
                                        if (!xmlnode_get_val(x2, "hip-report-interval", &hip_interval)) {
                                                int sec = atoi(hip_interval);
-                                               if (!vpninfo->csd_wrapper)
-                                                       vpn_progress(vpninfo, PRG_INFO, _("Ignoring portal's HIP report interval (%d minutes), because no HIP report script provided.\n"),
-                                                                                sec/60);
-                                               else if (vpninfo->trojan_interval)
+                                               if (vpninfo->trojan_interval)
                                                        vpn_progress(vpninfo, PRG_INFO, _("Ignoring portal's HIP report interval (%d minutes), because interval is already set to %d minutes.\n"),
                                                                                 sec/60, vpninfo->trojan_interval/60);
                                                else {

diff --git a/gpst.c b/gpst.c
index 989fc9b425c9cc9010c15491538aa8c73ee51f20..359e00d05aecc6fab947ab868c82418547e137a9 100644 (file)
--- a/gpst.c
+++ b/gpst.c
@@ -915,17 +915,20 @@ static int run_hip_script(struct openconnect_info *vpninfo)
 #endif
 
        if (!vpninfo->csd_wrapper) {
-               vpn_progress(vpninfo, PRG_ERR,
-                            _("WARNING: Server asked us to submit HIP report with md5sum %s.\n"
-                              "    VPN connectivity may be disabled or limited without HIP report submission.\n    %s\n"),
-                            vpninfo->csd_token,
+               /* Only warn once */
+               if (!vpninfo->last_trojan) {
+                       vpn_progress(vpninfo, PRG_ERR,
+                                    _("WARNING: Server asked us to submit HIP report with md5sum %s.\n"
+                                      "    VPN connectivity may be disabled or limited without HIP report submission.\n    %s\n"),
+                                    vpninfo->csd_token,
 #if defined(_WIN32) || defined(__native_client__)
-                            _("However, running the HIP report submission script on this platform is not yet implemented.")
+                                    _("However, running the HIP report submission script on this platform is not yet implemented.")
 #else
-                            _("You need to provide a --csd-wrapper argument with the HIP report submission script.")
+                                    _("You need to provide a --csd-wrapper argument with the HIP report submission script.")
 #endif
-                       );
-               /* XXX: Many GlobalProtect VPNs work fine despite allegedly requiring HIP report submission */
+                               );
+                       /* XXX: Many GlobalProtect VPNs work fine despite allegedly requiring HIP report submission */
+               }
                return 0;
        }
 
@@ -1044,21 +1047,22 @@ int gpst_setup(struct openconnect_info *vpninfo)
        if (ret)
                goto out;
 
-       /* Always check HIP once (even if no --csd-wrapper specified) */
-       if (!vpninfo->last_trojan) {
-               ret = check_and_maybe_submit_hip_report(vpninfo);
-               if (ret)
-                       goto out;
-       }
+       /* Always check HIP after getting configuration */
+       ret = check_and_maybe_submit_hip_report(vpninfo);
+       if (ret)
+               goto out;
+
+        /* XX: last_trojan is used both as a sentinel to detect the
+         * first time we check/submit HIP, and for the mainloop to timeout
+         * when periodic re-checking is required.
+         */
+       vpninfo->last_trojan = time(NULL);
 
        /* Default HIP re-checking to 3600 seconds unless already set by
-        * --force-trojan or portal config. There's no point to rechecking
-        * HIP if --csd-wrapper wasn't specified: either the VPN will
-        * work despite lack of HIP submission, or it won't.
+        * --force-trojan or portal config.
         */
-       if (vpninfo->csd_wrapper && !vpninfo->trojan_interval)
+       if (!vpninfo->trojan_interval)
                vpninfo->trojan_interval = 3600;
-       vpninfo->last_trojan = time(NULL);
 
        /* Connect tunnel immediately if ESP is not going to be used */
        ret = gpst_connect(vpninfo);