sessiond: enforce user-exclusive session access in session_access_ok

The current session_access_ok logic disallows the access to a session
when:
  uid != session->uid && gid != session->gid && uid != 0

This means that any user that is part of the same primary group as the
session's owner can access the session. The primary group is not
necessarily (and most likely) not the `tracing` group. Moreover, the
`tracing` group is not meant to provide shared access to sessions, but
to allow interactions with a root session daemon.

For instance:
  - the session has uid = 1000, gid = 100
  - the current user has uid = 1001, gid = 100

access to the session is granted.

This is way too broad and unexpected from most users as the LTTng
documentation never mentions this "primary group share tracing sessions"
behaviour. The documentation only alludes to the fact that separate
users have "their own set of sessions".

On most distributions, this change will have no impact as `useradd`
creates a new group for every user. Users will never share a primary
group and thus can't control each others' sessions.

However, it is not unusual to have users share a primary group (e.g.
`users`) and set the default umask to `0700`. In that case, there is no
expectation that every user will share files and there would be no
reasonable expectation that they should share all sessions.

For instance, it would be unexpected for one user to tear down the
sessions of other users with a single `lttng destroy -a` command.

If this type of session sharing is desirable to some users, then the
default umask of users could be checked or sessions could be created as
part of a group. However, in doubt, it is preferable to be strict.

This is not marked as a fix since this was most likely deliberate and
the change could, although unlikely, break existing deployment
scenarios.

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

diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c
index 689dc258a604cff0f3d7816f72ff8778523f8cd5..8a2ef85b7345c33d89cd2914134bd075a71b4e9e 100644 (file)
--- a/src/bin/lttng-sessiond/client.c
+++ b/src/bin/lttng-sessiond/client.c
@@ -579,15 +579,14 @@ static unsigned int lttng_sessions_count(uid_t uid, gid_t gid)
        struct ltt_session *session;
        const struct ltt_session_list *session_list = session_get_list();
 
-       DBG("Counting number of available session for UID %d GID %d",
-                       uid, gid);
+       DBG("Counting number of available session for UID %d", uid);
        cds_list_for_each_entry(session, &session_list->head, list) {
                if (!session_get(session)) {
                        continue;
                }
                session_lock(session);
                /* Only count the sessions the user can control. */
-               if (session_access_ok(session, uid, gid) &&
+               if (session_access_ok(session, uid) &&
                                !session->destroyed) {
                        i++;
                }
@@ -1106,13 +1105,12 @@ skip_domain:
        }
 
        /*
-        * Check that the UID or GID match that of the tracing session.
+        * Check that the UID matches that of the tracing session.
         * The root user can interact with all sessions.
         */
        if (need_tracing_session) {
                if (!session_access_ok(cmd_ctx->session,
-                               LTTNG_SOCK_GET_UID_CRED(&cmd_ctx->creds),
-                               LTTNG_SOCK_GET_GID_CRED(&cmd_ctx->creds)) ||
+                               LTTNG_SOCK_GET_UID_CRED(&cmd_ctx->creds)) ||
                                cmd_ctx->session->destroyed) {
                        ret = LTTNG_ERR_EPERM;
                        goto error;

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index b1b22012c2701ff8e8b3c2accf9be27674353786..dc442035a3fe21e6b4c487a1007c9f05d1b38c56 100644 (file)
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -3715,7 +3715,7 @@ void cmd_list_lttng_sessions(struct lttng_session *sessions,
                /*
                 * Only list the sessions the user can control.
                 */
-               if (!session_access_ok(session, uid, gid) ||
+               if (!session_access_ok(session, uid) ||
                                session->destroyed) {
                        session_put(session);
                        continue;

diff --git a/src/bin/lttng-sessiond/save.c b/src/bin/lttng-sessiond/save.c
index 09194008a834ae2fc0111bee26bf7b89cf3b54ee..052928c89e9298f60ddc12a03a16e8cd8584fe28 100644 (file)
--- a/src/bin/lttng-sessiond/save.c
+++ b/src/bin/lttng-sessiond/save.c
@@ -2633,8 +2633,7 @@ int save_session(struct ltt_session *session,
        memset(config_file_path, 0, sizeof(config_file_path));
 
        if (!session_access_ok(session,
-               LTTNG_SOCK_GET_UID_CRED(creds),
-               LTTNG_SOCK_GET_GID_CRED(creds)) || session->destroyed) {
+               LTTNG_SOCK_GET_UID_CRED(creds)) || session->destroyed) {
                ret = LTTNG_ERR_EPERM;
                goto end;
        }

diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c
index 95395c282a2ab79d89b23450a3cc531bea7e28ad..f8134f8ad966a950b0c97f13bbb18f2ba12d89b9 100644 (file)
--- a/src/bin/lttng-sessiond/session.c
+++ b/src/bin/lttng-sessiond/session.c
@@ -1296,18 +1296,13 @@ error:
 }
 
 /*
- * Check if the UID or GID match the session. Root user has access to all
+ * Check if the UID matches the session. Root user has access to all
  * sessions.
  */
-int session_access_ok(struct ltt_session *session, uid_t uid, gid_t gid)
+bool session_access_ok(struct ltt_session *session, uid_t uid)
 {
        assert(session);
-
-       if (uid != session->uid && gid != session->gid && uid != 0) {
-               return 0;
-       } else {
-               return 1;
-       }
+       return (uid == session->uid) || uid == 0;
 }
 
 /*

diff --git a/src/bin/lttng-sessiond/session.h b/src/bin/lttng-sessiond/session.h
index 34e51fe5a85850445e126453c3fd869c0e4d4790..e6ed40cc2927f624ce3255492a91e129edcb5a8c 100644 (file)
--- a/src/bin/lttng-sessiond/session.h
+++ b/src/bin/lttng-sessiond/session.h
@@ -240,7 +240,7 @@ struct ltt_session *session_find_by_id(uint64_t id);
 struct ltt_session_list *session_get_list(void);
 void session_list_wait_empty(void);
 
-int session_access_ok(struct ltt_session *session, uid_t uid, gid_t gid);
+bool session_access_ok(struct ltt_session *session, uid_t uid);
 
 int session_reset_rotation_state(struct ltt_session *session,
                enum lttng_rotation_state result);