GP server may send only a Legacy IP client address, but both Legacy and IPv6 magic...

In this corner case, we need to use the Legacy IP magic address.  The
inverse corner case would be if the server sends ESP ping magic addresses of
both types, but only sends an IPv6 client address; we were already handling
that one correctly, because we had observed that GlobalProtect servers
require the client to use the IPv6 magic ping address if they want to send
both IPv6 and Legacy IP traffic.

The easiest and most straightforward way to handle all these cases robustly
is simply to save both versions of the ESP magic address, just as we save
both versions of the client address, until after we have parsed the whole
config. At that point we decide which ESP magic address should be used.

See logs attached to
https://lists.infradead.org/pipermail/openconnect-devel/2024-July/005447.html
for an example of this:

    POST https://vpnhost.example.com/ssl-vpn/getconfig.esp
    …
    <               <gw-address>REDACTEDIPV4ADDRESS7</gw-address>
    <               <gw-address-v6>REDACTEDIPV6ADDRESS7</gw-address-v6>
    <               <ipv6-connection>no</ipv6-connection>
    <               <ip-address>REDACTEDIPV4ADDRESS0</ip-address>
    <               <netmask>255.255.255.255</netmask>
    …
    <               <ipsec>…</ipsec>
    Did not receive ESP keys and matching gateway in GlobalProtect config; tunnel will be TLS only.

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

diff --git a/gpst.c b/gpst.c
index 32d0810de0aafdcf498c6075d8bc0b3ecfd4c607..0450107ad4ecaf8f07ef51527c839c9cf2b02660 100644 (file)
--- a/gpst.c
+++ b/gpst.c
@@ -412,11 +412,9 @@ static int gpst_parse_config_xml(struct openconnect_info *vpninfo, xmlNode *xml_
        int ii;
 
 #ifdef HAVE_ESP
-       int esp_keys = 0, esp_magic_af = 0;
-       union {
-               struct in6_addr v6;
-               struct in_addr v4;
-       } esp_magic;
+       int esp_keys = 0, have_esp_magic_v4 = 0, have_esp_magic_v6 = 0;
+       struct in6_addr esp_magic_v6;
+       struct in_addr esp_magic_v4;
 #endif /* HAVE_ESP */
 
        if (!xml_node || !xmlnode_is_named(xml_node, "response"))
@@ -426,7 +424,6 @@ static int gpst_parse_config_xml(struct openconnect_info *vpninfo, xmlNode *xml_
        struct oc_vpn_option *new_opts = NULL;
        struct oc_ip_info new_ip_info = {};
 
-       memset(vpninfo->esp_magic, 0, sizeof(vpninfo->esp_magic));
        vpninfo->esp_replay_protect = 1;
        vpninfo->ssl_times.rekey_method = REKEY_NONE;
 
@@ -464,28 +461,27 @@ static int gpst_parse_config_xml(struct openconnect_info *vpninfo, xmlNode *xml_
                        vpninfo->ssl_times.last_rekey = time(NULL);
                        vpninfo->ssl_times.rekey = sec - 60;
                        vpninfo->ssl_times.rekey_method = REKEY_TUNNEL;
-               } else if (!xmlnode_get_val(xml_node, "gw-address", &s) ||
-                          !xmlnode_get_val(xml_node, "gw-address-v6", &s)) {
+               } else if (!xmlnode_get_val(xml_node, "gw-address", &s)) {
                        /* As remarked in oncp.c, "this is a tunnel; having a
                         * gateway is meaningless." See esp_send_probes_gp for the
                         * gory details of what this field actually means.
-                        *
-                        * If we get both Legacy IP and IPv6 tags, then we must use the IPv6
-                        * value in order for both AFs to work over the tunnel (see 5b98b628
-                        * "GlobalProtect IPv6 ESP support").
                         */
-                       int legacy = xmlnode_is_named(xml_node, "gw-address");
-                       if (vpninfo->peer_addr->sa_family == (legacy ? IPPROTO_IP : IPPROTO_IPV6) &&
+                       if (vpninfo->peer_addr->sa_family == IPPROTO_IP &&
                            vpninfo->ip_info.gateway_addr && strcmp(s, vpninfo->ip_info.gateway_addr))
                                vpn_progress(vpninfo, PRG_DEBUG,
-                                            _("Gateway address in config XML (%s) differs from external gateway address (%s).\n"), s, vpninfo->ip_info.gateway_addr);
+                                            _("Legacy IP gateway address in config XML (%s) differs from external gateway address (%s).\n"), s, vpninfo->ip_info.gateway_addr);
 #ifdef HAVE_ESP
-                       if (esp_magic_af == AF_INET6 && legacy) {
-                               /* We ignore the Legacy IP tag <gw-address> if we've already gotten the IPv6 tag <gw-address-v6>. */
-                       } else {
-                               esp_magic_af = legacy ? AF_INET : AF_INET6;
-                               inet_pton(esp_magic_af, s, &esp_magic);
-                       }
+                       have_esp_magic_v4 = 1;
+                       inet_pton(AF_INET, s, &esp_magic_v4);
+#endif /* HAVE_ESP */
+               } else if (!xmlnode_get_val(xml_node, "gw-address-v6", &s)) {
+                       if (vpninfo->peer_addr->sa_family == IPPROTO_IPV6 &&
+                           vpninfo->ip_info.gateway_addr && strcmp(s, vpninfo->ip_info.gateway_addr))
+                               vpn_progress(vpninfo, PRG_DEBUG,
+                                            _("IPv6 gateway address in config XML (%s) differs from external gateway address (%s).\n"), s, vpninfo->ip_info.gateway_addr);
+#ifdef HAVE_ESP
+                       have_esp_magic_v6 = 1;
+                       inet_pton(AF_INET6, s, &esp_magic_v6);
 #endif /* HAVE_ESP */
                } else if (!xmlnode_get_val(xml_node, "connected-gw-ip", &s)) {
                        if (vpninfo->ip_info.gateway_addr && strcmp(s, vpninfo->ip_info.gateway_addr))
@@ -602,12 +598,21 @@ static int gpst_parse_config_xml(struct openconnect_info *vpninfo, xmlNode *xml_
                             _("GlobalProtect IPv6 support is experimental. Please report results to <%s>.\n"),
                             "openconnect-devel@lists.infradead.org");
 #ifdef HAVE_ESP
-       if (esp_keys &&
-           ((esp_magic_af == AF_INET6 && new_ip_info.addr6) ||
-            (esp_magic_af == AF_INET && new_ip_info.addr))) {
-               /* We got ESP keys, an IPvX esp_magic address, and an IPvX address */
-               vpninfo->esp_magic_af = esp_magic_af;
-               memcpy(vpninfo->esp_magic, &esp_magic, sizeof(esp_magic));
+       if (esp_keys) {
+               /* If we get both Legacy IP and IPv6 magic addresses, then we must use the IPv6
+                * value in order for both AFs to work over the tunnel (see 5b98b628
+                * "GlobalProtect IPv6 ESP support").
+                */
+               if (have_esp_magic_v6 && new_ip_info.addr6) {
+                       /* We got ESP keys, an IPv6 esp_magic address, and an IPv6 client address */
+                       vpninfo->esp_magic_af = AF_INET6;
+                       memcpy(vpninfo->esp_magic, &esp_magic_v6, sizeof(esp_magic_v6));
+               } else if (have_esp_magic_v4 && new_ip_info.addr) {
+                       /* We got ESP keys, a Legacy IP esp_magic address, and a Legacy IP client address */
+                       vpninfo->esp_magic_af = AF_INET;
+                       memcpy(vpninfo->esp_magic, &esp_magic_v4, sizeof(esp_magic_v4));
+               } else
+                       goto cannot_esp;
 
                if (openconnect_setup_esp_keys(vpninfo, 0)) {
                        vpn_progress(vpninfo, PRG_ERR, "Failed to setup ESP keys.\n");
@@ -617,6 +622,7 @@ static int gpst_parse_config_xml(struct openconnect_info *vpninfo, xmlNode *xml_
                        vpninfo->delay_tunnel_reason = "awaiting GPST ESP connection";
                }
        } else if (vpninfo->dtls_state != DTLS_DISABLED)
+cannot_esp:
                vpn_progress(vpninfo, PRG_ERR,
                             _("Did not receive ESP keys and matching gateway in GlobalProtect config; tunnel will be TLS only.\n"));
 #endif