Blame SOURCES/0013-Avoids-user-session-hijacking.patch

ad412c
From 5d9881309d0aeeebbc177d8af6dc26aa2ba56cfc Mon Sep 17 00:00:00 2001
ad412c
From: Frediano Ziglio <freddy77@gmail.com>
ad412c
Date: Sun, 20 Sep 2020 08:06:16 +0100
ad412c
Subject: [PATCH vd_agent_linux 13/17] Avoids user session hijacking
ad412c
ad412c
Avoids user hijacking sessions by reusing PID.
ad412c
In theory an attacker could:
ad412c
- open a connection to the daemon;
ad412c
- fork and exit the process but keep the file descriptor open
ad412c
  (inheriting or duplicating it in forked process);
ad412c
- force OS to recycle the initial PID, by creating many short lived
ad412c
  processes.
ad412c
Daemon would detect the old PID as having the new session.
ad412c
Check the user to avoid such replacements.
ad412c
ad412c
This issue was reported by SUSE security team.
ad412c
ad412c
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
ad412c
Acked-by: Uri Lublin <uril@redhat.com>
ad412c
---
ad412c
 src/vdagent-connection.c | 13 +++++++------
ad412c
 src/vdagent-connection.h | 13 +++++++++----
ad412c
 src/vdagentd/vdagentd.c  | 31 +++++++++++++++++++++++++++----
ad412c
 3 files changed, 43 insertions(+), 14 deletions(-)
ad412c
ad412c
diff --git a/src/vdagent-connection.c b/src/vdagent-connection.c
ad412c
index b1d4db6..fb331be 100644
ad412c
--- a/src/vdagent-connection.c
ad412c
+++ b/src/vdagent-connection.c
ad412c
@@ -142,24 +142,25 @@ void vdagent_connection_destroy(gpointer p)
ad412c
     g_object_unref(self);
ad412c
 }
ad412c
 
ad412c
-gint vdagent_connection_get_peer_pid(VDAgentConnection *self,
ad412c
-                                     GError           **err)
ad412c
+PidUid vdagent_connection_get_peer_pid_uid(VDAgentConnection *self,
ad412c
+                                           GError           **err)
ad412c
 {
ad412c
     VDAgentConnectionPrivate *priv = vdagent_connection_get_instance_private(self);
ad412c
     GSocket *sock;
ad412c
     GCredentials *cred;
ad412c
-    gint pid = -1;
ad412c
+    PidUid pid_uid = { -1, -1 };
ad412c
 
ad412c
-    g_return_val_if_fail(G_IS_SOCKET_CONNECTION(priv->io_stream), pid);
ad412c
+    g_return_val_if_fail(G_IS_SOCKET_CONNECTION(priv->io_stream), pid_uid);
ad412c
 
ad412c
     sock = g_socket_connection_get_socket(G_SOCKET_CONNECTION(priv->io_stream));
ad412c
     cred = g_socket_get_credentials(sock, err);
ad412c
     if (cred) {
ad412c
-        pid = g_credentials_get_unix_pid(cred, err);
ad412c
+        pid_uid.pid = g_credentials_get_unix_pid(cred, err);
ad412c
+        pid_uid.uid = g_credentials_get_unix_user(cred, err);
ad412c
         g_object_unref(cred);
ad412c
     }
ad412c
 
ad412c
-    return pid;
ad412c
+    return pid_uid;
ad412c
 }
ad412c
 
ad412c
 /* Performs single write operation,
ad412c
diff --git a/src/vdagent-connection.h b/src/vdagent-connection.h
ad412c
index 9d5a212..c515a79 100644
ad412c
--- a/src/vdagent-connection.h
ad412c
+++ b/src/vdagent-connection.h
ad412c
@@ -92,10 +92,15 @@ void vdagent_connection_write(VDAgentConnection *self,
ad412c
 /* Synchronously write all queued messages to the output stream. */
ad412c
 void vdagent_connection_flush(VDAgentConnection *self);
ad412c
 
ad412c
-/* Returns the PID of the foreign process connected to the socket
ad412c
- * or -1 with @err set. */
ad412c
-gint vdagent_connection_get_peer_pid(VDAgentConnection *self,
ad412c
-                                     GError           **err);
ad412c
+typedef struct PidUid {
ad412c
+    pid_t pid;
ad412c
+    uid_t uid;
ad412c
+} PidUid;
ad412c
+
ad412c
+/* Returns the PID and UID of the foreign process connected to the socket
ad412c
+ * or fill @err set. */
ad412c
+PidUid vdagent_connection_get_peer_pid_uid(VDAgentConnection *self,
ad412c
+                                           GError           **err);
ad412c
 
ad412c
 G_END_DECLS
ad412c
 
ad412c
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
ad412c
index b31941d..e98fbe5 100644
ad412c
--- a/src/vdagentd/vdagentd.c
ad412c
+++ b/src/vdagentd/vdagentd.c
ad412c
@@ -955,16 +955,28 @@ static gboolean remove_active_xfers(gpointer key, gpointer value, gpointer conn)
ad412c
         return 0;
ad412c
 }
ad412c
 
ad412c
+/* Check a given process has a given UID */
ad412c
+static bool check_uid_of_pid(pid_t pid, uid_t uid)
ad412c
+{
ad412c
+    char fn[128];
ad412c
+    struct stat st;
ad412c
+
ad412c
+    snprintf(fn, sizeof(fn), "/proc/%u/status", (unsigned) pid);
ad412c
+    if (stat(fn, &st) != 0 || st.st_uid != uid) {
ad412c
+        return false;
ad412c
+    }
ad412c
+    return true;
ad412c
+}
ad412c
+
ad412c
 static void agent_connect(UdscsConnection *conn)
ad412c
 {
ad412c
     struct agent_data *agent_data;
ad412c
     agent_data = g_new0(struct agent_data, 1);
ad412c
     GError *err = NULL;
ad412c
-    gint pid;
ad412c
 
ad412c
     if (session_info) {
ad412c
-        pid = vdagent_connection_get_peer_pid(VDAGENT_CONNECTION(conn), &err;;
ad412c
-        if (err || pid <= 0) {
ad412c
+        PidUid pid_uid = vdagent_connection_get_peer_pid_uid(VDAGENT_CONNECTION(conn), &err;;
ad412c
+        if (err || pid_uid.pid <= 0) {
ad412c
             static const char msg[] = "Could not get peer PID, disconnecting new client";
ad412c
             if (err) {
ad412c
                 syslog(LOG_ERR, "%s: %s", msg, err->message);
ad412c
@@ -977,7 +989,18 @@ static void agent_connect(UdscsConnection *conn)
ad412c
             return;
ad412c
         }
ad412c
 
ad412c
-        agent_data->session = session_info_session_for_pid(session_info, pid);
ad412c
+        agent_data->session = session_info_session_for_pid(session_info, pid_uid.pid);
ad412c
+
ad412c
+        /* Check that the UID of the PID did not change, this should be done after
ad412c
+         * computing the session to avoid race conditions.
ad412c
+         * This can happen as vdagent_connection_get_peer_pid_uid get information
ad412c
+         * from the time of creating the socket, but the process in the meantime
ad412c
+         * have been replaced */
ad412c
+        if (!check_uid_of_pid(pid_uid.pid, pid_uid.uid)) {
ad412c
+            agent_data_destroy(agent_data);
ad412c
+            udscs_server_destroy_connection(server, conn);
ad412c
+            return;
ad412c
+        }
ad412c
     }
ad412c
 
ad412c
     g_object_set_data_full(G_OBJECT(conn), "agent_data", agent_data,
ad412c
-- 
ad412c
2.26.2
ad412c