Bug #1212: Gethostname return FQDN and lttng-relayd does not accept "." in hostname - LTTng - LTTng bugs repository

Actions

Copy link

Bug #1212

closed

Gethostname return FQDN and lttng-relayd does not accept "." in hostname

Added by Jonathan Rajotte Julien over 4 years ago. Updated about 4 years ago.

Status:

Resolved

Priority:

Normal

Assignee:

Jérémie Galarneau

Target version:

Start date:

01/07/2020

Due date:

% Done:

100%

Estimated time:

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?

Actions

Copy link

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.

[1] http://man7.org/linux/man-pages/man7/hostname.7.html

Actions

Copy link

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

Actions

Copy link