From c6eb18ee05170727e55a25eb9897e36d65bc9f13 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 1 Dec 2017 11:52:40 +0100 Subject: [PATCH] ifcfg-rh: persist the wep key type 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 interpreted both as 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 --- libnm-core/nm-setting-wireless-security.c | 5 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 97 +++++++++++-------- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 19 +++- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 4 +- 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 or KEY_PASSPHRASE(+) + * variable: KEY or KEY_PASSPHRASE(+); 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..665839fbbf 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");