ifcfg-rh: fix reading HWADDR_BLACKLIST

Adding GSList to the property may crash NetworkManager. Also check MAC
addresses and filter out the invalid ones.

(broken by commit 6a4127cfa063a77e3ee886a999441c71f6a0e11c)

4 files changed, 84 insertions, 23 deletions

diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index dfc8d289c1..139d158c48 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -15,7 +15,7 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
- * Copyright (C) 2008 - 2013 Red Hat, Inc.
+ * Copyright 2008 - 2014 Red Hat, Inc.
  */
 
 #include <config.h>
@@ -49,6 +49,7 @@
 #include <nm-setting-dcb.h>
 #include <nm-setting-generic.h>
 #include "nm-core-internal.h"
+#include "nm-utils-private.h"
 #include <nm-utils.h>
 
 #include "nm-platform.h"
@@ -3238,6 +3239,28 @@ make_wireless_security_setting (shvarFile *ifcfg,
 	return NULL; /* unencrypted */
 }
 
+static char **
+transform_hwddr_blacklist (const char *blacklist)
+{
+	char **strv, **iter;
+	int shift = 0;
+
+	strv = _nm_utils_strsplit_set (blacklist, " \t", 0);
+	for (iter = strv; iter && *iter; iter++) {
+		if (shift) {
+			*(iter-shift) = *iter;
+			*iter = NULL;
+		}
+		if (!nm_utils_hwaddr_valid (*(iter-shift), ETH_ALEN)) {
+			PARSE_WARNING ("invalid MAC in HWADDR_BLACKLIST '%s'", *(iter-shift));
+			g_free (*(iter-shift));
+			*(iter-shift) = NULL;
+			shift++;
+		}
+	}
+	return strv;
+}
+
 static NMSetting *
 make_wireless_setting (shvarFile *ifcfg,
                        GError **error)
@@ -3264,11 +3287,11 @@ make_wireless_setting (shvarFile *ifcfg,
 
 	value = svGetValue (ifcfg, "HWADDR_BLACKLIST", FALSE);
 	if (value) {
-		char **list;
+		char **strv;
 
-		list = g_strsplit_set (value, " \t", 0);
-		g_object_set (s_wireless, NM_SETTING_WIRELESS_MAC_ADDRESS_BLACKLIST, list, NULL);
-		g_strfreev (list);
+		strv = transform_hwddr_blacklist (value);
+		g_object_set (s_wireless, NM_SETTING_WIRELESS_MAC_ADDRESS_BLACKLIST, strv, NULL);
+		g_strfreev (strv);
 		g_free (value);
 	}
 
@@ -3489,7 +3512,6 @@ make_wired_setting (shvarFile *ifcfg,
 	NMSettingWired *s_wired;
 	char *value = NULL;
 	int mtu;
-	GSList *macaddr_blacklist = NULL;
 	char *nettype;
 
 	s_wired = NM_SETTING_WIRED (nm_setting_wired_new ());
@@ -3594,25 +3616,12 @@ make_wired_setting (shvarFile *ifcfg,
 
 	value = svGetValue (ifcfg, "HWADDR_BLACKLIST", FALSE);
 	if (value) {
-		char **list = NULL, **iter;
+		char **strv;
 
-		list = g_strsplit_set (value, " \t", 0);
-		for (iter = list; iter && *iter; iter++) {
-			if (**iter == '\0')
-				continue;
-			if (!nm_utils_hwaddr_valid (*iter, ETH_ALEN)) {
-				PARSE_WARNING ("invalid MAC in HWADDR_BLACKLIST '%s'", *iter);
-				continue;
-			}
-			macaddr_blacklist = g_slist_prepend (macaddr_blacklist, *iter);
-		}
-		if (macaddr_blacklist) {
-			macaddr_blacklist = g_slist_reverse (macaddr_blacklist);
-			g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS_BLACKLIST, macaddr_blacklist, NULL);
-			g_slist_free (macaddr_blacklist);
-		}
+		strv = transform_hwddr_blacklist (value);
+		g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS_BLACKLIST, strv, NULL);
+		g_strfreev (strv);
 		g_free (value);
-		g_strfreev (list);
 	}
 
 	value = svGetValue (ifcfg, "KEY_MGMT", FALSE);
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
index 09e47281c4..782ad8e045 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
@@ -1,5 +1,6 @@
 EXTRA_DIST = \
 	ifcfg-test-minimal \
+	ifcfg-test-misc-variables \
 	ifcfg-test-variables-corner-cases-1 \
 	ifcfg-test-nm-controlled \
 	ifcfg-test-nm-controlled-unrecognized \
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-misc-variables b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-misc-variables
new file mode 100644
index 0000000000..5649a84892
--- /dev/null
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-misc-variables
@@ -0,0 +1,3 @@
+DEVICE=eth0
+HWADDR_BLACKLIST="00:16:41:11:22:88 00:16:41:11:22:99"
+
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 3be7d721e0..39d4ffdd4e 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -238,6 +238,53 @@ test_read_basic (void)
 }
 
 static void
+test_read_miscellaneous_variables (void)
+{
+	NMConnection *connection;
+	NMSettingConnection *s_con;
+	NMSettingWired *s_wired;
+	NMSettingIP4Config *s_ip4;
+	GError *error = NULL;
+	char *expected_mac_blacklist[2] = { "00:16:41:11:22:88", "00:16:41:11:22:99" };
+	int mac_blacklist_num, i;
+	guint64 expected_timestamp = 0;
+	gboolean success;
+
+	connection = connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-misc-variables",
+	                                   NULL, TYPE_ETHERNET, NULL, NULL, NULL, NULL, &error, NULL);
+	g_assert_no_error (error);
+	g_assert (connection);
+	success = nm_connection_verify (connection, &error);
+	g_assert_no_error (error);
+	g_assert (success);
+
+	/* ===== CONNECTION SETTING ===== */
+	s_con = nm_connection_get_setting_connection (connection);
+	g_assert (s_con);
+	g_assert_cmpint (nm_setting_connection_get_timestamp (s_con), ==, expected_timestamp);
+	g_assert (nm_setting_connection_get_autoconnect (s_con));
+
+	/* ===== WIRED SETTING ===== */
+	s_wired = nm_connection_get_setting_wired (connection);
+	g_assert (s_wired);
+	g_assert_cmpint (nm_setting_wired_get_mtu (s_wired), ==, 0);
+
+	/* MAC blacklist */
+	mac_blacklist_num = nm_setting_wired_get_num_mac_blacklist_items (s_wired);
+	g_assert_cmpint (mac_blacklist_num, ==, 2);
+	for (i = 0; i < mac_blacklist_num; i++)
+		g_assert (nm_utils_hwaddr_matches (nm_setting_wired_get_mac_blacklist_item (s_wired, i), -1, expected_mac_blacklist[i], -1));
+
+	/* ===== IPv4 SETTING ===== */
+	s_ip4 = nm_connection_get_setting_ip4_config (connection);
+	g_assert (s_ip4);
+	g_assert_cmpstr (nm_setting_ip4_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_DISABLED);
+	g_assert (nm_setting_ip4_config_get_never_default (s_ip4) == FALSE);
+
+	g_object_unref (connection);
+}
+
+static void
 test_read_variables_corner_cases (void)
 {
 	NMConnection *connection;
@@ -13631,6 +13678,7 @@ int main (int argc, char **argv)
 	g_test_add_func (TPATH "unmanaged-unrecognized", test_read_unmanaged_unrecognized);
 	g_test_add_func (TPATH "unrecognized", test_read_unrecognized);
 	g_test_add_func (TPATH "basic", test_read_basic);
+	g_test_add_func (TPATH "miscellaneous-variables", test_read_miscellaneous_variables);
 	g_test_add_func (TPATH "variables-corner-cases", test_read_variables_corner_cases);
 	g_test_add_data_func (TPATH "no-prefix/8", GUINT_TO_POINTER (8), test_read_wired_static_no_prefix);
 	g_test_add_data_func (TPATH "no-prefix/16", GUINT_TO_POINTER (16), test_read_wired_static_no_prefix);