ssh-agent: Allow opening multiple connections to inferior ssh-agent

Previously, it keeps only one connection to the inferior ssh-agent
process.  That prevented simultaneous access to gnome-keyring's
ssh-agent service.  With this patch, it always opens a new connection
to the inferior ssh-agent process when a new client connects.

https://bugzilla.gnome.org/show_bug.cgi?id=794369

6 files changed, 51 insertions, 46 deletions

diff --git a/daemon/ssh-agent/gkd-ssh-agent-process.c b/daemon/ssh-agent/gkd-ssh-agent-process.c
index b6585d70..d3bb3a7e 100644
--- a/daemon/ssh-agent/gkd-ssh-agent-process.c
+++ b/daemon/ssh-agent/gkd-ssh-agent-process.c
@@ -47,7 +47,6 @@ struct _GkdSshAgentProcess
 {
 	GObject object;
 	gchar *path;
-	GSocketConnection *connection;
 	gint output;
 	GMutex lock;
 	GPid pid;
@@ -70,7 +69,6 @@ gkd_ssh_agent_process_finalize (GObject *object)
 {
 	GkdSshAgentProcess *self = GKD_SSH_AGENT_PROCESS (object);
 
-	g_clear_object (&self->connection);
 	if (self->output != -1)
 		close (self->output);
 	if (self->output_id)
@@ -206,7 +204,7 @@ on_timeout (gpointer user_data)
 	return TRUE;
 }
 
-gboolean
+GSocketConnection *
 gkd_ssh_agent_process_connect (GkdSshAgentProcess *self,
 			       GCancellable *cancellable,
 			       GError **error)
@@ -223,7 +221,7 @@ gkd_ssh_agent_process_connect (GkdSshAgentProcess *self,
 	if (self->pid == 0) {
 		if (!agent_start_inlock (self, error)) {
 			g_mutex_unlock (&self->lock);
-			return FALSE;
+			return NULL;
 		}
 		started = TRUE;
 	}
@@ -239,7 +237,7 @@ gkd_ssh_agent_process_connect (GkdSshAgentProcess *self,
 		g_mutex_unlock (&self->lock);
 		g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
 			     "ssh-agent process is not ready");
-		return FALSE;
+		return NULL;
 	}
 
 	address = g_unix_socket_address_new (self->path);
@@ -251,29 +249,10 @@ gkd_ssh_agent_process_connect (GkdSshAgentProcess *self,
 					      error);
 	g_object_unref (address);
 	g_object_unref (client);
-	if (!connection) {
-		g_mutex_unlock (&self->lock);
-		return FALSE;
-	}
-
-	g_clear_object (&self->connection);
-	self->connection = connection;
 
 	g_mutex_unlock (&self->lock);
 
-	return TRUE;
-}
-
-gboolean
-gkd_ssh_agent_process_call (GkdSshAgentProcess *self,
-                            EggBuffer*req,
-                            EggBuffer *resp,
-			    GCancellable *cancellable,
-			    GError **error)
-{
-	g_return_val_if_fail (self->connection != NULL, FALSE);
-	return _gkd_ssh_agent_write_packet (self->connection, req, cancellable, error) &&
-		_gkd_ssh_agent_read_packet (self->connection, resp, cancellable, error);
+	return connection;
 }
 
 GkdSshAgentProcess *
diff --git a/daemon/ssh-agent/gkd-ssh-agent-process.h b/daemon/ssh-agent/gkd-ssh-agent-process.h
index 350bc959..b1d8ffe0 100644
--- a/daemon/ssh-agent/gkd-ssh-agent-process.h
+++ b/daemon/ssh-agent/gkd-ssh-agent-process.h
@@ -32,12 +32,7 @@
 G_DECLARE_FINAL_TYPE(GkdSshAgentProcess, gkd_ssh_agent_process, GKD, SSH_AGENT_PROCESS, GObject)
 
 GkdSshAgentProcess *gkd_ssh_agent_process_new         (const gchar *path);
-gboolean            gkd_ssh_agent_process_connect     (GkdSshAgentProcess *self,
-                                                       GCancellable *cancellable,
-                                                       GError **error);
-gboolean            gkd_ssh_agent_process_call        (GkdSshAgentProcess *self,
-                                                       EggBuffer *req,
-                                                       EggBuffer *resp,
+GSocketConnection  *gkd_ssh_agent_process_connect     (GkdSshAgentProcess *self,
                                                        GCancellable *cancellable,
                                                        GError **error);
 GPid                gkd_ssh_agent_process_get_pid     (GkdSshAgentProcess *self);
diff --git a/daemon/ssh-agent/gkd-ssh-agent-service.c b/daemon/ssh-agent/gkd-ssh-agent-service.c
index 451c6d01..e9c20975 100644
--- a/daemon/ssh-agent/gkd-ssh-agent-service.c
+++ b/daemon/ssh-agent/gkd-ssh-agent-service.c
@@ -43,7 +43,7 @@
 
 EGG_SECURE_DECLARE (ssh_agent);
 
-typedef gboolean (*GkdSshAgentOperation) (GkdSshAgentService *agent, EggBuffer *req, EggBuffer *resp, GCancellable *cancellable, GError **error);
+typedef gboolean (*GkdSshAgentOperation) (GkdSshAgentService *agent, GSocketConnection *connection, EggBuffer *req, EggBuffer *resp, GCancellable *cancellable, GError **error);
 static const GkdSshAgentOperation operations[GKD_SSH_OP_MAX];
 
 enum {
@@ -159,16 +159,18 @@ gkd_ssh_agent_service_class_init (GkdSshAgentServiceClass *klass)
 
 static gboolean
 relay_request (GkdSshAgentService *self,
+	       GSocketConnection *connection,
 	       EggBuffer *req,
 	       EggBuffer *resp,
 	       GCancellable *cancellable,
 	       GError **error)
 {
-	return gkd_ssh_agent_process_call (self->process, req, resp, cancellable, error);
+	return _gkd_ssh_agent_call (connection, req, resp, cancellable, error);
 }
 
 static gboolean
 handle_request (GkdSshAgentService *self,
+		GSocketConnection *connection,
 		EggBuffer *req,
 		EggBuffer *resp,
 		GCancellable *cancellable,
@@ -187,7 +189,7 @@ handle_request (GkdSshAgentService *self,
 	else
 		func = relay_request;
 
-	return func (self, req, resp, cancellable, error);
+	return func (self, connection, req, resp, cancellable, error);
 }
 
 static void
@@ -287,13 +289,15 @@ on_run (GThreadedSocketService *service,
 	EggBuffer req;
 	EggBuffer resp;
 	GError *error;
+	GSocketConnection *agent_connection;
 	gboolean ret;
 
 	egg_buffer_init_full (&req, 128, egg_secure_realloc);
 	egg_buffer_init_full (&resp, 128, (EggBufferAllocator)g_realloc);
 
 	error = NULL;
-	if (!gkd_ssh_agent_process_connect (self->process, self->cancellable, &error)) {
+	agent_connection = gkd_ssh_agent_process_connect (self->process, self->cancellable, &error);
+	if (!agent_connection) {
 		g_warning ("couldn't connect to ssh-agent: %s", error->message);
 		g_error_free (error);
 		goto out;
@@ -311,7 +315,7 @@ on_run (GThreadedSocketService *service,
 
 		/* Handle the request */
 		error = NULL;
-		while (!(ret = handle_request (self, &req, &resp, self->cancellable, &error))) {
+		while (!(ret = handle_request (self, agent_connection, &req, &resp, self->cancellable, &error))) {
 			if (gkd_ssh_agent_process_get_pid (self->process) != 0) {
 				if (error->code != G_IO_ERROR_CANCELLED)
 					g_message ("couldn't handle client request: %s", error->message);
@@ -320,8 +324,10 @@ on_run (GThreadedSocketService *service,
 			}
 
 			/* Reconnect to the ssh-agent */
+			g_clear_object (&agent_connection);
 			g_clear_error (&error);
-			if (!gkd_ssh_agent_process_connect (self->process, self->cancellable, &error)) {
+			agent_connection = gkd_ssh_agent_process_connect (self->process, self->cancellable, &error);
+			if (!agent_connection) {
 				if (error->code != G_IO_ERROR_CANCELLED)
 					g_message ("couldn't connect to ssh-agent: %s", error->message);
 				g_error_free (error);
@@ -343,6 +349,7 @@ on_run (GThreadedSocketService *service,
 	egg_buffer_uninit (&req);
 	egg_buffer_uninit (&resp);
 
+	g_object_unref (agent_connection);
 	g_object_unref (self);
 
 	return TRUE;
@@ -445,6 +452,7 @@ gkd_ssh_agent_service_lookup_key (GkdSshAgentService *self,
 
 static gboolean
 op_add_identity (GkdSshAgentService *self,
+		 GSocketConnection *connection,
 		 EggBuffer *req,
 		 EggBuffer *resp,
 		 GCancellable *cancellable,
@@ -463,7 +471,7 @@ op_add_identity (GkdSshAgentService *self,
 	else
 		g_message ("got unparseable add identity request for ssh-agent");
 
-	ret = relay_request (self, req, resp, cancellable, error);
+	ret = relay_request (self, connection, req, resp, cancellable, error);
 	if (key) {
 		if (ret)
 			add_key (self, key);
@@ -510,6 +518,7 @@ parse_identities_answer (EggBuffer *resp)
 
 static gboolean
 op_request_identities (GkdSshAgentService *self,
+		       GSocketConnection *connection,
 		       EggBuffer *req,
 		       EggBuffer *resp,
 		       GCancellable *cancellable,
@@ -524,7 +533,7 @@ op_request_identities (GkdSshAgentService *self,
 	GList *l;
 	GkdSshAgentPreload *preload;
 
-	if (!relay_request (self, req, resp, cancellable, error))
+	if (!relay_request (self, connection, req, resp, cancellable, error))
 		return FALSE;
 
 	/* Parse all the keys, and if it fails, just fall through */
@@ -565,6 +574,7 @@ op_request_identities (GkdSshAgentService *self,
 
 static gboolean
 op_sign_request (GkdSshAgentService *self,
+		 GSocketConnection *connection,
 		 EggBuffer *req,
 		 EggBuffer *resp,
 		 GCancellable *cancellable,
@@ -584,11 +594,12 @@ op_sign_request (GkdSshAgentService *self,
 		g_message ("got unparseable sign request for ssh-agent");
 	}
 
-	return relay_request (self, req, resp, cancellable, error);
+	return relay_request (self, connection, req, resp, cancellable, error);
 }
 
 static gboolean
 op_remove_identity (GkdSshAgentService *self,
+		    GSocketConnection *connection,
 		    EggBuffer *req,
 		    EggBuffer *resp,
 		    GCancellable *cancellable,
@@ -608,7 +619,7 @@ op_remove_identity (GkdSshAgentService *self,
 		g_message ("got unparseable remove request for ssh-agent");
 
 	/* Call out ssh-agent anyway to make sure that the key is removed */
-	ret = relay_request (self, req, resp, cancellable, error);
+	ret = relay_request (self, connection, req, resp, cancellable, error);
 	if (key) {
 		if (ret)
 			remove_key (self, key);
@@ -619,6 +630,7 @@ op_remove_identity (GkdSshAgentService *self,
 
 static gboolean
 op_remove_all_identities (GkdSshAgentService *self,
+			  GSocketConnection *connection,
 			  EggBuffer *req,
 			  EggBuffer *resp,
 			  GCancellable *cancellable,
@@ -626,7 +638,7 @@ op_remove_all_identities (GkdSshAgentService *self,
 {
 	gboolean ret;
 
-	ret = relay_request (self, req, resp, cancellable, error);
+	ret = relay_request (self, connection, req, resp, cancellable, error);
 	if (ret)
 		clear_keys (self);
 
diff --git a/daemon/ssh-agent/gkd-ssh-agent-util.c b/daemon/ssh-agent/gkd-ssh-agent-util.c
index 4d745056..a35da152 100644
--- a/daemon/ssh-agent/gkd-ssh-agent-util.c
+++ b/daemon/ssh-agent/gkd-ssh-agent-util.c
@@ -75,6 +75,17 @@ _gkd_ssh_agent_write_packet (GSocketConnection *connection,
 	return g_output_stream_write_all (stream, buffer->buf, buffer->len, &bytes_written, cancellable, error);
 }
 
+gboolean
+_gkd_ssh_agent_call (GSocketConnection *connection,
+		     EggBuffer*req,
+		     EggBuffer *resp,
+		     GCancellable *cancellable,
+		     GError **error)
+{
+	return _gkd_ssh_agent_write_packet (connection, req, cancellable, error) &&
+		_gkd_ssh_agent_read_packet (connection, resp, cancellable, error);
+}
+
 GBytes *
 _gkd_ssh_agent_parse_public_key (GBytes *input,
 				 gchar **comment)
diff --git a/daemon/ssh-agent/gkd-ssh-agent-util.h b/daemon/ssh-agent/gkd-ssh-agent-util.h
index f8aba8b5..4836507b 100644
--- a/daemon/ssh-agent/gkd-ssh-agent-util.h
+++ b/daemon/ssh-agent/gkd-ssh-agent-util.h
@@ -36,6 +36,12 @@ gboolean _gkd_ssh_agent_write_packet     (GSocketConnection  *connection,
                                           GCancellable       *cancellable,
                                           GError            **error);
 
+gboolean _gkd_ssh_agent_call             (GSocketConnection  *connection,
+                                          EggBuffer          *req,
+                                          EggBuffer          *resp,
+                                          GCancellable       *cancellable,
+                                          GError            **error);
+
 GBytes  *_gkd_ssh_agent_parse_public_key (GBytes             *input,
                                           gchar             **comment);
 
diff --git a/daemon/ssh-agent/test-gkd-ssh-agent-process.c b/daemon/ssh-agent/test-gkd-ssh-agent-process.c
index 329eda10..a116b8e0 100644
--- a/daemon/ssh-agent/test-gkd-ssh-agent-process.c
+++ b/daemon/ssh-agent/test-gkd-ssh-agent-process.c
@@ -35,6 +35,7 @@ typedef struct {
 	EggBuffer req;
 	EggBuffer resp;
 	GkdSshAgentProcess *process;
+	GSocketConnection *connection;
 	GMainLoop *loop;
 } Test;
 
@@ -52,12 +53,14 @@ setup (Test *test, gconstpointer unused)
 	test->process = gkd_ssh_agent_process_new (path);
 	g_free (path);
 	g_assert_nonnull (test->process);
+	test->connection = NULL;
 }
 
 static void
 teardown (Test *test, gconstpointer unused)
 {
 	g_clear_object (&test->process);
+	g_clear_object (&test->connection);
 
 	egg_buffer_uninit (&test->req);
 	egg_buffer_uninit (&test->resp);
@@ -70,11 +73,10 @@ static void
 connect_to_process (Test *test)
 {
 	GError *error;
-	gboolean ret;
 
 	error = NULL;
-	ret = gkd_ssh_agent_process_connect (test->process, NULL, &error);
-	g_assert_true (ret);
+	test->connection = gkd_ssh_agent_process_connect (test->process, NULL, &error);
+	g_assert_nonnull (test->connection);
 	g_assert_no_error (error);
 }
 
@@ -91,7 +93,7 @@ call (Test *test)
 	gboolean ret;
 
 	error = NULL;
-	ret = gkd_ssh_agent_process_call (test->process, &test->req, &test->resp, NULL, &error);
+	ret = _gkd_ssh_agent_call (test->connection, &test->req, &test->resp, NULL, &error);
 	g_assert_true (ret);
 	g_assert_no_error (error);
 }