From: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Tue, 14 Jan 2020 22:08:13 +0000 (-0500)
Subject: Fix: relayd: hostname check is too restrictive
X-Git-Tag: v2.12.0-rc1~81
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=6ec9dc48cf7f3d5e1fc01f741197c0bacc94bbf0;p=lttng-tools.git

Fix: relayd: hostname check is too restrictive

The check performed by the relay daemon on hostnames is too
restrictive. Since existing session daemons directly use the hostname
returned by gethostname(), the relay daemon must correctly handle FQDN
hostnames, which may contain dots.

This has been observed on the LTTng CI's RHEL8 nodes which report an
FQDN hostname.

The new function 'is_name_path_safe' is used for both session and host
names. It does not check for every problematic path names (reserved
names on Windows, per-platform illegal characters, etc.) Those
restrictions are assumed to be handled when open() and similar
syscalls fail.

However, the objective of this check is to prevent malicious (or at
least unexpected), but legal, names from being used, namely:
  - names that contain a path separator,
  - empty names,
  - hidden names (starting with a dot).

Ideally, illegal names would be automatically escaped in the future.
This is, however, beyond the scope of this fix.

Fixes #1212

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ieeb8b60d22c27b390b51ef7fb52cea0d0ac0f188
---

diff --git a/src/bin/lttng-relayd/session.c b/src/bin/lttng-relayd/session.c
index df6bc102a..00dfbd7d2 100644
--- a/src/bin/lttng-relayd/session.c
+++ b/src/bin/lttng-relayd/session.c
@@ -23,6 +23,7 @@
 #include <common/time.h>
 #include <common/utils.h>
 #include <common/uuid.h>
+#include <common/compat/path.h>
 #include <urcu/rculist.h>
 
 #include <sys/stat.h>
@@ -217,6 +218,37 @@ end:
 	return ret;
 }
 
+/*
+ * Check if a name is safe to use in a path.
+ *
+ * A name that is deemed "path-safe":
+ *   - Does not contains a path separator (/ or \, platform dependant),
+ *   - Does not start with a '.' (hidden file/folder),
+ *   - Is not empty.
+ */
+static bool is_name_path_safe(const char *name)
+{
+	const size_t name_len = strlen(name);
+
+	/* Not empty. */
+	if (name_len == 0) {
+		WARN("An empty name is not allowed to be used in a path");
+		return false;
+	}
+	/* Does not start with '.'. */
+	if (name[0] == '.') {
+		WARN("Name \"%s\" is not allowed to be used in a path since it starts with '.'", name);
+		return false;
+	}
+	/* Does not contain a path-separator. */
+	if (strchr(name, LTTNG_PATH_SEPARATOR)) {
+		WARN("Name \"%s\" is not allowed to be used in a path since it contains a path separator", name);
+		return false;
+	}
+
+	return true;
+}
+
 /*
  * Create a new session by assigning a new session ID.
  *
@@ -241,9 +273,12 @@ struct relay_session *session_create(const char *session_name,
 	assert(hostname);
 	assert(base_path);
 
-	if (strstr(session_name, ".")) {
-		ERR("Illegal character in session name: \"%s\"",
-				session_name);
+	if (!is_name_path_safe(session_name)) {
+		ERR("Refusing to create session as the provided session name is not path-safe");
+		goto error;
+	}
+	if (!is_name_path_safe(hostname)) {
+		ERR("Refusing to create session as the provided hostname is not path-safe");
 		goto error;
 	}
 	if (strstr(base_path, "../")) {
@@ -251,11 +286,6 @@ struct relay_session *session_create(const char *session_name,
 				base_path);
 		goto error;
 	}
-	if (strstr(hostname, ".")) {
-		ERR("Invalid character in hostname: \"%s\"",
-				hostname);
-		goto error;
-	}
 
 	session = zmalloc(sizeof(*session));
 	if (!session) {
diff --git a/src/common/compat/path.h b/src/common/compat/path.h
new file mode 100644
index 000000000..5c367d5a0
--- /dev/null
+++ b/src/common/compat/path.h
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2020 - Jérémie Galarneau <jeremie.galarneau@efficios.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2 only,
+ * as published by the Free Software Foundation.
+ *
+ * This program 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 General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/* Build platform's preferred path separator. */
+#if defined(_WIN32) || defined(__CYGWIN__)
+#define LTTNG_PATH_SEPARATOR '\\'
+#else
+#define LTTNG_PATH_SEPARATOR '/'
+#endif