core: merge branch 'th/file-is-in-path'

https://github.com/NetworkManager/NetworkManager/pull/238

12 files changed, 239 insertions, 68 deletions

diff --git a/Makefile.am b/Makefile.am
index f99481ab7f..d735627b67 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1453,6 +1453,8 @@ src_libsystemd_nm_la_libadd = \
 src_libsystemd_nm_la_SOURCES = \
 	src/systemd/nm-sd.c \
 	src/systemd/nm-sd.h \
+	src/systemd/nm-sd-utils.c \
+	src/systemd/nm-sd-utils.h \
 	src/systemd/sd-adapt/nm-sd-adapt.c \
 	src/systemd/sd-adapt/nm-sd-adapt.h \
 	src/systemd/sd-adapt/architecture.h \
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index 1404854d71..ae17ee5179 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -34,6 +34,7 @@
 
 #include "platform/nm-platform.h"
 #include "nm-auth-utils.h"
+#include "systemd/nm-sd-utils.h"
 
 /*****************************************************************************/
 
@@ -998,3 +999,62 @@ nm_shutdown_wait_obj_unregister (NMShutdownWaitObjHandle *handle)
 	g_object_weak_unref (handle->watched_obj, _shutdown_waitobj_cb, handle);
 	_shutdown_waitobj_unregister (handle);
 }
+
+/*****************************************************************************/
+
+/**
+ * nm_utils_file_is_in_path:
+ * @abs_filename: the absolute filename to test
+ * @abs_path: the absolute path, to check whether filename is in.
+ *
+ * This tests, whether @abs_filename is a file which lies inside @abs_path.
+ * Basically, this checks whether @abs_filename is the same as @abs_path +
+ * basename(@abs_filename). It allows simple normalizations, like coalescing
+ * multiple "//".
+ *
+ * However, beware that this function is purely filename based. That means,
+ * it will reject files that reference the same file (i.e. inode) via
+ * symlinks or bind mounts. Maybe one would like to check for file (inode)
+ * identity, but that is not really possible based on the file name alone.
+ *
+ * This means, that nm_utils_file_is_in_path("/var/run/some-file", "/var/run")
+ * will succeed, but nm_utils_file_is_in_path("/run/some-file", "/var/run")
+ * will not (although, it's well known that they reference the same path).
+ *
+ * Also, note that @abs_filename must not have trailing slashes itself.
+ * So, this will reject nm_utils_file_is_in_path("/usr/lib/", "/usr") as
+ * invalid, because the function searches for file names (and "lib/" is
+ * clearly a directory).
+ *
+ * Returns: if @abs_filename is a file inside @abs_path, returns the
+ *   trailing part of @abs_filename which is the filename. Otherwise
+ *   %NULL.
+ */
+const char *
+nm_utils_file_is_in_path (const char *abs_filename,
+                          const char *abs_path)
+{
+	const char *path;
+
+	g_return_val_if_fail (abs_filename && abs_filename[0] == '/', NULL);
+	g_return_val_if_fail (abs_path && abs_path[0] == '/', NULL);
+
+	path = nm_sd_utils_path_startswith (abs_filename, abs_path);
+	if (!path)
+		return NULL;
+
+	nm_assert (path[0] != '/');
+	nm_assert (path > abs_filename);
+	nm_assert (path <= &abs_filename[strlen (abs_filename)]);
+
+	/* we require a non-empty remainder with no slashes. That is, only a filename.
+	 *
+	 * Note this will reject "/var/run/" as not being in "/var",
+	 * while "/var/run" would pass. The function searches for files
+	 * only, so a trailing slash (indicating a directory) is not allowed).
+	 * This is despite that the function cannot determine whether "/var/run"
+	 * is itself a file or a directory. "*/
+	return path[0] && !strchr (path, '/')
+	       ? path
+	       : NULL;
+}
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index 26907243f6..93edb912dd 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -86,4 +86,10 @@ void nm_shutdown_wait_obj_unregister (NMShutdownWaitObjHandle *handle);
 
 /*****************************************************************************/
 
+const char *
+nm_utils_file_is_in_path (const char *abs_filename,
+                          const char *abs_path);
+
+/*****************************************************************************/
+
 #endif /* __NETWORKMANAGER_UTILS_H__ */
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 50f127c090..75dd2e2c22 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -47,6 +47,10 @@
 #include "nm-setting-wireless.h"
 #include "nm-setting-wireless-security.h"
 
+#ifdef __NM_SD_UTILS_H__
+#error "nm-core-utils.c should stay independent of systemd utils. Are you looking for NetworkMangerUtils.c? "
+#endif
+
 G_STATIC_ASSERT (sizeof (NMUtilsTestFlags) <= sizeof (int));
 static int _nm_utils_testing = 0;
 
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c
index 509d41734a..05d4d7386b 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c
@@ -602,12 +602,9 @@ load_connection (NMSettingsPlugin *config,
 {
 	SettingsPluginIfcfg *plugin = SETTINGS_PLUGIN_IFCFG (config);
 	NMIfcfgConnection *connection;
-	int dir_len = strlen (IFCFG_DIR);
 	char *ifcfg_path;
 
-	if (   strncmp (filename, IFCFG_DIR, dir_len) != 0
-	    || filename[dir_len] != '/'
-	    || strchr (filename + dir_len + 1, '/') != NULL)
+	if (!nm_utils_file_is_in_path (filename, IFCFG_DIR))
 		return FALSE;
 
 	/* get the real ifcfg-path. This allows us to properly
diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
index c7d6efd728..1c9a06b1e0 100644
--- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c
+++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
@@ -171,7 +171,6 @@ update_connection (NMSKeyfilePlugin *self,
 	NMSKeyfileConnection *connection_by_uuid;
 	GError *local = NULL;
 	const char *uuid;
-	int dir_len;
 
 	g_return_val_if_fail (!source || NM_IS_CONNECTION (source), NULL);
 	g_return_val_if_fail (full_path || source, NULL);
@@ -179,17 +178,8 @@ update_connection (NMSKeyfilePlugin *self,
 	if (full_path)
 		_LOGD ("loading from file \"%s\"...", full_path);
 
-	if (g_str_has_prefix (full_path, nms_keyfile_utils_get_path ())) {
-		dir_len = strlen (nms_keyfile_utils_get_path ());
-	} else if (g_str_has_prefix (full_path, NM_CONFIG_KEYFILE_PATH_IN_MEMORY)) {
-		dir_len = NM_STRLEN (NM_CONFIG_KEYFILE_PATH_IN_MEMORY);
-	} else {
-		/* Just make sure the file name is not going go pass the following check. */
-		dir_len = strlen (full_path);
-	}
-
-	if (   full_path[dir_len] != '/'
-	    || strchr (full_path + dir_len + 1, '/') != NULL) {
+	if (   !nm_utils_file_is_in_path (full_path, nms_keyfile_utils_get_path ())
+	    && !nm_utils_file_is_in_path (full_path, NM_CONFIG_KEYFILE_PATH_IN_MEMORY)) {
 		g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
 		                     "File not in recognized system-connections directory");
 		return FALSE;
@@ -523,35 +513,6 @@ get_connections (NMSettingsPlugin *config)
 }
 
 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)
 {
@@ -559,20 +520,9 @@ load_connection (NMSettingsPlugin *config,
 	NMSKeyfileConnection *connection;
 	gboolean require_extension;
 
-	/* 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 ()))
+	if (nm_utils_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))
+	else if (nm_utils_file_is_in_path (filename, NM_CONFIG_KEYFILE_PATH_IN_MEMORY))
 		require_extension = TRUE;
 	else
 		return FALSE;
diff --git a/src/systemd/meson.build b/src/systemd/meson.build
index 870721b0d7..1bf1ea41d4 100644
--- a/src/systemd/meson.build
+++ b/src/systemd/meson.build
@@ -49,7 +49,8 @@ sources = files(
   'src/libsystemd/sd-id128/id128-util.c',
   'src/libsystemd/sd-id128/sd-id128.c',
   'src/shared/dns-domain.c',
-  'nm-sd.c'
+  'nm-sd.c',
+  'nm-sd-utils.c',
 )
 
 incs = [
diff --git a/src/systemd/nm-sd-utils.c b/src/systemd/nm-sd-utils.c
new file mode 100644
index 0000000000..914b4b448c
--- /dev/null
+++ b/src/systemd/nm-sd-utils.c
@@ -0,0 +1,45 @@
+/* This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ */
+
+#include "nm-default.h"
+
+#include "nm-sd-utils.h"
+
+#include "nm-sd-adapt.h"
+
+#include "path-util.h"
+
+/*****************************************************************************/
+
+gboolean
+nm_sd_utils_path_equal (const char *a, const char *b)
+{
+	return path_equal (a, b);
+}
+
+char *
+nm_sd_utils_path_simplify (char *path, gboolean kill_dots)
+{
+	return path_simplify (path, kill_dots);
+}
+
+const char *
+nm_sd_utils_path_startswith (const char *path, const char *prefix)
+{
+	return path_startswith (path, prefix);
+}
diff --git a/src/systemd/nm-sd-utils.h b/src/systemd/nm-sd-utils.h
new file mode 100644
index 0000000000..8ca920e273
--- /dev/null
+++ b/src/systemd/nm-sd-utils.h
@@ -0,0 +1,33 @@
+/* This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ */
+
+#ifndef __NM_SD_UTILS_H__
+#define __NM_SD_UTILS_H__
+
+/*****************************************************************************/
+
+gboolean nm_sd_utils_path_equal (const char *a, const char *b);
+
+char *nm_sd_utils_path_simplify (char *path, gboolean kill_dots);
+
+const char *nm_sd_utils_path_startswith (const char *path, const char *prefix);
+
+/*****************************************************************************/
+
+#endif /* __NM_SD_UTILS_H__ */
+
diff --git a/src/systemd/src/basic/path-util.c b/src/systemd/src/basic/path-util.c
index df0fe2bcce..5877957822 100644
--- a/src/systemd/src/basic/path-util.c
+++ b/src/systemd/src/basic/path-util.c
@@ -337,12 +337,15 @@ char *path_simplify(char *path, bool kill_dots) {
         /* Removes redundant inner and trailing slashes. Also removes unnecessary dots
          * if kill_dots is true. Modifies the passed string in-place.
          *
-         * ///foo//./bar/.   becomes /foo/./bar/.  (if kill_dots is false)
-         * ///foo//./bar/.   becomes /foo/bar      (if kill_dots is true)
-         * .//./foo//./bar/. becomes ./foo/bar     (if kill_dots is false)
-         * .//./foo//./bar/. becomes foo/bar       (if kill_dots is true)
+         * ///foo//./bar/.   becomes /foo/./bar/.      (if kill_dots is false)
+         * ///foo//./bar/.   becomes /foo/bar          (if kill_dots is true)
+         * .//./foo//./bar/. becomes ././foo/./bar/.   (if kill_dots is false)
+         * .//./foo//./bar/. becomes foo/bar           (if kill_dots is true)
          */
 
+        if (isempty(path))
+            return path;
+
         absolute = path_is_absolute(path);
 
         f = path;
@@ -372,15 +375,19 @@ char *path_simplify(char *path, bool kill_dots) {
                 *(t++) = *f;
         }
 
-        /* Special rule, if we are talking of the root directory, a trailing slash is good */
-        if (absolute && t == path)
-                *(t++) = '/';
+        /* Special rule, if we stripped everything, we either need a "/" (for the root directory)
+         * or "." for the current directory */
+        if (t == path) {
+                if (absolute)
+                        *(t++) = '/';
+                else
+                        *(t++) = '.';
+        }
 
         *t = 0;
         return path;
 }
 
-#if 0 /* NM_IGNORED */
 char* path_startswith(const char *path, const char *prefix) {
         assert(path);
         assert(prefix);
@@ -423,7 +430,6 @@ char* path_startswith(const char *path, const char *prefix) {
                 prefix += b;
         }
 }
-#endif /* NM_IGNORED */
 
 int path_compare(const char *a, const char *b) {
         int d;
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index d8586cc7f5..e436b36d3a 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -1845,6 +1845,25 @@ test_nm_utils_exp10 (void)
 
 /*****************************************************************************/
 
+static void
+test_utils_file_is_in_path (void)
+{
+	g_assert (!nm_utils_file_is_in_path ("/", "/"));
+	g_assert (!nm_utils_file_is_in_path ("//", "/"));
+	g_assert (!nm_utils_file_is_in_path ("/a/", "/"));
+	g_assert ( nm_utils_file_is_in_path ("/a", "/"));
+	g_assert ( nm_utils_file_is_in_path ("///a", "/"));
+	g_assert ( nm_utils_file_is_in_path ("//b/a", "/b//"));
+	g_assert ( nm_utils_file_is_in_path ("//b///a", "/b//"));
+	g_assert (!nm_utils_file_is_in_path ("//b///a/", "/b//"));
+	g_assert (!nm_utils_file_is_in_path ("//b///a/", "/b/a/"));
+	g_assert (!nm_utils_file_is_in_path ("//b///a", "/b/a/"));
+	g_assert ( nm_utils_file_is_in_path ("//b///a/.", "/b/a/"));
+	g_assert ( nm_utils_file_is_in_path ("//b///a/..", "/b/a/"));
+}
+
+/*****************************************************************************/
+
 NMTST_DEFINE ();
 
 int
@@ -1891,6 +1910,8 @@ main (int argc, char **argv)
 	g_test_add_func ("/general/stable-id/parse", test_stable_id_parse);
 	g_test_add_func ("/general/stable-id/generated-complete", test_stable_id_generated_complete);
 
+	g_test_add_func ("/general/test_utils_file_is_in_path", test_utils_file_is_in_path);
+
 	return g_test_run ();
 }
 
diff --git a/src/tests/test-systemd.c b/src/tests/test-systemd.c
index ab5fed22a9..6015e9df07 100644
--- a/src/tests/test-systemd.c
+++ b/src/tests/test-systemd.c
@@ -20,6 +20,7 @@
 #include "nm-default.h"
 
 #include "systemd/nm-sd.h"
+#include "systemd/nm-sd-utils.h"
 
 #include "nm-test-utils-core.h"
 
@@ -173,6 +174,50 @@ test_sd_event (void)
 
 /*****************************************************************************/
 
+static void
+test_path_equal (void)
+{
+#define _path_equal_check1(path, kill_dots, expected) \
+	G_STMT_START { \
+		const gboolean _kill_dots = (kill_dots); \
+		const char *_path0 = (path); \
+		const char *_expected = (expected); \
+		gs_free char *_path = g_strdup (_path0); \
+		const char *_path_result; \
+		\
+		if (   !_kill_dots \
+		    && !nm_sd_utils_path_equal (_path0, _expected)) \
+			g_error ("Paths \"%s\" and \"%s\" don't compare equal", _path0, _expected); \
+		\
+		_path_result = nm_sd_utils_path_simplify (_path, _kill_dots); \
+		g_assert (_path_result == _path); \
+		g_assert_cmpstr (_path, ==, _expected); \
+	} G_STMT_END
+
+#define _path_equal_check(path, expected_no_kill_dots, expected_kill_dots) \
+	G_STMT_START { \
+		_path_equal_check1 (path, FALSE, expected_no_kill_dots); \
+		_path_equal_check1 (path, TRUE,  expected_kill_dots ?: expected_no_kill_dots); \
+	} G_STMT_END
+
+	_path_equal_check ("",                  "",                NULL);
+	_path_equal_check (".",                 ".",               NULL);
+	_path_equal_check ("..",                "..",              NULL);
+	_path_equal_check ("/..",               "/..",             NULL);
+	_path_equal_check ("//..",              "/..",             NULL);
+	_path_equal_check ("/.",                "/.",              "/");
+	_path_equal_check ("./",                ".",               ".");
+	_path_equal_check ("./.",               "./.",             ".");
+	_path_equal_check (".///.",             "./.",             ".");
+	_path_equal_check (".///./",            "./.",             ".");
+	_path_equal_check (".////",             ".",               ".");
+	_path_equal_check ("//..//foo/",        "/../foo",         NULL);
+	_path_equal_check ("///foo//./bar/.",   "/foo/./bar/.",    "/foo/bar");
+	_path_equal_check (".//./foo//./bar/.", "././foo/./bar/.", "foo/bar");
+}
+
+/*****************************************************************************/
+
 NMTST_DEFINE ();
 
 int
@@ -183,6 +228,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/systemd/dhcp/create", test_dhcp_create);
 	g_test_add_func ("/systemd/lldp/create", test_lldp_create);
 	g_test_add_func ("/systemd/sd-event", test_sd_event);
+	g_test_add_func ("/systemd/test_path_equal", test_path_equal);
 
 	return g_test_run ();
 }