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
(cherry picked from commit a823217b1f)

src/settings/plugins/keyfile/tests/test-keyfile.c

@@ -33,6 +33,7 @@
 
 #include "reader.h"
 #include "writer.h"
+#include "utils.h"
 
 #include "nm-test-utils.h"
 
@@ -3566,6 +3567,56 @@ test_write_flags_property (void)
 	g_object_unref (connection);
 }
 
+/*****************************************************************************/
+
+static void
+_escape_filename (const char *filename, gboolean would_be_ignored)
+{
+	gs_free char *esc = NULL;
+
+	g_assert (filename && filename[0]);
+
+	if (!!would_be_ignored != !!nm_keyfile_plugin_utils_should_ignore_file (filename)) {
+		if (would_be_ignored)
+			g_error ("We expect filename \"%s\" to be ignored, but it isn't", filename);
+		else
+			g_error ("We expect filename \"%s\" not to be ignored, but it is", filename);
+	}
+
+	esc = nm_keyfile_plugin_utils_escape_filename (filename);
+	g_assert (esc && esc[0]);
+	g_assert (!strchr (esc, '/'));
+
+	if (nm_keyfile_plugin_utils_should_ignore_file (esc))
+		g_error ("Escaping filename \"%s\" yielded \"%s\", but this is ignored", filename, esc);
+}
+
+static void
+test_nm_keyfile_plugin_utils_escape_filename (void)
+{
+	_escape_filename ("ab", FALSE);
+	_escape_filename (".vim-file.swp", TRUE);
+	_escape_filename (".vim-file.Swp", TRUE);
+	_escape_filename (".vim-file.SWP", TRUE);
+	_escape_filename (".vim-file.swpx", TRUE);
+	_escape_filename (".vim-file.Swpx", TRUE);
+	_escape_filename (".vim-file.SWPX", TRUE);
+	_escape_filename (".pem-file.pem", TRUE);
+	_escape_filename (".pem-file.Pem", TRUE);
+	_escape_filename (".pem-file.PEM", TRUE);
+	_escape_filename (".pem-file.der", TRUE);
+	_escape_filename (".pem-file.Der", TRUE);
+	_escape_filename (".mkstemp.ABCEDF", TRUE);
+	_escape_filename (".mkstemp.abcdef", TRUE);
+	_escape_filename (".mkstemp.123456", TRUE);
+	_escape_filename (".mkstemp.A23456", TRUE);
+	_escape_filename (".#emacs-locking", TRUE);
+	_escape_filename ("file-with-tilde~", TRUE);
+	_escape_filename (".file-with-dot", FALSE);
+}
+
+/*****************************************************************************/
+
 NMTST_DEFINE ();
 
 int main (int argc, char **argv)
@@ -3636,6 +3687,8 @@ int main (int argc, char **argv)
 	g_test_add_func ("/keyfile/test_read_flags_property ", test_read_flags_property);
 	g_test_add_func ("/keyfile/test_write_flags_property ", test_write_flags_property);
 
+	g_test_add_func ("/keyfile/test_nm_keyfile_plugin_utils_escape_filename ", test_nm_keyfile_plugin_utils_escape_filename);
+
 	return g_test_run ();
 }
 

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 = '*';
+
+	/* 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 {
 	const char *setting;
 	const char *alias;

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);

src/settings/plugins/keyfile/writer.c

@@ -778,26 +778,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,
@@ -811,7 +791,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;
@@ -839,8 +819,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
@@ -856,27 +838,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);
 		}
 	}
@@ -919,7 +909,6 @@ _internal_write_connection (NMConnection *connection,
 	g_free (path);
 
 out:
-	g_free (filename);
 	g_free (data);
 	g_key_file_free (key_file);
 	return success;