From 5a43b733b9897e1ba7563e3555edfdd2b168183a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Oct 2016 18:19:20 +0200 Subject: [PATCH] ifcfg-rh: change ESSID handling Let shvar.h do the escaping/unescaping of the ESSID. We should not treat a value differently whether it is quoted or not. Also, cutting away double quotes and calling svUnescape() is just wrong. Now, we write a value in hex if it contains non-printable characters or if the reader would treat it like a hex value. Reader treats ESSID now as hex if it starts with "0x" followed by pairs of hex digits. --- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 40 ++++------------- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 45 ++++++++++--------- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 27 ++++++++++- 3 files changed, 57 insertions(+), 55 deletions(-) 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 5ad1f43ad6..dbfdf74474 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -3437,39 +3437,19 @@ make_wireless_setting (shvarFile *ifcfg, g_free (value); } - value = svGetValue (ifcfg, "ESSID", TRUE); + value = svGetValue (ifcfg, "ESSID", FALSE); if (value) { - GBytes *bytes = NULL; + gs_unref_bytes GBytes *bytes = NULL; gsize ssid_len = 0; gsize value_len = strlen (value); - if ( (value_len >= 2) - && (value[0] == '"') - && (value[value_len - 1] == '"')) { - /* Strip the quotes and unescape */ - char *p = value + 1; - - value[value_len - 1] = '\0'; - svUnescape (p); - bytes = g_bytes_new (p, strlen (p)); - } else if ((value_len > 2) && (strncmp (value, "0x", 2) == 0)) { - /* Hex representation */ - if (value_len % 2) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, - "Invalid SSID '%s' size (looks like hex but length not multiple of 2)", - value); - g_free (value); - goto error; - } - - bytes = nm_utils_hexstr2bin (value); - if (!bytes) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, - "Invalid SSID '%s' (looks like hex SSID but isn't)", - value); - g_free (value); - goto error; - } + if ( value_len > 2 + && (value_len % 2) == 0 + && g_str_has_prefix (value, "0x") + && NM_STRCHAR_ALL (&value[2], ch, g_ascii_isxdigit (ch))) { + /* interpret the value as hex-digits iff value starts + * with "0x" followed by pairs of hex digits */ + bytes = nm_utils_hexstr2bin (&value[2]); } else bytes = g_bytes_new (value, value_len); @@ -3478,13 +3458,11 @@ make_wireless_setting (shvarFile *ifcfg, g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Invalid SSID '%s' (size %zu not between 1 and 32 inclusive)", value, ssid_len); - g_bytes_unref (bytes); g_free (value); goto error; } g_object_set (s_wireless, NM_SETTING_WIRELESS_SSID, bytes, NULL); - g_bytes_unref (bytes); g_free (value); } 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 0b6ed52cdc..ffdce42dda 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -774,13 +774,12 @@ write_wireless_setting (NMConnection *connection, GError **error) { NMSettingWireless *s_wireless; - char *tmp, *tmp2; + char *tmp; GBytes *ssid; const guint8 *ssid_data; gsize ssid_len; const char *mode, *bssid; const char *device_mac, *cloned_mac; - char buf[33]; guint32 mtu, chan, i; gboolean adhoc = FALSE, hex_ssid = FALSE; const char * const *macaddr_blacklist; @@ -836,10 +835,23 @@ write_wireless_setting (NMConnection *connection, /* If the SSID contains any non-printable characters, we need to use the * hex notation of the SSID instead. */ - for (i = 0; i < ssid_len; i++) { - if (!g_ascii_isprint (ssid_data[i])) { - hex_ssid = TRUE; - break; + if ( ssid_len > 2 + && ssid_data[0] == '0' + && ssid_data[1] == 'x') { + hex_ssid = TRUE; + for (i = 2; i < ssid_len; i++) { + if (!g_ascii_isxdigit (ssid_data[i])) { + hex_ssid = FALSE; + break; + } + } + } + if (!hex_ssid) { + for (i = 0; i < ssid_len; i++) { + if (!g_ascii_isprint (ssid_data[i])) { + hex_ssid = TRUE; + break; + } } } @@ -851,26 +863,15 @@ write_wireless_setting (NMConnection *connection, g_string_append (str, "0x"); for (i = 0; i < ssid_len; i++) g_string_append_printf (str, "%02X", ssid_data[i]); - svSetValue (ifcfg, "ESSID", str->str, TRUE); + svSetValue (ifcfg, "ESSID", str->str, FALSE); g_string_free (str, TRUE); } else { - const char *tmp_escaped; + char buf[33]; - /* Printable SSIDs always get quoted */ - memset (buf, 0, sizeof (buf)); + nm_assert (ssid_len <= 32); memcpy (buf, ssid_data, ssid_len); - tmp_escaped = svEscape (buf, &tmp); - - /* svEscape will usually quote the string, but just for consistency, - * if svEscape doesn't quote the ESSID, we quote it ourselves. - */ - if (tmp_escaped[0] != '"' && tmp_escaped[strlen (tmp_escaped) - 1] != '"') { - tmp2 = g_strdup_printf ("\"%s\"", tmp_escaped); - svSetValue (ifcfg, "ESSID", tmp2, TRUE); - g_free (tmp2); - } else - svSetValue (ifcfg, "ESSID", tmp_escaped, TRUE); - g_free (tmp); + buf[ssid_len] = '\0'; + svSetValue (ifcfg, "ESSID", buf, FALSE); } mode = nm_setting_wireless_get_mode (s_wireless); 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 d5e47e8e33..cd0a698ec9 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -2010,6 +2010,30 @@ test_read_wifi_open_ssid_hex (void) g_object_unref (connection); } +static void +test_read_wifi_open_ssid_hex_bad (void) +{ + gs_unref_object NMConnection *connection = NULL; + NMSettingConnection *s_con; + NMSettingWireless *s_wireless; + GBytes *ssid; + const char *expected_ssid = "0x626cxx"; + + connection = _connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wifi-open-ssid-bad-hex", + NULL, TYPE_WIRELESS, NULL); + + s_con = nm_connection_get_setting_connection (connection); + g_assert (s_con); + g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "System 0x626cxx (test-wifi-open-ssid-bad-hex)"); + + s_wireless = nm_connection_get_setting_wireless (connection); + g_assert (s_wireless); + + ssid = nm_setting_wireless_get_ssid (s_wireless); + g_assert (ssid); + g_assert_cmpmem (g_bytes_get_data (ssid, NULL), g_bytes_get_size (ssid), expected_ssid, strlen (expected_ssid)); +} + static void test_read_wifi_open_ssid_bad (gconstpointer data) { @@ -8946,7 +8970,6 @@ test_utils_ignore (void) #define TPATH "/settings/plugins/ifcfg-rh/" -#define TEST_IFCFG_WIFI_OPEN_SSID_BAD_HEX TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wifi-open-ssid-bad-hex" #define TEST_IFCFG_WIFI_OPEN_SSID_LONG_QUOTED TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wifi-open-ssid-long-quoted" #define TEST_IFCFG_WIFI_OPEN_SSID_LONG_HEX TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wifi-open-ssid-long-hex" @@ -9023,7 +9046,7 @@ int main (int argc, char **argv) g_test_add_func (TPATH "wifi/read/open", test_read_wifi_open); g_test_add_func (TPATH "wifi/read/open/auto", test_read_wifi_open_auto); g_test_add_func (TPATH "wifi/read/open/hex-ssid", test_read_wifi_open_ssid_hex); - g_test_add_data_func (TPATH "wifi/read/open-ssid/bad-hex", TEST_IFCFG_WIFI_OPEN_SSID_BAD_HEX, test_read_wifi_open_ssid_bad); + g_test_add_func (TPATH "wifi/read/open-ssid/bad-hex", test_read_wifi_open_ssid_hex_bad); g_test_add_data_func (TPATH "wifi/read/open-ssid/long-hex", TEST_IFCFG_WIFI_OPEN_SSID_LONG_HEX, test_read_wifi_open_ssid_bad); g_test_add_data_func (TPATH "wifi/read/open-ssid/long-quoted", TEST_IFCFG_WIFI_OPEN_SSID_LONG_QUOTED, test_read_wifi_open_ssid_bad); g_test_add_func (TPATH "wifi/read/open/quoted-ssid", test_read_wifi_open_ssid_quoted);