Bug #1212
closedGethostname return FQDN and lttng-relayd does not accept "." in hostname
100%
Description
Since commit:
commit 590f0324d6dfd382f79229a7934fa8b5b661641f Author: Jérémie Galarneau <jeremie.galarneau@efficios.com> Date: Fri Aug 30 18:10:56 2019 -0400 Fix: validate that session, host and basepath are legal Ensure that session name, hostname and the session's base path do not contain dots ('.') to safeguard against malformed names that could be used to walk-up the relay daemon output path hierarchy. Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> ──────────────────────────────────────────────────────────────────────────────────────────────────────── modified: src/bin/lttng-relayd/session.c ──────────────────────────────────────────────────────────────────────────────────────────────────────── @ session.c:101 @ struct relay_session *session_create(const char *session_name, uint32_t minor) { int ret; struct relay_session *session; struct relay_session *session = NULL; if (session_name && strstr(session_name, ".")) { ERR("Illegal character in session name: \"%s\"", session_name); goto error; } if (base_path && strstr(base_path, "../")) { ERR("Invalid session base path walks up the path hierarchy: \"%s\"", base_path); goto error; } if (hostname && strstr(hostname, ".")) { ERR("Invalid character in hostname: \"%s\"", hostname); goto error; } session = zmalloc(sizeof(*session)); if (!session) {
lttng-relayd does not accept dots in the hostname.
We have a host for which gethostname return the FQDN (e.g ci-node-el8-amd64-03-01.internal.efficios.com). This is legal. We can either allow dot in hostname or go fetch the "localname" (hostname --short). A basic implementation of the last solution is available here as a WIP here: https://review.lttng.org/c/lttng-tools/+/2740
The "downside" with getting the shortname is that we would need to do the same for the lttng-ust metadata env generator in lttng-ust. Do we really want to restrict hostname with dots? I imagine that this is to prevent stuff ala hostname = ../../../myhostname?
Opinion?
Updated by Jonathan Rajotte Julien over 4 years ago
We might want to check for "separator" instead that are illegal anyway or have a complete hostname validator instead [1].
Each element of the hostname must be from 1 to 63 characters long and the entire hostname, including the dots, can be at most 253 characters long. Valid characters for hostnames are ASCII(7) letters from a to z, the digits from 0 to 9, and the hyphen (-). A hostname may not start with a hyphen.
Updated by Mathieu Desnoyers over 4 years ago
I think disallowing all dots is too strict.
For the purpose of this discussion, let's use regular expressions.
If we wish to disallow "../", "./", and ".*" (hidden files), then we should rather identify patterns matching the regular expressions "\/\." or "^\.".
E.g., we want to disallow:
"../abc/def"
"abc/.."
"abc/../def"
"abc/./def"
"./def"
".abc/def"
"abc/.def"
But we want to allow:
"abc.def"
"abc/def.gh"
"abc/def."
Changing the check to match the regular expressions "\/\." and "^\." rather than just "\." should do the trick.
Alternatively, if we decide that we want to disallow "../" and "./", but allow hidden directories, then we should match the following regular expressions:
"\/\.[\.$\/]" or "^\.[\.$\/]"
I'm not sure whether we want to allow hidden directories or not.
Thanks,
Mathieu
Updated by Jonathan Rajotte Julien over 4 years ago
We might also have to check how these "paths" are expressed under a lttng-relayd windows host.
Updated by Jérémie Galarneau over 4 years ago
I'm proposing this looser check, comments welcome:
https://review.lttng.org/c/lttng-tools/+/2804
Updated by Jérémie Galarneau about 4 years ago
- Status changed from Feedback to Resolved
- % Done changed from 0 to 100
Applied in changeset lttng-tools:lttng-tools|6ec9dc48cf7f3d5e1fc01f741197c0bacc94bbf0.