keyfile: read MAC addresses and a test framework

Clean up handling of "special" keys in keyfiles, ie ones that need more processing than the basic GKeyFile API supports. Add MAC address reading (writing support to come). Additionally, add some test bits for the keyfile plugin that get run on 'make check'.
2024-07-21 18:24:49 +00:00 · 2009-01-12 14:21:44 -05:00 · 2009-01-12 14:21:44 -05:00 · 45f2f1144d
parent 0400af7d03
commit 45f2f1144d
15 changed files with 783 additions and 97 deletions
--- a/Makefile.am
+++ b/Makefile.am
@ -51,9 +51,10 @@ EXTRA_DIST =				\
 	intltool-merge.in		\
 	intltool-update.in		

-DISTCHECK_CONFIGURE_FLAGS = --enable-gtk-doc
+DISTCHECK_CONFIGURE_FLAGS = --enable-gtk-doc --with-tests=yes

 DISTCLEANFILES = intltool-extract intltool-merge intltool-update

 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = NetworkManager.pc
+
--- a/configure.in
+++ b/configure.in
@ -477,6 +477,21 @@ fi

 GTK_DOC_CHECK(1.0)

+dnl
+dnl Tests
+dnl
+AC_ARG_WITH(tests, AC_HELP_STRING([--with-tests], [Build NetworkManager documentation]))
+AM_CONDITIONAL(WITH_TESTS, test "x$with_tests" = "xyes")
+case $with_tests in
+    yes)
+        with_tests=yes
+        ;;
+    *)
+        with_tests=no
+        ;;
+esac
+
+
 AC_CONFIG_FILES([
 Makefile
 include/Makefile
@ -504,6 +519,9 @@ system-settings/plugins/ifupdown/Makefile
 system-settings/plugins/ifcfg-rh/Makefile
 system-settings/plugins/ifcfg-suse/Makefile
 system-settings/plugins/keyfile/Makefile
+system-settings/plugins/keyfile/io/Makefile
+system-settings/plugins/keyfile/tests/Makefile
+system-settings/plugins/keyfile/tests/keyfiles/Makefile
 test/Makefile
 initscript/Makefile
 initscript/RedHat/Makefile
@ -541,4 +559,6 @@ echo 'if this is not correct, please specifiy your distro with --with-distro=DIS
 echo
 echo Building documentation: ${with_docs}
 echo
+echo Building tests: ${with_tests}
+echo

--- a/system-settings/plugins/keyfile/Makefile.am
+++ b/system-settings/plugins/keyfile/Makefile.am
@ -1,3 +1,11 @@
+SUBDIRS=io tests
+
+INCLUDES = \
+	-I$(top_srcdir)/system-settings/src \
+	-I$(top_srcdir)/include \
+	-I$(top_srcdir)/libnm-util \
+	-I$(top_srcdir)/libnm-glib \
+	-I$(top_srcdir)/system-settings/plugins/keyfile/io

 pkglib_LTLIBRARIES = libnm-settings-plugin-keyfile.la

@ -5,11 +13,7 @@ libnm_settings_plugin_keyfile_la_SOURCES = \
 	nm-keyfile-connection.c \
 	nm-keyfile-connection.h \
 	plugin.c \
-	plugin.h \
-	reader.c \
-	reader.h \
-	writer.c \
-	writer.h
+	plugin.h

 keyfiledir=$(sysconfdir)/NetworkManager/system-connections

@ -19,10 +23,6 @@ libnm_settings_plugin_keyfile_la_CPPFLAGS = \
 	$(DBUS_CFLAGS) \
 	-DSYSCONFDIR=\"$(sysconfdir)\" \
 	-DG_DISABLE_DEPRECATED \
-	-I${top_srcdir}/system-settings/src \
-	-I$(top_srcdir)/include \
-	-I$(top_srcdir)/libnm-util \
-	-I${top_srcdir}/libnm-glib \
 	-DKEYFILE_DIR=\""$(keyfiledir)"\"

 libnm_settings_plugin_keyfile_la_LDFLAGS = -module -avoid-version
@ -31,7 +31,8 @@ libnm_settings_plugin_keyfile_la_LIBADD = \
 	$(GMODULE_LIBS) \
 	$(DBUS_LIBS) \
 	$(top_builddir)/libnm-util/libnm-util.la \
-	$(top_builddir)/libnm-glib/libnm_glib.la
+	$(top_builddir)/libnm-glib/libnm_glib.la \
+	$(top_builddir)/system-settings/plugins/keyfile/io/libkeyfile-io.la

 if NO_GIO
 libnm_settings_plugin_keyfile_la_LIBADD += \
--- a/system-settings/plugins/keyfile/io/Makefile.am
+++ b/system-settings/plugins/keyfile/io/Makefile.am
@ -0,0 +1,20 @@
+INCLUDES = \
+	-I$(top_srcdir)/system-settings/src \
+	-I$(top_srcdir)/include \
+	-I$(top_srcdir)/libnm-util \
+	-I$(top_srcdir)/libnm-glib
+
+noinst_LTLIBRARIES = libkeyfile-io.la
+
+libkeyfile_io_la_SOURCES = \
+	reader.h \
+	reader.c \
+	writer.h \
+	writer.c
+
+libkeyfile_io_la_CPPFLAGS = \
+	$(GLIB_CFLAGS) \
+	$(DBUS_CFLAGS)
+
+libkeyfile_io_la_LIBADD = $(GTHREAD_LIBS)
+
--- a/system-settings/plugins/keyfile/io/reader.c
+++ b/system-settings/plugins/keyfile/io/reader.c
@ -29,7 +29,10 @@
 #include <nm-setting-ip4-config.h>
 #include <nm-setting-vpn.h>
 #include <nm-setting-connection.h>
+#include <nm-setting-wired.h>
+#include <nm-setting-wireless.h>
 #include <arpa/inet.h>
+#include <netinet/ether.h>
 #include <string.h>

 #include "nm-dbus-glib-types.h"
@ -43,36 +46,12 @@ read_array_of_uint (GKeyFile *file,
 	GArray *array = NULL;
 	gsize length;
 	int i;
+	gint *tmp;

-	if (NM_IS_SETTING_IP4_CONFIG (setting) && !strcmp (key, NM_SETTING_IP4_CONFIG_DNS)) {
-		char **list, **iter;
-		int ret;
-
-		list = g_key_file_get_string_list (file, nm_setting_get_name (setting), key, &length, NULL);
-		if (!list || !g_strv_length (list))
-			return TRUE;
-
-		array = g_array_sized_new (FALSE, FALSE, sizeof (guint32), length);
-		for (iter = list; *iter; iter++) {
-			struct in_addr addr;
-
-			ret = inet_pton (AF_INET, *iter, &addr);
-			if (ret <= 0) {
-				g_warning ("%s: ignoring invalid DNS server address '%s'", __func__, *iter);
-				continue;
-			}
-
-			g_array_append_val (array, addr.s_addr);			
-		}
-	} else {
-		gint *tmp;
-
-		tmp = g_key_file_get_integer_list (file, nm_setting_get_name (setting), key, &length, NULL);
-
-		array = g_array_sized_new (FALSE, FALSE, sizeof (guint32), length);
-		for (i = 0; i < length; i++)
-			g_array_append_val (array, tmp[i]);
-	}
+	tmp = g_key_file_get_integer_list (file, nm_setting_get_name (setting), key, &length, NULL);
+	array = g_array_sized_new (FALSE, FALSE, sizeof (guint32), length);
+	for (i = 0; i < length; i++)
+		g_array_append_val (array, tmp[i]);

 	if (array) {
 		g_object_set (setting, key, array, NULL);
@ -181,6 +160,25 @@ next:
 	return addresses;
 }

+static void
+ip4_addr_parser (NMSetting *setting, const char *key, GKeyFile *keyfile)
+{
+	GPtrArray *addresses;
+	const char *setting_name = nm_setting_get_name (setting);
+
+	addresses = read_addresses (keyfile, setting_name, key);
+
+	/* Work around for previous syntax */
+	if (!addresses && !strcmp (key, NM_SETTING_IP4_CONFIG_ADDRESSES))
+		addresses = read_addresses (keyfile, setting_name, "address");
+
+	if (addresses) {
+		g_object_set (setting, key, addresses, NULL);
+		g_ptr_array_foreach (addresses, free_one_address, NULL);
+		g_ptr_array_free (addresses, TRUE);
+	}
+}
+
 static void
 free_one_route (gpointer data, gpointer user_data)
 {
@ -236,7 +234,7 @@ read_routes (GKeyFile *file,
 			} else if (j == 3) {
 				guint32 metric = 0;

-				/* prefix */
+				/* metric */
 				if (!get_one_int (*iter, G_MAXUINT32, key_name, &metric)) {
 					g_array_free (route, TRUE);
 					goto next;
@ -268,48 +266,110 @@ next:
 	return routes;
 }

-static gboolean
-read_array_of_array_of_uint (GKeyFile *file,
-                             NMSetting *setting,
-                             const char *key)
+static void
+ip4_route_parser (NMSetting *setting, const char *key, GKeyFile *keyfile)
 {
-	gboolean success = FALSE;
-	const char *setting_name;
+	GPtrArray *routes;
+	const char *setting_name = nm_setting_get_name (setting);

-	/* Only handle IPv4 addresses and routes for now */
-	if (!NM_IS_SETTING_IP4_CONFIG (setting))
-		return FALSE;
+	routes = read_routes (keyfile, setting_name, key);
+	if (routes) {
+		g_object_set (setting, key, routes, NULL);
+		g_ptr_array_foreach (routes, free_one_route, NULL);
+		g_ptr_array_free (routes, TRUE);
+	}
+}

-	setting_name = nm_setting_get_name (setting);
+static void
+ip4_dns_parser (NMSetting *setting, const char *key, GKeyFile *keyfile)
+{
+	const char *setting_name = nm_setting_get_name (setting);
+	GArray *array = NULL;
+	gsize length;
+	char **list, **iter;
+	int ret;

-	if (!strcmp (key, NM_SETTING_IP4_CONFIG_ADDRESSES)) {
-		GPtrArray *addresses;
+	list = g_key_file_get_string_list (keyfile, setting_name, key, &length, NULL);
+	if (!list || !g_strv_length (list))
+		return;

-		addresses = read_addresses (file, setting_name, key);
+	array = g_array_sized_new (FALSE, FALSE, sizeof (guint32), length);
+	for (iter = list; *iter; iter++) {
+		struct in_addr addr;

-		/* Work around for previous syntax */
-		if (!addresses && !strcmp (key, NM_SETTING_IP4_CONFIG_ADDRESSES))
-			addresses = read_addresses (file, setting_name, "address");
-
-		if (addresses) {
-			g_object_set (setting, key, addresses, NULL);
-			g_ptr_array_foreach (addresses, free_one_address, NULL);
-			g_ptr_array_free (addresses, TRUE);
+		ret = inet_pton (AF_INET, *iter, &addr);
+		if (ret <= 0) {
+			g_warning ("%s: ignoring invalid DNS server address '%s'", __func__, *iter);
+			continue;
 		}
-		success = TRUE;
-	} else if (!strcmp (key, NM_SETTING_IP4_CONFIG_ROUTES)) {
-		GPtrArray *routes;

-		routes = read_routes (file, setting_name, key);
-		if (routes) {
-			g_object_set (setting, key, routes, NULL);
-			g_ptr_array_foreach (routes, free_one_route, NULL);
-			g_ptr_array_free (routes, TRUE);
-		}
-		success = TRUE;
+		g_array_append_val (array, addr.s_addr);			
 	}

-	return success;
+	if (array) {
+		g_object_set (setting, key, array, NULL);
+		g_array_free (array, TRUE);
+	}
+}
+
+
+static void
+mac_address_parser (NMSetting *setting, const char *key, GKeyFile *keyfile)
+{
+	const char *setting_name = nm_setting_get_name (setting);
+	struct ether_addr *eth;
+	char *tmp_string = NULL, *p;
+	gint *tmp_list;
+	GByteArray *array = NULL;
+	gsize length;
+	int i;
+
+	p = tmp_string = g_key_file_get_string (keyfile, setting_name, key, NULL);
+	if (tmp_string) {
+		/* Look for enough ':' characters to signify a MAC address */
+		i = 0;
+		while (*p) {
+			if (*p == ':')
+				i++;
+			p++;
+		}
+		if (i == 5) {
+			/* parse as a MAC address */
+			eth = ether_aton (tmp_string);
+			if (eth) {
+				g_free (tmp_string);
+				array = g_byte_array_sized_new (ETH_ALEN);
+				g_byte_array_append (array, eth->ether_addr_octet, ETH_ALEN);
+				goto done;
+			}
+		}
+	}
+	g_free (tmp_string);
+
+	/* Old format; list of ints */
+	tmp_list = g_key_file_get_integer_list (keyfile, setting_name, key, &length, NULL);
+	array = g_byte_array_sized_new (length);
+	for (i = 0; i < length; i++) {
+		int val = tmp_list[i];
+		unsigned char v = (unsigned char) (val & 0xFF);
+
+		if (val < 0 || val > 255) {
+			g_warning ("%s: %s / %s ignoring invalid byte element '%d' (not "
+			           " between 0 and 255 inclusive)", __func__, setting_name,
+			           key, val);
+		} else
+			g_byte_array_append (array, (const unsigned char *) &v, sizeof (v));
+	}
+	g_free (tmp_list);
+
+done:
+	if (array->len == ETH_ALEN) {
+		g_object_set (setting, key, array, NULL);
+	} else {
+		g_warning ("%s: ignoring invalid MAC address for %s / %s",
+		           __func__, setting_name, key);
+	}
+	g_byte_array_free (array, TRUE);
 }

 static void
@ -337,11 +397,48 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key)
 	g_strfreev (keys);
 }

+
 typedef struct {
 	GKeyFile *keyfile;
 	gboolean secrets;
 } ReadSettingInfo;

+typedef struct {
+	const char *setting_name;
+	const char *key;
+	gboolean check_for_key;
+	void (*parser) (NMSetting *setting, const char *key, GKeyFile *keyfile);
+} KeyParser;
+
+/* A table of keys that require further parsing/conversion becuase they are
+ * stored in a format that can't be automatically read using the key's type.
+ * i.e. IP addresses, which are stored in NetworkManager as guint32, but are
+ * stored in keyfiles as strings, eg "10.1.1.2".
+ */
+static KeyParser key_parsers[] = {
+	{ NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	  NM_SETTING_IP4_CONFIG_ADDRESSES,
+	  FALSE,
+	  ip4_addr_parser },
+	{ NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	  NM_SETTING_IP4_CONFIG_ROUTES,
+	  FALSE,
+	  ip4_route_parser },
+	{ NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	  NM_SETTING_IP4_CONFIG_DNS,
+	  FALSE,
+	  ip4_dns_parser },
+	{ NM_SETTING_WIRED_SETTING_NAME,
+	  NM_SETTING_WIRED_MAC_ADDRESS,
+	  TRUE,
+	  mac_address_parser },
+	{ NM_SETTING_WIRELESS_SETTING_NAME,
+	  NM_SETTING_WIRELESS_MAC_ADDRESS,
+	  TRUE,
+	  mac_address_parser },
+	{ NULL, NULL, FALSE }
+};
+
 static void
 read_one_setting_value (NMSetting *setting,
                        const char *key,
@ -355,6 +452,7 @@ read_one_setting_value (NMSetting *setting,
 	GType type;
 	GError *err = NULL;
 	gboolean check_for_key = TRUE;
+	KeyParser *parser = &key_parsers[0];

 	/* Property is not writable */
 	if (!(flags & G_PARAM_WRITABLE))
@ -375,18 +473,38 @@ read_one_setting_value (NMSetting *setting,

 	setting_name = nm_setting_get_name (setting);

-	/* IPv4 addresses and VPN properties don't have the exact key name */
-	if (NM_IS_SETTING_IP4_CONFIG (setting) && !strcmp (key, NM_SETTING_IP4_CONFIG_ADDRESSES))
-		check_for_key = FALSE;
-	else if (NM_IS_SETTING_VPN (setting))
+	/* Look through the list of handlers for non-standard format key values */
+	while (parser->setting_name) {
+		if (!strcmp (parser->setting_name, setting_name) && !strcmp (parser->key, key)) {
+			check_for_key = parser->check_for_key;
+			break;
+		}
+		parser++;
+	}
+
+	/* VPN properties don't have the exact key name */
+	if (NM_IS_SETTING_VPN (setting))
 		check_for_key = FALSE;

+	/* Check for the exact key in the GKeyFile if required.  Most setting
+	 * properties map 1:1 to a key in the GKeyFile, but for those properties
+	 * like IP addresses and routes where more than one value is actually
+	 * encoded by the setting property, this won't be true.
+	 */
 	if (check_for_key && !g_key_file_has_key (file, setting_name, key, &err)) {
+		/* Key doesn't exist or an error ocurred, thus nothing to do. */
 		if (err) {
 			g_warning ("Error loading setting '%s' value: %s", setting_name, err->message);
 			g_error_free (err);
 		}
+		return;
+	}

+	/* If there's a custom parser for this key, handle that before the generic
+	 * parsers below.
+	 */
+	if (parser && parser->setting_name) {
+		(*parser->parser) (setting, key, file);
 		return;
 	}

@ -444,9 +562,11 @@ read_one_setting_value (NMSetting *setting,
 			int val = tmp[i];
 			unsigned char v = (unsigned char) (val & 0xFF);

-			if (val < 0 || val > 255)
-				g_warning ("Value out of range for a byte value");
-			else
+			if (val < 0 || val > 255) {
+				g_warning ("%s: %s / %s ignoring invalid byte element '%d' (not "
+				           " between 0 and 255 inclusive)", __func__, setting_name,
+				           key, val);
+			} else
 				g_byte_array_append (array, (const unsigned char *) &v, sizeof (v));
 		}

@ -475,11 +595,6 @@ read_one_setting_value (NMSetting *setting,
 			g_warning ("Unhandled setting property type (read): '%s/%s' : '%s'",
 					 setting_name, key, G_VALUE_TYPE_NAME (value));
 		}
-	} else if (type == DBUS_TYPE_G_ARRAY_OF_ARRAY_OF_UINT) {
-		if (!read_array_of_array_of_uint (file, setting, key)) {
-			g_warning ("Unhandled setting property type (read): '%s/%s' : '%s'",
-					 setting_name, key, G_VALUE_TYPE_NAME (value));
-		}
 	} else {
 		g_warning ("Unhandled setting property type (read): '%s/%s' : '%s'",
 				 setting_name, key, G_VALUE_TYPE_NAME (value));
--- a/system-settings/plugins/keyfile/io/reader.h
+++ b/system-settings/plugins/keyfile/io/reader.h
--- a/system-settings/plugins/keyfile/io/writer.c
+++ b/system-settings/plugins/keyfile/io/writer.c
@ -30,6 +30,7 @@
 #include <nm-utils.h>
 #include <string.h>
 #include <arpa/inet.h>
+#include <nm-settings.h>

 #include "nm-dbus-glib-types.h"
 #include "writer.h"
@ -315,7 +316,12 @@ writer_id_to_filename (const char *id)
 }

 gboolean
-write_connection (NMConnection *connection, char **out_path, GError **error)
+write_connection (NMConnection *connection,
+                  const char *keyfile_dir,
+                  uid_t owner_uid,
+                  pid_t owner_grp,
+                  char **out_path,
+                  GError **error)
 {
 	NMSettingConnection *s_con;
 	GKeyFile *key_file;
@ -323,6 +329,7 @@ write_connection (NMConnection *connection, char **out_path, GError **error)
 	gsize len;
 	gboolean success = FALSE;
 	char *filename, *path;
+	int err;

 	if (out_path)
 		g_return_val_if_fail (*out_path == NULL, FALSE);
@ -338,18 +345,27 @@ write_connection (NMConnection *connection, char **out_path, GError **error)
 		goto out;

 	filename = writer_id_to_filename (nm_setting_connection_get_id (s_con));
-	path = g_build_filename (KEYFILE_DIR, filename, NULL);
+	path = g_build_filename (keyfile_dir, filename, NULL);
 	g_free (filename);

 	g_file_set_contents (path, data, len, error);
-	chmod (path, S_IRUSR | S_IWUSR);
-	if (chown (path, 0, 0) < 0) {
-		g_warning ("Error chowning '%s': %d", path, errno);
+	if (chown (path, owner_uid, owner_grp) < 0) {
+		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INTERNAL_ERROR,
+		             "%s.%d: error chowning '%s': %d", __FILE__, __LINE__,
+		             path, errno);
 		unlink (path);
 	} else {
-		if (out_path)
-			*out_path = g_strdup (path);
-		success = TRUE;
+		err = chmod (path, S_IRUSR | S_IWUSR);
+		if (err > 0) {
+			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INTERNAL_ERROR,
+			             "%s.%d: error setting permissions on '%s': %d", __FILE__,
+			             __LINE__, path, errno);
+			unlink (path);
+		} else {
+			if (out_path)
+				*out_path = g_strdup (path);
+			success = TRUE;
+		}
 	}
 	g_free (path);

--- a/system-settings/plugins/keyfile/io/writer.h
+++ b/system-settings/plugins/keyfile/io/writer.h
@ -25,7 +25,12 @@
 #include <glib.h>
 #include <nm-connection.h>

-gboolean write_connection (NMConnection *connection, char **out_path, GError **error);
+gboolean write_connection (NMConnection *connection,
+                           const char *keyfile_dir,
+                           uid_t owner_uid,
+                           pid_t owner_grp,
+                           char **out_path,
+                           GError **error);

 char *writer_id_to_filename (const char *id);

--- a/system-settings/plugins/keyfile/nm-keyfile-connection.c
+++ b/system-settings/plugins/keyfile/nm-keyfile-connection.c
@ -224,7 +224,7 @@ update (NMExportedConnection *exported,
 		connection = nm_exported_connection_get_connection (exported);
 		success = nm_connection_replace_settings (connection, new_settings, error);
 		if (success) {
-			success = write_connection (connection, &filename, error);
+			success = write_connection (connection, KEYFILE_DIR, 0, 0, &filename, error);
 			if (success && filename && strcmp (priv->filename, filename)) {
 				/* Update the filename if it changed */
 				g_free (priv->filename);
@ -294,7 +294,7 @@ constructor (GType type,
 		g_object_set (s_con, NM_SETTING_CONNECTION_UUID, uuid, NULL);
 		g_free (uuid);

-		if (!write_connection (wrapped, NULL, &error)) {
+		if (!write_connection (wrapped, KEYFILE_DIR, 0, 0, NULL, &error)) {
 			g_warning ("Couldn't update connection %s with a UUID: (%d) %s",
 			           nm_setting_connection_get_id (s_con), error ? error->code : 0,
 			           error ? error->message : "unknown");
--- a/system-settings/plugins/keyfile/plugin.c
+++ b/system-settings/plugins/keyfile/plugin.c
@ -350,7 +350,7 @@ add_connection (NMSystemConfigInterface *config,
                NMConnection *connection,
                GError **error)
 {
-	return write_connection (connection, NULL, error);
+	return write_connection (connection, KEYFILE_DIR, 0, 0, NULL, error);
 }

 static GSList *
--- a/system-settings/plugins/keyfile/tests/Makefile.am
+++ b/system-settings/plugins/keyfile/tests/Makefile.am
@ -0,0 +1,32 @@
+SUBDIRS=keyfiles
+
+INCLUDES = \
+	-I$(top_srcdir)/include \
+	-I$(top_srcdir)/libnm-util \
+	-I$(top_srcdir)/libnm-glib \
+	-I$(top_srcdir)/system-settings/plugins/keyfile/io
+
+noinst_PROGRAMS = test-keyfile
+
+test_keyfile_SOURCES = \
+	test-keyfile.c
+
+test_keyfile_CPPFLAGS = \
+	$(GLIB_CFLAGS) \
+	$(DBUS_CFLAGS) \
+	-DTEST_KEYFILES_DIR=\"$(abs_srcdir)/keyfiles\" \
+	-DTEST_SCRATCH_DIR=\"$(abs_builddir)/keyfiles\"
+
+test_keyfile_LDADD = \
+	$(DBUS_LIBS) \
+	$(top_builddir)/libnm-glib/libnm_glib.la \
+	$(top_builddir)/libnm-util/libnm-util.la \
+	$(top_builddir)/system-settings/plugins/keyfile/io/libkeyfile-io.la
+
+if WITH_TESTS
+
+check-local: test-keyfile
+	$(abs_builddir)/test-keyfile
+
+endif
+
--- a/system-settings/plugins/keyfile/tests/keyfiles/Makefile.am
+++ b/system-settings/plugins/keyfile/tests/keyfiles/Makefile.am
@ -0,0 +1,9 @@
+EXTRA_DIST = \
+	Test_Wired_Connection \
+	Test_GSM_Connection
+
+check-local:
+	@for f in $(EXTRA_DIST); do \
+		chmod 0600 $(abs_srcdir)/$$f; \
+	done
+
--- a/system-settings/plugins/keyfile/tests/keyfiles/Test_GSM_Connection
+++ b/system-settings/plugins/keyfile/tests/keyfiles/Test_GSM_Connection
@ -0,0 +1,41 @@
+
+[serial]
+baud=115200
+bits=8
+parity=110
+stopbits=1
+send-delay=0
+
+[connection]
+id=Test GSM Connection
+uuid=05a5ec81-fa72-4b7c-9f85-4a0dfd36c84f
+type=gsm
+autoconnect=false
+timestamp=0
+
+[gsm]
+number=*99#
+username=username
+apn=my.apn
+network-type=0
+band=0
+
+[ppp]
+noauth=false
+refuse-eap=false
+refuse-pap=false
+refuse-chap=false
+refuse-mschap=false
+refuse-mschapv2=false
+nobsdcomp=false
+nodeflate=false
+no-vj-comp=false
+require-mppe=false
+require-mppe-128=false
+mppe-stateful=false
+crtscts=false
+baud=0
+mru=0
+mtu=0
+lcp-echo-failure=5
+lcp-echo-interval=30
--- a/system-settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection
+++ b/system-settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection
@ -0,0 +1,22 @@
+
+[connection]
+id=Test Wired Connection
+uuid=4e80a56d-c99f-4aad-a6dd-b449bc398c57
+type=802-3-ethernet
+autoconnect=true
+timestamp=6654332
+
+[802-3-ethernet]
+mac-address=00:11:22:33:44:55
+speed=0
+duplex=full
+auto-negotiate=true
+mtu=1400
+
+[ipv4]
+method=manual
+dns=4.2.2.1;4.2.2.2;
+addresses1=192.168.0.5;24;192.168.0.1;
+addresses2=1.2.3.4;16;1.2.1.1;
+ignore-auto-routes=false
+ignore-auto-dns=false
--- a/system-settings/plugins/keyfile/tests/test-keyfile.c
+++ b/system-settings/plugins/keyfile/tests/test-keyfile.c
@ -0,0 +1,404 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/* NetworkManager system settings service - keyfile plugin
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * 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 Red Hat, Inc.
+ */
+
+#include <stdio.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <string.h>
+#include <netinet/ether.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <sys/socket.h>
+
+#include <dbus/dbus-glib.h>
+
+#include <nm-utils.h>
+#include <nm-setting-connection.h>
+#include <nm-setting-wired.h>
+#include <nm-setting-wireless.h>
+#include <nm-setting-ip4-config.h>
+
+#include "reader.h"
+#include "writer.h"
+
+#define TEST_WIRED_FILE TEST_KEYFILES_DIR"/Test_Wired_Connection"
+
+static void
+FAIL(const char *test_name, const char *fmt, ...)
+{
+    va_list args;
+    char buf[500];
+
+	snprintf (buf, 500, "FAIL: (%s) %s\n", test_name, fmt);
+
+    va_start (args, fmt);
+	vfprintf (stderr, buf, args);
+    va_end (args);
+	_exit (1);
+}
+
+#define ASSERT(x, test_name, fmt, ...) \
+	if (!(x)) { \
+		FAIL (test_name, fmt, ## __VA_ARGS__); \
+	}
+
+static void
+test_read_valid_wired_connection (void)
+{
+	NMConnection *connection;
+	NMSettingConnection *s_con;
+	NMSettingWired *s_wired;
+	NMSettingIP4Config *s_ip4;
+	GError *error = NULL;
+	const GByteArray *array;
+	char expected_mac_address[ETH_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55 };
+	const char *tmp;
+	const char *expected_id = "Test Wired Connection";
+	const char *expected_uuid = "4e80a56d-c99f-4aad-a6dd-b449bc398c57";
+	const guint64 expected_timestamp = 6654332;
+	guint64 timestamp;
+	const char *expected_dns1 = "4.2.2.1";
+	const char *expected_dns2 = "4.2.2.2";
+	struct in_addr addr;
+	const char *expected_address1 = "192.168.0.5";
+	const char *expected_address2 = "1.2.3.4";
+	const char *expected_address1_gw = "192.168.0.1";
+	const char *expected_address2_gw = "1.2.1.1";
+	NMIP4Address *ip4_addr;
+
+	connection = connection_from_file (TEST_WIRED_FILE, TRUE);
+	ASSERT (connection != NULL,
+			"connection-read", "failed to read %s", TEST_WIRED_FILE);
+
+	ASSERT (nm_connection_verify (connection, &error),
+	        "connection-verify", "failed to verify %s: %s", TEST_WIRED_FILE, error->message);
+
+	/* ===== CONNECTION SETTING ===== */
+
+	s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION));
+	ASSERT (s_con != NULL,
+	        "connection-verify-connection", "failed to verify %s: missing %s setting",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_CONNECTION_SETTING_NAME);
+
+	/* ID */
+	tmp = nm_setting_connection_get_id (s_con);
+	ASSERT (tmp != NULL,
+	        "connection-verify-connection", "failed to verify %s: missing %s / %s key",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_CONNECTION_SETTING_NAME,
+	        NM_SETTING_CONNECTION_ID);
+	ASSERT (strcmp (tmp, expected_id) == 0,
+	        "connection-verify-connection", "failed to verify %s: unexpected %s / %s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_CONNECTION_SETTING_NAME,
+	        NM_SETTING_CONNECTION_ID);
+
+	/* UUID */
+	tmp = nm_setting_connection_get_uuid (s_con);
+	ASSERT (tmp != NULL,
+	        "connection-verify-connection", "failed to verify %s: missing %s / %s key",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_CONNECTION_SETTING_NAME,
+	        NM_SETTING_CONNECTION_UUID);
+	ASSERT (strcmp (tmp, expected_uuid) == 0,
+	        "connection-verify-connection", "failed to verify %s: unexpected %s / %s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_CONNECTION_SETTING_NAME,
+	        NM_SETTING_CONNECTION_UUID);
+
+	/* Timestamp */
+	timestamp = nm_setting_connection_get_timestamp (s_con);
+	ASSERT (timestamp == expected_timestamp,
+	        "connection-verify-connection", "failed to verify %s: unexpected %s /%s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_CONNECTION_SETTING_NAME,
+	        NM_SETTING_CONNECTION_TIMESTAMP);
+
+	/* Autoconnect */
+	ASSERT (nm_setting_connection_get_autoconnect (s_con) == TRUE,
+	        "connection-verify-connection", "failed to verify %s: unexpected %s /%s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_CONNECTION_SETTING_NAME,
+	        NM_SETTING_CONNECTION_AUTOCONNECT);
+
+	/* ===== WIRED SETTING ===== */
+
+	s_wired = NM_SETTING_WIRED (nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED));
+	ASSERT (s_wired != NULL,
+	        "connection-verify-wired", "failed to verify %s: missing %s setting",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_WIRED_SETTING_NAME);
+
+	/* MAC address */
+	array = nm_setting_wired_get_mac_address (s_wired);
+	ASSERT (array != NULL,
+	        "connection-verify-wired", "failed to verify %s: missing %s / %s key",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_WIRED_SETTING_NAME,
+	        NM_SETTING_WIRED_MAC_ADDRESS);
+	ASSERT (array->len == ETH_ALEN,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value length",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_WIRED_SETTING_NAME,
+	        NM_SETTING_WIRED_MAC_ADDRESS);
+	ASSERT (memcmp (array->data, &expected_mac_address[0], sizeof (expected_mac_address)) == 0,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_WIRED_SETTING_NAME,
+	        NM_SETTING_WIRED_MAC_ADDRESS);
+
+	ASSERT (nm_setting_wired_get_mtu (s_wired) == 1400,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_WIRED_SETTING_NAME,
+	        NM_SETTING_WIRED_MTU);
+
+	/* ===== IPv4 SETTING ===== */
+
+	s_ip4 = NM_SETTING_IP4_CONFIG (nm_connection_get_setting (connection, NM_TYPE_SETTING_IP4_CONFIG));
+	ASSERT (s_ip4 != NULL,
+	        "connection-verify-ip4", "failed to verify %s: missing %s setting",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME);
+
+	/* Method */
+	tmp = nm_setting_ip4_config_get_method (s_ip4);
+	ASSERT (strcmp (tmp, NM_SETTING_IP4_CONFIG_METHOD_MANUAL) == 0,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_METHOD);
+
+	/* DNS Addresses */
+	ASSERT (nm_setting_ip4_config_get_num_dns (s_ip4) == 2,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+
+	ASSERT (inet_pton (AF_INET, expected_dns1, &addr) > 0,
+	        "connection-verify-wired", "failed to verify %s: couldn't convert DNS IP address #1",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+	ASSERT (nm_setting_ip4_config_get_dns (s_ip4, 0) == addr.s_addr,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value #1",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+
+	ASSERT (inet_pton (AF_INET, expected_dns2, &addr) > 0,
+	        "connection-verify-wired", "failed to verify %s: couldn't convert DNS IP address #2",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+	ASSERT (nm_setting_ip4_config_get_dns (s_ip4, 1) == addr.s_addr,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value #2",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+
+	ASSERT (nm_setting_ip4_config_get_num_addresses (s_ip4) == 2,
+	        "connection-verify-wired", "failed to verify %s: unexpected %s / %s key value",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+
+	/* Address #1 */
+	ip4_addr = nm_setting_ip4_config_get_address (s_ip4, 0);
+	ASSERT (ip4_addr,
+	        "connection-verify-wired", "failed to verify %s: missing IP4 address #1",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+
+	ASSERT (nm_ip4_address_get_prefix (ip4_addr) == 24,
+	        "connection-verify-wired", "failed to verify %s: unexpected IP4 address #1 gateway",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+
+	ASSERT (inet_pton (AF_INET, expected_address1, &addr) > 0,
+	        "connection-verify-wired", "failed to verify %s: couldn't convert IP address #1",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+	ASSERT (nm_ip4_address_get_address (ip4_addr) == addr.s_addr,
+	        "connection-verify-wired", "failed to verify %s: unexpected IP4 address #1",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+
+	ASSERT (inet_pton (AF_INET, expected_address1_gw, &addr) > 0,
+	        "connection-verify-wired", "failed to verify %s: couldn't convert IP address #1 gateway",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+	ASSERT (nm_ip4_address_get_gateway (ip4_addr) == addr.s_addr,
+	        "connection-verify-wired", "failed to verify %s: unexpected IP4 address #1 gateway",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+	
+	/* Address #2 */
+	ip4_addr = nm_setting_ip4_config_get_address (s_ip4, 1);
+	ASSERT (ip4_addr,
+	        "connection-verify-wired", "failed to verify %s: missing IP4 address #2",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+
+	ASSERT (nm_ip4_address_get_prefix (ip4_addr) == 16,
+	        "connection-verify-wired", "failed to verify %s: unexpected IP4 address #2 gateway",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+
+	ASSERT (inet_pton (AF_INET, expected_address2, &addr) > 0,
+	        "connection-verify-wired", "failed to verify %s: couldn't convert IP address #2",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_DNS);
+	ASSERT (nm_ip4_address_get_address (ip4_addr) == addr.s_addr,
+	        "connection-verify-wired", "failed to verify %s: unexpected IP4 address #2",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+
+	ASSERT (inet_pton (AF_INET, expected_address2_gw, &addr) > 0,
+	        "connection-verify-wired", "failed to verify %s: couldn't convert IP address #2 gateway",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+	ASSERT (nm_ip4_address_get_gateway (ip4_addr) == addr.s_addr,
+	        "connection-verify-wired", "failed to verify %s: unexpected IP4 address #2 gateway",
+	        TEST_WIRED_FILE,
+	        NM_SETTING_IP4_CONFIG_SETTING_NAME,
+	        NM_SETTING_IP4_CONFIG_ADDRESSES);
+
+	g_object_unref (connection);
+}
+
+static void
+test_write_wired_connection (void)
+{
+	NMConnection *connection;
+	NMSettingConnection *s_con;
+	NMSettingWired *s_wired;
+	NMSettingIP4Config *s_ip4;
+	char *uuid;
+	GByteArray *mac;
+	unsigned char tmpmac[] = { 0x99, 0x88, 0x77, 0x66, 0x55, 0x44 };
+	gboolean success;
+	NMConnection *reread;
+	char *testfile = NULL;
+	GError *error = NULL;
+	pid_t owner_grp;
+	uid_t owner_uid;
+
+	connection = nm_connection_new ();
+	ASSERT (connection != NULL,
+			"connection-write", "failed to allocate new connection");
+
+	s_con = NM_SETTING_CONNECTION (nm_setting_connection_new ());
+	ASSERT (s_con != NULL,
+			"connection-write", "failed to allocate new %s setting",
+			NM_SETTING_CONNECTION_SETTING_NAME);
+	nm_connection_add_setting (connection, NM_SETTING (s_con));
+
+	uuid = nm_utils_uuid_generate ();
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_ID, "Work Wireless",
+	              NM_SETTING_CONNECTION_UUID, uuid,
+	              NM_SETTING_CONNECTION_AUTOCONNECT, FALSE,
+	              NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME,
+	              NM_SETTING_CONNECTION_TIMESTAMP, 0x12345678L,
+	              NULL);
+	g_free (uuid);
+
+	s_wired = NM_SETTING_WIRED (nm_setting_wired_new ());
+	ASSERT (s_wired != NULL,
+			"connection-write", "failed to allocate new %s setting",
+			NM_SETTING_WIRED_SETTING_NAME);
+	nm_connection_add_setting (connection, NM_SETTING (s_wired));
+
+	mac = g_byte_array_sized_new (ETH_ALEN);
+	g_byte_array_append (mac, &tmpmac[0], sizeof (tmpmac));
+	g_object_set (s_wired,
+	              NM_SETTING_WIRED_MAC_ADDRESS, mac,
+	              NM_SETTING_WIRED_MTU, 900,
+	              NULL);
+	g_byte_array_free (mac, TRUE);
+
+	s_ip4 = NM_SETTING_IP4_CONFIG (nm_setting_ip4_config_new ());
+	ASSERT (s_ip4 != NULL,
+			"connection-write", "failed to allocate new %s setting",
+			NM_SETTING_WIRED_SETTING_NAME);
+	nm_connection_add_setting (connection, NM_SETTING (s_ip4));
+
+	/* Write out the connection */
+	owner_uid = geteuid ();
+	owner_grp = getegid ();
+	success = write_connection (connection, TEST_SCRATCH_DIR, owner_uid, owner_grp, &testfile, &error);
+	ASSERT (success == TRUE,
+			"connection-write", "failed to allocate write keyfile: %s",
+			error ? error->message : "(none)");
+
+	ASSERT (testfile != NULL,
+			"connection-write", "didn't get keyfile name back after writing connection");
+
+	/* Read the connection back in and compare it to the one we just wrote out */
+	reread = connection_from_file (testfile, TRUE);
+	ASSERT (reread != NULL, "connection-write", "failed to re-read test connection");
+
+	ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE,
+			"connection-write", "written and re-read connection weren't the same");
+
+	g_clear_error (&error);
+	unlink (testfile);
+	g_free (testfile);
+
+	g_object_unref (reread);
+	g_object_unref (connection);
+}
+
+int main (int argc, char **argv)
+{
+	GError *error = NULL;
+	DBusGConnection *bus;
+	char *basename;
+
+	g_type_init ();
+	bus = dbus_g_bus_get (DBUS_BUS_SESSION, NULL);
+
+	if (!nm_utils_init (&error))
+		FAIL ("nm-utils-init", "failed to initialize libnm-util: %s", error->message);
+
+	/* The tests */
+	test_read_valid_wired_connection ();
+	test_write_wired_connection ();
+
+	basename = g_path_get_basename (argv[0]);
+	fprintf (stdout, "%s: SUCCESS\n", basename);
+	g_free (basename);
+	return 0;
+}
+