From dc4d1b8e3e315933c82b23b2806a9cf973e78e78 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio 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 Acked-by: Uri Lublin --- 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