On 3 May 2017 at 22:35, Stefan Sperling <[email protected]> wrote:
> On Wed, May 03, 2017 at 10:05:01PM +0200, Lukas Jirkovsky wrote:
>> New version. Instead of reimplementing the gpg-agent logic, "gpgconf
>> --list-dir agent-socket" is used. If the gpgconf fails, the code falls
>> back to the original solution to stay compatible with old Gnupg
>> versions.
>
> Thanks Lukas, this patch looks good to me.
>
> I have just one minor stylistic nit:
> The local scratch_pool created in your new function is not necessary.
> This code could be simplified further by just using 'pool' throughout.
Hello Stefan,
thank you for comments. Attached is the updated version without scratch_pool.
> In any case, I would not object to having your patch committed as it is.
> The scratch_pool is just a small detail.
>
> It would be good if you could send patches as a unidiff in the future.
> 'svn diff' produces such output by default, and 'diff -u' works, too.
No problem, will do. I was just blindly following the Patch submission
guidelines from the Community Guide. I like the unified diff output
more as well.
Index: subversion/libsvn_subr/gpg_agent.c
===================================================================
--- subversion/libsvn_subr/gpg_agent.c (revision 1793701)
+++ subversion/libsvn_subr/gpg_agent.c (working copy)
@@ -64,10 +64,13 @@
#include <sys/socket.h>
#include <sys/un.h>
+#include <apr_file_io.h>
#include <apr_pools.h>
+#include <apr_strings.h>
#include "svn_auth.h"
#include "svn_config.h"
#include "svn_error.h"
+#include "svn_io.h"
#include "svn_pools.h"
#include "svn_cmdline.h"
#include "svn_checksum.h"
@@ -225,6 +228,46 @@
close(sd);
}
+/* Find gpg-agent socket location using gpgconf. Returns the path to socket, or
+ * NULL if the socket path cannot be determined using gpgconf.
+ */
+static const char *
+find_gpgconf_agent_socket(apr_pool_t *pool)
+{
+ apr_proc_t proc;
+ svn_stringbuf_t *line;
+ svn_error_t *err;
+ const char *gpgargv[] = { "gpgconf", "--list-dir", "agent-socket", NULL };
+
+ /* execute "gpgconf --list-dir agent-socket" */
+ err = svn_io_start_cmd3(&proc, NULL, "gpgconf", (const char* const*)gpgargv,
+ NULL, TRUE, FALSE, NULL, TRUE, NULL, FALSE, NULL,
+ pool);
+ if (err != SVN_NO_ERROR)
+ {
+ svn_error_clear(err);
+ return NULL;
+ }
+
+ /* read the gpgconf output */
+ err = svn_io_file_readline(proc.out, &line, NULL, NULL, APR_SIZE_MAX,
+ pool, pool);
+ if (err != SVN_NO_ERROR)
+ {
+ svn_error_clear(err);
+ return NULL;
+ }
+ apr_file_close(proc.out);
+ err = svn_io_wait_for_cmd(&proc, "gpgconf", NULL, NULL, pool);
+ if (err != SVN_NO_ERROR)
+ {
+ svn_error_clear(err);
+ return NULL;
+ }
+
+ return line->data;
+}
+
/* Locate a running GPG Agent, and return an open file descriptor
* for communication with the agent in *NEW_SD. If no running agent
* can be found, set *NEW_SD to -1. */
@@ -242,37 +285,43 @@
*new_sd = -1;
- /* This implements the method of finding the socket as described in
- * the gpg-agent man page under the --use-standard-socket option.
- * The manage page says the standard socket is "named 'S.gpg-agent' located
- * in the home directory." GPG's home directory is either the directory
- * specified by $GNUPGHOME or ~/.gnupg. */
- gpg_agent_info = getenv("GPG_AGENT_INFO");
- if (gpg_agent_info != NULL)
- {
- apr_array_header_t *socket_details;
+ /* Query socket location using gpgconf if possible */
+ socket_name = find_gpgconf_agent_socket(pool);
- /* For reference GPG_AGENT_INFO consists of 3 : separated fields.
- * The path to the socket, the pid of the gpg-agent process and
- * finally the version of the protocol the agent talks. */
- socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE,
- pool);
- socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
- }
- else if ((gnupghome = getenv("GNUPGHOME")) != NULL)
- {
- const char *homedir = svn_dirent_canonicalize(gnupghome, pool);
- socket_name = svn_dirent_join(homedir, "S.gpg-agent", pool);
- }
- else
+ /* fallback to the old method used with Gnupg 1.x */
+ if (socket_name == NULL)
{
- const char *homedir = svn_user_get_homedir(pool);
+ /* This implements the method of finding the socket as described in
+ * the gpg-agent man page under the --use-standard-socket option.
+ * The manage page says the standard socket is "named 'S.gpg-agent'
located
+ * in the home directory." GPG's home directory is either the directory
+ * specified by $GNUPGHOME or ~/.gnupg. */
+ if ((gpg_agent_info = getenv("GPG_AGENT_INFO")) != NULL)
+ {
+ apr_array_header_t *socket_details;
- if (!homedir)
- return SVN_NO_ERROR;
+ /* For reference GPG_AGENT_INFO consists of 3 : separated fields.
+ * The path to the socket, the pid of the gpg-agent process and
+ * finally the version of the protocol the agent talks. */
+ socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE,
+ pool);
+ socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
+ }
+ else if ((gnupghome = getenv("GNUPGHOME")) != NULL)
+ {
+ const char *homedir = svn_dirent_canonicalize(gnupghome, pool);
+ socket_name = svn_dirent_join(homedir, "S.gpg-agent", pool);
+ }
+ else
+ {
+ const char *homedir = svn_user_get_homedir(pool);
- socket_name = svn_dirent_join_many(pool, homedir, ".gnupg",
- "S.gpg-agent", SVN_VA_NULL);
+ if (!homedir)
+ return SVN_NO_ERROR;
+
+ socket_name = svn_dirent_join_many(pool, homedir, ".gnupg",
+ "S.gpg-agent", SVN_VA_NULL);
+ }
}
if (socket_name != NULL)
