[NetworkManager-openconnect/sni-resolve-authgroup] Support openconnect_get_connect_url() to fix SNI and authgroup problems



commit eeda887d09e716c6651c16bfa50d8752e14c3625
Author: David Woodhouse <dwmw2 infradead org>
Date:   Fri May 14 13:20:04 2021 +0100

    Support openconnect_get_connect_url() to fix SNI and authgroup problems
    
    Fixes: #53
    Fixes: #46
    
    A long time back, OpenConnect started returning the IP address when we
    call openconnect_get_hostname(), to ensure that it ends up establishing
    the connection to precisely the same host as it authenticated to. Since
    we passed on the server certificate fingerprint explicitly it didn't
    need to revalidate that anyway.
    
    However, that breaks virtualhost servers which rely on either a Host:
    header or SNI to provide the actual hostname. So where OpenConnect is
    new enough to understand the --resolve argument, use that and go back
    to giving it the *actual* hostname in the connect URL.
    
    Meanwhile, the Pulse protocol started actually caring about the *path*
    for the connection; it's the only one for which the path part of the
    URL actually matters after authentication, and isn't just noise left
    behind by the last form we authenticated to. So for *Pulse* only, add
    the path too.
    
    The next OpenConnect release will have openconnect_get_connect_url()
    and we won't need to do that by hand, but for now we *can* support
    versions going back to v7.07 where the --resolve argument was added,
    so let's do so.
    
    We need to construct the --resolve argument too, and everything we
    need to do that is already available, although it's a bit icky that
    we have to strip the [] from around IPv6 literals.

 auth-dialog/main.c           | 74 ++++++++++++++++++++++++++++++++++++++++++--
 shared/nm-service-defines.h  |  1 +
 src/nm-openconnect-service.c |  9 +++++-
 3 files changed, 80 insertions(+), 4 deletions(-)
---
diff --git a/auth-dialog/main.c b/auth-dialog/main.c
index 5c1518e..411fc72 100644
--- a/auth-dialog/main.c
+++ b/auth-dialog/main.c
@@ -154,6 +154,7 @@ static void keyring_store_passwords(gpointer key, gpointer value, gpointer user_
 typedef struct auth_ui_data {
        char *vpn_name;
        char *vpn_uuid;
+       gboolean connect_urlpath;
        GHashTable *options;
        GHashTable *secrets;
        GHashTable *success_secrets;
@@ -1084,6 +1085,9 @@ static int get_config (auth_ui_data *ui_data,
 #endif
                return -EINVAL;
 
+       if (!g_strcmp0(protocol, "pulse"))
+               ui_data->connect_urlpath = TRUE;
+
        cafile = g_hash_table_lookup (options, NM_OPENCONNECT_KEY_CACERT);
        if (cafile)
                openconnect_set_cafile(vpninfo, OC3DUP (cafile));
@@ -1294,6 +1298,66 @@ static void hash_table_merge (GHashTable *old_hash, GHashTable *new_hash)
        g_hash_table_foreach_steal (old_hash, &hash_merge_one, new_hash);
 }
 
+
+static char *oc_server_url(auth_ui_data *ui_data)
+{
+#if OPENCONNECT_CHECK_VER(5,7)
+       return g_strdup(openconnect_get_connect_url(ui_data->vpninfo));
+#elif OPENCONNECT_CHECK_VER(5,3)
+       /* This is basically what OpenConnect does to create it for us,
+        * in newer versions. Ideally we wouldn't have to get involved
+        * in any of this, but for backward compatibiity we can tolerate
+        * it because it lets us use the --resolve trick, and thus fix
+        * things for users whose servers need SNI, without having to
+        * upgrade openconnect first. */
+       const char *dnsname = openconnect_get_dnsname(ui_data->vpninfo);
+       int port = openconnect_get_port(ui_data->vpninfo);
+       const char *urlpath = NULL;
+
+       if (ui_data->connect_urlpath)
+               urlpath = openconnect_get_urlpath();
+
+       if (port == 443)
+               return g_strdup_printf("https://%s/%s";,
+                                      dnsname, urlpath ? urlpath : "");
+       else
+               return g_strdup_printf("https://%s:%d/%s";,
+                                      dnsname, port, urlpath ? urlpath : "");
+#else
+       /* Prior to OpenConnect v7.07 (API 5.3) openconnect didn't have the
+        * --resolve argument, so needs the IP address — which confusingly
+        * is what it returns from openconnect_get_hostname() — in order to
+        * ensure that it connects to the same server we authenticated to,
+        * even when round robin or geo DNS would give a different lookup.
+        *
+        * This does mean that SNI-based proxies on the server end are going
+        * to fail to find the right target, for older OpenConnect. */
+       return g_strdup_printf ("%s:%d",
+                               openconnect_get_hostname(ui_data->vpninfo),
+                               openconnect_get_port(ui_data->vpninfo));
+#endif
+}
+
+static char *oc_server_resolve(struct openconnect_info *vpninfo)
+{
+       /* Versions older than OpenConnect v7.07 (API 5.3) didn't
+        * support the --resolve argument. Don't confuse them. */
+#if OPENCONNECT_CHECK_VER(5,3)
+       const char *ipaddr = openconnect_get_hostname(vpninfo);
+       const char *dnsname = openconnect_get_dnsname(vpninfo);
+
+       if (g_strcmp0(ipaddr, dnsname)) {
+               /* Strip the [] surrounding IPv6 literals. */
+               int l = strlen(ipaddr);
+               if (ipaddr[0] == '[' && ipaddr[l-1] == ']') {
+                       ipaddr++;
+                       l -=2;
+               }
+               return g_strdup_printf("%s:%.*s", dnsname, l, ipaddr);
+       }
+#endif
+       return NULL;
+}
 static gboolean cookie_obtained(auth_ui_data *ui_data)
 {
        ui_data->getting_cookie = FALSE;
@@ -1331,11 +1395,15 @@ static gboolean cookie_obtained(auth_ui_data *ui_data)
                /* Merge in the three *real* secrets that are actually used
                   by nm-openconnect-service to make the connection */
                key = g_strdup (NM_OPENCONNECT_KEY_GATEWAY);
-               value = g_strdup_printf ("%s:%d",
-                                        openconnect_get_hostname(ui_data->vpninfo),
-                                        openconnect_get_port(ui_data->vpninfo));
+               value = oc_server_url(ui_data);
                g_hash_table_insert (ui_data->secrets, key, value);
 
+               value = oc_server_resolve(ui_data->vpninfo);
+               if (value) {
+                       key = g_strdup (NM_OPENCONNECT_KEY_RESOLVE);
+                       g_hash_table_insert (ui_data->secrets, key, value);
+               }
+
                key = g_strdup (NM_OPENCONNECT_KEY_COOKIE);
                value = g_strdup (openconnect_get_cookie (ui_data->vpninfo));
                g_hash_table_insert (ui_data->secrets, key, value);
diff --git a/shared/nm-service-defines.h b/shared/nm-service-defines.h
index e598bcc..4e7d481 100644
--- a/shared/nm-service-defines.h
+++ b/shared/nm-service-defines.h
@@ -34,6 +34,7 @@
 #define NM_OPENCONNECT_KEY_GATEWAY "gateway"
 #define NM_OPENCONNECT_KEY_COOKIE "cookie"
 #define NM_OPENCONNECT_KEY_GWCERT "gwcert"
+#define NM_OPENCONNECT_KEY_RESOLVE "resolve"
 #define NM_OPENCONNECT_KEY_AUTHTYPE "authtype"
 #define NM_OPENCONNECT_KEY_USERCERT "usercert"
 #define NM_OPENCONNECT_KEY_CACERT "cacert"
diff --git a/src/nm-openconnect-service.c b/src/nm-openconnect-service.c
index 348c19a..c567747 100644
--- a/src/nm-openconnect-service.c
+++ b/src/nm-openconnect-service.c
@@ -102,6 +102,7 @@ static const ValidProperty valid_secrets[] = {
        { NM_OPENCONNECT_KEY_COOKIE,  G_TYPE_STRING, 0, 0 },
        { NM_OPENCONNECT_KEY_GATEWAY, G_TYPE_STRING, 0, 0 },
        { NM_OPENCONNECT_KEY_GWCERT,  G_TYPE_STRING, 0, 0 },
+       { NM_OPENCONNECT_KEY_RESOLVE, G_TYPE_STRING, 0, 0 },
        { NULL,                       G_TYPE_NONE, 0, 0 }
 };
 
@@ -395,7 +396,7 @@ nm_openconnect_start_openconnect_binary (NMOpenconnectPlugin *plugin,
        gint stdin_fd;
        char csd_user_arg[60];
        const char *props_vpn_gw, *props_cookie, *props_cacert, *props_mtu, *props_gwcert, *props_proxy;
-       const char *props_csd_enable, *props_csd_wrapper;
+       const char *props_csd_enable, *props_csd_wrapper, *props_resolve;
        const char *protocol;
 
        /* Find openconnect */
@@ -437,6 +438,7 @@ nm_openconnect_start_openconnect_binary (NMOpenconnectPlugin *plugin,
                return -1;
        }
        props_gwcert = nm_setting_vpn_get_secret (s_vpn, NM_OPENCONNECT_KEY_GWCERT);
+       props_resolve = nm_setting_vpn_get_secret (s_vpn, NM_OPENCONNECT_KEY_RESOLVE);
 
        props_cacert = nm_setting_vpn_get_data_item (s_vpn, NM_OPENCONNECT_KEY_CACERT);
        props_mtu = nm_setting_vpn_get_data_item (s_vpn, NM_OPENCONNECT_KEY_MTU);
@@ -457,6 +459,11 @@ nm_openconnect_start_openconnect_binary (NMOpenconnectPlugin *plugin,
                }
        }
 
+       if (props_resolve && strlen(props_resolve)) {
+               g_ptr_array_add (openconnect_argv, (gpointer) "--resolve");
+               g_ptr_array_add (openconnect_argv, (gpointer) props_resolve);
+       }
+
        if (props_gwcert && strlen(props_gwcert)) {
                g_ptr_array_add (openconnect_argv, (gpointer) "--servercert");
                g_ptr_array_add (openconnect_argv, (gpointer) props_gwcert);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]