145 lines
5.2 KiB
Diff
145 lines
5.2 KiB
Diff
From 5d9881309d0aeeebbc177d8af6dc26aa2ba56cfc Mon Sep 17 00:00:00 2001
|
|
From: Frediano Ziglio <freddy77@gmail.com>
|
|
Date: Sun, 20 Sep 2020 08:06:16 +0100
|
|
Subject: [PATCH vd_agent_linux 13/17] Avoids user session hijacking
|
|
|
|
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>
|
|
---
|
|
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 b31941d..e98fbe5 100644
|
|
--- a/src/vdagentd/vdagentd.c
|
|
+++ b/src/vdagentd/vdagentd.c
|
|
@@ -955,16 +955,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(UdscsConnection *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);
|
|
@@ -977,7 +989,18 @@ static void agent_connect(UdscsConnection *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.26.2
|
|
|