[gnome-keyring] ssh-agent: Allow opening multiple connections to inferior ssh-agent

[Daiki Ueno <dueno src gnome org>]

commit a19966ba0ef403e8eed733cc9be8b1d45a3787e4
Author: Daiki Ueno <dueno src gnome org>
Date:   Fri Mar 16 12:26:40 2018 +0100

    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

 daemon/ssh-agent/gkd-ssh-agent-process.c      |   29 +++------------------
 daemon/ssh-agent/gkd-ssh-agent-process.h      |    7 +----
 daemon/ssh-agent/gkd-ssh-agent-service.c      |   34 +++++++++++++++++--------
 daemon/ssh-agent/gkd-ssh-agent-util.c         |   11 ++++++++
 daemon/ssh-agent/gkd-ssh-agent-util.h         |    6 ++++
 daemon/ssh-agent/test-gkd-ssh-agent-process.c |   10 ++++---
 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 b6585d7..d3bb3a7 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 350bc95..b1d8ffe 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 451c6d0..e9c2097 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 4d74505..a35da15 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 f8aba8b..4836507 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 329eda1..a116b8e 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);
 }