keyfile: add nm_keyfile_plugin_utils_escape_filename() function

We have nm_keyfile_plugin_utils_should_ignore_file() to ignore certain
files based on patterns. We also need a matching escape function to
avoid saving connections with a name we would ignore later.

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

3 files changed, 60 insertions, 30 deletions

diff --git a/src/settings/plugins/keyfile/utils.c b/src/settings/plugins/keyfile/utils.c
index 48eb0878be..e76c9da073 100644
--- a/src/settings/plugins/keyfile/utils.c
+++ b/src/settings/plugins/keyfile/utils.c
@@ -96,6 +96,7 @@ nm_keyfile_plugin_utils_should_ignore_file (const char *filename)
 	g_return_val_if_fail (base != NULL, TRUE);
 
 	/* Ignore files with certain patterns */
+	/* should_ignore_file() must mirror escape_filename() */
 	if (   (check_prefix (base, ".") && check_suffix (base, SWP_TAG))   /* vim temporary files: .filename.swp */
 	    || (check_prefix (base, ".") && check_suffix (base, SWPX_TAG))  /* vim temporary files: .filename.swpx */
 	    || check_suffix (base, PEM_TAG)                                 /* 802.1x certificates and keys */
@@ -109,6 +110,44 @@ nm_keyfile_plugin_utils_should_ignore_file (const char *filename)
 	return ignore;
 }
 
+char *
+nm_keyfile_plugin_utils_escape_filename (const char *filename)
+{
+	GString *str;
+	const char *f = filename;
+	const char ESCAPE_CHAR = '*';
+
+	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++) {
+		switch (f[0]) {
+		case '/':
+			g_string_append_c (str, ESCAPE_CHAR);
+			break;
+		default:
+			g_string_append_c (str, f[0]);
+			break;
+		}
+	}
+
+	/* 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_CHAR;
+	if (check_suffix (str->str, "~"))
+		str->str[str->len - 1] = ESCAPE_CHAR;
+	if (   check_mkstemp_suffix (str->str)
+	    || check_suffix (str->str, PEM_TAG)
+	    || check_suffix (str->str, DER_TAG))
+		g_string_append_c (str, ESCAPE_CHAR);
+
+	return g_string_free (str, FALSE);;
+}
+
+
 typedef struct {
 	const char *setting;
 	const char *alias;
diff --git a/src/settings/plugins/keyfile/utils.h b/src/settings/plugins/keyfile/utils.h
index d153367103..456cd1a88f 100644
--- a/src/settings/plugins/keyfile/utils.h
+++ b/src/settings/plugins/keyfile/utils.h
@@ -33,6 +33,8 @@
 
 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);
 
 const char *nm_keyfile_plugin_get_setting_name_for_alias (const char *alias);
diff --git a/src/settings/plugins/keyfile/writer.c b/src/settings/plugins/keyfile/writer.c
index 7f3751faae..5b6fdad4f4 100644
--- a/src/settings/plugins/keyfile/writer.c
+++ b/src/settings/plugins/keyfile/writer.c
@@ -775,26 +775,6 @@ write_setting_value (NMSetting *setting,
 	}
 }
 
-static char *
-_writer_id_to_filename (const char *id)
-{
-	char *filename, *f;
-	const char *i = id;
-
-	f = filename = g_malloc0 (strlen (id) + 1);
-
-	/* Convert '/' to '*' */
-	while (*i) {
-		if (*i == '/')
-			*f++ = '*';
-		else
-			*f++ = *i;
-		i++;
-	}
-
-	return filename;
-}
-
 static gboolean
 _internal_write_connection (NMConnection *connection,
                             const char *keyfile_dir,
@@ -808,7 +788,7 @@ _internal_write_connection (NMConnection *connection,
 	char *data;
 	gsize len;
 	gboolean success = FALSE;
-	char *filename = NULL, *path;
+	char *path;
 	const char *id;
 	WriteInfo info;
 	GError *local_err = NULL;
@@ -836,8 +816,10 @@ _internal_write_connection (NMConnection *connection,
 	if (existing_path != NULL) {
 		path = g_strdup (existing_path);
 	} else {
-		filename = _writer_id_to_filename (id);
-		path = g_build_filename (keyfile_dir, filename, NULL);
+		char *filename_escaped = nm_keyfile_plugin_utils_escape_filename (id);
+
+		path = g_build_filename (keyfile_dir, filename_escaped, NULL);
+		g_free (filename_escaped);
 	}
 
 	/* If a file with this path already exists (but isn't the existing path
@@ -853,27 +835,35 @@ _internal_write_connection (NMConnection *connection,
 
 		/* A keyfile with this connection's ID already exists. Pick another name. */
 		for (i = 0; i < 100; i++) {
-			g_free (path);
+			char *filename, *filename_escaped;
+
 			if (i == 0)
-				path = g_strdup_printf ("%s/%s-%s", keyfile_dir, filename, nm_connection_get_uuid (connection));
+				filename = g_strdup_printf ("%s-%s", id, nm_connection_get_uuid (connection));
 			else
-				path = g_strdup_printf ("%s/%s-%s-%u", keyfile_dir, filename, nm_connection_get_uuid (connection), i);
+				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) {
-			g_free (path);
 			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,
-				                    "%s.%d: could not find suitable keyfile file name (%s/%s already used)",
-				                    __FILE__, __LINE__, keyfile_dir, filename);
+				                    "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);
 		}
 	}
@@ -916,7 +906,6 @@ _internal_write_connection (NMConnection *connection,
 	g_free (path);
 
 out:
-	g_free (filename);
 	g_free (data);
 	g_key_file_free (key_file);
 	return success;