|
|
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 |
|