NetworkManager-openvpn/0001-tls-remote-workaround-rh1421241.patch

From 40ee847d32c11d0bc7c1b06fefa9a9ef8e2b0570 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 13 Feb 2017 12:30:16 +0100
Subject: [PATCH 1/4] service: avoid strlen() for checking whether a string is
 empty

Possibly the compiler can optimize it not to evaluate the full string length,
just to verify whether the string is empty. Still, I think it's bad style.

(cherry picked from commit 2a4a4a49d8b97e3cbe37307f6b6c1053df946ce4)
---
 src/nm-openvpn-service.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c
index d7bd29f..d6e36a6 100644
--- a/src/nm-openvpn-service.c
+++ b/src/nm-openvpn-service.c
@@ -1406,7 +1406,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 
 	/* Cipher */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_CIPHER);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--cipher");
 		add_openvpn_arg (args, tmp);
 	}
@@ -1419,7 +1419,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 
 	/* Keysize */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_KEYSIZE);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--keysize");
 		if (!add_openvpn_arg_int (args, tmp)) {
 			g_set_error (error,
@@ -1440,25 +1440,25 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 
 	/* TA */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_TA);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--tls-auth");
 		add_openvpn_arg_utf8safe (args, tmp);
 
 		tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_TA_DIR);
-		if (tmp && strlen (tmp))
+		if (tmp && tmp[0])
 			add_openvpn_arg (args, tmp);
 	}
 
 	/* tls-remote */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_TLS_REMOTE);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--tls-remote");
 		add_openvpn_arg (args, tmp);
 	}
 
 	/* verify-x509-name */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_VERIFY_X509_NAME);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		const char *name;
 		gs_free char *type = NULL;
 
@@ -1483,7 +1483,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 
 	/* remote-cert-tls */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_REMOTE_CERT_TLS);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--remote-cert-tls");
 		add_openvpn_arg (args, tmp);
 	}
@@ -1500,7 +1500,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 	if (!connection_type_is_tls_mode (connection_type)) {
 		/* Ignore --reneg-sec option if we are not in TLS mode (as enabled
 		 * by --client below). openvpn will error out otherwise, see bgo#749050. */
-	} else if (tmp && strlen (tmp)) {
+	} else if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--reneg-sec");
 		if (!add_openvpn_arg_int (args, tmp)) {
 			g_set_error (error,
@@ -1532,7 +1532,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 
 	/* TUN MTU size */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_TUNNEL_MTU);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--tun-mtu");
 		if (!add_openvpn_arg_int (args, tmp)) {
 			g_set_error (error,
@@ -1546,7 +1546,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 
 	/* fragment size */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_FRAGMENT_SIZE);
-	if (tmp && strlen (tmp)) {
+	if (tmp && tmp[0]) {
 		add_openvpn_arg (args, "--fragment");
 		if (!add_openvpn_arg_int (args, tmp)) {
 			g_set_error (error,
@@ -1620,12 +1620,12 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 		add_cert_args (args, s_vpn);
 	} else if (!strcmp (connection_type, NM_OPENVPN_CONTYPE_STATIC_KEY)) {
 		tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_STATIC_KEY);
-		if (tmp && strlen (tmp)) {
+		if (tmp && tmp[0]) {
 			add_openvpn_arg (args, "--secret");
 			add_openvpn_arg_utf8safe (args, tmp);
 
 			tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_STATIC_KEY_DIRECTION);
-			if (tmp && strlen (tmp))
+			if (tmp && tmp[0])
 				add_openvpn_arg (args, tmp);
 		}
 
@@ -1659,7 +1659,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 		add_openvpn_arg (args, "--auth-user-pass");
 
 		tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_CA);
-		if (tmp && strlen (tmp)) {
+		if (tmp && tmp[0]) {
 			add_openvpn_arg (args, "--ca");
 			add_openvpn_arg_utf8safe (args, tmp);
 		}
-- 
2.9.3


From 1a21babccc3eb77c5b4a2953e7c45aaec670b120 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 13 Feb 2017 11:31:40 +0100
Subject: [PATCH 2/4] service: minor refactoring of nm_find_openvpn()

And rename to openvpn_binary_find_exepath().
The prefix "openvpn_binary_" will be used for related functions.

(cherry picked from commit 05cb6356bb4d27fb1c2ca5f8a7bfdf23fe424f0c)
---
 src/nm-openvpn-service.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c
index d6e36a6..d88ed19 100644
--- a/src/nm-openvpn-service.c
+++ b/src/nm-openvpn-service.c
@@ -188,6 +188,25 @@ _LOGD_enabled (void)
 
 /*****************************************************************************/
 
+static const char *
+openvpn_binary_find_exepath (void)
+{
+	static const char *paths[] = {
+		"/usr/sbin/openvpn",
+		"/sbin/openvpn",
+		"/usr/local/sbin/openvpn",
+	};
+	int i;
+
+	for (i = 0; i < G_N_ELEMENTS (paths); i++) {
+		if (g_file_test (paths[i], G_FILE_TEST_EXISTS))
+			return paths[i];
+	}
+	return NULL;
+}
+
+/*****************************************************************************/
+
 static void
 pids_pending_data_free (PidsPendingData *pid_data)
 {
@@ -886,26 +905,6 @@ connection_type_is_tls_mode (const char *connection_type)
 	    || strcmp (connection_type, NM_OPENVPN_CONTYPE_PASSWORD_TLS) == 0;
 }
 
-static const char *
-nm_find_openvpn (void)
-{
-	static const char *openvpn_binary_paths[] = {
-		"/usr/sbin/openvpn",
-		"/sbin/openvpn",
-		"/usr/local/sbin/openvpn",
-		NULL
-	};
-	const char  **openvpn_binary = openvpn_binary_paths;
-
-	while (*openvpn_binary != NULL) {
-		if (g_file_test (*openvpn_binary, G_FILE_TEST_EXISTS))
-			break;
-		openvpn_binary++;
-	}
-
-	return *openvpn_binary;
-}
-
 static void
 add_openvpn_arg (GPtrArray *args, const char *arg)
 {
@@ -1154,7 +1153,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 		return FALSE;
 
 	/* Find openvpn */
-	openvpn_binary = nm_find_openvpn ();
+	openvpn_binary = openvpn_binary_find_exepath ();
 	if (!openvpn_binary) {
 		g_set_error_literal (error,
 		                     NM_VPN_PLUGIN_ERROR,
-- 
2.9.3


From adc7dd5148c12917eee1c2c92ddb605e2ecd6b2c Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 13 Feb 2017 12:30:26 +0100
Subject: [PATCH 3/4] service: for OpenVPN 2.4 and newer, handle --tls-remote
 option via --verify-x509-name

The tls-remote option got removed from OpenVPN 2.4. This requires users
to fix their existing configurations to use verify-x509-name instead.

Using tls-remote on a recent OpenVPN binary thus fails to establish
the connection, which is an annoyance for the user. Let the plugin
automatically convert the "tls-remote $NAME" option to "verify-x509-name
$NAME name". Note that the two options are not entirely equivalent, thus
the is a chance that this wrongly rejects a server that would have worked
before, or ever worse, that it wronlgy accepts a server that would have
been rejected.

But in most common cases, the workaround should work fine.
The user is still strongly encouraged to update his configuration.

https://bugzilla.gnome.org/show_bug.cgi?id=776045
https://bugzilla.redhat.com/show_bug.cgi?id=1421241
(cherry picked from commit f7421ef277222bd640c432afefc21ef5a98477bc)
---
 src/nm-openvpn-service.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 85 insertions(+), 2 deletions(-)

diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c
index d88ed19..fa59537 100644
--- a/src/nm-openvpn-service.c
+++ b/src/nm-openvpn-service.c
@@ -70,6 +70,13 @@ G_DEFINE_TYPE (NMOpenvpnPlugin, nm_openvpn_plugin, NM_TYPE_VPN_SERVICE_PLUGIN)
 
 #define NM_OPENVPN_PLUGIN_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_OPENVPN_PLUGIN, NMOpenvpnPluginPrivate))
 
+typedef enum {
+	OPENVPN_BINARY_VERSION_INVALID,
+	OPENVPN_BINARY_VERSION_UNKNOWN,
+	OPENVPN_BINARY_VERSION_2_3_OR_OLDER,
+	OPENVPN_BINARY_VERSION_2_4_OR_NEWER,
+} OpenvpnBinaryVersion;
+
 typedef struct {
 	char *default_username;
 	char *username;
@@ -205,6 +212,64 @@ openvpn_binary_find_exepath (void)
 	return NULL;
 }
 
+static OpenvpnBinaryVersion
+openvpn_binary_detect_version (const char *exepath)
+{
+	gs_free char *s_stdout = NULL;
+	const char *s;
+	int exit_code;
+	int n;
+
+	g_return_val_if_fail (exepath && exepath[0] == '/', OPENVPN_BINARY_VERSION_UNKNOWN);
+
+	if (!g_spawn_sync (NULL,
+	                   (char *[]) { (char *) exepath, "--version", NULL },
+	                   NULL,
+	                   G_SPAWN_STDERR_TO_DEV_NULL,
+	                   NULL,
+	                   NULL,
+	                   &s_stdout,
+	                   NULL,
+	                   &exit_code,
+	                   NULL))
+		return OPENVPN_BINARY_VERSION_UNKNOWN;
+
+	if (   !WIFEXITED (exit_code)
+	    || WEXITSTATUS (exit_code) != 1) {
+		/* expect return code 1 (OPENVPN_EXIT_STATUS_USAGE) */
+		return OPENVPN_BINARY_VERSION_UNKNOWN;
+	}
+
+	/* the output for --version starts with title_string, which starts with PACKAGE_STRING,
+	 * which looks like "OpenVPN 2.#...". Do a strict parsing here... */
+	if (   !s_stdout
+	    || !g_str_has_prefix (s_stdout, "OpenVPN 2."))
+		return OPENVPN_BINARY_VERSION_UNKNOWN;
+	s = &s_stdout[NM_STRLEN ("OpenVPN 2.")];
+
+	if (!g_ascii_isdigit (s[0]))
+		return OPENVPN_BINARY_VERSION_UNKNOWN;
+
+	n = 0;
+	do {
+		if (n > G_MAXINT / 100)
+			return OPENVPN_BINARY_VERSION_UNKNOWN;
+		n = (n * 10) + (s[0] - '0');
+	} while (g_ascii_isdigit ((++s)[0]));
+
+	if (n <= 3)
+		return OPENVPN_BINARY_VERSION_2_3_OR_OLDER;
+	return OPENVPN_BINARY_VERSION_2_4_OR_NEWER;
+}
+
+static OpenvpnBinaryVersion
+openvpn_binary_detect_version_cached (const char *exepath, OpenvpnBinaryVersion *cached)
+{
+	if (G_UNLIKELY (*cached == OPENVPN_BINARY_VERSION_INVALID))
+		*cached = openvpn_binary_detect_version (exepath);
+	return *cached;
+}
+
 /*****************************************************************************/
 
 static void
@@ -1119,12 +1184,14 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 	gboolean dev_type_is_tap;
 	char *stmp;
 	const char *defport, *proto_tcp;
+	const char *tls_remote = NULL;
 	const char *nm_openvpn_user, *nm_openvpn_group, *nm_openvpn_chroot;
 	gs_free char *bus_name = NULL;
 	NMSettingVpn *s_vpn;
 	const char *connection_type;
 	gint64 v_int64;
 	char sbuf_64[65];
+	OpenvpnBinaryVersion openvpn_binary_version = OPENVPN_BINARY_VERSION_INVALID;
 
 	s_vpn = nm_connection_get_setting_vpn (connection);
 	if (!s_vpn) {
@@ -1451,8 +1518,17 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 	/* tls-remote */
 	tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_TLS_REMOTE);
 	if (tmp && tmp[0]) {
-		add_openvpn_arg (args, "--tls-remote");
-		add_openvpn_arg (args, tmp);
+		if (openvpn_binary_detect_version_cached (openvpn_binary, &openvpn_binary_version) != OPENVPN_BINARY_VERSION_2_4_OR_NEWER) {
+			_LOGW ("the tls-remote option is deprecated and removed from OpenVPN 2.4. Update your connection to use verify-x509-name");
+			add_openvpn_arg (args, "--tls-remote");
+			add_openvpn_arg (args, tmp);
+		} else {
+			_LOGW ("the tls-remote option is deprecated and removed from OpenVPN 2.4. For compatibility, the plugin uses \"verify-x509-name\" \"%s\" \"name\" instead. Update your connection to use verify-x509-name", tmp);
+			add_openvpn_arg (args, "--verify-x509-name");
+			add_openvpn_arg (args, tmp);
+			add_openvpn_arg (args, "name");
+		}
+		tls_remote = tmp;
 	}
 
 	/* verify-x509-name */
@@ -1461,6 +1537,13 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
 		const char *name;
 		gs_free char *type = NULL;
 
+		if (tls_remote) {
+			g_set_error (error, NM_VPN_PLUGIN_ERROR,
+			             NM_VPN_PLUGIN_ERROR_BAD_ARGUMENTS,
+			             _("Invalid configuration with tls-remote and verify-x509-name."));
+			return FALSE;
+		}
+
 		name = strchr (tmp, ':');
 		if (name) {
 			type = g_strndup (tmp, name - tmp);
-- 
2.9.3


From 11049e7c888fcc74896b34ea86f09d38a561fc35 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 13 Feb 2017 12:56:27 +0100
Subject: [PATCH 4/4] properties: discourage use of tls-remote in GUI

Mark the entry as "error" when selecting the deprecated
tls-remote option.

This is to make it more apparent to the user that he
should avoid this setting.

(cherry picked from commit 1c2986b8881b3b28d493f66cc804da12712cc2a7)
---
 properties/auth-helpers.c       | 14 ++++++++++++--
 properties/import-export.c      |  2 +-
 properties/nm-openvpn-dialog.ui |  2 ++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/properties/auth-helpers.c b/properties/auth-helpers.c
index 4d1e1ce..2f880dd 100644
--- a/properties/auth-helpers.c
+++ b/properties/auth-helpers.c
@@ -1211,7 +1211,7 @@ populate_tls_remote_mode_entry_combo (GtkEntry* entry, GtkComboBox *box,
 
 	gtk_list_store_append (store, &iter);
 	gtk_list_store_set (store, &iter,
-	                    TLS_REMOTE_MODE_COL_NAME, _("Verify subject partially (legacy mode)"),
+	                    TLS_REMOTE_MODE_COL_NAME, _("Verify subject partially (legacy mode, strongly discouraged)"),
 	                    TLS_REMOTE_MODE_COL_VALUE, TLS_REMOTE_MODE_LEGACY,
 	                    -1);
 
@@ -1250,6 +1250,7 @@ tls_remote_changed (GtkWidget *widget, gpointer user_data)
 	GtkWidget *entry, *combo, *ok_button;
 	GtkTreeIter iter;
 	gboolean entry_enabled = TRUE, entry_has_error = FALSE;
+	gboolean legacy_tls_remote = FALSE;
 
 	entry     = GTK_WIDGET (gtk_builder_get_object (builder, "tls_remote_entry"));
 	combo     = GTK_WIDGET (gtk_builder_get_object (builder, "tls_remote_mode_combo"));
@@ -1272,6 +1273,7 @@ tls_remote_changed (GtkWidget *widget, gpointer user_data)
 
 			entry_enabled = TRUE;
 			entry_has_error = !subject || !subject[0];
+			legacy_tls_remote = nm_streq (tls_remote_mode, TLS_REMOTE_MODE_LEGACY);
 		}
 	}
 
@@ -1280,9 +1282,17 @@ tls_remote_changed (GtkWidget *widget, gpointer user_data)
 		widget_set_error (entry);
 		gtk_widget_set_sensitive (ok_button, FALSE);
 	} else {
-		widget_unset_error (entry);
+		if (legacy_tls_remote) {
+			/* selecting tls-remote is not an error, but strongly discouraged. I wish
+			 * there would be a warning-class as well. Anyway, mark the widget as
+			 * erroneous, although this doesn't make the connection invalid (which
+			 * is an ugly inconsistency). */
+			widget_set_error (entry);
+		} else
+			widget_unset_error (entry);
 		gtk_widget_set_sensitive (ok_button, TRUE);
 	}
+
 }
 
 static void
diff --git a/properties/import-export.c b/properties/import-export.c
index 1993026..7b42e0b 100644
--- a/properties/import-export.c
+++ b/properties/import-export.c
@@ -1256,7 +1256,7 @@ do_import (const char *path, const char *contents, gsize contents_len, GError **
 		}
 
 		if (NM_IN_STRSET (params[0], NMV_OVPN_TAG_VERIFY_X509_NAME)) {
-			const char *type = "subject";
+			const char *type = NM_OPENVPN_VERIFY_X509_NAME_TYPE_SUBJECT;
 			gs_free char *item = NULL;
 
 			if (!args_params_check_nargs_minmax (params, 1, 2, &line_error))
diff --git a/properties/nm-openvpn-dialog.ui b/properties/nm-openvpn-dialog.ui
index b2ca176..5558b70 100644
--- a/properties/nm-openvpn-dialog.ui
+++ b/properties/nm-openvpn-dialog.ui
@@ -1918,6 +1918,8 @@ When enabled, connection will only succeed if the server certificate matches som
 Matching can either apply to the whole certificate subject (all the fields),
 or just the Common Name (CN field).
 
+The legacy option tls-remote is deprecated and removed from OpenVPN 2.4 and newer. Do not use it anymore.
+
 config: verify-x509-name subject-or-name [mode]
 config (legacy mode): tls-remote subject-or-name</property>
                             <property name="model">model9</property>
-- 
2.9.3