packages/spice-vdagent
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Upgrade to 0.21.0

Browse Source
This commit is contained in:
yangcheng1203 2021-12-03 11:16:13 +08:00
parent e284dedc36
commit 08fc7dab42
14 changed files with 6 additions and 803 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
0001-vdagentd-work-around-GLib-s-fork-issues.patch
View File

@ -1,103 +0,0 @@
From 9b8c0ebb9fb573e6ce3c5416371509f416503d0c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Jank=C5=AF?= <jjanku@redhat.com>
Date: Fri, 20 Mar 2020 10:36:03 +0100
Subject: [PATCH 1/2] vdagentd: work around GLib's fork issues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Creating threads is not compatible with forking as only the thread
that calls fork() is inherited.
Handlers registered with g_unix_signal_add() create a thread so
move these calls after fork.
Also call g_socket_service_start() after fork to avoid creation of
new threads before it is necessary.
Fixes: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/18
Also see: https://gitlab.gnome.org/GNOME/glib/issues/2073
Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
src/udscs.c | 6 ++++++
src/udscs.h | 2 ++
src/vdagentd/vdagentd.c | 9 +++++----
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/udscs.c b/src/udscs.c
index 4de75f8..7c99eed 100644
--- a/src/udscs.c
+++ b/src/udscs.c
@@ -186,6 +186,7 @@ struct udscs_server *udscs_server_new(
server->read_callback = read_callback;
server->error_cb = error_cb;
server->service = g_socket_service_new();
+ g_socket_service_stop(server->service);
g_signal_connect(server->service, "incoming",
G_CALLBACK(udscs_server_accept_cb), server);
@@ -223,6 +224,11 @@ void udscs_server_listen_to_address(struct udscs_server *server,
g_object_unref(sock_addr);
}
+void udscs_server_start(struct udscs_server *server)
+{
+ g_socket_service_start(server->service);
+}
+
void udscs_server_destroy_connection(struct udscs_server *server,
UdscsConnection *conn)
{
diff --git a/src/udscs.h b/src/udscs.h
index 45ebd3f..4f7ea36 100644
--- a/src/udscs.h
+++ b/src/udscs.h
@@ -98,6 +98,8 @@ void udscs_server_listen_to_address(struct udscs_server *server,
const gchar *addr,
GError **err);
+void udscs_server_start(struct udscs_server *server);
+
void udscs_server_destroy_connection(struct udscs_server *server,
UdscsConnection *conn);
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index cfd0a51..1b63ec8 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -1184,10 +1184,6 @@ int main(int argc, char *argv[])
uinput_device = g_strdup(DEFAULT_UINPUT_DEVICE);
}
- g_unix_signal_add(SIGINT, signal_handler, NULL);
- g_unix_signal_add(SIGHUP, signal_handler, NULL);
- g_unix_signal_add(SIGTERM, signal_handler, NULL);
-
openlog("spice-vdagentd", do_daemonize ? 0 : LOG_PERROR, LOG_USER);
/* Setup communication with vdagent process(es) */
@@ -1240,6 +1236,10 @@ int main(int argc, char *argv[])
}
#endif
+ g_unix_signal_add(SIGINT, signal_handler, NULL);
+ g_unix_signal_add(SIGHUP, signal_handler, NULL);
+ g_unix_signal_add(SIGTERM, signal_handler, NULL);
+
if (want_session_info)
session_info = session_info_create(debug);
if (session_info) {
@@ -1252,6 +1252,7 @@ int main(int argc, char *argv[])
active_xfers = g_hash_table_new(g_direct_hash, g_direct_equal);
+ udscs_server_start(server);
loop = g_main_loop_new(NULL, FALSE);
g_main_loop_run(loop);
--
2.25.1

44
0002-vdagentd-init-static-uinput-before-fork.patch
View File

@ -1,44 +0,0 @@
From 7b0435ef66af088c1a1be20b6bc6b0fcb76e4e1a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Jank=C5=AF?= <jjanku@redhat.com>
Date: Fri, 20 Mar 2020 17:18:32 +0100
Subject: [PATCH 2/2] vdagentd: init static uinput before fork
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Otherwise the caller doesn't know that the init failed
because we're returning 0 in the parent and 1 in child.
Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
src/vdagentd/vdagentd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 1b63ec8..753c9bf 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -1224,9 +1224,6 @@ int main(int argc, char *argv[])
}
}
- if (do_daemonize)
- daemonize();
-
#ifdef WITH_STATIC_UINPUT
uinput = vdagentd_uinput_create(uinput_device, 1024, 768, NULL, 0,
debug > 1, uinput_fake);
@@ -1236,6 +1233,9 @@ int main(int argc, char *argv[])
}
#endif
+ if (do_daemonize)
+ daemonize();
+
g_unix_signal_add(SIGINT, signal_handler, NULL);
g_unix_signal_add(SIGHUP, signal_handler, NULL);
g_unix_signal_add(SIGTERM, signal_handler, NULL);
--
2.25.1

82
backport-0001-CVE-2020-25650.patch
View File

@ -1,82 +0,0 @@
From 1a8b93ca6ac0b690339ab7f0afc6fc45d198d332 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Sat, 19 Sep 2020 15:13:42 +0100
Subject: [PATCH] Avoids unchecked file transfer IDs allocation and usage
Avoid agents allocating file transfers.
The "active_xfers" entries are now inserted when client start sending
files.
Also different agents cannot mess with other agent transfers as a
transfer is bound to a single agent.
This issue was reported by SUSE security team.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
---
src/vdagentd/vdagentd.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index a2b74bb..f15989d 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -381,9 +381,11 @@ static void do_client_file_xfer(VirtioPort *vport,
s->id, VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED, NULL, 0);
return;
}
- udscs_write(active_session_conn, VDAGENTD_FILE_XFER_START, 0, 0,
- data, message_header->size);
- return;
+ msg_type = VDAGENTD_FILE_XFER_START;
+ id = s->id;
+ // associate the id with the active connection
+ g_hash_table_insert(active_xfers, GUINT_TO_POINTER(id), active_session_conn);
+ break;
}
case VD_AGENT_FILE_XFER_STATUS: {
VDAgentFileXferStatusMessage *s = (VDAgentFileXferStatusMessage *)data;
@@ -408,6 +410,12 @@ static void do_client_file_xfer(VirtioPort *vport,
return;
}
udscs_write(conn, msg_type, 0, 0, data, message_header->size);
+
+ // client told that transfer is ended, agents too stop the transfer
+ // and release resources
+ if (message_header->type == VD_AGENT_FILE_XFER_STATUS) {
+ g_hash_table_remove(active_xfers, GUINT_TO_POINTER(id));
+ }
}
static void forward_data_to_session_agent(uint32_t type, uint8_t *data, size_t size)
@@ -1012,6 +1020,15 @@ static void do_agent_file_xfer_status(UdscsConnection *conn,
const gchar *log_msg = NULL;
guint data_size = 0;
+ UdscsConnection *task_conn = g_hash_table_lookup(active_xfers, task_id);
+ if (task_conn == NULL || task_conn != conn) {
+ // Protect against misbehaving agent.
+ // Ignore the message, but do not disconnect the agent, to protect against
+ // a misbehaving client that tries to disconnect a good agent
+ // e.g. by sending a new task and immediately cancelling it.
+ return;
+ }
+
/* header->arg1 = file xfer task id, header->arg2 = file xfer status */
switch (header->arg2) {
case VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE:
@@ -1026,10 +1043,9 @@ static void do_agent_file_xfer_status(UdscsConnection *conn,
send_file_xfer_status(virtio_port, log_msg, header->arg1, header->arg2,
data, data_size);
- if (header->arg2 == VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA)
- g_hash_table_insert(active_xfers, task_id, conn);
- else
+ if (header->arg2 != VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA) {
g_hash_table_remove(active_xfers, task_id);
+ }
}
static void agent_read_complete(UdscsConnection *conn,
--
GitLab

25
backport-0001-CVE-2020-25651.patch
View File

@ -1,25 +0,0 @@
From e4bfd1b632b6c14e8411dbe3565115a78cd3d256 Mon Sep 17 00:00:00 2001
From: Uri Lublin <uril@redhat.com>
Date: Wed, 7 Oct 2020 19:34:57 +0300
Subject: [PATCH] cleanup active_xfers when the client disconnects
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
src/vdagentd/vdagentd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 92885b5..8437779 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -168,6 +168,7 @@ static void send_capabilities(VirtioPort *vport,
static void do_client_disconnect(void)
{
+ g_hash_table_remove_all(active_xfers);
if (client_connected) {
udscs_server_write_all(server, VDAGENTD_CLIENT_DISCONNECTED, 0, 0,
NULL, 0);
--
GitLab

49
backport-0001-CVE-2020-25652.patch
View File

@ -1,49 +0,0 @@
From 91caa9223857708475d29df1768208fed1675340 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Sun, 20 Sep 2020 08:05:37 +0100
Subject: [PATCH] Avoids unlimited agent connections
Limit the number of agents that can be connected.
Avoids reaching the maximum number of files in a process.
Beside one file descriptor per agent the daemon open just some
other fixed number of files.
This issue was reported by SUSE security team.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
src/udscs.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/udscs.c b/src/udscs.c
index 7c99eed..3df67b3 100644
--- a/src/udscs.c
+++ b/src/udscs.c
@@ -30,6 +30,12 @@
#include "vdagentd-proto-strings.h"
#include "vdagent-connection.h"
+// Maximum number of connected agents.
+// Avoid DoS from agents.
+// As each connection end up taking a file descriptor is good to have a limit
+// less than the number of file descriptors in the process (by default 1024).
+#define MAX_CONNECTED_AGENTS 128
+
struct _UdscsConnection {
VDAgentConnection parent_instance;
int debug;
@@ -254,6 +260,12 @@ static gboolean udscs_server_accept_cb(GSocketService *service,
struct udscs_server *server = user_data;
UdscsConnection *new_conn;
+ /* prevents DoS having too many agents attached */
+ if (g_list_length(server->connections) >= MAX_CONNECTED_AGENTS) {
+ syslog(LOG_ERR, "Too many agents connected");
+ return TRUE;
+ }
+
new_conn = g_object_new(UDSCS_TYPE_CONNECTION, NULL);
new_conn->debug = server->debug;
new_conn->read_callback = server->read_callback;
--
GitLab

143
backport-0001-CVE-2020-25653.patch
View File

@ -1,143 +0,0 @@
From 51c415df82a52e9ec033225783c77df95f387891 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Sun, 20 Sep 2020 08:06:16 +0100
Subject: [PATCH] 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 ff8b88d..8e47e79 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, NULL);
+ 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 8462889..fc22338 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -952,16 +952,28 @@ static gboolean remove_active_xfers(gpointer key, gpointer value, gpointer conn)
return 0;
}
+/* Check a given process has a given UID */
+static gboolean 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);
@@ -974,7 +986,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)) {
+ g_free(agent_data);
+ udscs_server_destroy_connection(server, conn);
+ return;
+ }
}
g_object_set_data(G_OBJECT(conn), "agent_data", agent_data);
--
GitLab

49
backport-0002-CVE-2020-25650.patch
View File

@ -1,49 +0,0 @@
From 9d35d8a86fb310fc1f29d428c0a96995948d2357 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Fri, 2 Oct 2020 12:27:59 +0100
Subject: [PATCH] Avoids uncontrolled "active_xfers" allocations
Limit the number of active file transfers possibly causing DoSes
consuming memory in "active_xfers".
This issue was reported by SUSE security team.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
---
src/vdagentd/vdagentd.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -46,6 +46,14 @@
#define DEFAULT_UINPUT_DEVICE "/dev/uinput"
+// Maximum number of transfers active at any time.
+// Avoid DoS from client.
+// As each transfer could likely end up taking a file descriptor
+// it is good to have a limit less than the number of file descriptors
+// in the process (by default 1024). The daemon do not open file
+// descriptors for the transfers but the agents do.
+#define MAX_ACTIVE_TRANSFERS 128
+
struct agent_data {
char *session;
int width;
@@ -372,6 +380,12 @@ static void do_client_file_xfer(VirtioPo
"Cancelling client file-xfer request %u",
s->id, VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED, NULL, 0);
return;
+ } else if (g_hash_table_size(active_xfers) >= MAX_ACTIVE_TRANSFERS) {
+ send_file_xfer_status(vport,
+ "Too many transfers ongoing. "
+ "Cancelling client file-xfer request %u",
+ s->id, VD_AGENT_FILE_XFER_STATUS_ERROR, NULL, 0);
+ return;
}
msg_type = VDAGENTD_FILE_XFER_START;
id = s->id;
--
GitLab

31
backport-0002-CVE-2020-25651.patch
View File

@ -1,31 +0,0 @@
From b7db1c20c9f80154fb54392eb44add3486d3e427 Mon Sep 17 00:00:00 2001
From: Uri Lublin <uril@redhat.com>
Date: Sun, 11 Oct 2020 20:59:17 +0300
Subject: [PATCH] vdagentd: do not allow to use an already used file-xfer id
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
src/vdagentd/vdagentd.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 8437779..78378aa 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -404,6 +404,13 @@ static void do_client_file_xfer(VirtioPort *vport,
"Cancelling client file-xfer request %u",
s->id, VD_AGENT_FILE_XFER_STATUS_ERROR, NULL, 0);
return;
+ } else if (g_hash_table_lookup(active_xfers, GUINT_TO_POINTER(s->id)) != NULL) {
+ // id is already used -- client is confused
+ send_file_xfer_status(vport,
+ "File transfer ID is already used. "
+ "Cancelling client file-xfer request %u",
+ s->id, VD_AGENT_FILE_XFER_STATUS_ERROR, NULL, 0);
+ return;
}
msg_type = VDAGENTD_FILE_XFER_START;
id = s->id;
--
GitLab

55
backport-0002-CVE-2020-25652.patch
View File

@ -1,55 +0,0 @@
From 812ca777469a377c84b9861d7d326bfc72563304 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Thu, 24 Sep 2020 12:13:44 +0100
Subject: [PATCH] vdagentd: Limit number of agents per session to 1
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <uril@redhat.com>
---
src/vdagentd/vdagentd.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 59aa523..92885b5 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -952,6 +952,20 @@ static gboolean remove_active_xfers(gpointer key, gpointer value, gpointer conn)
return 0;
}
+/* Check if this connection matches the passed session */
+static int connection_matches_session(UdscsConnection *conn, void *priv)
+{
+ const char *session = priv;
+ const struct agent_data *agent_data = g_object_get_data(G_OBJECT(conn), "agent_data");
+
+ if (!agent_data || !agent_data->session ||
+ strcmp(agent_data->session, session) != 0) {
+ return 0;
+ }
+
+ return 1;
+}
+
/* Check a given process has a given UID */
static gboolean check_uid_of_pid(pid_t pid, uid_t uid)
{
@@ -1006,6 +1020,16 @@ static void agent_connect(UdscsConnection *conn)
udscs_server_destroy_connection(server, conn);
return;
}
+
+ // Check there are no other connection for this session
+ // Note that "conn" is not counted as "agent_data" is still not attached to it
+ if (udscs_server_for_all_clients(server, connection_matches_session,
+ agent_data->session) > 0) {
+ syslog(LOG_ERR, "An agent is already connected for this session");
+ g_free(agent_data);
+ udscs_server_destroy_connection(server, conn);
+ return;
+ }
}
g_object_set_data(G_OBJECT(conn), "agent_data", agent_data);
--
GitLab

163
backport-0002-CVE-2020-25653.patch
View File

@ -1,163 +0,0 @@
From 5c50131797e985d0a5654c1fd7000ae945ed29a7 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Mon, 21 Sep 2020 07:06:09 +0100
Subject: [PATCH] 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 <freddy77@gmail.com>
Acked-by: Uri Lublin <uril@redhat.com>
---
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 fcdb0b6..77429bc 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 2d2311c..42ccc5f 100644
--- a/src/vdagentd/systemd-login.c
+++ b/src/vdagentd/systemd-login.c
@@ -394,3 +394,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 fc22338..59aa523 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -988,12 +988,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);
g_free(agent_data);
udscs_server_destroy_connection(server, conn);
return;
--
GitLab

43
backport-Better-check-for-vdagent_connection_get_peer_pid-results.patch
View File

@ -1,43 +0,0 @@
From 7e924bcbf0bb6b300c6518499c05e87cea13ac51 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Mon, 21 Sep 2020 16:42:26 +0100
Subject: [PATCH] vdagentd: Better check for vdagent_connection_get_peer_pid
results
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The function can return -1 and leave "err" to NULL in some cases,
do not check only for "err".
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Rop茅 <jrope@redhat.com>
---
src/vdagentd/vdagentd.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index cd6340e..560f2ce 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -933,10 +933,14 @@ static void agent_connect(UdscsConnection *conn)
if (session_info) {
pid = vdagent_connection_get_peer_pid(VDAGENT_CONNECTION(conn), &err);
- if (err) {
- syslog(LOG_ERR, "Could not get peer PID, disconnecting new client: %s",
- err->message);
- g_error_free(err);
+ if (err || pid <= 0) {
+ static const char msg[] = "Could not get peer PID, disconnecting new client";
+ if (err) {
+ syslog(LOG_ERR, "%s: %s", msg, err->message);
+ g_error_free(err);
+ } else {
+ syslog(LOG_ERR, "%s", msg);
+ }
g_free(agent_data);
udscs_server_destroy_connection(server, conn);
return;
--
GitLab

BIN
spice-vdagent-0.20.0.tar.bz2
View File

Binary file not shown.

BIN
spice-vdagent-0.21.0.tar.bz2 Normal file
View File

Binary file not shown.

22
spice-vdagent.spec
View File

@ -1,6 +1,6 @@
Name: spice-vdagent
Version: 0.20.0
Release: 2
Version: 0.21.0
Release: 1
Summary: Agent for Spice guests
License: GPLv3+
URL: https://spice-space.org/
@ -8,7 +8,7 @@ Source0: https://spice-space.org/download/releases/%{name}-%{version}.tar
BuildRequires: systemd-devel
BuildRequires: glib2-devel >= 2.50
BuildRequires: spice-protocol >= 0.14.1
BuildRequires: spice-protocol >= 0.14.3
BuildRequires: libpciaccess-devel libXrandr-devel libXinerama-devel
BuildRequires: libXfixes-devel systemd desktop-file-utils libtool
BuildRequires: alsa-lib-devel dbus-devel libdrm-devel
@ -16,19 +16,6 @@ BuildRequires: alsa-lib-devel dbus-devel libdrm-devel
BuildRequires: automake autoconf
%{?systemd_requires}
Patch0001: 0001-vdagentd-work-around-GLib-s-fork-issues.patch
Patch0002: 0002-vdagentd-init-static-uinput-before-fork.patch
Patch6000: backport-0001-CVE-2020-25650.patch
Patch6001: backport-0001-CVE-2020-25652.patch
Patch6002: backport-Better-check-for-vdagent_connection_get_peer_pid-results.patch
Patch6003: backport-0001-CVE-2020-25653.patch
Patch6004: backport-0002-CVE-2020-25653.patch
Patch6005: backport-0002-CVE-2020-25652.patch
Patch6006: backport-0002-CVE-2020-25650.patch
Patch6007: backport-0001-CVE-2020-25651.patch
Patch6008: backport-0002-CVE-2020-25651.patch
%description
%{name} is an optional component for enhancing user experience and performing guest-oriented management tasks.
@ -77,6 +64,9 @@ make install DESTDIR=$RPM_BUILD_ROOT V=2
%{_mandir}/man1/*.1.gz
%changelog
* Fri Dec 3 2021 yangcheng <yangcheng87@huawei.com> - 0.21.0-1
- upgrade to 0.21.0
* Mon Feb 22 2021 jinzhimin <jinzhimin2@huawei.com> - 0.20.0-2
- fix CVE-2020-25650 CVE-2020-25651 CVE-2020-25652 CVE-2020-25653