Fix: unprivilieged sessiond agent port clashes with root sessiond

This fix addresses the same problem as reported in f28f9e44.

The session daemon now tries to bind the agent TCP socket to a
port within a range (10 ports by default). The session daemon
will use the first available TCP port within that range when
binding to "localhost". It is still possible to restrict the
session daemon to the broken behaviour by specifying an agent
port using the --agent-tcp-port PORT. If that option is used,
the session daemon will attempt to bind to that part. If it
fails, agent tracing will be marked as disabled.

This fix is backported since the current logic of binding to a
set port means that the default configuration on Ubuntu, Debian,
and other distributions that launch an lttng-sessiond on boot does
not allow the tracing of agent domains (Java Util Logging, log4j,
and Python logging back-ends).

By default, users are not part of the tracing group and it is
not reasonable to expect users to be part of that group for
userspace tracing.

The behaviour of the "system" lttng-sessiond does not change
as it will bind on the first available port within the range.
The non-privilieged session daemons that will be launched after
will be able to bind on other ports available within the range.

Reported-by: Deborah Barnard <starfallprojects@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/configure.ac b/configure.ac
index 232cdf14174b6a4dc0fa43e02cb015e37d31e3c6..06cf38ec4038f5db9c3b6d86464bb36f90f32710 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -326,7 +326,8 @@ m4_define([_DEFAULT_CHANNEL_LIVE_TIMER], [0])
 m4_define([_DEFAULT_CHANNEL_READ_TIMER], [200000])
 m4_define([_DEFAULT_CHANNEL_MONITOR_TIMER], [1000000])
 m4_define([_DEFAULT_CHANNEL_BLOCKING_TIMEOUT], [0])
-_AC_DEFINE_AND_SUBST([DEFAULT_AGENT_TCP_PORT], [5345])
+_AC_DEFINE_AND_SUBST([DEFAULT_AGENT_TCP_PORT_RANGE_BEGIN], [5345])
+_AC_DEFINE_AND_SUBST([DEFAULT_AGENT_TCP_PORT_RANGE_END], [5354])
 _AC_DEFINE_AND_SUBST([DEFAULT_APP_SOCKET_RW_TIMEOUT], [5])
 _AC_DEFINE_AND_SUBST([DEFAULT_CHANNEL_SUBBUF_SIZE], [_DEFAULT_CHANNEL_SUBBUF_SIZE])
 _AC_DEFINE_AND_SUBST([DEFAULT_CHANNEL_TRACEFILE_COUNT], [0])

diff --git a/doc/man/asciidoc-attrs.conf.in b/doc/man/asciidoc-attrs.conf.in
index bf6333c9127152f7f4ffadb44c4063d729b8182e..f9d918e4063db6a268874c1aaef497505d0927b6 100644 (file)
--- a/doc/man/asciidoc-attrs.conf.in
+++ b/doc/man/asciidoc-attrs.conf.in
@@ -1,6 +1,7 @@
 [attributes]
 # default values
-default_agent_tcp_port="@DEFAULT_AGENT_TCP_PORT@"
+default_agent_tcp_port_range_begin="@DEFAULT_AGENT_TCP_PORT_RANGE_BEGIN@"
+default_agent_tcp_port_range_end="@DEFAULT_AGENT_TCP_PORT_RANGE_END@"
 default_app_socket_rw_timeout="@DEFAULT_APP_SOCKET_RW_TIMEOUT@"
 default_channel_subbuf_size="@DEFAULT_CHANNEL_SUBBUF_SIZE@"
 default_channel_tracefile_count="@DEFAULT_CHANNEL_TRACEFILE_COUNT@"

diff --git a/doc/man/lttng-sessiond.8.txt b/doc/man/lttng-sessiond.8.txt
index 2bcfe52375e2f8403495b66c30f19c36b4a23844..e84aecc4cc59e424602504fc585cdecd563b4660 100644 (file)
--- a/doc/man/lttng-sessiond.8.txt
+++ b/doc/man/lttng-sessiond.8.txt
@@ -152,7 +152,8 @@ Paths and ports
 ~~~~~~~~~~~~~~~
 option:--agent-tcp-port='PORT'::
     Listen on TCP port 'PORT' for agent application registrations
-    (default: {default_agent_tcp_port}).
+    (default: a port withinin the range
+    [{default_agent_tcp_port_range_begin},{nbsp}{default_agent_tcp_port_range_end}]).
 
 option:-a 'PATH', option:--apps-sock='PATH'::
     Set application Unix socket path to 'PATH'.

diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
index a5016ac33da707eba9b2939ce6c1c2d10d887f04..ea3e8fb00c224305289f3f7de8308820933003ed 100644 (file)
--- a/src/bin/lttng-relayd/live.c
+++ b/src/bin/lttng-relayd/live.c
@@ -452,10 +452,11 @@ struct lttcomm_sock *init_socket(struct lttng_uri *uri)
        if (ret < 0) {
                goto error;
        }
-       DBG("Listening on sock %d for live", sock->fd);
+       DBG("Listening on sock %d for lttng-live", sock->fd);
 
        ret = sock->ops->bind(sock);
        if (ret < 0) {
+               PERROR("Failed to bind lttng-live socket");
                goto error;
        }
 

diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
index cd4f058bf0243df63dd78e573e97fca202b751d3..0253997f7bb9d849d968811962601d51ad8cb43e 100644 (file)
--- a/src/bin/lttng-relayd/main.c
+++ b/src/bin/lttng-relayd/main.c
@@ -749,6 +749,7 @@ static struct lttcomm_sock *relay_socket_create(struct lttng_uri *uri)
 
        ret = sock->ops->bind(sock);
        if (ret < 0) {
+               PERROR("Failed to bind socket");
                goto error;
        }
 

diff --git a/src/bin/lttng-sessiond/agent-thread.c b/src/bin/lttng-sessiond/agent-thread.c
index d53e0b1c749e9c8291e5b6630ec4f55dde718257..f8456b428d49882945c2089bf12e3c4121c7420b 100644 (file)
--- a/src/bin/lttng-sessiond/agent-thread.c
+++ b/src/bin/lttng-sessiond/agent-thread.c
@@ -83,6 +83,8 @@ static struct lttcomm_sock *init_tcp_socket(void)
        int ret;
        struct lttng_uri *uri = NULL;
        struct lttcomm_sock *sock = NULL;
+       unsigned int port;
+       bool bind_succeeded = false;
 
        /*
         * This should never fail since the URI is hardcoded and the port is set
@@ -90,8 +92,8 @@ static struct lttcomm_sock *init_tcp_socket(void)
         */
        ret = uri_parse(default_reg_uri, &uri);
        assert(ret);
-       assert(config.agent_tcp_port);
-       uri->port = config.agent_tcp_port;
+       assert(config.agent_tcp_port.begin > 0);
+       uri->port = config.agent_tcp_port.begin;
 
        sock = lttcomm_alloc_sock_from_uri(uri);
        uri_free(uri);
@@ -105,11 +107,43 @@ static struct lttcomm_sock *init_tcp_socket(void)
                goto error;
        }
 
-       ret = sock->ops->bind(sock);
-       if (ret < 0) {
-               WARN("Another session daemon is using this agent port. Agent support "
-                               "will be deactivated to prevent interfering with the tracing.");
-               goto error;
+       for (port = config.agent_tcp_port.begin;
+                       port <= config.agent_tcp_port.end; port++) {
+               ret = lttcomm_sock_set_port(sock, (uint16_t) port);
+               if (ret) {
+                       ERR("[agent-thread] Failed to set port %u on socket",
+                                       port);
+                       goto error;
+               }
+               DBG3("[agent-thread] Trying to bind on port %u", port);
+               ret = sock->ops->bind(sock);
+               if (!ret) {
+                       bind_succeeded = true;
+                       break;
+               }
+
+               if (errno == EADDRINUSE) {
+                       DBG("Failed to bind to port %u since it is already in use",
+                                       port);
+               } else {
+                       PERROR("Failed to bind to port %u", port);
+                       goto error;
+               }
+       }
+
+       if (!bind_succeeded) {
+               if (config.agent_tcp_port.begin == config.agent_tcp_port.end) {
+                       WARN("Another process is already using the agent port %i. "
+                                       "Agent support will be deactivated.",
+                                       config.agent_tcp_port.begin);
+                       goto error;
+               } else {
+                       WARN("All ports in the range [%i, %i] are already in use. "
+                                       "Agent support will be deactivated.",
+                                       config.agent_tcp_port.begin,
+                                       config.agent_tcp_port.end);
+                       goto error;
+               }
        }
 
        ret = sock->ops->listen(sock, -1);
@@ -118,7 +152,7 @@ static struct lttcomm_sock *init_tcp_socket(void)
        }
 
        DBG("[agent-thread] Listening on TCP port %u and socket %d",
-                       config.agent_tcp_port, sock->fd);
+                       port, sock->fd);
 
        return sock;
 
@@ -134,9 +168,19 @@ error:
  */
 static void destroy_tcp_socket(struct lttcomm_sock *sock)
 {
+       int ret;
+       uint16_t port;
+
        assert(sock);
 
-       DBG3("[agent-thread] Destroy TCP socket on port %u", config.agent_tcp_port);
+       ret = lttcomm_sock_get_port(sock, &port);
+       if (ret) {
+               ERR("[agent-thread] Failed to get port of agent TCP socket");
+               port = 0;
+       }
+
+       DBG3("[agent-thread] Destroy TCP socket on port %" PRIu16,
+                       port);
 
        /* This will return gracefully if fd is invalid. */
        sock->ops->close(sock);
@@ -234,6 +278,15 @@ bool agent_tracing_is_enabled(void)
        return enabled == 1;
 }
 
+/*
+ * Write agent TCP port using the rundir.
+ */
+static int write_agent_port(uint16_t port)
+{
+       return utils_create_pid_file((pid_t) port,
+                       config.agent_port_file_path.value);
+}
+
 /*
  * This thread manage application notify communication.
  */
@@ -259,16 +312,30 @@ void *agent_thread_manage_registration(void *data)
        }
 
        reg_sock = init_tcp_socket();
-       uatomic_set(&agent_tracing_enabled, !!reg_sock);
+       if (reg_sock) {
+               uint16_t port;
+
+               assert(lttcomm_sock_get_port(reg_sock, &port) == 0);
+
+               ret = write_agent_port(port);
+               if (ret) {
+                       ERR("[agent-thread] Failed to create agent port file: agent tracing will be unavailable");
+                       /* Don't prevent the launch of the sessiond on error. */
+                       sessiond_notify_ready();
+                       goto error;
+               }
+       } else {
+               /* Don't prevent the launch of the sessiond on error. */
+               sessiond_notify_ready();
+               goto error_tcp_socket;
+       }
 
        /*
         * Signal that the agent thread is ready. The command thread
         * may start to query whether or not agent tracing is enabled.
         */
+       uatomic_set(&agent_tracing_enabled, 1);
        sessiond_notify_ready();
-       if (!reg_sock) {
-               goto error_tcp_socket;
-       }
 
        /* Add TCP socket to poll set. */
        ret = lttng_poll_add(&events, reg_sock->fd,
@@ -365,7 +432,6 @@ restart:
        }
 
 exit:
-       uatomic_set(&agent_tracing_enabled, 0);
        /* Whatever happens, try to delete it and exit. */
        (void) lttng_poll_del(&events, reg_sock->fd);
 error:
@@ -373,6 +439,7 @@ error:
 error_tcp_socket:
        lttng_poll_clean(&events);
 error_poll_create:
+       uatomic_set(&agent_tracing_enabled, 0);
        DBG("[agent-thread] is cleaning up and stopping.");
 
        rcu_thread_offline();

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 3feb3298927218354ea6e8d0974d46499f325ee6..e727888181d8ae1089e967297adec076a6a8585e 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -4853,8 +4853,8 @@ static int set_option(int opt, const char *arg, const char *optname)
                                ERR("Port overflow in --agent-tcp-port parameter: %s", arg);
                                return -1;
                        }
-                       config.agent_tcp_port = (uint32_t) v;
-                       DBG3("Agent TCP port set to non default: %u", config.agent_tcp_port);
+                       config.agent_tcp_port.begin = config.agent_tcp_port.end = (int) v;
+                       DBG3("Agent TCP port set to non default: %i", (int) v);
                }
        } else if (string_match(optname, "load") || opt == 'l') {
                if (!arg || *arg == '\0') {
@@ -5446,15 +5446,6 @@ static int write_pidfile(void)
         return utils_create_pid_file(getpid(), config.pid_file_path.value);
 }
 
-/*
- * Write agent TCP port using the rundir.
- */
-static int write_agent_port(void)
-{
-       return utils_create_pid_file(config.agent_tcp_port,
-                       config.agent_port_file_path.value);
-}
-
 static int set_clock_plugin_env(void)
 {
        int ret = 0;
@@ -5811,12 +5802,6 @@ int main(int argc, char **argv)
                retval = -1;
                goto exit_init_data;
        }
-       ret = write_agent_port();
-       if (ret) {
-               ERR("Error in write_agent_port");
-               retval = -1;
-               goto exit_init_data;
-       }
 
        /* Initialize communication library */
        lttcomm_init();

diff --git a/src/bin/lttng-sessiond/sessiond-config.c b/src/bin/lttng-sessiond/sessiond-config.c
index d710eb4e5689ae8ee60a66db81a1c17d04ec83c3..587f2f82dfd1e676bd63ff40494ae98bd80962ec 100644 (file)
--- a/src/bin/lttng-sessiond/sessiond-config.c
+++ b/src/bin/lttng-sessiond/sessiond-config.c
@@ -32,7 +32,7 @@ struct sessiond_config sessiond_config_build_defaults = {
        .verbose =                              0,
        .verbose_consumer =                     0,
 
-       .agent_tcp_port =                       DEFAULT_AGENT_TCP_PORT,
+       .agent_tcp_port =                       { .begin = DEFAULT_AGENT_TCP_PORT_RANGE_BEGIN, .end = DEFAULT_AGENT_TCP_PORT_RANGE_END },
        .app_socket_timeout =                   DEFAULT_APP_SOCKET_RW_TIMEOUT,
 
        .no_kernel =                            false,
@@ -494,7 +494,13 @@ void sessiond_config_log(struct sessiond_config *config)
        DBG_NO_LOC("\tverbose:                      %i", config->verbose);
        DBG_NO_LOC("\tverbose consumer:             %i", config->verbose_consumer);
        DBG_NO_LOC("\tquiet mode:                   %s", config->quiet ? "True" : "False");
-       DBG_NO_LOC("\tagent_tcp_port:               %i", config->agent_tcp_port);
+       if (config->agent_tcp_port.begin == config->agent_tcp_port.end) {
+               DBG_NO_LOC("\tagent_tcp_port:               %i", config->agent_tcp_port.begin);
+       } else {
+               DBG_NO_LOC("\tagent_tcp_port:               [%i, %i]",
+                               config->agent_tcp_port.begin,
+                               config->agent_tcp_port.end);
+       }
        DBG_NO_LOC("\tapplication socket timeout:   %i", config->app_socket_timeout);
        DBG_NO_LOC("\tno-kernel:                    %s", config->no_kernel ? "True" : "False");
        DBG_NO_LOC("\tbackground:                   %s", config->background ? "True" : "False");

diff --git a/src/bin/lttng-sessiond/sessiond-config.h b/src/bin/lttng-sessiond/sessiond-config.h
index 86b360f55a9a1627c5565221370951982687f93f..af129eb23b57848bb7dc120f7874e09b7ea923c9 100644 (file)
--- a/src/bin/lttng-sessiond/sessiond-config.h
+++ b/src/bin/lttng-sessiond/sessiond-config.h
@@ -26,6 +26,10 @@ struct config_string {
        bool should_free;
 };
 
+struct config_int_range {
+       int begin, end;
+};
+
 /* Config string takes ownership of value. */
 LTTNG_HIDDEN
 void config_string_set(struct config_string *string, char *value);
@@ -33,8 +37,8 @@ void config_string_set(struct config_string *string, char *value);
 struct sessiond_config {
        int verbose;
        int verbose_consumer;
-       /* Agent TCP port for registration. Used by the agent thread. */
-       int agent_tcp_port;
+       /* Agent TCP port range for registration. Used by the agent thread. */
+       struct config_int_range agent_tcp_port;
        /* Socket timeout for receiving and sending (in seconds). */
        int app_socket_timeout;
 

diff --git a/src/common/defaults.h b/src/common/defaults.h
index 083e1abc9f715d86b02101ec3259b47c9b4a9069..e8ce419e63f6f2617d91401e0726a53ce17dcd84 100644 (file)
--- a/src/common/defaults.h
+++ b/src/common/defaults.h
@@ -269,8 +269,9 @@
 #define DEFAULT_NETWORK_DATA_PORT           CONFIG_DEFAULT_NETWORK_DATA_PORT
 #define DEFAULT_NETWORK_VIEWER_PORT         CONFIG_DEFAULT_NETWORK_VIEWER_PORT
 
-/* Agent registration TCP port. */
-#define DEFAULT_AGENT_TCP_PORT              CONFIG_DEFAULT_AGENT_TCP_PORT
+/* Agent registration TCP port range. */
+#define DEFAULT_AGENT_TCP_PORT_RANGE_BEGIN  CONFIG_DEFAULT_AGENT_TCP_PORT_RANGE_BEGIN
+#define DEFAULT_AGENT_TCP_PORT_RANGE_END    CONFIG_DEFAULT_AGENT_TCP_PORT_RANGE_END
 
 /*
  * If a thread stalls for this amount of time, it will be considered bogus (bad

diff --git a/src/common/sessiond-comm/inet.c b/src/common/sessiond-comm/inet.c
index ed7f5dc165e1b3961a2f53ef3107a0f54db85472..e48f2c9ad8195d7cf90bc76d48715e414a6d6d26 100644 (file)
--- a/src/common/sessiond-comm/inet.c
+++ b/src/common/sessiond-comm/inet.c
@@ -100,15 +100,9 @@ error:
 LTTNG_HIDDEN
 int lttcomm_bind_inet_sock(struct lttcomm_sock *sock)
 {
-       int ret;
-
-       ret = bind(sock->fd, (const struct sockaddr *) &sock->sockaddr.addr.sin,
+       return bind(sock->fd,
+                       (const struct sockaddr *) &sock->sockaddr.addr.sin,
                        sizeof(sock->sockaddr.addr.sin));
-       if (ret < 0) {
-               PERROR("bind inet");
-       }
-
-       return ret;
 }
 
 static

diff --git a/src/common/sessiond-comm/inet6.c b/src/common/sessiond-comm/inet6.c
index 1fd18a96258382e408df8c30fe8e250b456cd11d..e71b55ad1c3651defdbd6ef1df187309ce8825ff 100644 (file)
--- a/src/common/sessiond-comm/inet6.c
+++ b/src/common/sessiond-comm/inet6.c
@@ -98,15 +98,9 @@ error:
 LTTNG_HIDDEN
 int lttcomm_bind_inet6_sock(struct lttcomm_sock *sock)
 {
-       int ret;
-
-       ret = bind(sock->fd, (const struct sockaddr *) &sock->sockaddr.addr.sin6,
+       return bind(sock->fd,
+                       (const struct sockaddr *) &sock->sockaddr.addr.sin6,
                        sizeof(sock->sockaddr.addr.sin6));
-       if (ret < 0) {
-               PERROR("bind inet6");
-       }
-
-       return ret;
 }
 
 static

diff --git a/src/common/sessiond-comm/sessiond-comm.c b/src/common/sessiond-comm/sessiond-comm.c
index 336902a60f597d6c7820f34032f29d6a77b8396f..0067be1636ecca662e52e5eb569adfbd0cd5fa98 100644 (file)
--- a/src/common/sessiond-comm/sessiond-comm.c
+++ b/src/common/sessiond-comm/sessiond-comm.c
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <errno.h>
+#include <inttypes.h>
 
 #include <common/common.h>
 
@@ -413,6 +414,53 @@ int lttcomm_setsockopt_snd_timeout(int sock, unsigned int msec)
        return ret;
 }
 
+LTTNG_HIDDEN
+int lttcomm_sock_get_port(const struct lttcomm_sock *sock, uint16_t *port)
+{
+       assert(sock);
+       assert(port);
+       assert(sock->sockaddr.type == LTTCOMM_INET ||
+                       sock->sockaddr.type == LTTCOMM_INET6);
+       assert(sock->proto == LTTCOMM_SOCK_TCP ||
+                       sock->proto == LTTCOMM_SOCK_UDP);
+
+       switch (sock->sockaddr.type) {
+       case LTTCOMM_INET:
+               *port = ntohs(sock->sockaddr.addr.sin.sin_port);
+               break;
+       case LTTCOMM_INET6:
+               *port = ntohs(sock->sockaddr.addr.sin6.sin6_port);
+               break;
+       default:
+               abort();
+       }
+
+       return 0;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sock_set_port(struct lttcomm_sock *sock, uint16_t port)
+{
+       assert(sock);
+       assert(sock->sockaddr.type == LTTCOMM_INET ||
+                       sock->sockaddr.type == LTTCOMM_INET6);
+       assert(sock->proto == LTTCOMM_SOCK_TCP ||
+                       sock->proto == LTTCOMM_SOCK_UDP);
+
+       switch (sock->sockaddr.type) {
+       case LTTCOMM_INET:
+               sock->sockaddr.addr.sin.sin_port = htons(port);
+               break;
+       case LTTCOMM_INET6:
+               sock->sockaddr.addr.sin6.sin6_port = htons(port);
+               break;
+       default:
+               abort();
+       }
+
+       return 0;
+}
+
 LTTNG_HIDDEN
 void lttcomm_init(void)
 {

diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index ab18540e0e93048a91ecc3dc8ec4ce970aa7768a..fb959750d1a2290db4cd6758ac99e06c8af47296 100644 (file)
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -626,6 +626,14 @@ LTTNG_HIDDEN struct lttcomm_relayd_sock *lttcomm_alloc_relayd_sock(
 LTTNG_HIDDEN int lttcomm_setsockopt_rcv_timeout(int sock, unsigned int msec);
 LTTNG_HIDDEN int lttcomm_setsockopt_snd_timeout(int sock, unsigned int msec);
 
+LTTNG_HIDDEN int lttcomm_sock_get_port(const struct lttcomm_sock *sock,
+               uint16_t *port);
+/*
+ * Set a port to an lttcomm_sock. This will have no effect is the socket is
+ * already bound.
+ */
+LTTNG_HIDDEN int lttcomm_sock_set_port(struct lttcomm_sock *sock, uint16_t port);
+
 LTTNG_HIDDEN void lttcomm_init(void);
 /* Get network timeout, in milliseconds */
 LTTNG_HIDDEN unsigned long lttcomm_get_network_timeout(void);