ifcfg-rh: persist the wep key typebg/ifcfg-rh-wep-key-type-rh1518177

The wireless-security setting has a 'wep-key-type' property that is
used to specify the WEP key type and is needed because some keys could
be both a passphrase or a hex/ascii key.

The ifcfg-rh plugin currently stores the key type implicitly: if
wep-key-type is 'passphrase' it uses the KEY_PASSPHRASE%d variable, if
it's 'key' the KEY%d variable and when it's 'unknown' it uses either
variables depending on the detected type (preferring 'key' in case
both are compatible).

This means that some connections will be read differently from how
they were written, because once the KEY (or KEY_PASSPHRASE) is read
there is no way to know whether the 'wep-key-type' property was 'key'
(or 'passphrase') or 'unknown'.

Fix this by persisting the key type explicitly in the file. The new
variable is redundant in most cases because the variables used for
keys also determine the key type.

https://bugzilla.redhat.com/show_bug.cgi?id=1518177

4 files changed, 78 insertions, 47 deletions

diff --git a/libnm-core/nm-setting-wireless-security.c b/libnm-core/nm-setting-wireless-security.c
index 91bcbe7a23..de77a4938d 100644
--- a/libnm-core/nm-setting-wireless-security.c
+++ b/libnm-core/nm-setting-wireless-security.c
@@ -1814,10 +1814,11 @@ nm_setting_wireless_security_class_init (NMSettingWirelessSecurityClass *setting
 	 **/
 	/* ---ifcfg-rh---
 	 * property: wep-key-type
-	 * variable: KEY<i> or KEY_PASSPHRASE<i>(+)
+	 * variable: KEY<i> or KEY_PASSPHRASE<i>(+); KEY_TYPE(+)
 	 * description: KEY is used for "key" type (10 or 26 hexadecimal characters,
 	 *   or 5 or 13 character string prefixed with "s:"). KEY_PASSPHRASE is used
-	 *   for WEP passphrases.
+	 *   for WEP passphrases. KEY_TYPE specifies the key type and can be either
+	 *   'key' or 'passphrase'. KEY_TYPE is redundant and can be omitted.
 	 * example: KEY1=s:ahoj, KEY1=0a1c45bc02, KEY_PASSPHRASE1=mysupersecretkey
 	 * ---end---
 	 */
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 8ba04e474e..621f1c7683 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -2336,9 +2336,8 @@ add_one_wep_key (shvarFile *ifcfg,
                  NMSettingWirelessSecurity *s_wsec,
                  GError **error)
 {
-	char *key = NULL;
-	char *value = NULL;
-	gboolean success = FALSE;
+	gs_free char *key = NULL;
+	gs_free char *value = NULL;
 
 	g_return_val_if_fail (ifcfg != NULL, FALSE);
 	g_return_val_if_fail (shvar_key != NULL, FALSE);
@@ -2351,13 +2350,8 @@ add_one_wep_key (shvarFile *ifcfg,
 
 	/* Validate keys */
 	if (passphrase) {
-		if (strlen (value) && strlen (value) < 64) {
+		if (strlen (value) && strlen (value) < 64)
 			key = g_strdup (value);
-			g_object_set (G_OBJECT (s_wsec),
-			              NM_SETTING_WIRELESS_SECURITY_WEP_KEY_TYPE,
-			              NM_WEP_KEY_TYPE_PASSPHRASE,
-			              NULL);
-		}
 	} else {
 		if (strlen (value) == 10 || strlen (value) == 26) {
 			/* Hexadecimal WEP key */
@@ -2367,7 +2361,7 @@ add_one_wep_key (shvarFile *ifcfg,
 				if (!g_ascii_isxdigit (*p)) {
 					g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
 					             "Invalid hexadecimal WEP key.");
-					goto out;
+					return FALSE;
 				}
 				p++;
 			}
@@ -2381,7 +2375,7 @@ add_one_wep_key (shvarFile *ifcfg,
 				if (!g_ascii_isprint ((int) (*p))) {
 					g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
 					             "Invalid ASCII WEP key.");
-					goto out;
+					return FALSE;
 				}
 				p++;
 			}
@@ -2396,47 +2390,47 @@ add_one_wep_key (shvarFile *ifcfg,
 		}
 	}
 
-	if (key) {
-		nm_setting_wireless_security_set_wep_key (s_wsec, key_idx, key);
-		g_free (key);
-		success = TRUE;
-	} else {
+	if (!key) {
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
 		             "Invalid WEP key length.");
+		return FALSE;
 	}
 
-out:
-	g_free (value);
-	return success;
+	nm_setting_wireless_security_set_wep_key (s_wsec, key_idx, key);
+
+	return TRUE;
 }
 
 static gboolean
 read_wep_keys (shvarFile *ifcfg,
+               NMWepKeyType key_type,
                guint8 def_idx,
                NMSettingWirelessSecurity *s_wsec,
                GError **error)
 {
-	/* Try hex/ascii keys first */
-	if (!add_one_wep_key (ifcfg, "KEY1", 0, FALSE, s_wsec, error))
-		return FALSE;
-	if (!add_one_wep_key (ifcfg, "KEY2", 1, FALSE, s_wsec, error))
-		return FALSE;
-	if (!add_one_wep_key (ifcfg, "KEY3", 2, FALSE, s_wsec, error))
-		return FALSE;
-	if (!add_one_wep_key (ifcfg, "KEY4", 3, FALSE, s_wsec, error))
-		return FALSE;
-	if (!add_one_wep_key (ifcfg, "KEY", def_idx, FALSE, s_wsec, error))
-		return FALSE;
+	if (key_type != NM_WEP_KEY_TYPE_PASSPHRASE) {
+		if (!add_one_wep_key (ifcfg, "KEY1", 0, FALSE, s_wsec, error))
+			return FALSE;
+		if (!add_one_wep_key (ifcfg, "KEY2", 1, FALSE, s_wsec, error))
+			return FALSE;
+		if (!add_one_wep_key (ifcfg, "KEY3", 2, FALSE, s_wsec, error))
+			return FALSE;
+		if (!add_one_wep_key (ifcfg, "KEY4", 3, FALSE, s_wsec, error))
+			return FALSE;
+		if (!add_one_wep_key (ifcfg, "KEY", def_idx, FALSE, s_wsec, error))
+			return FALSE;
+	}
 
-	/* And then passphrases */
-	if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE1", 0, TRUE, s_wsec, error))
-		return FALSE;
-	if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE2", 1, TRUE, s_wsec, error))
-		return FALSE;
-	if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE3", 2, TRUE, s_wsec, error))
-		return FALSE;
-	if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE4", 3, TRUE, s_wsec, error))
-		return FALSE;
+	if (key_type != NM_WEP_KEY_TYPE_KEY) {
+		if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE1", 0, TRUE, s_wsec, error))
+			return FALSE;
+		if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE2", 1, TRUE, s_wsec, error))
+			return FALSE;
+		if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE3", 2, TRUE, s_wsec, error))
+			return FALSE;
+		if (!add_one_wep_key (ifcfg, "KEY_PASSPHRASE4", 3, TRUE, s_wsec, error))
+			return FALSE;
+	}
 
 	return TRUE;
 }
@@ -2501,19 +2495,40 @@ make_wep_setting (shvarFile *ifcfg,
 
 	/* Read keys in the ifcfg file if they are system-owned */
 	if (key_flags == NM_SETTING_SECRET_FLAG_NONE) {
-		if (!read_wep_keys (ifcfg, default_key_idx, s_wsec, error))
+		NMWepKeyType key_type;
+		const char *v;
+		gs_free char *to_free = NULL;
+
+		v = svGetValueStr (ifcfg, "KEY_TYPE", &to_free);
+		if (!v) {
+			key_type = NM_WEP_KEY_TYPE_UNKNOWN;
+		} else if (nm_streq (v, "key"))
+			key_type = NM_WEP_KEY_TYPE_KEY;
+		else if (nm_streq (v, "passphrase")) {
+			key_type = NM_WEP_KEY_TYPE_PASSPHRASE;
+		} else {
+			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
+			             "Invalid KEY_TYPE value '%s'", v);
+			return FALSE;
+		}
+
+		if (!read_wep_keys (ifcfg, key_type, default_key_idx, s_wsec, error))
 			return NULL;
 
 		/* Try to get keys from the "shadow" key file */
 		keys_ifcfg = utils_get_keys_ifcfg (file, FALSE);
 		if (keys_ifcfg) {
-			if (!read_wep_keys (keys_ifcfg, default_key_idx, s_wsec, error)) {
+			if (!read_wep_keys (keys_ifcfg, key_type, default_key_idx, s_wsec, error)) {
 				svCloseFile (keys_ifcfg);
 				return NULL;
 			}
 			svCloseFile (keys_ifcfg);
 			g_assert (error == NULL || *error == NULL);
 		}
+
+		g_object_set (G_OBJECT (s_wsec),
+		              NM_SETTING_WIRELESS_SECURITY_WEP_KEY_TYPE, key_type,
+		              NULL);
 	}
 
 	value = svGetValueStr_cp (ifcfg, "SECURITYMODE");
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index 8c3d14b7af..f614376fd5 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -662,12 +662,26 @@ write_wireless_security_setting (NMConnection *connection,
 
 	/* And write the new ones out */
 	if (wep) {
+		NMWepKeyType key_type;
+		const char *key_type_str = NULL;
+
 		/* Default WEP TX key index */
 		svSetValueInt64 (ifcfg, "DEFAULTKEY", nm_setting_wireless_security_get_wep_tx_keyidx(s_wsec) + 1);
 
-		for (i = 0; i < 4; i++) {
-			NMWepKeyType key_type;
+		key_type = nm_setting_wireless_security_get_wep_key_type (s_wsec);
+		switch (key_type) {
+		case NM_WEP_KEY_TYPE_KEY:
+			key_type_str = "key";
+			break;
+		case NM_WEP_KEY_TYPE_PASSPHRASE:
+			key_type_str = "passphrase";
+			break;
+		case NM_WEP_KEY_TYPE_UNKNOWN:
+			break;
+		}
+		svSetValue (ifcfg, "KEY_TYPE", key_type_str);
 
+		for (i = 0; i < 4; i++) {
 			key = nm_setting_wireless_security_get_wep_key (s_wsec, i);
 			if (key) {
 				gs_free char *ascii_key = NULL;
@@ -678,7 +692,6 @@ write_wireless_security_setting (NMConnection *connection,
 				 * are some passphrases that are indistinguishable from WEP hex
 				 * keys.
 				 */
-				key_type = nm_setting_wireless_security_get_wep_key_type (s_wsec);
 				if (key_type == NM_WEP_KEY_TYPE_UNKNOWN) {
 					if (nm_utils_wep_key_valid (key, NM_WEP_KEY_TYPE_KEY))
 						key_type = NM_WEP_KEY_TYPE_KEY;
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 07b121b261..284886a7ae 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -2686,7 +2686,7 @@ test_read_wifi_wep_passphrase (void)
 	g_assert (s_wsec);
 	g_assert_cmpstr (nm_setting_wireless_security_get_key_mgmt (s_wsec), ==, "none");
 	g_assert_cmpint (nm_setting_wireless_security_get_wep_tx_keyidx (s_wsec), ==, 0);
-	g_assert_cmpint (nm_setting_wireless_security_get_wep_key_type (s_wsec), ==, NM_WEP_KEY_TYPE_PASSPHRASE);
+	g_assert_cmpint (nm_setting_wireless_security_get_wep_key_type (s_wsec), ==, NM_WEP_KEY_TYPE_UNKNOWN);
 	g_assert_cmpstr (nm_setting_wireless_security_get_wep_key (s_wsec, 0), ==, "foobar222blahblah");
 	g_assert (!nm_setting_wireless_security_get_wep_key (s_wsec, 1));
 	g_assert (!nm_setting_wireless_security_get_wep_key (s_wsec, 2));
@@ -5758,6 +5758,7 @@ test_write_wifi_wep_40_ascii (void)
 	g_object_set (s_wsec,
 	              NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "none",
 	              NM_SETTING_WIRELESS_SECURITY_WEP_TX_KEYIDX, 2,
+	              NM_SETTING_WIRELESS_SECURITY_WEP_KEY_TYPE, NM_WEP_KEY_TYPE_KEY,
 	              NM_SETTING_WIRELESS_SECURITY_AUTH_ALG, "shared",
 	              NULL);
 	nm_setting_wireless_security_set_wep_key (s_wsec, 0, "lorem");
@@ -5845,6 +5846,7 @@ test_write_wifi_wep_104_ascii (void)
 	g_object_set (s_wsec,
 	              NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "none",
 	              NM_SETTING_WIRELESS_SECURITY_WEP_TX_KEYIDX, 0,
+	              NM_SETTING_WIRELESS_SECURITY_WEP_KEY_TYPE, NM_WEP_KEY_TYPE_UNKNOWN,
 	              NM_SETTING_WIRELESS_SECURITY_AUTH_ALG, "open",
 	              NULL);
 	nm_setting_wireless_security_set_wep_key (s_wsec, 0, "LoremIpsumSit");