libnm-core: hwaddr-related fixes (bgo 739023)

15 files changed, 252 insertions, 126 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 928dd628c9..45c0827d6b 100644
--- a/libnm-core/nm-setting-wired.c
+++ b/libnm-core/nm-setting-wired.c
@@ -50,7 +50,7 @@ typedef struct {
 	gboolean auto_negotiate;
 	char *device_mac_address;
 	char *cloned_mac_address;
-	GSList *mac_address_blacklist;
+	GArray *mac_address_blacklist;
 	guint32 mtu;
 	char **s390_subchannels;
 	char *s390_nettype;
@@ -185,15 +185,17 @@ nm_setting_wired_get_cloned_mac_address (NMSettingWired *setting)
  * nm_setting_wired_get_mac_address_blacklist:
  * @setting: the #NMSettingWired
  *
- * Returns: (element-type utf8): the #NMSettingWired:mac-address-blacklist
- * property of the setting
+ * Returns: the #NMSettingWired:mac-address-blacklist property of the setting
  **/
-const GSList *
+const char * const *
 nm_setting_wired_get_mac_address_blacklist (NMSettingWired *setting)
 {
+	NMSettingWiredPrivate *priv;
+
 	g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), NULL);
 
-	return NM_SETTING_WIRED_GET_PRIVATE (setting)->mac_address_blacklist;
+	priv = NM_SETTING_WIRED_GET_PRIVATE (setting);
+	return (const char * const *) priv->mac_address_blacklist->data;
 }
 
 /**
@@ -207,7 +209,7 @@ nm_setting_wired_get_num_mac_blacklist_items (NMSettingWired *setting)
 {
 	g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), 0);
 
-	return g_slist_length (NM_SETTING_WIRED_GET_PRIVATE (setting)->mac_address_blacklist);
+	return NM_SETTING_WIRED_GET_PRIVATE (setting)->mac_address_blacklist->len;
 }
 
 /**
@@ -226,9 +228,9 @@ nm_setting_wired_get_mac_blacklist_item (NMSettingWired *setting, guint32 idx)
 	g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), NULL);
 
 	priv = NM_SETTING_WIRED_GET_PRIVATE (setting);
-	g_return_val_if_fail (idx <= g_slist_length (priv->mac_address_blacklist), NULL);
+	g_return_val_if_fail (idx <= priv->mac_address_blacklist->len, NULL);
 
-	return (const char *) g_slist_nth_data (priv->mac_address_blacklist, idx);
+	return g_array_index (priv->mac_address_blacklist, const char *, idx);
 }
 
 /**
@@ -245,7 +247,8 @@ gboolean
 nm_setting_wired_add_mac_blacklist_item (NMSettingWired *setting, const char *mac)
 {
 	NMSettingWiredPrivate *priv;
-	GSList *iter;
+	const char *candidate;
+	int i;
 
 	g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), FALSE);
 	g_return_val_if_fail (mac != NULL, FALSE);
@@ -254,13 +257,14 @@ nm_setting_wired_add_mac_blacklist_item (NMSettingWired *setting, const char *ma
 		return FALSE;
 
 	priv = NM_SETTING_WIRED_GET_PRIVATE (setting);
-	for (iter = priv->mac_address_blacklist; iter; iter = g_slist_next (iter)) {
-		if (!strcasecmp (mac, (char *) iter->data))
+	for (i = 0; i < priv->mac_address_blacklist->len; i++) {
+		candidate = g_array_index (priv->mac_address_blacklist, char *, i);
+		if (nm_utils_hwaddr_matches (mac, -1, candidate, -1))
 			return FALSE;
 	}
 
-	priv->mac_address_blacklist = g_slist_append (priv->mac_address_blacklist,
-	                                              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;
 }
@@ -276,16 +280,13 @@ void
 nm_setting_wired_remove_mac_blacklist_item (NMSettingWired *setting, guint32 idx)
 {
 	NMSettingWiredPrivate *priv;
-	GSList *elt;
 
 	g_return_if_fail (NM_IS_SETTING_WIRED (setting));
 
 	priv = NM_SETTING_WIRED_GET_PRIVATE (setting);
-	elt = g_slist_nth (priv->mac_address_blacklist, idx);
-	g_return_if_fail (elt != NULL);
+	g_return_if_fail (idx < priv->mac_address_blacklist->len);
 
-	g_free (elt->data);
-	priv->mac_address_blacklist = g_slist_delete_link (priv->mac_address_blacklist, elt);
+	g_array_remove_index (priv->mac_address_blacklist, idx);
 	g_object_notify (G_OBJECT (setting), NM_SETTING_WIRED_MAC_ADDRESS_BLACKLIST);
 }
 
@@ -303,18 +304,17 @@ gboolean
 nm_setting_wired_remove_mac_blacklist_item_by_value (NMSettingWired *setting, const char *mac)
 {
 	NMSettingWiredPrivate *priv;
-	GSList *iter;
+	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 (iter = priv->mac_address_blacklist; iter; iter = g_slist_next (iter)) {
-		if (!strcasecmp (mac, (char *) iter->data)) {
-			priv->mac_address_blacklist = g_slist_delete_link (priv->mac_address_blacklist, iter);
+	for (i = 0; i < priv->mac_address_blacklist->len; 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;
 		}
@@ -333,8 +333,7 @@ nm_setting_wired_clear_mac_blacklist_items (NMSettingWired *setting)
 {
 	g_return_if_fail (NM_IS_SETTING_WIRED (setting));
 
-	g_slist_free_full (NM_SETTING_WIRED_GET_PRIVATE (setting)->mac_address_blacklist, g_free);
-	NM_SETTING_WIRED_GET_PRIVATE (setting)->mac_address_blacklist = NULL;
+	g_array_set_size (NM_SETTING_WIRED_GET_PRIVATE (setting)->mac_address_blacklist, 0);
 	g_object_notify (G_OBJECT (setting), NM_SETTING_WIRED_MAC_ADDRESS_BLACKLIST);
 }
 
@@ -563,8 +562,8 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 	const char *valid_duplex[] = { "half", "full", NULL };
 	const char *valid_nettype[] = { "qeth", "lcs", "ctc", NULL };
 	GHashTableIter iter;
-	GSList* mac_blacklist_iter;
 	const char *key, *value;
+	int i;
 
 	if (priv->port && !_nm_utils_string_in_list (priv->port, valid_ports)) {
 		g_set_error (error,
@@ -595,14 +594,15 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		return FALSE;
 	}
 
-	for (mac_blacklist_iter = priv->mac_address_blacklist; mac_blacklist_iter;
-	     mac_blacklist_iter = mac_blacklist_iter->next) {
-		if (!nm_utils_hwaddr_valid (mac_blacklist_iter->data, ETH_ALEN)) {
+	for (i = 0; i < priv->mac_address_blacklist->len; i++) {
+		const char *mac = g_array_index (priv->mac_address_blacklist, const char *, i);
+
+		if (!nm_utils_hwaddr_valid (mac, ETH_ALEN)) {
 			g_set_error (error,
 			             NM_CONNECTION_ERROR,
 			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
 			             _("'%s' is not a valid MAC address"),
-			             (const char *) mac_blacklist_iter->data);
+			             mac);
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRED_SETTING_NAME, NM_SETTING_WIRED_MAC_ADDRESS_BLACKLIST);
 			return FALSE;
 		}
@@ -658,11 +658,21 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 }
 
 static void
+clear_blacklist_item (char **item_p)
+{
+	g_free (*item_p);
+}
+
+static void
 nm_setting_wired_init (NMSettingWired *setting)
 {
 	NMSettingWiredPrivate *priv = NM_SETTING_WIRED_GET_PRIVATE (setting);
 
 	priv->s390_options = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+
+	/* We use GArray rather than GPtrArray so it will automatically be NULL-terminated */
+	priv->mac_address_blacklist = g_array_new (TRUE, FALSE, sizeof (char *));
+	g_array_set_clear_func (priv->mac_address_blacklist, (GDestroyNotify) clear_blacklist_item);
 }
 
 static void
@@ -678,7 +688,7 @@ finalize (GObject *object)
 
 	g_free (priv->device_mac_address);
 	g_free (priv->cloned_mac_address);
-	g_slist_free_full (priv->mac_address_blacklist, g_free);
+	g_array_unref (priv->mac_address_blacklist);
 
 	if (priv->s390_subchannels)
 		g_strfreev (priv->s390_subchannels);
@@ -691,6 +701,9 @@ set_property (GObject *object, guint prop_id,
               const GValue *value, GParamSpec *pspec)
 {
 	NMSettingWiredPrivate *priv = NM_SETTING_WIRED_GET_PRIVATE (object);
+	const char * const *blacklist;
+	const char *mac;
+	int i;
 
 	switch (prop_id) {
 	case PROP_PORT:
@@ -709,15 +722,23 @@ 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:
-		g_slist_free_full (priv->mac_address_blacklist, g_free);
-		priv->mac_address_blacklist = _nm_utils_strv_to_slist (g_value_get_boxed (value));
+		blacklist = g_value_get_boxed (value);
+		g_array_set_size (priv->mac_address_blacklist, 0);
+		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:
 		priv->mtu = g_value_get_uint (value);
@@ -768,7 +789,7 @@ get_property (GObject *object, guint prop_id,
 		g_value_set_string (value, nm_setting_wired_get_cloned_mac_address (setting));
 		break;
 	case PROP_MAC_ADDRESS_BLACKLIST:
-		g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->mac_address_blacklist));
+		g_value_set_boxed (value, (char **) priv->mac_address_blacklist->data);
 		break;
 	case PROP_MTU:
 		g_value_set_uint (value, nm_setting_wired_get_mtu (setting));
diff --git a/libnm-core/nm-setting-wired.h b/libnm-core/nm-setting-wired.h
index ba8b4303fc..0439291cc5 100644
--- a/libnm-core/nm-setting-wired.h
+++ b/libnm-core/nm-setting-wired.h
@@ -73,7 +73,7 @@ gboolean          nm_setting_wired_get_auto_negotiate   (NMSettingWired *setting
 const char *      nm_setting_wired_get_mac_address      (NMSettingWired *setting);
 const char *      nm_setting_wired_get_cloned_mac_address (NMSettingWired *setting);
 
-const GSList     *nm_setting_wired_get_mac_address_blacklist   (NMSettingWired *setting);
+const char * const *nm_setting_wired_get_mac_address_blacklist   (NMSettingWired *setting);
 guint32           nm_setting_wired_get_num_mac_blacklist_items (NMSettingWired *setting);
 const char *      nm_setting_wired_get_mac_blacklist_item      (NMSettingWired *setting,
                                                                 guint32 idx);
diff --git a/libnm-core/nm-setting-wireless.c b/libnm-core/nm-setting-wireless.c
index 2c0dd58455..5ca1169116 100644
--- a/libnm-core/nm-setting-wireless.c
+++ b/libnm-core/nm-setting-wireless.c
@@ -54,7 +54,7 @@ typedef struct {
 	guint32 tx_power;
 	char *device_mac_address;
 	char *cloned_mac_address;
-	GSList *mac_address_blacklist;
+	GArray *mac_address_blacklist;
 	guint32 mtu;
 	GSList *seen_bssids;
 	gboolean hidden;
@@ -419,15 +419,17 @@ nm_setting_wireless_get_cloned_mac_address (NMSettingWireless *setting)
  * nm_setting_wireless_get_mac_address_blacklist:
  * @setting: the #NMSettingWireless
  *
- * Returns: (element-type utf8): the
- * #NMSettingWireless:mac-address-blacklist property of the setting
+ * Returns: the #NMSettingWireless:mac-address-blacklist property of the setting
  **/
-const GSList *
+const char * const *
 nm_setting_wireless_get_mac_address_blacklist (NMSettingWireless *setting)
 {
+	NMSettingWirelessPrivate *priv;
+
 	g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), NULL);
 
-	return NM_SETTING_WIRELESS_GET_PRIVATE (setting)->mac_address_blacklist;
+	priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
+	return (const char * const *) priv->mac_address_blacklist->data;
 }
 
 /**
@@ -441,7 +443,7 @@ nm_setting_wireless_get_num_mac_blacklist_items (NMSettingWireless *setting)
 {
 	g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), 0);
 
-	return g_slist_length (NM_SETTING_WIRELESS_GET_PRIVATE (setting)->mac_address_blacklist);
+	return NM_SETTING_WIRELESS_GET_PRIVATE (setting)->mac_address_blacklist->len;
 }
 
 /**
@@ -460,9 +462,9 @@ nm_setting_wireless_get_mac_blacklist_item (NMSettingWireless *setting, guint32
 	g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), NULL);
 
 	priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
-	g_return_val_if_fail (idx <= g_slist_length (priv->mac_address_blacklist), NULL);
+	g_return_val_if_fail (idx <= priv->mac_address_blacklist->len, NULL);
 
-	return (const char *) g_slist_nth_data (priv->mac_address_blacklist, idx);
+	return g_array_index (priv->mac_address_blacklist, const char *, idx);
 }
 
 /**
@@ -479,7 +481,8 @@ gboolean
 nm_setting_wireless_add_mac_blacklist_item (NMSettingWireless *setting, const char *mac)
 {
 	NMSettingWirelessPrivate *priv;
-	GSList *iter;
+	const char *candidate;
+	int i;
 
 	g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), FALSE);
 	g_return_val_if_fail (mac != NULL, FALSE);
@@ -488,13 +491,14 @@ nm_setting_wireless_add_mac_blacklist_item (NMSettingWireless *setting, const ch
 		return FALSE;
 
 	priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
-	for (iter = priv->mac_address_blacklist; iter; iter = g_slist_next (iter)) {
-		if (!strcasecmp (mac, (char *) iter->data))
+	for (i = 0; i < priv->mac_address_blacklist->len; i++) {
+		candidate = g_array_index (priv->mac_address_blacklist, char *, i);
+		if (nm_utils_hwaddr_matches (mac, -1, candidate, -1))
 			return FALSE;
 	}
 
-	priv->mac_address_blacklist = g_slist_append (priv->mac_address_blacklist,
-	                                              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;
 }
@@ -510,16 +514,13 @@ void
 nm_setting_wireless_remove_mac_blacklist_item (NMSettingWireless *setting, guint32 idx)
 {
 	NMSettingWirelessPrivate *priv;
-	GSList *elt;
 
 	g_return_if_fail (NM_IS_SETTING_WIRELESS (setting));
 
 	priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
-	elt = g_slist_nth (priv->mac_address_blacklist, idx);
-	g_return_if_fail (elt != NULL);
+	g_return_if_fail (idx < priv->mac_address_blacklist->len);
 
-	g_free (elt->data);
-	priv->mac_address_blacklist = g_slist_delete_link (priv->mac_address_blacklist, elt);
+	g_array_remove_index (priv->mac_address_blacklist, idx);
 	g_object_notify (G_OBJECT (setting), NM_SETTING_WIRELESS_MAC_ADDRESS_BLACKLIST);
 }
 
@@ -537,18 +538,17 @@ gboolean
 nm_setting_wireless_remove_mac_blacklist_item_by_value (NMSettingWireless *setting, const char *mac)
 {
 	NMSettingWirelessPrivate *priv;
-	GSList *iter;
+	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 (iter = priv->mac_address_blacklist; iter; iter = g_slist_next (iter)) {
-		if (!strcasecmp (mac, (char *) iter->data)) {
-			priv->mac_address_blacklist = g_slist_delete_link (priv->mac_address_blacklist, iter);
+	for (i = 0; i < priv->mac_address_blacklist->len; 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;
 		}
@@ -567,8 +567,7 @@ nm_setting_wireless_clear_mac_blacklist_items (NMSettingWireless *setting)
 {
 	g_return_if_fail (NM_IS_SETTING_WIRELESS (setting));
 
-	g_slist_free_full (NM_SETTING_WIRELESS_GET_PRIVATE (setting)->mac_address_blacklist, g_free);
-	NM_SETTING_WIRELESS_GET_PRIVATE (setting)->mac_address_blacklist = NULL;
+	g_array_set_size (NM_SETTING_WIRELESS_GET_PRIVATE (setting)->mac_address_blacklist, 0);
 	g_object_notify (G_OBJECT (setting), NM_SETTING_WIRELESS_MAC_ADDRESS_BLACKLIST);
 }
 
@@ -683,6 +682,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 	const char *valid_modes[] = { NM_SETTING_WIRELESS_MODE_INFRA, NM_SETTING_WIRELESS_MODE_ADHOC, NM_SETTING_WIRELESS_MODE_AP, NULL };
 	const char *valid_bands[] = { "a", "bg", NULL };
 	GSList *iter;
+	int i;
 	gsize length;
 
 	if (!priv->ssid) {
@@ -773,13 +773,15 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		return FALSE;
 	}
 
-	for (iter = priv->mac_address_blacklist; iter; iter = iter->next) {
-		if (!nm_utils_hwaddr_valid (iter->data, ETH_ALEN)) {
+	for (i = 0; i < priv->mac_address_blacklist->len; i++) {
+		const char *mac = g_array_index (priv->mac_address_blacklist, const char *, i);
+
+		if (!nm_utils_hwaddr_valid (mac, ETH_ALEN)) {
 			g_set_error (error,
 			             NM_CONNECTION_ERROR,
 			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
 			             _("'%s' is not a valid MAC address"),
-			             (const char *) iter->data);
+			             mac);
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SETTING_NAME, NM_SETTING_WIRELESS_MAC_ADDRESS_BLACKLIST);
 			return FALSE;
 		}
@@ -812,8 +814,19 @@ nm_setting_wireless_get_security (NMSetting    *setting,
 }
 
 static void
+clear_blacklist_item (char **item_p)
+{
+	g_free (*item_p);
+}
+
+static void
 nm_setting_wireless_init (NMSettingWireless *setting)
 {
+	NMSettingWirelessPrivate *priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
+
+	/* We use GArray rather than GPtrArray so it will automatically be NULL-terminated */
+	priv->mac_address_blacklist = g_array_new (TRUE, FALSE, sizeof (char *));
+	g_array_set_clear_func (priv->mac_address_blacklist, (GDestroyNotify) clear_blacklist_item);
 }
 
 static void
@@ -829,7 +842,7 @@ finalize (GObject *object)
 	g_free (priv->bssid);
 	g_free (priv->device_mac_address);
 	g_free (priv->cloned_mac_address);
-	g_slist_free_full (priv->mac_address_blacklist, g_free);
+	g_array_unref (priv->mac_address_blacklist);
 	g_slist_free_full (priv->seen_bssids, g_free);
 
 	G_OBJECT_CLASS (nm_setting_wireless_parent_class)->finalize (object);
@@ -840,6 +853,9 @@ set_property (GObject *object, guint prop_id,
               const GValue *value, GParamSpec *pspec)
 {
 	NMSettingWirelessPrivate *priv = NM_SETTING_WIRELESS_GET_PRIVATE (object);
+	const char * const *blacklist;
+	const char *mac;
+	int i;
 
 	switch (prop_id) {
 	case PROP_SSID:
@@ -870,15 +886,23 @@ 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:
-		g_slist_free_full (priv->mac_address_blacklist, g_free);
-		priv->mac_address_blacklist = _nm_utils_strv_to_slist (g_value_get_boxed (value));
+		blacklist = g_value_get_boxed (value);
+		g_array_set_size (priv->mac_address_blacklist, 0);
+		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:
 		priv->mtu = g_value_get_uint (value);
@@ -932,7 +956,7 @@ get_property (GObject *object, guint prop_id,
 		g_value_set_string (value, nm_setting_wireless_get_cloned_mac_address (setting));
 		break;
 	case PROP_MAC_ADDRESS_BLACKLIST:
-		g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->mac_address_blacklist));
+		g_value_set_boxed (value, (char **) priv->mac_address_blacklist->data);
 		break;
 	case PROP_MTU:
 		g_value_set_uint (value, nm_setting_wireless_get_mtu (setting));
diff --git a/libnm-core/nm-setting-wireless.h b/libnm-core/nm-setting-wireless.h
index b402934cf7..e9e6ae7dab 100644
--- a/libnm-core/nm-setting-wireless.h
+++ b/libnm-core/nm-setting-wireless.h
@@ -104,7 +104,7 @@ guint32           nm_setting_wireless_get_tx_power           (NMSettingWireless
 const char       *nm_setting_wireless_get_mac_address        (NMSettingWireless *setting);
 const char       *nm_setting_wireless_get_cloned_mac_address (NMSettingWireless *setting);
 
-const GSList     *nm_setting_wireless_get_mac_address_blacklist   (NMSettingWireless *setting);
+const char * const *nm_setting_wireless_get_mac_address_blacklist   (NMSettingWireless *setting);
 guint32           nm_setting_wireless_get_num_mac_blacklist_items (NMSettingWireless *setting);
 const char *      nm_setting_wireless_get_mac_blacklist_item      (NMSettingWireless *setting,
                                                                    guint32 idx);
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 d3c3bef1dc..017c343b0d 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -2353,6 +2353,59 @@ nm_utils_hwaddr_valid (const char *asc, gssize length)
 }
 
 /**
+ * nm_utils_hwaddr_canonical:
+ * @asc: the ASCII representation of a hardware address
+ * @length: the length of address that @asc is expected to convert to
+ *   (or -1 to accept any length up to %NM_UTILS_HWADDR_LEN_MAX)
+ *
+ * Parses @asc to see if it is a valid hardware address of the given
+ * length, and if so, returns it in canonical form (uppercase, with
+ * leading 0s as needed, and with colons rather than hyphens).
+ *
+ * Return value: (transfer full): the canonicalized address if @asc appears to
+ *   be a valid hardware address of the indicated length, %NULL if not.
+ */
+char *
+nm_utils_hwaddr_canonical (const char *asc, gssize length)
+{
+	guint8 buf[NM_UTILS_HWADDR_LEN_MAX];
+
+	g_return_val_if_fail (asc != NULL, NULL);
+	g_return_val_if_fail (length == -1 || (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX), NULL);
+
+	if (length == -1) {
+		length = hwaddr_binary_len (asc);
+		if (length == 0 || length > NM_UTILS_HWADDR_LEN_MAX)
+			return NULL;
+	}
+
+	if (nm_utils_hwaddr_aton (asc, buf, length) == NULL)
+		return NULL;
+
+	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
  * @hwaddr1_len: size of @hwaddr1, or -1 if @hwaddr1 is ASCII
diff --git a/libnm-core/nm-utils.h b/libnm-core/nm-utils.h
index 7402cd921d..a94730793e 100644
--- a/libnm-core/nm-utils.h
+++ b/libnm-core/nm-utils.h
@@ -141,17 +141,18 @@ const char *nm_utils_wifi_strength_bars (guint8 strength);
  */
 #define NM_UTILS_HWADDR_LEN_MAX 20 /* INFINIBAND_ALEN */
 
-gsize       nm_utils_hwaddr_len   (int type) G_GNUC_PURE;
-
-char       *nm_utils_hwaddr_ntoa  (gconstpointer addr, gsize length);
-GByteArray *nm_utils_hwaddr_atoba (const char *asc, gsize length);
-guint8     *nm_utils_hwaddr_aton  (const char *asc, gpointer buffer, gsize length);
-
-gboolean    nm_utils_hwaddr_valid (const char *asc, gssize length);
-gboolean    nm_utils_hwaddr_matches (gconstpointer hwaddr1,
-                                     gssize        hwaddr1_len,
-                                     gconstpointer hwaddr2,
-                                     gssize        hwaddr2_len);
+gsize       nm_utils_hwaddr_len       (int type) G_GNUC_PURE;
+
+char       *nm_utils_hwaddr_ntoa      (gconstpointer addr, gsize length);
+GByteArray *nm_utils_hwaddr_atoba     (const char *asc, gsize length);
+guint8     *nm_utils_hwaddr_aton      (const char *asc, gpointer buffer, gsize length);
+
+gboolean    nm_utils_hwaddr_valid     (const char *asc, gssize length);
+char       *nm_utils_hwaddr_canonical (const char *asc, gssize length);
+gboolean    nm_utils_hwaddr_matches   (gconstpointer hwaddr1,
+                                       gssize        hwaddr1_len,
+                                       gconstpointer hwaddr2,
+                                       gssize        hwaddr2_len);
 
 char *nm_utils_bin2hexstr (const char *bytes, int len, int final_len);
 int   nm_utils_hex2byte   (const char *hex);
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index 32061426ae..88a296640e 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -2239,6 +2239,40 @@ test_hwaddr_equal (void)
 }
 
 static void
+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;
+
+	canonical = nm_utils_hwaddr_canonical (string, ETH_ALEN);
+	g_assert_cmpstr (canonical, ==, string);
+	g_free (canonical);
+
+	canonical = nm_utils_hwaddr_canonical (lower_string, ETH_ALEN);
+	g_assert_cmpstr (canonical, ==, string);
+	g_free (canonical);
+
+	canonical = nm_utils_hwaddr_canonical (short_string, ETH_ALEN);
+	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);
+
+	canonical = nm_utils_hwaddr_canonical (invalid_string, -1);
+	g_assert_cmpstr (canonical, ==, invalid_string);
+	g_free (canonical);
+}
+
+static void
 test_connection_changed_cb (NMConnection *connection, gboolean *data)
 {
 	*data = TRUE;
@@ -3372,6 +3406,7 @@ int main (int argc, char **argv)
 	g_test_add_func ("/core/general/test_hwaddr_aton_no_leading_zeros", test_hwaddr_aton_no_leading_zeros);
 	g_test_add_func ("/core/general/test_hwaddr_aton_malformed", test_hwaddr_aton_malformed);
 	g_test_add_func ("/core/general/test_hwaddr_equal", test_hwaddr_equal);
+	g_test_add_func ("/core/general/test_hwaddr_canonical", test_hwaddr_canonical);
 
 	g_test_add_func ("/core/general/test_ip4_prefix_to_netmask", test_ip4_prefix_to_netmask);
 	g_test_add_func ("/core/general/test_ip4_netmask_to_prefix", test_ip4_netmask_to_prefix);
diff --git a/libnm/libnm.ver b/libnm/libnm.ver
index 64a3b507df..bc08e7fd01 100644
--- a/libnm/libnm.ver
+++ b/libnm/libnm.ver
@@ -809,6 +809,7 @@ global:
 	nm_utils_hexstr2bin;
 	nm_utils_hwaddr_atoba;
 	nm_utils_hwaddr_aton;
+	nm_utils_hwaddr_canonical;
 	nm_utils_hwaddr_len;
 	nm_utils_hwaddr_matches;
 	nm_utils_hwaddr_ntoa;
diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c
index e4b726a221..1ee689261c 100644
--- a/src/devices/nm-device-ethernet.c
+++ b/src/devices/nm-device-ethernet.c
@@ -442,7 +442,8 @@ check_connection_compatible (NMDevice *device, NMConnection *connection)
 	if (s_wired) {
 		const char *mac;
 		gboolean try_mac = TRUE;
-		const GSList *mac_blacklist, *mac_blacklist_iter;
+		const char * const *mac_blacklist;
+		int i;
 
 		if (!match_subchans (self, s_wired, &try_mac))
 			return FALSE;
@@ -453,14 +454,13 @@ check_connection_compatible (NMDevice *device, NMConnection *connection)
 
 		/* Check for MAC address blacklist */
 		mac_blacklist = nm_setting_wired_get_mac_address_blacklist (s_wired);
-		for (mac_blacklist_iter = mac_blacklist; mac_blacklist_iter;
-			 mac_blacklist_iter = g_slist_next (mac_blacklist_iter)) {
-			if (!nm_utils_hwaddr_valid (mac_blacklist_iter->data, ETH_ALEN)) {
+		for (i = 0; mac_blacklist[i]; i++) {
+			if (!nm_utils_hwaddr_valid (mac_blacklist[i], ETH_ALEN)) {
 				g_warn_if_reached ();
 				return FALSE;
 			}
 
-			if (nm_utils_hwaddr_matches (mac_blacklist_iter->data, -1, priv->perm_hw_addr, -1))
+			if (nm_utils_hwaddr_matches (mac_blacklist[i], -1, priv->perm_hw_addr, -1))
 				return FALSE;
 		}
 	}
diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c
index e3e927a5c7..094b78ca01 100644
--- a/src/devices/wifi/nm-device-wifi.c
+++ b/src/devices/wifi/nm-device-wifi.c
@@ -801,7 +801,8 @@ check_connection_compatible (NMDevice *device, NMConnection *connection)
 	NMSettingConnection *s_con;
 	NMSettingWireless *s_wireless;
 	const char *mac;
-	const GSList *mac_blacklist, *mac_blacklist_iter;
+	const char * const *mac_blacklist;
+	int i;
 	const char *mode;
 
 	if (!NM_DEVICE_CLASS (nm_device_wifi_parent_class)->check_connection_compatible (device, connection))
@@ -823,14 +824,13 @@ check_connection_compatible (NMDevice *device, NMConnection *connection)
 
 	/* Check for MAC address blacklist */
 	mac_blacklist = nm_setting_wireless_get_mac_address_blacklist (s_wireless);
-	for (mac_blacklist_iter = mac_blacklist; mac_blacklist_iter;
-	     mac_blacklist_iter = g_slist_next (mac_blacklist_iter)) {
-		if (!nm_utils_hwaddr_valid (mac_blacklist_iter->data, ETH_ALEN)) {
+	for (i = 0; mac_blacklist[i]; i++) {
+		if (!nm_utils_hwaddr_valid (mac_blacklist[i], ETH_ALEN)) {
 			g_warn_if_reached ();
 			return FALSE;
 		}
 
-		if (nm_utils_hwaddr_matches (mac_blacklist_iter->data, -1, priv->perm_hw_addr, -1))
+		if (nm_utils_hwaddr_matches (mac_blacklist[i], -1, priv->perm_hw_addr, -1))
 			return FALSE;
 	}
 
diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c
index 90bbe71eb2..c088525dac 100644
--- a/src/settings/plugins/ifcfg-rh/writer.c
+++ b/src/settings/plugins/ifcfg-rh/writer.c
@@ -811,7 +811,7 @@ write_wireless_setting (NMConnection *connection,
 	char buf[33];
 	guint32 mtu, chan, i;
 	gboolean adhoc = FALSE, hex_ssid = FALSE;
-	const GSList *macaddr_blacklist;
+	const char * const *macaddr_blacklist;
 
 	s_wireless = nm_connection_get_setting_wireless (connection);
 	if (!s_wireless) {
@@ -828,19 +828,12 @@ write_wireless_setting (NMConnection *connection,
 
 	svSetValue (ifcfg, "HWADDR_BLACKLIST", NULL, FALSE);
 	macaddr_blacklist = nm_setting_wireless_get_mac_address_blacklist (s_wireless);
-	if (macaddr_blacklist) {
-		const GSList *iter;
-		GString *blacklist_str = g_string_new (NULL);
+	if (macaddr_blacklist[0]) {
+		char *blacklist_str;
 
-		for (iter = macaddr_blacklist; iter; iter = g_slist_next (iter)) {
-			g_string_append (blacklist_str, iter->data);
-			g_string_append_c (blacklist_str, ' ');
-
-		}
-		if (blacklist_str->len > 0)
-			g_string_truncate (blacklist_str, blacklist_str->len - 1);
-		svSetValue (ifcfg, "HWADDR_BLACKLIST", blacklist_str->str, FALSE);
-		g_string_free (blacklist_str, TRUE);
+		blacklist_str = g_strjoinv (" ", (char **) macaddr_blacklist);
+		svSetValue (ifcfg, "HWADDR_BLACKLIST", blacklist_str, FALSE);
+		g_free (blacklist_str);
 	}
 
 	svSetValue (ifcfg, "MTU", NULL, FALSE);
@@ -1040,7 +1033,7 @@ write_wired_setting (NMConnection *connection, shvarFile *ifcfg, GError **error)
 	guint32 mtu, num_opts, i;
 	const char *const *s390_subchannels;
 	GString *str;
-	const GSList *macaddr_blacklist;
+	const char * const *macaddr_blacklist;
 
 	s_wired = nm_connection_get_setting_wired (connection);
 	if (!s_wired) {
@@ -1057,19 +1050,12 @@ write_wired_setting (NMConnection *connection, shvarFile *ifcfg, GError **error)
 
 	svSetValue (ifcfg, "HWADDR_BLACKLIST", NULL, FALSE);
 	macaddr_blacklist = nm_setting_wired_get_mac_address_blacklist (s_wired);
-	if (macaddr_blacklist) {
-		const GSList *iter;
-		GString *blacklist_str = g_string_new (NULL);
+	if (macaddr_blacklist[0]) {
+		char *blacklist_str;
 
-		for (iter = macaddr_blacklist; iter; iter = g_slist_next (iter)) {
-			g_string_append (blacklist_str, iter->data);
-			g_string_append_c (blacklist_str, ' ');
-
-		}
-		if (blacklist_str->len > 0)
-			g_string_truncate (blacklist_str, blacklist_str->len - 1);
-		svSetValue (ifcfg, "HWADDR_BLACKLIST", blacklist_str->str, FALSE);
-		g_string_free (blacklist_str, TRUE);
+		blacklist_str = g_strjoinv (" ", (char **) macaddr_blacklist);
+		svSetValue (ifcfg, "HWADDR_BLACKLIST", blacklist_str, FALSE);
+		g_free (blacklist_str);
 	}
 
 	svSetValue (ifcfg, "MTU", NULL, FALSE);