fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-03-17 18:20:33 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

keyfile: write keyfiles to "/run" directory with ".nmconnection" file suffix

Browse source 
For profiles in "/etc/NetworkManager/system-connections", we did not enforce
that the keyfiles have a special suffix, nor did we generate the
filenames in such a manner. In hindsight, I think that was a mistake.

Recently we added "/run/NetworkManager/system-connections" as additional
keyfile directory. Enforce a suffix and write keyfiles with such a name.

In principle, we could also start writing keyfiles in /etc with the
same suffix. But let's not do that, because we anyway cannot enforce
it.

An ugly part is, that during `nmcli connection load` we need to
determine whether the to-be-loaded connection is under /etc or /run.
Preferably, we would allow any kind of symlinking as what matters
is the file object (inode) and not the path. Anyway, we don't do
that but compare plain paths. That means, paths which are not
in an expected form, will be rejected. In particular, the paths
starting with "/run/..." and "/var/run/..." will be treated differently,
and one of them will be rejected.

Note that ifcfg-rh plugin strictly enforces that the path
starts with IFCFG_DIR as well. So, while this is a breaking
change for keyfile, I think it's reasonable.

(cherry picked from commit 648c256b90)
This commit is contained in:
Thomas Haller 2018-10-18 16:36:51 +02:00
parent ae5a09d720
commit 7685cf2840
6 changed files with 125 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
src/settings/plugins/keyfile/nms-keyfile-plugin.c
View file

@ -322,7 +322,7 @@ dir_changed (GFileMonitor *monitor,
gboolean exists;
full_path = g_file_get_path (file);
if (nms_keyfile_utils_should_ignore_file (full_path)) {
if (nms_keyfile_utils_should_ignore_file (full_path, FALSE)) {
g_free (full_path);
return;
}
@ -428,7 +428,9 @@ _sort_paths (const char **f1, const char **f2, GHashTable *paths)
}
static void
_read_dir (GPtrArray *filenames, const char *path)
_read_dir (GPtrArray *filenames,
const char *path,
gboolean require_extension)
{
GDir *dir;
const char *item;
@ -442,7 +444,7 @@ _read_dir (GPtrArray *filenames, const char *path)
}
while ((item = g_dir_read_name (dir))) {
if (nms_keyfile_utils_should_ignore_file (item))
if (nms_keyfile_utils_should_ignore_file (item, require_extension))
continue;
g_ptr_array_add (filenames, g_build_filename (path, item, NULL));
}
@ -465,8 +467,8 @@ read_connections (NMSettingsPlugin *config)
filenames = g_ptr_array_new_with_free_func (g_free);
_read_dir (filenames, NM_CONFIG_KEYFILE_PATH_IN_MEMORY);
_read_dir (filenames, nms_keyfile_utils_get_path ());
_read_dir (filenames, NM_CONFIG_KEYFILE_PATH_IN_MEMORY, TRUE);
_read_dir (filenames, nms_keyfile_utils_get_path (), FALSE);
alive_connections = g_hash_table_new (nm_direct_hash, NULL);
@ -520,14 +522,62 @@ get_connections (NMSettingsPlugin *config)
return _nm_utils_hash_values_to_slist (priv->connections);
}
static gboolean
_file_is_in_path (const char *abs_filename,
const char *abs_path)
{
gsize l;
/* FIXME: ensure that both paths are at least normalized (coalescing ".",
* duplicate '/', and trailing '/'). */
nm_assert (abs_filename && abs_filename[0] == '/');
nm_assert (abs_path && abs_path[0] == '/');
l = strlen (abs_path);
if (strncmp (abs_filename, abs_path, l) != 0)
return FALSE;
abs_filename += l;
while (abs_filename[0] == '/')
abs_filename++;
if (!abs_filename[0])
return FALSE;
if (strchr (abs_filename, '/'))
return FALSE;
return TRUE;
}
static gboolean
load_connection (NMSettingsPlugin *config,
const char *filename)
{
NMSKeyfilePlugin *self = NMS_KEYFILE_PLUGIN ((NMSKeyfilePlugin *) config);
NMSKeyfileConnection *connection;
gboolean require_extension;
if (nms_keyfile_utils_should_ignore_file (filename))
/* the test whether to require a file extension tries to figure out whether
* the provided filename is inside /etc or /run.
*
* However, on Posix a filename just resolves to an Inode, and there can
* be any kind of paths that point to the same Inode. It's not generally possible
* to check for that (unless, we would stat all files in the target directory
* and see whether their inode matches).
*
* So, when loading the file do something simpler: require that the path
* starts with the well-known prefix. This rejects symlinks or hard links
* which would actually also point to the same file. */
if (_file_is_in_path (filename, nms_keyfile_utils_get_path ()))
require_extension = FALSE;
else if (_file_is_in_path (filename, NM_CONFIG_KEYFILE_PATH_IN_MEMORY))
require_extension = TRUE;
else
return FALSE;
if (nms_keyfile_utils_should_ignore_file (filename, require_extension))
return FALSE;
connection = update_connection (self, NULL, filename, find_by_path (self, filename), TRUE, NULL, NULL);

10
src/settings/plugins/keyfile/nms-keyfile-reader.c
View file

@ -114,6 +114,7 @@ nms_keyfile_reader_from_keyfile (GKeyFile *key_file,
};
gs_free char *base_dir_free = NULL;
gs_free char *profile_filename_free = NULL;
gs_free char *filename_id = NULL;
const char *profile_filename = NULL;
nm_assert (filename && filename[0]);
@ -141,7 +142,14 @@ nms_keyfile_reader_from_keyfile (GKeyFile *key_file,
if (!connection)
return NULL;
nm_keyfile_read_ensure_id (connection, filename);
if (g_str_has_suffix (filename, NMS_KEYFILE_PATH_SUFFIX_NMCONNECTION)) {
gsize l = strlen (filename);
if (l > NM_STRLEN (NMS_KEYFILE_PATH_SUFFIX_NMCONNECTION))
filename_id = g_strndup (filename, l - NM_STRLEN (NMS_KEYFILE_PATH_SUFFIX_NMCONNECTION));
}
nm_keyfile_read_ensure_id (connection, filename_id ?: filename);
if (!profile_filename) {
profile_filename_free = g_build_filename (profile_dir ?: base_dir, filename, NULL);

23
src/settings/plugins/keyfile/nms-keyfile-utils.c
View file

@ -84,7 +84,7 @@ check_suffix (const char *base, const char *tag)
#define DER_TAG ".der"
gboolean
nms_keyfile_utils_should_ignore_file (const char *filename)
nms_keyfile_utils_should_ignore_file (const char *filename, gboolean require_extension)
{
gs_free char *base = NULL;
@ -104,6 +104,14 @@ nms_keyfile_utils_should_ignore_file (const char *filename)
if (check_suffix (base, PEM_TAG) || check_suffix (base, DER_TAG))
return TRUE;
if (require_extension) {
gsize l = strlen (base);
if ( l <= NM_STRLEN (NMS_KEYFILE_PATH_SUFFIX_NMCONNECTION)
|| !g_str_has_suffix (base, NMS_KEYFILE_PATH_SUFFIX_NMCONNECTION))
return TRUE;
}
return FALSE;
}
@ -167,14 +175,16 @@ nms_keyfile_utils_check_file_permissions (const char *filename,
/*****************************************************************************/
char *
nms_keyfile_utils_escape_filename (const char *filename)
nms_keyfile_utils_escape_filename (const char *filename,
gboolean with_extension)
{
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. */
*
* But for newly added escapings, use '_' instead.
* Also, @with_extension is new-style. */
const char ESCAPE_CHAR = with_extension ? '_' : '*';
const char ESCAPE_CHAR2 = '_';
g_return_val_if_fail (filename && filename[0], NULL);
@ -200,6 +210,9 @@ nms_keyfile_utils_escape_filename (const char *filename)
|| check_suffix (str->str, DER_TAG))
g_string_append_c (str, ESCAPE_CHAR2);
if (with_extension)
g_string_append (str, NMS_KEYFILE_PATH_SUFFIX_NMCONNECTION);
return g_string_free (str, FALSE);;
}

6
src/settings/plugins/keyfile/nms-keyfile-utils.h
View file

@ -25,15 +25,17 @@
#define NM_CONFIG_KEYFILE_PATH_IN_MEMORY NMRUNDIR "/system-connections"
#define NMS_KEYFILE_PATH_SUFFIX_NMCONNECTION ".nmconnection"
#define NMS_KEYFILE_CONNECTION_LOG_PATH(path) ((path) ?: "in-memory")
#define NMS_KEYFILE_CONNECTION_LOG_FMT "%s (%s,\"%s\")"
#define NMS_KEYFILE_CONNECTION_LOG_ARG(con) NMS_KEYFILE_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_settings_connection_get_uuid ((NMSettingsConnection *) (con)), nm_settings_connection_get_id ((NMSettingsConnection *) (con))
#define NMS_KEYFILE_CONNECTION_LOG_FMTD "%s (%s,\"%s\",%p)"
#define NMS_KEYFILE_CONNECTION_LOG_ARGD(con) NMS_KEYFILE_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_settings_connection_get_uuid ((NMSettingsConnection *) (con)), nm_settings_connection_get_id ((NMSettingsConnection *) (con)), (con)
gboolean nms_keyfile_utils_should_ignore_file (const char *filename);
gboolean nms_keyfile_utils_should_ignore_file (const char *filename, gboolean require_extension);
char *nms_keyfile_utils_escape_filename (const char *filename);
char *nms_keyfile_utils_escape_filename (const char *filename, gboolean with_extension);
const char *nms_keyfile_utils_get_path (void);

18
src/settings/plugins/keyfile/nms-keyfile-writer.c
View file

@ -173,6 +173,7 @@ static gboolean
_internal_write_connection (NMConnection *connection,
const char *keyfile_dir,
const char *profile_dir,
gboolean with_extension,
uid_t owner_uid,
pid_t owner_grp,
const char *existing_path,
@ -229,7 +230,7 @@ _internal_write_connection (NMConnection *connection,
if (existing_path != NULL && !rename) {
path = g_strdup (existing_path);
} else {
char *filename_escaped = nms_keyfile_utils_escape_filename (id);
char *filename_escaped = nms_keyfile_utils_escape_filename (id, with_extension);
path = g_build_filename (keyfile_dir, filename_escaped, NULL);
g_free (filename_escaped);
@ -255,7 +256,7 @@ _internal_write_connection (NMConnection *connection,
else
filename = g_strdup_printf ("%s-%s-%u", id, nm_connection_get_uuid (connection), i);
filename_escaped = nms_keyfile_utils_escape_filename (filename);
filename_escaped = nms_keyfile_utils_escape_filename (filename, with_extension);
g_free (path);
path = g_strdup_printf ("%s/%s", keyfile_dir, filename_escaped);
@ -351,16 +352,21 @@ nms_keyfile_writer_connection (NMConnection *connection,
GError **error)
{
const char *keyfile_dir;
gboolean with_extension = FALSE;
if (save_to_disk)
keyfile_dir = nms_keyfile_utils_get_path ();
else
else {
keyfile_dir = NM_CONFIG_KEYFILE_PATH_IN_MEMORY;
with_extension = TRUE;
}
return _internal_write_connection (connection,
keyfile_dir,
nms_keyfile_utils_get_path (),
0, 0,
with_extension,
0,
0,
existing_path,
force_rename,
out_path,
@ -382,7 +388,9 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
return _internal_write_connection (connection,
keyfile_dir,
keyfile_dir,
owner_uid, owner_grp,
FALSE,
owner_uid,
owner_grp,
NULL,
FALSE,
out_path,

48
src/settings/plugins/keyfile/tests/test-keyfile.c
View file

@ -2460,49 +2460,51 @@ test_write_tc_config (void)
/*****************************************************************************/
static void
_escape_filename (const char *filename, gboolean would_be_ignored)
_escape_filename (gboolean with_extension, const char *filename, gboolean would_be_ignored)
{
gs_free char *esc = NULL;
g_assert (filename && filename[0]);
if (!!would_be_ignored != !!nms_keyfile_utils_should_ignore_file (filename)) {
if (!!would_be_ignored != !!nms_keyfile_utils_should_ignore_file (filename, with_extension)) {
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 = nms_keyfile_utils_escape_filename (filename);
esc = nms_keyfile_utils_escape_filename (filename, with_extension);
g_assert (esc && esc[0]);
g_assert (!strchr (esc, '/'));
if (nms_keyfile_utils_should_ignore_file (esc))
if (nms_keyfile_utils_should_ignore_file (esc, with_extension))
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);
_escape_filename (FALSE, "ab", FALSE);
_escape_filename (FALSE, ".vim-file.swp", TRUE);
_escape_filename (FALSE, ".vim-file.Swp", TRUE);
_escape_filename (FALSE, ".vim-file.SWP", TRUE);
_escape_filename (FALSE, ".vim-file.swpx", TRUE);
_escape_filename (FALSE, ".vim-file.Swpx", TRUE);
_escape_filename (FALSE, ".vim-file.SWPX", TRUE);
_escape_filename (FALSE, ".pem-file.pem", TRUE);
_escape_filename (FALSE, ".pem-file.Pem", TRUE);
_escape_filename (FALSE, ".pem-file.PEM", TRUE);
_escape_filename (FALSE, ".pem-file.der", TRUE);
_escape_filename (FALSE, ".pem-file.Der", TRUE);
_escape_filename (FALSE, ".mkstemp.ABCEDF", TRUE);
_escape_filename (FALSE, ".mkstemp.abcdef", TRUE);
_escape_filename (FALSE, ".mkstemp.123456", TRUE);
_escape_filename (FALSE, ".mkstemp.A23456", TRUE);
_escape_filename (FALSE, ".#emacs-locking", TRUE);
_escape_filename (FALSE, "file-with-tilde~", TRUE);
_escape_filename (FALSE, ".file-with-dot", TRUE);
_escape_filename (TRUE, "lala", TRUE);
}
/*****************************************************************************/