From 4ecbe9f176b6d29aa51bf7800d0495e1c949f5bc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Aug 2016 14:38:53 +0200 Subject: [PATCH 1/1] fix recent change in behavior for "comp-lzo=no" setting openvpn supports 4 different ways for --comp-lzo: 1) no --comp-lzo option 2) --comp-lzo yes 3) --comp-lzo [adaptive] 4) --comp-lzo no Before commit 2ecf18c25a7bee7f0122d9d666a7e11cd8b55ea3, nm-openvpn only supported 1) and 2). Those were expressed in NM's connection by either omitting the comp-lzo setting or setting "comp-lzo=yes". Arguably due to a bug, old plasma-nm would configure connections with comp-lzo=no to mean 1), so after update of nm-openvpn to 2ecf18c2 those connections changed to mean 4), which broke some existing configurations. That was later attemted to be fixed in plasma-nm by commit [1], which however only affects new connections and cannot fix existing connections for users. Ultimatley, the "comp-lzo=no" setting is spoiled due to that. The fix is to add a new setting "comp-lzo=no-by-default" which shall have the meaning 4) and pass "--comp-lzo no" to openvpn. A connection with "comp-lzo=no" is again treated as 1). This fixes old connections that were created by old plasma-nm before commit [1] by restoring the old meaning. This however now breaks users of recent nm-openvpn which were deliberately setting "comp-lzo=no" to mean option 4), most notably users of recent plasma-nm (since commit [1]). Users of the properties plugin for nm-connection-editor/gnome-control-center are anyway unable to configure "comp-lzo=no" in the UI, so probably isn't a real issue for many users. plasma-nm bugs: [1] https://quickgit.kde.org/?p=plasma-nm.git&a=commit&h=31bcd5f2cffd1c19fbd10ab0f4172f2d82eff194 https://bugs.kde.org/show_bug.cgi?id=365816 https://bugzilla.redhat.com/show_bug.cgi?id=1365663 https://bugzilla.gnome.org/show_bug.cgi?id=769177 https://bugzilla.redhat.com/show_bug.cgi?id=1355688 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833166 (cherry picked from commit 62bdd278d69cc396479af9d05a0b776d5ad386bf) --- properties/auth-helpers.c | 5 ++++- properties/import-export.c | 14 ++++++++++++-- src/nm-openvpn-service.c | 24 ++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/properties/auth-helpers.c b/properties/auth-helpers.c index 85985c8..7244df1 100644 --- a/properties/auth-helpers.c +++ b/properties/auth-helpers.c @@ -2010,7 +2010,10 @@ advanced_dialog_new_hash_from_dialog (GtkWidget *dialog, GError **error) if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (widget))) { /* we only have a checkbox, which we either map to "--comp-lzo yes" or * no "--comp-lzo" flag. In the UI, we cannot express "--comp-lzo [adaptive]" - * or "--comp-lzo no". */ + * or "--comp-lzo no". + * + * Note that "--comp-lzo no" must be encoded as "comp-lzo=no-by-default" (bgo#769177). + */ g_hash_table_insert (hash, g_strdup (NM_OPENVPN_KEY_COMP_LZO), g_strdup ("yes")); } diff --git a/properties/import-export.c b/properties/import-export.c index 90b4631..ddb1cd3 100644 --- a/properties/import-export.c +++ b/properties/import-export.c @@ -951,13 +951,23 @@ do_import (const char *path, const char *contents, gsize contents_len, GError ** } if (NM_IN_STRSET (params[0], NMV_OVPN_TAG_COMP_LZO)) { + const char *v; + if (!args_params_check_nargs_minmax (params, 0, 1, &line_error)) goto handle_line_error; - if (!NM_IN_STRSET (params[1], NULL, "no", "yes", "adaptive")) { + + v = params[1] ?: "adaptive"; + + if (nm_streq (v, "no")) { + /* old plasma-nm used to set "comp-lzo=no" to mean unset, thus it spoiled + * to "no" option to be used in the connection. Workaround that, by instead + * using "no-by-default" (bgo#769177). */ + v = "no-by-default"; + } else if (!NM_IN_STRSET (v, "yes", "adaptive")) { line_error = g_strdup_printf (_("unsupported comp-lzo argument")); goto handle_line_error; } - setting_vpn_add_data_item (s_vpn, NM_OPENVPN_KEY_COMP_LZO, params[1] ?: "adaptive"); + setting_vpn_add_data_item (s_vpn, NM_OPENVPN_KEY_COMP_LZO, v); continue; } diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c index e5a1aa9..9238a45 100644 --- a/src/nm-openvpn-service.c +++ b/src/nm-openvpn-service.c @@ -1250,6 +1250,30 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin, } tmp = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_COMP_LZO); + + /* openvpn understands 4 different modes for --comp-lzo, which have + * different meaning: + * 1) no --comp-lzo option + * 2) --comp-lzo yes + * 3) --comp-lzo [adaptive] + * 4) --comp-lzo no + * + * In the past, nm-openvpn only supported 1) and 2) by having no + * comp-lzo connection setting or "comp-lzo=yes", respectively. + * + * However, old plasma-nm would set "comp-lzo=no" in the connection + * to mean 1). Thus, "comp-lzo=no" is spoiled to mean 4) in order + * to preserve backward compatibily. + * We use instead a special value "no-by-default" to express "no". + * + * See bgo#769177 + */ + if (NM_IN_STRSET (tmp, "no")) { + /* means no --comp-lzo option. */ + tmp = NULL; + } else if (NM_IN_STRSET (tmp, "no-by-default")) + tmp = "no"; + if (NM_IN_STRSET (tmp, "yes", "no", "adaptive")) { add_openvpn_arg (args, "--comp-lzo"); add_openvpn_arg (args, tmp); -- 2.7.4