Re: [PATCH] gpg-agent storage - add support for /run based sockets

On Fri, Apr 28, 2017 at 05:46:21PM +0200, Lukas Jirkovsky wrote:
> Add support for sockets in /run used by recent gpg-agent

Thanks for the patch! Aside from a few comments on the code, I wonder
if it would be better to use "gpgconf --list-dir agent-socket" instead
of reimplementing the various bits of logic ourselves? That's only
usable with the gnupg 2.x series, though.

> Index: subversion/libsvn_subr/gpg_agent.c
> ===================================================================
> *** subversion/libsvn_subr/gpg_agent.c (revision 1792920)
> --- subversion/libsvn_subr/gpg_agent.c (working copy)
> *************** find_running_gpg_agent(int *new_sd, apr_
> *** 266,278 ****
> }
> else
> {
> const char *homedir = svn_user_get_homedir(pool);
> !
> ! 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)
> --- 269,309 ----
> }
> else
> {
> + int isockmax = 0;
> + const char* socketpaths[3] = {0,0,0};
> + /* GnuPG since 2.1.13 uses /run/user/UID/gnupg based sockets */
> + apr_uid_t uid;
> + apr_gid_t gid;
> + if(apr_uid_current(&uid, &gid, pool) == APR_SUCCESS) {

apr_uid_current is only available when APR_HAS_USER is defined.

> + char* uidbuf = apr_psprintf(pool, "%lu", (unsigned long)uid);
> + socketpaths[isockmax++] = svn_dirent_join_many(pool, "/run/user",
> + uidbuf, "gnupg",
> + "S.gpg-agent",
> + SVN_VA_NULL);
> + socketpaths[isockmax++] = svn_dirent_join_many(pool, "/var/run/user",
> + uidbuf, "gnupg",
> + "S.gpg-agent",
> + SVN_VA_NULL);
> + }
> + /* older GnuPG versions use /home/USER/.gnupg based sockets */
> const char *homedir = svn_user_get_homedir(pool);
> ! if(homedir) {
> ! socketpaths[isockmax++] = svn_dirent_join_many(pool, homedir,
> ! ".gnupg",
> ! "S.gpg-agent",
> ! SVN_VA_NULL);
> ! }
> !
> ! /* find which of the available socket paths actually exists */
> ! int isock = 0;
> ! for (isock = 0; isock < isockmax; isock++) {
> ! struct stat statbuf;
> ! stat(socketpaths[isock], &statbuf);

Should probably use svn_io_stat here instead, with wanted set to
APR_FINFO_TYPE.

> ! if(S_ISSOCK(statbuf.st_mode)) {
> ! socket_name = socketpaths[isock];
> ! break;
> ! }
> ! }
> }
>
> if (socket_name != NULL)

Cheers,

-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB