agent: Don't keep socket to ssh/gpg-agent open
authorTobias Brunner <tobias@strongswan.org>
Fri, 1 Feb 2019 08:48:43 +0000 (09:48 +0100)
committerTobias Brunner <tobias@strongswan.org>
Wed, 20 Feb 2019 09:43:19 +0000 (10:43 +0100)
Instead, create a socket when necessary.  Apparently, it can prevent
the agent from getting terminated (e.g. during system shutdown) if e.g.
charon-nm is still running with an open connection to the agent.

src/libstrongswan/plugins/agent/agent_private_key.c

index db87aff..8f6ea37 100644 (file)
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2013-2019 Tobias Brunner
  * Copyright (C) 2008-2009 Martin Willi
  * HSR Hochschule fuer Technik Rapperswil
  *
@@ -44,9 +45,9 @@ struct private_agent_private_key_t {
        agent_private_key_t public;
 
        /**
-        * ssh-agent unix socket connection
+        * Path to the UNIX socket
         */
-       int socket;
+       char *path;
 
        /**
         * public key encoded in SSH format
@@ -174,28 +175,35 @@ static int open_connection(char *path)
  */
 static bool read_key(private_agent_private_key_t *this, public_key_t *pubkey)
 {
-       int len;
+       int socket, len;
        char buf[2048];
        chunk_t blob, key;
+       bool success = FALSE;
+
+       socket = open_connection(this->path);
+       if (socket < 0)
+       {
+               return FALSE;
+       }
 
        len = htonl(1);
        buf[0] = SSH_AGENT_ID_REQUEST;
-       if (write(this->socket, &len, sizeof(len)) != sizeof(len) ||
-               write(this->socket, &buf, 1) != 1)
+       if (write(socket, &len, sizeof(len)) != sizeof(len) ||
+               write(socket, &buf, 1) != 1)
        {
                DBG1(DBG_LIB, "writing to ssh-agent failed");
-               return FALSE;
+               goto done;
        }
 
        blob = chunk_create(buf, sizeof(buf));
-       blob.len = read(this->socket, blob.ptr, blob.len);
+       blob.len = read(socket, blob.ptr, blob.len);
 
        if (blob.len < sizeof(uint32_t) + sizeof(u_char) ||
                read_uint32(&blob) != blob.len ||
                read_byte(&blob) != SSH_AGENT_ID_RESPONSE)
        {
                DBG1(DBG_LIB, "received invalid ssh-agent identity response");
-               return FALSE;
+               goto done;
        }
        read_uint32(&blob);
 
@@ -219,9 +227,12 @@ static bool read_key(private_agent_private_key_t *this, public_key_t *pubkey)
                        continue;
                }
                this->key = chunk_clone(key);
-               return TRUE;
+               success = TRUE;
+               break;
        }
-       return FALSE;
+done:
+       close(socket);
+       return success;
 }
 
 static bool scheme_supported(private_agent_private_key_t *this,
@@ -271,6 +282,8 @@ METHOD(private_key_t, sign, bool,
        uint32_t len, flags = 0;
        char buf[2048], *prefix = NULL;
        chunk_t blob;
+       int socket;
+       bool success = FALSE;
 
        if (!scheme_supported(this, scheme, &flags, &prefix))
        {
@@ -279,46 +292,52 @@ METHOD(private_key_t, sign, bool,
                return FALSE;
        }
 
+       socket = open_connection(this->path);
+       if (socket < 0)
+       {
+               return FALSE;
+       }
+
        len = htonl(1 + sizeof(uint32_t) * 3 + this->key.len + data.len);
        buf[0] = SSH_AGENT_SIGN_REQUEST;
-       if (write(this->socket, &len, sizeof(len)) != sizeof(len) ||
-               write(this->socket, &buf, 1) != 1)
+       if (write(socket, &len, sizeof(len)) != sizeof(len) ||
+               write(socket, &buf, 1) != 1)
        {
                DBG1(DBG_LIB, "writing to ssh-agent failed");
-               return FALSE;
+               goto done;
        }
 
        len = htonl(this->key.len);
-       if (write(this->socket, &len, sizeof(len)) != sizeof(len) ||
-               write(this->socket, this->key.ptr, this->key.len) != this->key.len)
+       if (write(socket, &len, sizeof(len)) != sizeof(len) ||
+               write(socket, this->key.ptr, this->key.len) != this->key.len)
        {
                DBG1(DBG_LIB, "writing to ssh-agent failed");
-               return FALSE;
+               goto done;
        }
 
        len = htonl(data.len);
-       if (write(this->socket, &len, sizeof(len)) != sizeof(len) ||
-               write(this->socket, data.ptr, data.len) != data.len)
+       if (write(socket, &len, sizeof(len)) != sizeof(len) ||
+               write(socket, data.ptr, data.len) != data.len)
        {
                DBG1(DBG_LIB, "writing to ssh-agent failed");
-               return FALSE;
+               goto done;
        }
 
        flags = htonl(flags);
-       if (write(this->socket, &flags, sizeof(flags)) != sizeof(flags))
+       if (write(socket, &flags, sizeof(flags)) != sizeof(flags))
        {
                DBG1(DBG_LIB, "writing to ssh-agent failed");
-               return FALSE;
+               goto done;
        }
 
        blob = chunk_create(buf, sizeof(buf));
-       blob.len = read(this->socket, blob.ptr, blob.len);
+       blob.len = read(socket, blob.ptr, blob.len);
        if (blob.len < sizeof(uint32_t) + sizeof(u_char) ||
                read_uint32(&blob) != blob.len ||
                read_byte(&blob) != SSH_AGENT_SIGN_RESPONSE)
        {
                DBG1(DBG_LIB, "received invalid ssh-agent signature response");
-               return FALSE;
+               goto done;
        }
        /* parse length */
        blob = read_string(&blob);
@@ -326,7 +345,7 @@ METHOD(private_key_t, sign, bool,
        if (prefix && !chunk_equals(read_string(&blob), chunk_from_str(prefix)))
        {
                DBG1(DBG_LIB, "ssh-agent didn't return requested %s signature", prefix);
-               return FALSE;
+               goto done;
        }
        type = this->pubkey->get_type(this->pubkey);
        if (type == KEY_RSA || type == KEY_ED25519 || type == KEY_ED448)
@@ -335,7 +354,7 @@ METHOD(private_key_t, sign, bool,
                if (blob.len)
                {
                        *signature = chunk_clone(blob);
-                       return TRUE;
+                       success = TRUE;
                }
        }
        else
@@ -350,12 +369,18 @@ METHOD(private_key_t, sign, bool,
                        if (r.len && s.len)
                        {
                                *signature = chunk_cat("cc", r, s);
-                               return TRUE;
+                               success = TRUE;
                        }
                }
        }
-       DBG1(DBG_LIB, "received invalid ssh-agent signature response");
-       return FALSE;
+       if (!success)
+       {
+               DBG1(DBG_LIB, "received invalid ssh-agent signature response");
+       }
+
+done:
+       close(socket);
+       return success;
 }
 
 METHOD(private_key_t, get_type, key_type_t,
@@ -483,9 +508,9 @@ METHOD(private_key_t, destroy, void,
 {
        if (ref_put(&this->ref))
        {
-               close(this->socket);
                chunk_free(&this->key);
                DESTROY_IF(this->pubkey);
+               free(this->path);
                free(this);
        }
 }
@@ -539,15 +564,10 @@ agent_private_key_t *agent_private_key_open(key_type_t type, va_list args)
                                .destroy = _destroy,
                        },
                },
+               .path = strdup(path),
                .ref = 1,
        );
 
-       this->socket = open_connection(path);
-       if (this->socket < 0)
-       {
-               free(this);
-               return NULL;
-       }
        if (!read_key(this, pubkey))
        {
                destroy(this);