From dc4d1b8e3e315933c82b23b2806a9cf973e78e78 Mon Sep 17 00:00:00 2001

From: Frediano Ziglio <freddy77@gmail.com>

Date: Mon, 21 Sep 2020 07:06:09 +0100

Subject: [PATCH vd_agent_linux 14/17] Better check for sessions

Do not allow other users to hijack a session checking that

the process is launched by the owner of the session.

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>

Acked-by: Uri Lublin <uril@redhat.com>

---

 src/vdagentd/console-kit.c        | 67 +++++++++++++++++++++++++++++++

 src/vdagentd/dummy-session-info.c |  5 +++

 src/vdagentd/session-info.h       |  3 ++

 src/vdagentd/systemd-login.c      |  9 +++++

 src/vdagentd/vdagentd.c           | 10 ++++-

 5 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/src/vdagentd/console-kit.c b/src/vdagentd/console-kit.c

index fc630f1..09a8402 100644

--- a/src/vdagentd/console-kit.c

+++ b/src/vdagentd/console-kit.c

@@ -568,3 +568,70 @@ exit:

     }

     return ret;

 }

+

+uid_t session_info_uid_for_session(struct session_info *info, const char *session)

+{

+    DBusError error;

+    DBusMessage *message = NULL;

+    DBusMessage *reply = NULL;

+    uint32_t uid;

+    uid_t ret = -1;

+    const char *err_msg;

+

+    g_return_val_if_fail(info != NULL, ret);

+    g_return_val_if_fail(info->connection != NULL, ret);

+    g_return_val_if_fail(info->active_session != NULL, ret);

+

+    dbus_error_init(&error);

+

+    err_msg = "(console-kit) Unable to create dbus message for GetUnixUser";

+    message = dbus_message_new_method_call(INTERFACE_CONSOLE_KIT,

+                                           session,

+                                           INTERFACE_CONSOLE_KIT_SESSION,

+                                           "GetUnixUser");

+    if (message == NULL) {

+        goto exit;

+    }

+

+    err_msg = "(console-kit) GetUnixUser failed";

+    reply = dbus_connection_send_with_reply_and_block(info->connection,

+                                                      message,

+                                                      -1,

+                                                      &error);

+    if (reply == NULL || dbus_error_is_set(&error)) {

+        goto exit;

+    }

+

+    dbus_error_init(&error);

+    err_msg = "(console-kit) fail to get session-type from reply";

+    if (!dbus_message_get_args(reply,

+                               &error,

+                               DBUS_TYPE_UINT32, &uid,

+                               DBUS_TYPE_INVALID)) {

+        goto exit;

+    }

+

+    if (info->verbose) {

+        syslog(LOG_DEBUG, "(console-kit) unix user is '%u'", (unsigned) uid);

+    }

+

+    err_msg = NULL;

+    ret = uid;

+

+exit:

+    if (err_msg) {

+        if (dbus_error_is_set(&error)) {

+            syslog(LOG_ERR, "%s: %s", err_msg, error.message);

+            dbus_error_free(&error);

+        } else {

+            syslog(LOG_ERR, "%s", err_msg);

+        }

+    }

+    if (reply != NULL) {

+        dbus_message_unref(reply);

+    }

+    if (message != NULL) {

+        dbus_message_unref(message);

+    }

+    return ret;

+}

diff --git a/src/vdagentd/dummy-session-info.c b/src/vdagentd/dummy-session-info.c

index 7fd1eea..137c01a 100644

--- a/src/vdagentd/dummy-session-info.c

+++ b/src/vdagentd/dummy-session-info.c

@@ -55,3 +55,8 @@ gboolean session_info_session_is_locked(G_GNUC_UNUSED struct session_info *si)

 {

     return FALSE;

 }

+

+uid_t session_info_uid_for_session(struct session_info *si, const char *session)

+{

+    return -1;

+}

diff --git a/src/vdagentd/session-info.h b/src/vdagentd/session-info.h

index c8edb86..96aa8d3 100644

--- a/src/vdagentd/session-info.h

+++ b/src/vdagentd/session-info.h

@@ -40,4 +40,7 @@ char *session_info_session_for_pid(struct session_info *ck, uint32_t pid);

 gboolean session_info_session_is_locked(struct session_info *si);

 gboolean session_info_is_user(struct session_info *si);

+/* get owner of a given session */

+uid_t session_info_uid_for_session(struct session_info *si, const char *session);

+

 #endif

diff --git a/src/vdagentd/systemd-login.c b/src/vdagentd/systemd-login.c

index 0b8f3c1..0e1ff3f 100644

--- a/src/vdagentd/systemd-login.c

+++ b/src/vdagentd/systemd-login.c

@@ -392,3 +392,12 @@ gboolean session_info_is_user(struct session_info *si)

     return ret;

 }

+

+uid_t session_info_uid_for_session(struct session_info *si, const char *session)

+{

+    uid_t ret = -1;

+    if (sd_session_get_uid(session, &ret) < 0) {

+        return -1;

+    }

+    return ret;

+}

diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c

index e98fbe5..bb39340 100644

--- a/src/vdagentd/vdagentd.c

+++ b/src/vdagentd/vdagentd.c

@@ -991,12 +991,20 @@ static void agent_connect(UdscsConnection *conn)

         agent_data->session = session_info_session_for_pid(session_info, pid_uid.pid);

+        uid_t session_uid = session_info_uid_for_session(session_info, agent_data->session);

+

         /* Check that the UID of the PID did not change, this should be done after

          * computing the session to avoid race conditions.

          * This can happen as vdagent_connection_get_peer_pid_uid get information

          * from the time of creating the socket, but the process in the meantime

          * have been replaced */

-        if (!check_uid_of_pid(pid_uid.pid, pid_uid.uid)) {

+        if (!check_uid_of_pid(pid_uid.pid, pid_uid.uid) ||

+            /* Check that the user launching the Agent is the same as session one

+             * or root user.

+             * This prevents session hijacks from other users. */

+            (pid_uid.uid != 0 && pid_uid.uid != session_uid)) {

+            syslog(LOG_ERR, "UID mismatch: UID=%u PID=%u suid=%u", pid_uid.uid,

+                   pid_uid.pid, session_uid);

             agent_data_destroy(agent_data);

             udscs_server_destroy_connection(server, conn);

             return;

-- 

2.26.2