keyfile: merge branch 'th/keyfile-escape-file-bgo735824'

This changes behavior, in that we now ignore keyfiles that
start with a dot ('.'). This means, that connection with ids
starting with a dot, will be ignored.

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

(cherry picked from commit 6ccb888831)

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", TRUE);
+}
+
+/*****************************************************************************/
+
 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

@@ -23,6 +23,7 @@
 #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>
@@ -59,7 +60,7 @@ check_prefix (const char *base, const char *tag)
 
 	len = strlen (base);
 	tag_len = strlen (tag);
-	if ((len > tag_len) && !strncasecmp (base, tag, tag_len))
+	if ((len > tag_len) && !g_ascii_strncasecmp (base, tag, tag_len))
 		return TRUE;
 	return FALSE;
 }
@@ -74,7 +75,7 @@ check_suffix (const char *base, const char *tag)
 
 	len = strlen (base);
 	tag_len = strlen (tag);
-	if ((len > tag_len) && !strcasecmp (base + len - tag_len, tag))
+	if ((len > tag_len) && !g_ascii_strcasecmp (base + len - tag_len, tag))
 		return TRUE;
 	return FALSE;
 }
@@ -87,28 +88,65 @@ check_suffix (const char *base, const char *tag)
 gboolean
 nm_keyfile_plugin_utils_should_ignore_file (const char *filename)
 {
-	char *base;
-	gboolean ignore = FALSE;
+	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 files with certain patterns */
-	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 */
-	    || check_suffix (base, DER_TAG)                                 /* 802.1x certificates and keys */
-	    || check_mkstemp_suffix (base)                                  /* temporary files created by mkstemp() */
-	    || check_prefix (base, ".#")                                    /* Emacs locking file (link) */
-	    || base[strlen (base) - 1] == '~')
-		ignore = 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;
 
-	g_free (base);
-	return ignore;
+	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 {
 	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;