From b084ad7f2b0d511867d82be0c9f8feb1f0a86bc2 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 28 Oct 2014 08:56:07 -0400 Subject: [PATCH] libnm-core: canonicalize hardware addresses in settings Convert hardware addresses to canonical form (uppercase, leading zeros, colons) when setting them on/adding them to NMSetting properties. --- libnm-core/nm-setting-bridge.c | 3 ++- libnm-core/nm-setting-infiniband.c | 3 ++- libnm-core/nm-setting-wimax.c | 3 ++- libnm-core/nm-setting-wired.c | 35 +++++++++++++++++------------- libnm-core/nm-setting-wireless.c | 35 +++++++++++++++++------------- libnm-core/nm-utils-private.h | 2 ++ libnm-core/nm-utils.c | 20 +++++++++++++++++ libnm-core/tests/test-general.c | 5 +++++ 8 files changed, 73 insertions(+), 33 deletions(-) diff --git a/libnm-core/nm-setting-bridge.c b/libnm-core/nm-setting-bridge.c index 082f8edb3d..1c43208847 100644 --- a/libnm-core/nm-setting-bridge.c +++ b/libnm-core/nm-setting-bridge.c @@ -278,7 +278,8 @@ set_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_MAC_ADDRESS: g_free (priv->mac_address); - priv->mac_address = g_value_dup_string (value); + priv->mac_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value), + ETH_ALEN); break; case PROP_STP: priv->stp = g_value_get_boolean (value); diff --git a/libnm-core/nm-setting-infiniband.c b/libnm-core/nm-setting-infiniband.c index b7ae1a318a..61e532e357 100644 --- a/libnm-core/nm-setting-infiniband.c +++ b/libnm-core/nm-setting-infiniband.c @@ -318,7 +318,8 @@ set_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_MAC_ADDRESS: g_free (priv->mac_address); - priv->mac_address = g_value_dup_string (value); + priv->mac_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value), + INFINIBAND_ALEN); break; case PROP_MTU: priv->mtu = g_value_get_uint (value); diff --git a/libnm-core/nm-setting-wimax.c b/libnm-core/nm-setting-wimax.c index 113e27fc45..9de966f3fd 100644 --- a/libnm-core/nm-setting-wimax.c +++ b/libnm-core/nm-setting-wimax.c @@ -167,7 +167,8 @@ set_property (GObject *object, guint prop_id, break; case PROP_MAC_ADDRESS: g_free (priv->mac_address); - priv->mac_address = g_value_dup_string (value); + priv->mac_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value), + ETH_ALEN); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); diff --git a/libnm-core/nm-setting-wired.c b/libnm-core/nm-setting-wired.c index b49812769b..45c0827d6b 100644 --- a/libnm-core/nm-setting-wired.c +++ b/libnm-core/nm-setting-wired.c @@ -247,6 +247,7 @@ gboolean nm_setting_wired_add_mac_blacklist_item (NMSettingWired *setting, const char *mac) { NMSettingWiredPrivate *priv; + const char *candidate; int i; g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), FALSE); @@ -257,11 +258,12 @@ nm_setting_wired_add_mac_blacklist_item (NMSettingWired *setting, const char *ma priv = NM_SETTING_WIRED_GET_PRIVATE (setting); for (i = 0; i < priv->mac_address_blacklist->len; i++) { - if (!strcasecmp (mac, g_array_index (priv->mac_address_blacklist, char *, i))) + candidate = g_array_index (priv->mac_address_blacklist, char *, i); + if (nm_utils_hwaddr_matches (mac, -1, candidate, -1)) return FALSE; } - mac = g_ascii_strup (mac, -1); + mac = nm_utils_hwaddr_canonical (mac, ETH_ALEN); g_array_append_val (priv->mac_address_blacklist, mac); g_object_notify (G_OBJECT (setting), NM_SETTING_WIRED_MAC_ADDRESS_BLACKLIST); return TRUE; @@ -302,17 +304,16 @@ gboolean nm_setting_wired_remove_mac_blacklist_item_by_value (NMSettingWired *setting, const char *mac) { NMSettingWiredPrivate *priv; + const char *candidate; int i; g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), FALSE); g_return_val_if_fail (mac != NULL, FALSE); - if (!nm_utils_hwaddr_valid (mac, ETH_ALEN)) - return FALSE; - priv = NM_SETTING_WIRED_GET_PRIVATE (setting); for (i = 0; i < priv->mac_address_blacklist->len; i++) { - if (!strcasecmp (mac, g_array_index (priv->mac_address_blacklist, char *, i))) { + candidate = g_array_index (priv->mac_address_blacklist, char *, i); + if (!nm_utils_hwaddr_matches (mac, -1, candidate, -1)) { g_array_remove_index (priv->mac_address_blacklist, i); g_object_notify (G_OBJECT (setting), NM_SETTING_WIRED_MAC_ADDRESS_BLACKLIST); return TRUE; @@ -700,7 +701,9 @@ set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { NMSettingWiredPrivate *priv = NM_SETTING_WIRED_GET_PRIVATE (object); - char **blacklist; + const char * const *blacklist; + const char *mac; + int i; switch (prop_id) { case PROP_PORT: @@ -719,20 +722,22 @@ set_property (GObject *object, guint prop_id, break; case PROP_MAC_ADDRESS: g_free (priv->device_mac_address); - priv->device_mac_address = g_value_dup_string (value); + priv->device_mac_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value), + ETH_ALEN); break; case PROP_CLONED_MAC_ADDRESS: g_free (priv->cloned_mac_address); - priv->cloned_mac_address = g_value_dup_string (value); + priv->cloned_mac_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value), + ETH_ALEN); break; case PROP_MAC_ADDRESS_BLACKLIST: - blacklist = g_value_dup_boxed (value); + blacklist = g_value_get_boxed (value); g_array_set_size (priv->mac_address_blacklist, 0); - if (blacklist) { - g_array_set_size (priv->mac_address_blacklist, g_strv_length (blacklist)); - memcpy (priv->mac_address_blacklist->data, blacklist, - priv->mac_address_blacklist->len * sizeof (char *)); - g_free (blacklist); + if (blacklist && *blacklist) { + for (i = 0; blacklist[i]; i++) { + mac = _nm_utils_hwaddr_canonical_or_invalid (blacklist[i], ETH_ALEN); + g_array_append_val (priv->mac_address_blacklist, mac); + } } break; case PROP_MTU: diff --git a/libnm-core/nm-setting-wireless.c b/libnm-core/nm-setting-wireless.c index baf9a36a0e..5ca1169116 100644 --- a/libnm-core/nm-setting-wireless.c +++ b/libnm-core/nm-setting-wireless.c @@ -481,6 +481,7 @@ gboolean nm_setting_wireless_add_mac_blacklist_item (NMSettingWireless *setting, const char *mac) { NMSettingWirelessPrivate *priv; + const char *candidate; int i; g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), FALSE); @@ -491,11 +492,12 @@ nm_setting_wireless_add_mac_blacklist_item (NMSettingWireless *setting, const ch priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting); for (i = 0; i < priv->mac_address_blacklist->len; i++) { - if (!strcasecmp (mac, g_array_index (priv->mac_address_blacklist, char *, i))) + candidate = g_array_index (priv->mac_address_blacklist, char *, i); + if (nm_utils_hwaddr_matches (mac, -1, candidate, -1)) return FALSE; } - mac = g_ascii_strup (mac, -1); + mac = nm_utils_hwaddr_canonical (mac, ETH_ALEN); g_array_append_val (priv->mac_address_blacklist, mac); g_object_notify (G_OBJECT (setting), NM_SETTING_WIRELESS_MAC_ADDRESS_BLACKLIST); return TRUE; @@ -536,17 +538,16 @@ gboolean nm_setting_wireless_remove_mac_blacklist_item_by_value (NMSettingWireless *setting, const char *mac) { NMSettingWirelessPrivate *priv; + const char *candidate; int i; g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), FALSE); g_return_val_if_fail (mac != NULL, FALSE); - if (!nm_utils_hwaddr_valid (mac, ETH_ALEN)) - return FALSE; - priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting); for (i = 0; i < priv->mac_address_blacklist->len; i++) { - if (!strcasecmp (mac, g_array_index (priv->mac_address_blacklist, char *, i))) { + candidate = g_array_index (priv->mac_address_blacklist, char *, i); + if (!nm_utils_hwaddr_matches (mac, -1, candidate, -1)) { g_array_remove_index (priv->mac_address_blacklist, i); g_object_notify (G_OBJECT (setting), NM_SETTING_WIRELESS_MAC_ADDRESS_BLACKLIST); return TRUE; @@ -852,7 +853,9 @@ set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { NMSettingWirelessPrivate *priv = NM_SETTING_WIRELESS_GET_PRIVATE (object); - char **blacklist; + const char * const *blacklist; + const char *mac; + int i; switch (prop_id) { case PROP_SSID: @@ -883,20 +886,22 @@ set_property (GObject *object, guint prop_id, break; case PROP_MAC_ADDRESS: g_free (priv->device_mac_address); - priv->device_mac_address = g_value_dup_string (value); + priv->device_mac_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value), + ETH_ALEN); break; case PROP_CLONED_MAC_ADDRESS: g_free (priv->cloned_mac_address); - priv->cloned_mac_address = g_value_dup_string (value); + priv->cloned_mac_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value), + ETH_ALEN); break; case PROP_MAC_ADDRESS_BLACKLIST: - blacklist = g_value_dup_boxed (value); + blacklist = g_value_get_boxed (value); g_array_set_size (priv->mac_address_blacklist, 0); - if (blacklist) { - g_array_set_size (priv->mac_address_blacklist, g_strv_length (blacklist)); - memcpy (priv->mac_address_blacklist->data, blacklist, - priv->mac_address_blacklist->len * sizeof (char *)); - g_free (blacklist); + if (blacklist && *blacklist) { + for (i = 0; blacklist[i]; i++) { + mac = _nm_utils_hwaddr_canonical_or_invalid (blacklist[i], ETH_ALEN); + g_array_append_val (priv->mac_address_blacklist, mac); + } } break; case PROP_MTU: diff --git a/libnm-core/nm-utils-private.h b/libnm-core/nm-utils-private.h index a5b25ccf9e..bc42921514 100644 --- a/libnm-core/nm-utils-private.h +++ b/libnm-core/nm-utils-private.h @@ -67,4 +67,6 @@ char ** _nm_utils_strsplit_set (const char *str, const char *delimiters, int max_tokens); +char * _nm_utils_hwaddr_canonical_or_invalid (const char *mac, gssize length); + #endif diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index bafc326c7b..017c343b0d 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -2385,6 +2385,26 @@ nm_utils_hwaddr_canonical (const char *asc, gssize length) return g_strdup (nm_utils_hwaddr_ntoa (buf, length)); } +/* This is used to possibly canonicalize values passed to MAC address property + * setters. Unlike nm_utils_hwaddr_canonical(), it accepts %NULL, and if you + * pass it an invalid MAC address, it just returns that string rather than + * returning %NULL (so that we can return a proper error from verify() later). + */ +char * +_nm_utils_hwaddr_canonical_or_invalid (const char *mac, gssize length) +{ + char *canonical; + + if (!mac) + return NULL; + + canonical = nm_utils_hwaddr_canonical (mac, length); + if (canonical) + return canonical; + else + return g_strdup (mac); +} + /** * nm_utils_hwaddr_matches: * @hwaddr1: pointer to a binary or ASCII hardware address, or %NULL diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 6c111d04e5..88a296640e 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -2244,6 +2244,7 @@ test_hwaddr_canonical (void) const char *string = "00:1A:2B:03:44:05"; const char *lower_string = "00:1a:2b:03:44:05"; const char *short_string = "0:1a:2b:3:44:5"; + const char *hyphen_string = "00-1a-2b-03-44-05"; const char *invalid_string = "00:1A:2B"; char *canonical; @@ -2259,6 +2260,10 @@ test_hwaddr_canonical (void) g_assert_cmpstr (canonical, ==, string); g_free (canonical); + canonical = nm_utils_hwaddr_canonical (hyphen_string, ETH_ALEN); + g_assert_cmpstr (canonical, ==, string); + g_free (canonical); + canonical = nm_utils_hwaddr_canonical (invalid_string, ETH_ALEN); g_assert_cmpstr (canonical, ==, NULL);