File Avoids-user-session-hijacking.patch of Package spice-vdagent.17026

Overview Repositories Revisions Requests Users Attributes Meta

File Avoids-user-session-hijacking.patch of Package spice-vdagent.17026

From 40af9a185d3ba7b32baea69393a6e353f66b2bfd Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Sun, 20 Sep 2020 08:06:16 +0100
Subject: [PATCH 16/20] Avoids user session hijacking

Git-commit: d1499d7da7bc33f885acf0e6d4bd95d95033d86e
References: bsc#1173749

Avoids user hijacking sessions by reusing PID.
In theory an attacker could:
- open a connection to the daemon;
- fork and exit the process but keep the file descriptor open
  (inheriting or duplicating it in forked process);
- force OS to recycle the initial PID, by creating many short lived
  processes.
Daemon would detect the old PID as having the new session.
Check the user to avoid such replacements.

This issue was reported by SUSE security team.

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <uril@redhat.com>
Signed-off-by: Bruce Rogers <brogers@suse.com>
---
 src/vdagent-connection.c | 13 +++++++------
 src/vdagent-connection.h | 13 +++++++++----
 src/vdagentd/vdagentd.c  | 31 +++++++++++++++++++++++++++----
 3 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/src/vdagent-connection.c b/src/vdagent-connection.c
index b1d4db6..fb331be 100644
--- a/src/vdagent-connection.c
+++ b/src/vdagent-connection.c
@@ -142,24 +142,25 @@ void vdagent_connection_destroy(gpointer p)
     g_object_unref(self);
 }
 
-gint vdagent_connection_get_peer_pid(VDAgentConnection *self,
-                                     GError           **err)
+PidUid vdagent_connection_get_peer_pid_uid(VDAgentConnection *self,
+                                           GError           **err)
 {
     VDAgentConnectionPrivate *priv = vdagent_connection_get_instance_private(self);
     GSocket *sock;
     GCredentials *cred;
-    gint pid = -1;
+    PidUid pid_uid = { -1, -1 };
 
-    g_return_val_if_fail(G_IS_SOCKET_CONNECTION(priv->io_stream), pid);
+    g_return_val_if_fail(G_IS_SOCKET_CONNECTION(priv->io_stream), pid_uid);
 
     sock = g_socket_connection_get_socket(G_SOCKET_CONNECTION(priv->io_stream));
     cred = g_socket_get_credentials(sock, err);
     if (cred) {
-        pid = g_credentials_get_unix_pid(cred, err);
+        pid_uid.pid = g_credentials_get_unix_pid(cred, err);
+        pid_uid.uid = g_credentials_get_unix_user(cred, err);
         g_object_unref(cred);
     }
 
-    return pid;
+    return pid_uid;
 }
 
 /* Performs single write operation,
diff --git a/src/vdagent-connection.h b/src/vdagent-connection.h
index 9d5a212..c515a79 100644
--- a/src/vdagent-connection.h
+++ b/src/vdagent-connection.h
@@ -92,10 +92,15 @@ void vdagent_connection_write(VDAgentConnection *self,
 /* Synchronously write all queued messages to the output stream. */
 void vdagent_connection_flush(VDAgentConnection *self);
 
-/* Returns the PID of the foreign process connected to the socket
- * or -1 with @err set. */
-gint vdagent_connection_get_peer_pid(VDAgentConnection *self,
-                                     GError           **err);
+typedef struct PidUid {
+    pid_t pid;
+    uid_t uid;
+} PidUid;
+
+/* Returns the PID and UID of the foreign process connected to the socket
+ * or fill @err set. */
+PidUid vdagent_connection_get_peer_pid_uid(VDAgentConnection *self,
+                                           GError           **err);
 
 G_END_DECLS
 
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index add3fab..06931ce 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -916,16 +916,28 @@ static gboolean remove_active_xfers(gpointer key, gpointer value, gpointer conn)
         return 0;
 }
 
+/* Check a given process has a given UID */
+static bool check_uid_of_pid(pid_t pid, uid_t uid)
+{
+    char fn[128];
+    struct stat st;
+
+    snprintf(fn, sizeof(fn), "/proc/%u/status", (unsigned) pid);
+    if (stat(fn, &st) != 0 || st.st_uid != uid) {
+        return false;
+    }
+    return true;
+}
+
 static void agent_connect(struct udscs_connection *conn)
 {
     struct agent_data *agent_data;
     agent_data = g_new0(struct agent_data, 1);
     GError *err = NULL;
-    gint pid;
 
     if (session_info) {
-        pid = vdagent_connection_get_peer_pid(VDAGENT_CONNECTION(conn), &err);
-        if (err || pid <= 0) {
+        PidUid pid_uid = vdagent_connection_get_peer_pid_uid(VDAGENT_CONNECTION(conn), &err);
+        if (err || pid_uid.pid <= 0) {
             static const char msg[] = "Could not get peer PID, disconnecting new client";
             if (err) {
                 syslog(LOG_ERR, "%s: %s", msg, err->message);
@@ -938,7 +950,18 @@ static void agent_connect(struct udscs_connection *conn)
             return;
         }
 
-        agent_data->session = session_info_session_for_pid(session_info, pid);
+        agent_data->session = session_info_session_for_pid(session_info, pid_uid.pid);
+
+        /* 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)) {
+            agent_data_destroy(agent_data);
+            udscs_server_destroy_connection(server, conn);
+            return;
+        }
     }
 
     g_object_set_data_full(G_OBJECT(conn), "agent_data", agent_data,
-- 
2.29.0

Places

File Avoids-user-session-hijacking.patch of Package spice-vdagent.17026

Places