From 2288467f63826a06d25ac361fa04ea92ec7ddfa3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Tue, 24 Apr 2018 15:58:41 -0400 Subject: [PATCH] Fix: unprivilieged sessiond agent port clashes with root sessiond MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jérémie Galarneau --- configure.ac | 3 +- doc/man/asciidoc-attrs.conf.in | 3 +- doc/man/lttng-sessiond.8.txt | 3 +- src/bin/lttng-relayd/live.c | 3 +- src/bin/lttng-relayd/main.c | 1 + src/bin/lttng-sessiond/agent-thread.c | 95 ++++++++++++++++++++---- src/bin/lttng-sessiond/main.c | 19 +---- src/bin/lttng-sessiond/sessiond-config.c | 10 ++- src/bin/lttng-sessiond/sessiond-config.h | 8 +- src/common/defaults.h | 5 +- src/common/sessiond-comm/inet.c | 10 +-- src/common/sessiond-comm/inet6.c | 10 +-- src/common/sessiond-comm/sessiond-comm.c | 48 ++++++++++++ src/common/sessiond-comm/sessiond-comm.h | 8 ++ 14 files changed, 169 insertions(+), 57 deletions(-) diff --git a/configure.ac b/configure.ac index 6112fbba9..5173b1ea9 100644 --- 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 354539001..e541c0c1c 100644 --- 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 2bcfe5237..e84aecc4c 100644 --- 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 a5016ac33..ea3e8fb00 100644 --- 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 a8164ff22..f5f1ad3b4 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -751,6 +751,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 d53e0b1c7..f8456b428 100644 --- 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 cec8e835e..fdea20ca4 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -5062,8 +5062,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') { @@ -5655,15 +5655,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; @@ -6125,12 +6116,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 d710eb4e5..587f2f82d 100644 --- 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 86b360f55..af129eb23 100644 --- 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 38ab1e146..e0f4438c8 100644 --- 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 ed7f5dc16..e48f2c9ad 100644 --- 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 1fd18a962..e71b55ad1 100644 --- 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 336902a60..0067be163 100644 --- a/src/common/sessiond-comm/sessiond-comm.c +++ b/src/common/sessiond-comm/sessiond-comm.c @@ -26,6 +26,7 @@ #include #include #include +#include #include @@ -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 8aca4d481..8c4fe0ee7 100644 --- a/src/common/sessiond-comm/sessiond-comm.h +++ b/src/common/sessiond-comm/sessiond-comm.h @@ -679,6 +679,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); -- 2.34.1