libnm: add keyfile support to libnm-core

https://bugzilla.gnome.org/show_bug.cgi?id=744699

libnm-core/Makefile.libnm-core

@@ -48,6 +48,9 @@ libnm_core_private_headers =			\
 	$(core)/crypto.h			\
 	$(core)/nm-connection-private.h		\
 	$(core)/nm-core-internal.h		\
+	$(core)/nm-keyfile-reader.h         \
+	$(core)/nm-keyfile-utils.h          \
+	$(core)/nm-keyfile-writer.h         \
 	$(core)/nm-property-compare.h		\
 	$(core)/nm-setting-private.h		\
 	$(core)/nm-utils-private.h
@@ -57,6 +60,9 @@ libnm_core_sources =				\
 	$(core)/crypto.c			\
 	$(core)/nm-connection.c			\
 	$(core)/nm-errors.c			\
+	$(core)/nm-keyfile-reader.c         \
+	$(core)/nm-keyfile-utils.c          \
+	$(core)/nm-keyfile-writer.c         \
 	$(core)/nm-property-compare.c		\
 	$(core)/nm-setting-8021x.c		\
 	$(core)/nm-setting-adsl.c		\

libnm-core/nm-keyfile-reader.h

@@ -16,15 +16,70 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
  * Copyright (C) 2008 Novell, Inc.
- * Copyright (C) 2008 Red Hat, Inc.
+ * Copyright (C) 2015 Red Hat, Inc.
  */
 
-#ifndef _KEYFILE_PLUGIN_READER_H
+#ifndef __NM_KEYFILE_READER_H__
-#define _KEYFILE_PLUGIN_READER_H
+#define __NM_KEYFILE_READER_H__
 
 #include <glib.h>
-#include <nm-connection.h>
 
-NMConnection *nm_keyfile_plugin_connection_from_file (const char *filename, GError **error);
+#include "nm-connection.h"
 
-#endif /* _KEYFILE_PLUGIN_READER_H */
+
+typedef enum {
+	NM_KEYFILE_READ_TYPE_WARN               = 1,
+}  NMKeyfileReadType;
+
+/**
+ * NMKeyfileReadHandler:
+ *
+ * Hook to nm_keyfile_read(). The user might fail the reading by setting
+ * @error.
+ *
+ * Returns: should return TRUE, if the reading was handled. Otherwise,
+ * a default action will be performed that depends on the @type.
+ * For %NM_KEYFILE_READ_TYPE_WARN type, the default action is doing nothing.
+ */
+typedef gboolean (*NMKeyfileReadHandler) (GKeyFile *keyfile,
+                                          NMConnection *connection,
+                                          NMKeyfileReadType type,
+                                          void *type_data,
+                                          void *user_data,
+                                          GError **error);
+
+typedef enum {
+	NM_KEYFILE_WARN_SEVERITY_DEBUG                  = 1000,
+	NM_KEYFILE_WARN_SEVERITY_INFO                   = 2000,
+	NM_KEYFILE_WARN_SEVERITY_WARN                   = 3000,
+} NMKeyfileWarnSeverity;
+
+/**
+ * NMKeyfileReadTypeDataWarn:
+ *
+ * this struct is passed as @type_data for the @NMKeyfileReadHandler of
+ * type %NM_KEYFILE_READ_TYPE_WARN.
+ */
+typedef struct {
+	/* might be %NULL, if the warning is not about a group. */
+	const char *group;
+
+	/* might be %NULL, if the warning is not about a setting. */
+	NMSetting *setting;
+
+	/* might be %NULL, if the warning is not about a property. */
+	const char *property_name;
+
+	NMKeyfileWarnSeverity severity;
+	const char *message;
+} NMKeyfileReadTypeDataWarn;
+
+
+NMConnection *nm_keyfile_read (GKeyFile *keyfile,
+                               const char *keyfile_name,
+                               const char *base_dir,
+                               NMKeyfileReadHandler handler,
+                               void *user_data,
+                               GError **error);
+
+#endif /* __NM_KEYFILE_READER_H__ */

libnm-core/nm-keyfile-utils.c

@@ -23,128 +23,11 @@
 #include <glib.h>
 #include <stdlib.h>
 #include <string.h>
-#include "gsystem-local-alloc.h"
-#include "utils.h"
-#include <nm-setting-wired.h>
-#include <nm-setting-wireless.h>
-#include <nm-setting-wireless-security.h>
 
-
+#include "nm-keyfile-utils.h"
-static const char temp_letters[] =
+#include "nm-setting-wired.h"
-"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+#include "nm-setting-wireless.h"
-
+#include "nm-setting-wireless-security.h"
-/*
- * Check '.[a-zA-Z0-9]{6}' file suffix used for temporary files by g_file_set_contents() (mkstemp()).
- */
-static gboolean
-check_mkstemp_suffix (const char *path)
-{
-	const char *ptr;
-
-	g_return_val_if_fail (path != NULL, FALSE);
-
-	/* Matches *.[a-zA-Z0-9]{6} suffix of mkstemp()'s temporary files */
-	ptr = strrchr (path, '.');
-	if (ptr && (strspn (ptr + 1, temp_letters) == 6) && (! ptr[7]))
-		return TRUE;
-	return FALSE;
-}
-
-static gboolean
-check_prefix (const char *base, const char *tag)
-{
-	int len, tag_len;
-
-	g_return_val_if_fail (base != NULL, TRUE);
-	g_return_val_if_fail (tag != NULL, TRUE);
-
-	len = strlen (base);
-	tag_len = strlen (tag);
-	if ((len > tag_len) && !g_ascii_strncasecmp (base, tag, tag_len))
-		return TRUE;
-	return FALSE;
-}
-
-static gboolean
-check_suffix (const char *base, const char *tag)
-{
-	int len, tag_len;
-
-	g_return_val_if_fail (base != NULL, TRUE);
-	g_return_val_if_fail (tag != NULL, TRUE);
-
-	len = strlen (base);
-	tag_len = strlen (tag);
-	if ((len > tag_len) && !g_ascii_strcasecmp (base + len - tag_len, tag))
-		return TRUE;
-	return FALSE;
-}
-
-#define SWP_TAG ".swp"
-#define SWPX_TAG ".swpx"
-#define PEM_TAG ".pem"
-#define DER_TAG ".der"
-
-gboolean
-nm_keyfile_plugin_utils_should_ignore_file (const char *filename)
-{
-	gs_free char *base = NULL;
-
-	g_return_val_if_fail (filename != NULL, TRUE);
-
-	base = g_path_get_basename (filename);
-	g_return_val_if_fail (base != NULL, TRUE);
-
-	/* Ignore hidden and backup files */
-	/* should_ignore_file() must mirror escape_filename() */
-	if (check_prefix (base, ".") || check_suffix (base, "~"))
-		return TRUE;
-	/* Ignore temporary files */
-	if (check_mkstemp_suffix (base))
-		return TRUE;
-	/* Ignore 802.1x certificates and keys */
-	if (check_suffix (base, PEM_TAG) || check_suffix (base, DER_TAG))
-		return TRUE;
-
-	return FALSE;
-}
-
-char *
-nm_keyfile_plugin_utils_escape_filename (const char *filename)
-{
-	GString *str;
-	const char *f = filename;
-	const char ESCAPE_CHAR = '*';
-
-	/* keyfile used to escape with '*', do not change that behavior.
-	 * But for newly added escapings, use '_' instead. */
-	const char ESCAPE_CHAR2 = '_';
-
-	g_return_val_if_fail (filename && filename[0], NULL);
-
-	str = g_string_sized_new (60);
-
-	/* Convert '/' to ESCAPE_CHAR */
-	for (f = filename; f[0]; f++) {
-		if (f[0] == '/')
-			g_string_append_c (str, ESCAPE_CHAR);
-		else
-			g_string_append_c (str, f[0]);
-	}
-
-	/* escape_filename() must avoid anything that should_ignore_file() would reject.
-	 * We can escape here more aggressivly then what we would read back. */
-	if (check_prefix (str->str, "."))
-		str->str[0] = ESCAPE_CHAR2;
-	if (check_suffix (str->str, "~"))
-		str->str[str->len - 1] = ESCAPE_CHAR2;
-	if (   check_mkstemp_suffix (str->str)
-	    || check_suffix (str->str, PEM_TAG)
-	    || check_suffix (str->str, DER_TAG))
-		g_string_append_c (str, ESCAPE_CHAR2);
-
-	return g_string_free (str, FALSE);;
-}
 
 
 typedef struct {

libnm-core/nm-keyfile-utils.h

@@ -15,25 +15,15 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
- * (C) Copyright 2010 Red Hat, Inc.
+ * (C) Copyright 2010-2015 Red Hat, Inc.
  */
 
-#ifndef _UTILS_H_
+#ifndef __NM_KEYFILE_UTILS_H__
-#define _UTILS_H_
+#define __NM_KEYFILE_UTILS_H__
 
 #include <glib.h>
-#include "common.h"
-#include "NetworkManagerUtils.h"
 
-#define NM_KEYFILE_CONNECTION_LOG_PATH(path)  str_if_set (path,"in-memory")
+#define VPN_SECRETS_GROUP "vpn-secrets"
-#define NM_KEYFILE_CONNECTION_LOG_FMT         "%s (%s,\"%s\")"
-#define NM_KEYFILE_CONNECTION_LOG_ARG(con)    NM_KEYFILE_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_connection_get_uuid ((NMConnection *) (con)), nm_connection_get_id ((NMConnection *) (con))
-#define NM_KEYFILE_CONNECTION_LOG_FMTD        "%s (%s,\"%s\",%p)"
-#define NM_KEYFILE_CONNECTION_LOG_ARGD(con)   NM_KEYFILE_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_connection_get_uuid ((NMConnection *) (con)), nm_connection_get_id ((NMConnection *) (con)), (con)
-
-gboolean nm_keyfile_plugin_utils_should_ignore_file (const char *filename);
-
-char *nm_keyfile_plugin_utils_escape_filename (const char *filename);
 
 const char *nm_keyfile_plugin_get_alias_for_setting_name (const char *setting_name);
 
@@ -85,5 +75,5 @@ gboolean nm_keyfile_plugin_kf_has_key     (GKeyFile *kf,
                                            const char *key,
                                            GError **error);
 
-#endif  /* _UTILS_H_ */
+#endif  /* __NM_KEYFILE_UTILS_H__ */
 

libnm-core/nm-keyfile-writer.c

@@ -16,7 +16,7 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
  * Copyright (C) 2008 Novell, Inc.
- * Copyright (C) 2008 - 2012 Red Hat, Inc.
+ * Copyright (C) 2008 - 2015 Red Hat, Inc.
  */
 
 #include "config.h"
@@ -26,26 +26,34 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <errno.h>
-
-#include <nm-setting.h>
-#include <nm-setting-connection.h>
-#include <nm-setting-ip4-config.h>
-#include <nm-setting-ip6-config.h>
-#include <nm-setting-vpn.h>
-#include <nm-setting-wired.h>
-#include <nm-setting-wireless.h>
-#include <nm-setting-ip4-config.h>
-#include <nm-setting-bluetooth.h>
-#include <nm-setting-8021x.h>
-#include <nm-utils.h>
-#include <string.h>
 #include <arpa/inet.h>
+#include <string.h>
+#include <glib/gi18n-lib.h>
+
+#include "nm-setting.h"
+#include "nm-setting-connection.h"
+#include "nm-setting-ip4-config.h"
+#include "nm-setting-ip6-config.h"
+#include "nm-setting-vpn.h"
+#include "nm-setting-wired.h"
+#include "nm-setting-wireless.h"
+#include "nm-setting-ip4-config.h"
+#include "nm-setting-bluetooth.h"
+#include "nm-setting-8021x.h"
+#include "nm-utils.h"
 
 #include "nm-glib-compat.h"
-#include "nm-logging.h"
+#include "nm-keyfile-writer.h"
-#include "writer.h"
+#include "nm-keyfile-utils.h"
-#include "common.h"
+
-#include "utils.h"
+typedef struct {
+	NMConnection *connection;
+	GKeyFile *keyfile;
+	GError *error;
+	NMKeyfileWriteHandler handler;
+	void *user_data;
+} KeyfileWriterInfo;
+
 
 /* Some setting properties also contain setting names, such as
  * NMSettingConnection's 'type' property (which specifies the base type of the
@@ -55,9 +63,7 @@
  * from the real setting name to the more-readable alias.
  */
 static void
-setting_alias_writer (GKeyFile *file,
+setting_alias_writer (KeyfileWriterInfo *info,
-                      const char *keyfile_dir,
-                      const char *uuid,
                       NMSetting *setting,
                       const char *key,
                       const GValue *value)
@@ -66,13 +72,13 @@ setting_alias_writer (GKeyFile *file,
 
 	str = g_value_get_string (value);
 	alias = nm_keyfile_plugin_get_alias_for_setting_name (str);
-	nm_keyfile_plugin_kf_set_string (file,
+	nm_keyfile_plugin_kf_set_string (info->keyfile,
 	                                 nm_setting_get_name (setting),
 	                                 key,
 	                                 alias ? alias : str);
 }
 
-static gboolean
+static void
 write_array_of_uint (GKeyFile *file,
                      NMSetting *setting,
                      const char *key,
@@ -84,7 +90,7 @@ write_array_of_uint (GKeyFile *file,
 
 	array = (GArray *) g_value_get_boxed (value);
 	if (!array || !array->len)
-		return TRUE;
+		return;
 
 	tmp_array = g_new (gint, array->len);
 	for (i = 0; i < array->len; i++)
@@ -92,13 +98,10 @@ write_array_of_uint (GKeyFile *file,
 
 	nm_keyfile_plugin_kf_set_integer_list (file, nm_setting_get_name (setting), key, tmp_array, array->len);
 	g_free (tmp_array);
-	return TRUE;
 }
 
 static void
-dns_writer (GKeyFile *file,
+dns_writer (KeyfileWriterInfo *info,
-            const char *keyfile_dir,
-            const char *uuid,
             NMSetting *setting,
             const char *key,
             const GValue *value)
@@ -107,7 +110,7 @@ dns_writer (GKeyFile *file,
 
 	list = g_value_get_boxed (value);
 	if (list && list[0]) {
-		nm_keyfile_plugin_kf_set_string_list (file, nm_setting_get_name (setting), key,
+		nm_keyfile_plugin_kf_set_string_list (info->keyfile, nm_setting_get_name (setting), key,
 		                                      (const char **) list, g_strv_length (list));
 	}
 }
@@ -178,9 +181,7 @@ write_ip_values (GKeyFile *file,
 }
 
 static void
-addr_writer (GKeyFile *file,
+addr_writer (KeyfileWriterInfo *info,
-             const char *keyfile_dir,
-             const char *uuid,
              NMSetting *setting,
              const char *key,
              const GValue *value)
@@ -191,13 +192,11 @@ addr_writer (GKeyFile *file,
 
 	array = (GPtrArray *) g_value_get_boxed (value);
 	if (array && array->len)
-		write_ip_values (file, setting_name, array, gateway, FALSE);
+		write_ip_values (info->keyfile, setting_name, array, gateway, FALSE);
 }
 
 static void
-ip4_addr_label_writer (GKeyFile *file,
+ip4_addr_label_writer (KeyfileWriterInfo *info,
-                       const char *keyfile_dir,
-                       const char *uuid,
                        NMSetting *setting,
                        const char *key,
                        const GValue *value)
@@ -206,9 +205,7 @@ ip4_addr_label_writer (GKeyFile *file,
 }
 
 static void
-gateway_writer (GKeyFile *file,
+gateway_writer (KeyfileWriterInfo *info,
-                const char *keyfile_dir,
-                const char *uuid,
                 NMSetting *setting,
                 const char *key,
                 const GValue *value)
@@ -217,9 +214,7 @@ gateway_writer (GKeyFile *file,
 }
 
 static void
-route_writer (GKeyFile *file,
+route_writer (KeyfileWriterInfo *info,
-              const char *keyfile_dir,
-              const char *uuid,
               NMSetting *setting,
               const char *key,
               const GValue *value)
@@ -229,7 +224,7 @@ route_writer (GKeyFile *file,
 
 	array = (GPtrArray *) g_value_get_boxed (value);
 	if (array && array->len)
-		write_ip_values (file, setting_name, array, NULL, TRUE);
+		write_ip_values (info->keyfile, setting_name, array, NULL, TRUE);
 }
 
 static void
@@ -271,9 +266,7 @@ write_hash_of_string (GKeyFile *file,
 }
 
 static void
-ssid_writer (GKeyFile *file,
+ssid_writer (KeyfileWriterInfo *info,
-             const char *keyfile_dir,
-             const char *uuid,
              NMSetting *setting,
              const char *key,
              const GValue *value)
@@ -323,21 +316,19 @@ ssid_writer (GKeyFile *file,
 				ssid[j++] = ssid_data[i];
 			}
 		}
-		nm_keyfile_plugin_kf_set_string (file, setting_name, key, ssid);
+		nm_keyfile_plugin_kf_set_string (info->keyfile, setting_name, key, ssid);
 		g_free (ssid);
 	} else {
 		tmp_array = g_new (gint, ssid_len);
 		for (i = 0; i < ssid_len; i++)
 			tmp_array[i] = (int) ssid_data[i];
-		nm_keyfile_plugin_kf_set_integer_list (file, setting_name, key, tmp_array, ssid_len);
+		nm_keyfile_plugin_kf_set_integer_list (info->keyfile, setting_name, key, tmp_array, ssid_len);
 		g_free (tmp_array);
 	}
 }
 
 static void
-password_raw_writer (GKeyFile *file,
+password_raw_writer (KeyfileWriterInfo *info,
-                     const char *keyfile_dir,
-                     const char *uuid,
                      NMSetting *setting,
                      const char *key,
                      const GValue *value)
@@ -360,7 +351,7 @@ password_raw_writer (GKeyFile *file,
 	tmp_array = g_new (gint, len);
 	for (i = 0; i < len; i++)
 		tmp_array[i] = (int) data[i];
-	nm_keyfile_plugin_kf_set_integer_list (file, setting_name, key, tmp_array, len);
+	nm_keyfile_plugin_kf_set_integer_list (info->keyfile, setting_name, key, tmp_array, len);
 	g_free (tmp_array);
 }
 
@@ -419,83 +410,15 @@ static const ObjectType objtypes[10] = {
 	{ NULL },
 };
 
-static gboolean
-write_cert_key_file (const char *path,
-                     const guint8 *data,
-                     gsize data_len,
-                     GError **error)
-{
-	char *tmppath;
-	int fd = -1, written;
-	gboolean success = FALSE;
-
-	tmppath = g_malloc0 (strlen (path) + 10);
-	g_assert (tmppath);
-	memcpy (tmppath, path, strlen (path));
-	strcat (tmppath, ".XXXXXX");
-
-	errno = 0;
-	fd = mkstemp (tmppath);
-	if (fd < 0) {
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "Could not create temporary file for '%s': %d",
-		             path, errno);
-		goto out;
-	}
-
-	/* Only readable by root */
-	errno = 0;
-	if (fchmod (fd, S_IRUSR | S_IWUSR) != 0) {
-		close (fd);
-		unlink (tmppath);
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "Could not set permissions for temporary file '%s': %d",
-		             path, errno);
-		goto out;
-	}
-
-	errno = 0;
-	written = write (fd, data, data_len);
-	if (written != data_len) {
-		close (fd);
-		unlink (tmppath);
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "Could not write temporary file for '%s': %d",
-		             path, errno);
-		goto out;
-	}
-	close (fd);
-
-	/* Try to rename */
-	errno = 0;
-	if (rename (tmppath, path) == 0)
-		success = TRUE;
-	else {
-		unlink (tmppath);
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "Could not rename temporary file to '%s': %d",
-		             path, errno);
-	}
-
-out:
-	g_free (tmppath);
-	return success;
-}
-
 static void
-cert_writer (GKeyFile *file,
+cert_writer (KeyfileWriterInfo *info,
-             const char *keyfile_dir,
-             const char *uuid,
              NMSetting *setting,
              const char *key,
              const GValue *value)
 {
-	const char *setting_name = nm_setting_get_name (setting);
-	NMSetting8021xCKScheme scheme;
-	NMSetting8021xCKFormat format;
-	const char *path = NULL, *ext = "pem";
 	const ObjectType *objtype = NULL;
-	int i;
+	guint i;
+	NMKeyfileWriteTypeDataCert type_data = { 0 };
 
 	for (i = 0; i < G_N_ELEMENTS (objtypes) && objtypes[i].key; i++) {
 		if (g_strcmp0 (objtypes[i].key, key) == 0) {
@@ -503,82 +426,48 @@ cert_writer (GKeyFile *file,
 			break;
 		}
 	}
-	if (!objtype) {
+	if (!objtype)
-		g_return_if_fail (objtype);
-		return;
-	}
-
-	scheme = objtype->scheme_func (NM_SETTING_802_1X (setting));
-	if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) {
-		path = objtype->path_func (NM_SETTING_802_1X (setting));
-		g_assert (path);
-
-		/* If the path is rooted in the keyfile directory, just use a
-		 * relative path instead of an absolute one.
-		 */
-		if (g_str_has_prefix (path, keyfile_dir)) {
-			path += strlen (keyfile_dir);
-			while (*path == '/')
-				path++;
-		}
-
-		nm_keyfile_plugin_kf_set_string (file, setting_name, key, path);
-	} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
-		GBytes *blob;
-		const guint8 *blob_data;
-		gsize blob_len;
-		gboolean success;
-		GError *error = NULL;
-		char *new_path;
-
-		blob = objtype->blob_func (NM_SETTING_802_1X (setting));
-		g_assert (blob);
-		blob_data = g_bytes_get_data (blob, &blob_len);
-
-		if (objtype->format_func) {
-			/* Get the extension for a private key */
-			format = objtype->format_func (NM_SETTING_802_1X (setting));
-			if (format == NM_SETTING_802_1X_CK_FORMAT_PKCS12)
-				ext = "p12";
-		} else {
-			/* DER or PEM format certificate? */
-			if (blob_len > 2 && blob_data[0] == 0x30 && blob_data[1] == 0x82)
-				ext = "der";
-		}
-
-		/* Write the raw data out to the standard file so that we can use paths
-		 * from now on instead of pushing around the certificate data.
-		 */
-		new_path = g_strdup_printf ("%s/%s-%s.%s", keyfile_dir, uuid, objtype->suffix, ext);
-		g_assert (new_path);
-
-		success = write_cert_key_file (new_path, blob_data, blob_len, &error);
-		if (success) {
-			/* Write the path value to the keyfile */
-			nm_keyfile_plugin_kf_set_string (file, setting_name, key, new_path);
-		} else {
-			nm_log_warn (LOGD_SETTINGS, "Failed to write certificate/key %s: %s",
-			             new_path, error->message);
-			g_error_free (error);
-		}
-		g_free (new_path);
-	} else {
-		/* scheme_func() returns UNKNOWN in all other cases. The only valid case
-		 * where a scheme is allowed to be UNKNOWN, is unsetting the value. In this
-		 * case, we don't expect the writer to be called, because the default value
-		 * will not be serialized.
-		 * The only other reason for the scheme to be UNKNOWN is an invalid cert.
-		 * But our connection verifies, so that cannot happen either. */
 		g_return_if_reached ();
+
+	if (!info->handler)
+		goto out_unhandled;
+
+	type_data.setting = NM_SETTING_802_1X (setting);
+	type_data.property_name = key;
+	type_data.suffix = objtype->suffix;
+	type_data.scheme_func = objtype->scheme_func;
+	type_data.format_func = objtype->format_func;
+	type_data.path_func = objtype->path_func;
+	type_data.blob_func = objtype->blob_func;
+
+	if (info->handler (info->connection,
+	                   info->keyfile,
+	                   NM_KEYFILE_WRITE_TYPE_CERT,
+	                   &type_data,
+	                   info->user_data,
+	                   &info->error))
+		return;
+
+out_unhandled:
+
+	/* scheme_func() would not return UNKNOWN, because UNKNOWN happens only
+	 * if the cert is unset (1) or if the cert is invalid (2).
+	 * (1) cannot happen, because we only reach cert_writer() for non-default
+	 * properties. (2) cannot happen, because we verified the connection.
+	 *
+	 * Hence, at this point we do have a certifiacte, but no default implementation
+	 * to write it. The handler *must* do something with these certifications. */
+	if (!info->error) {
+		g_set_error (&info->error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+		             _("Failed to write unhandled certificate property %s.%s"),
+		             nm_setting_get_name (setting), key);
 	}
 }
 
 typedef struct {
 	const char *setting_name;
 	const char *key;
-	void (*writer) (GKeyFile *keyfile,
+	void (*writer) (KeyfileWriterInfo *info,
-	                const char *keyfile_dir,
-	                const char *uuid,
 	                NMSetting *setting,
 	                const char *key,
 	                const GValue *value);
@@ -648,12 +537,6 @@ static KeyWriter key_writers[] = {
 	{ NULL, NULL, NULL }
 };
 
-typedef struct {
-	GKeyFile *keyfile;
-	const char *keyfile_dir;
-	const char *uuid;
-} WriteInfo;
-
 static void
 write_setting_value (NMSetting *setting,
                      const char *key,
@@ -661,12 +544,15 @@ write_setting_value (NMSetting *setting,
                      GParamFlags flag,
                      gpointer user_data)
 {
-	WriteInfo *info = user_data;
+	KeyfileWriterInfo *info = user_data;
 	const char *setting_name;
 	GType type = G_VALUE_TYPE (value);
 	KeyWriter *writer = &key_writers[0];
 	GParamSpec *pspec;
 
+	if (info->error)
+		return;
+
 	/* Setting name gets picked up from the keyfile's section name instead */
 	if (!strcmp (key, NM_SETTING_NAME))
 		return;
@@ -704,7 +590,7 @@ write_setting_value (NMSetting *setting,
 	/* Look through the list of handlers for non-standard format key values */
 	while (writer->setting_name) {
 		if (!strcmp (writer->setting_name, setting_name) && !strcmp (writer->key, key)) {
-			(*writer->writer) (info->keyfile, info->keyfile_dir, info->uuid, setting, key, value);
+			(*writer->writer) (info, setting, key, value);
 			return;
 		}
 		writer++;
@@ -763,184 +649,42 @@ write_setting_value (NMSetting *setting,
 	} else if (type == G_TYPE_HASH_TABLE) {
 		write_hash_of_string (info->keyfile, setting, key, value);
 	} else if (type == G_TYPE_ARRAY) {
-		if (!write_array_of_uint (info->keyfile, setting, key, value)) {
+		write_array_of_uint (info->keyfile, setting, key, value);
-			nm_log_warn (LOGD_SETTINGS, "Unhandled setting property type (write) '%s/%s' : '%s'", 
-			             setting_name, key, g_type_name (type));
-		}
 	} else if (G_VALUE_HOLDS_FLAGS (value)) {
 		/* Flags are guint but GKeyFile has no uint reader, just uint64 */
 		nm_keyfile_plugin_kf_set_uint64 (info->keyfile, setting_name, key, (guint64) g_value_get_flags (value));
 	} else if (G_VALUE_HOLDS_ENUM (value))
 		nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, (gint) g_value_get_enum (value));
-	else {
+	else
-		nm_log_warn (LOGD_SETTINGS, "Unhandled setting property type (write) '%s/%s' : '%s'", 
+		g_warn_if_reached ();
-		             setting_name, key, g_type_name (type));
-	}
 }
 
-static gboolean
+GKeyFile *
-_internal_write_connection (NMConnection *connection,
+nm_keyfile_write (NMConnection *connection,
-                            const char *keyfile_dir,
+                  NMKeyfileWriteHandler handler,
-                            uid_t owner_uid,
+                  void *user_data,
-                            pid_t owner_grp,
+                  GError **error)
-                            const char *existing_path,
-                            char **out_path,
-                            GError **error)
 {
-	GKeyFile *key_file;
+	KeyfileWriterInfo info = { 0 };
-	char *data;
-	gsize len;
-	gboolean success = FALSE;
-	char *path;
-	const char *id;
-	WriteInfo info;
-	GError *local_err = NULL;
 
-	g_return_val_if_fail (!out_path || !*out_path, FALSE);
+	g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
+	g_return_val_if_fail (!error || !*error, NULL);
 
 	if (!nm_connection_verify (connection, error))
-		g_return_val_if_reached (FALSE);
+		return NULL;
 
-	id = nm_connection_get_id (connection);
+	info.connection = connection;
-	g_assert (id && *id);
+	info.keyfile = g_key_file_new ();
-
+	info.error = NULL;
-	info.keyfile = key_file = g_key_file_new ();
+	info.handler = handler;
-	info.keyfile_dir = keyfile_dir;
+	info.user_data = user_data;
-	info.uuid = nm_connection_get_uuid (connection);
-	g_assert (info.uuid);
 	nm_connection_for_each_setting_value (connection, write_setting_value, &info);
-	data = g_key_file_to_data (key_file, &len, error);
-	if (!data)
-		goto out;
 
-	/* If we have existing file path, use it. Else generate one from
+	if (info.error) {
-	 * connection's ID.
+		g_propagate_error (error, info.error);
-	 */
+		g_key_file_unref (info.keyfile);
-	if (existing_path != NULL) {
+		return NULL;
-		path = g_strdup (existing_path);
-	} else {
-		char *filename_escaped = nm_keyfile_plugin_utils_escape_filename (id);
-
-		path = g_build_filename (keyfile_dir, filename_escaped, NULL);
-		g_free (filename_escaped);
 	}
-
+	return info.keyfile;
-	/* If a file with this path already exists (but isn't the existing path
-	 * of the connection) then we need another name.  Multiple connections
-	 * can have the same ID (ie if two connections with the same ID are visible
-	 * to different users) but of course can't have the same path.  Yeah,
-	 * there's a race here, but there's not a lot we can do about it, and
-	 * we shouldn't get more than one connection with the same UUID either.
-	 */
-	if (g_strcmp0 (path, existing_path) != 0 && g_file_test (path, G_FILE_TEST_EXISTS)) {
-		guint i;
-		gboolean name_found = FALSE;
-
-		/* A keyfile with this connection's ID already exists. Pick another name. */
-		for (i = 0; i < 100; i++) {
-			char *filename, *filename_escaped;
-
-			if (i == 0)
-				filename = g_strdup_printf ("%s-%s", id, nm_connection_get_uuid (connection));
-			else
-				filename = g_strdup_printf ("%s-%s-%u", id, nm_connection_get_uuid (connection), i);
-
-			filename_escaped = nm_keyfile_plugin_utils_escape_filename (filename);
-
-			g_free (path);
-			path = g_strdup_printf ("%s/%s", keyfile_dir, filename_escaped);
-			g_free (filename);
-			g_free (filename_escaped);
-			if (g_strcmp0 (path, existing_path) == 0 || !g_file_test (path, G_FILE_TEST_EXISTS)) {
-				name_found = TRUE;
-				break;
-			}
-		}
-		if (!name_found) {
-			if (existing_path == NULL) {
-				/* this really should not happen, we tried hard to find an unused name... bail out. */
-				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-				                    "could not find suitable keyfile file name (%s already used)", path);
-				g_free (path);
-				goto out;
-			}
-			/* Both our preferred path based on connection id and id-uuid are taken.
-			 * Fallback to @existing_path */
-			g_free (path);
-			path = g_strdup (existing_path);
-		}
-	}
-
-	/* In case of updating the connection and changing the file path,
-	 * we need to remove the old one, not to end up with two connections.
-	 */
-	if (existing_path != NULL && strcmp (path, existing_path) != 0)
-		unlink (existing_path);
-
-	g_file_set_contents (path, data, len, &local_err);
-	if (local_err) {
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "%s.%d: error writing to file '%s': %s", __FILE__, __LINE__,
-		             path, local_err->message);
-		g_error_free (local_err);
-		g_free (path);
-		goto out;
-	}
-
-	if (chown (path, owner_uid, owner_grp) < 0) {
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "%s.%d: error chowning '%s': %d", __FILE__, __LINE__,
-		             path, errno);
-		unlink (path);
-	} else {
-		if (chmod (path, S_IRUSR | S_IWUSR) < 0) {
-			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-			             "%s.%d: error setting permissions on '%s': %d", __FILE__,
-			             __LINE__, path, errno);
-			unlink (path);
-		} else {
-			if (out_path && g_strcmp0 (existing_path, path)) {
-				*out_path = path;  /* pass path out to caller */
-				path = NULL;
-			}
-			success = TRUE;
-		}
-	}
-	g_free (path);
-
-out:
-	g_free (data);
-	g_key_file_free (key_file);
-	return success;
-}
-
-gboolean
-nm_keyfile_plugin_write_connection (NMConnection *connection,
-                                    const char *existing_path,
-                                    char **out_path,
-                                    GError **error)
-{
-	return _internal_write_connection (connection,
-	                                   KEYFILE_DIR,
-	                                   0, 0,
-	                                   existing_path,
-	                                   out_path,
-	                                   error);
-}
-
-gboolean
-nm_keyfile_plugin_write_test_connection (NMConnection *connection,
-                                         const char *keyfile_dir,
-                                         uid_t owner_uid,
-                                         pid_t owner_grp,
-                                         char **out_path,
-                                         GError **error)
-{
-	return _internal_write_connection (connection,
-	                                   keyfile_dir,
-	                                   owner_uid, owner_grp,
-	                                   NULL,
-	                                   out_path,
-	                                   error);
 }
 

libnm-core/nm-keyfile-writer.h

@@ -19,23 +19,73 @@
  * Copyright (C) 2008 - 2011 Red Hat, Inc.
  */
 
-#ifndef _KEYFILE_PLUGIN_WRITER_H
+#ifndef __NM_KEYFILE_WRITER_H__
-#define _KEYFILE_PLUGIN_WRITER_H
+#define __NM_KEYFILE_WRITER_H__
 
 #include <sys/types.h>
 #include <glib.h>
-#include <nm-connection.h>
 
-gboolean nm_keyfile_plugin_write_connection (NMConnection *connection,
+#include "nm-connection.h"
-                                             const char *existing_path,
+#include "nm-setting-8021x.h"
-                                             char **out_path,
-                                             GError **error);
 
-gboolean nm_keyfile_plugin_write_test_connection (NMConnection *connection,
-                                                  const char *keyfile_dir,
-                                                  uid_t owner_uid,
-                                                  pid_t owner_grp,
-                                                  char **out_path,
-                                                  GError **error);
 
-#endif /* _KEYFILE_PLUGIN_WRITER_H */
+typedef enum {
+	NM_KEYFILE_WRITE_TYPE_CERT              = 1,
+} NMKeyfileWriteType;
+
+/**
+ * NMKeyfileWriteHandler:
+ *
+ * This is a hook to tweak the serialization.
+ *
+ * Handler for certain properties or events that are not entirely contained
+ * within the keyfile or that might be serialized differently. The @type and
+ * @type_data arguments tell which kind of argument we have at hand.
+ *
+ * Currently only the type %NM_KEYFILE_WRITE_TYPE_CERT is supported, which provides
+ * @type_data as %NMKeyfileWriteTypeDataCert. However, this handler should be generic enough
+ * to support other types as well.
+ *
+ * This don't have to be only "properties". For example, nm_keyfile_read() uses
+ * a similar handler to push warnings to the caller.
+ *
+ * If the handler raises an error, it should set the @error value. This causes
+ * the an overall failure.
+ *
+ * Returns: whether the issue was handled. If the type was unhandled,
+ * a default action will be performed. This might be raise an error,
+ * do some fallback parsing, or do nothing.
+ */
+typedef gboolean (*NMKeyfileWriteHandler) (NMConnection *connection,
+                                           GKeyFile *keyfile,
+                                           NMKeyfileWriteType type,
+                                           void *type_data,
+                                           void *user_data,
+                                           GError **error);
+
+/**
+ * NMKeyfileWriteTypeDataCert:
+ *
+ * this struct is passed as @type_data for the @NMKeyfileWriteHandler of
+ * type %NM_KEYFILE_WRITE_TYPE_CERT.
+ */
+typedef struct {
+	NMSetting8021x *setting;
+	const char *property_name;
+
+	/* The following functions are helpers that simplify the implementation
+	 * of the handler. */
+	const char *suffix;
+	NMSetting8021xCKScheme (*scheme_func) (NMSetting8021x *setting);
+	NMSetting8021xCKFormat (*format_func) (NMSetting8021x *setting);
+	const char *           (*path_func)   (NMSetting8021x *setting);
+	GBytes *               (*blob_func)   (NMSetting8021x *setting);
+} NMKeyfileWriteTypeDataCert;
+
+
+GKeyFile *nm_keyfile_write (NMConnection *connection,
+                            NMKeyfileWriteHandler handler,
+                            void *user_data,
+                            GError **error);
+
+#endif /* __NM_KEYFILE_WRITER_H__ */

po/POTFILES.in

@@ -48,6 +48,8 @@ libnm-core/crypto.c
 libnm-core/crypto_gnutls.c
 libnm-core/crypto_nss.c
 libnm-core/nm-connection.c
+libnm-core/nm-keyfile-reader.c
+libnm-core/nm-keyfile-writer.c
 libnm-core/nm-setting-8021x.c
 libnm-core/nm-setting-adsl.c
 libnm-core/nm-setting-bluetooth.c