diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index edece1b2bc..8155aed567 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/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 (); } diff --git a/src/settings/plugins/keyfile/utils.c b/src/settings/plugins/keyfile/utils.c index 48eb0878be..5ff14e1f9e 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 = '*'; + + /* 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; 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 26818a50c0..314ff11608 100644 --- a/src/settings/plugins/keyfile/writer.c +++ b/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;