Check that the socket is well and served by us.

NEWS

@@ -1,42 +1,46 @@
 Noteworthy changes in version 2.0.10 (unreleased)
 -------------------------------------------------
 
- * New keyserver helper gpg2keys_kdns as generic DNS CERT lookup.  Run
+ * [gpg] New keyserver helper gpg2keys_kdns as generic DNS CERT
-   with --help for a short description.  Requires the ADNS library.
+   lookup.  Run with --help for a short description.  Requires the
+   ADNS library.
 
  * [gpg] New mechanisms "local" and "nodefault" for --auto-key-locate.
    Fixed a few problems with this option.
 
- * [w32] Initialized the socket subsystem for all keyserver helpers.
-
- * [w32] The sysconf directory has been moved from a subdirectory of
-   the installation directory to %CSIDL_COMMON_APPDATA%/GNU/etc/gnupg.
-
  * [gpg] New command --locate-keys.
 
  * [gpg] New options --with-sig-list and --with-sig-check.
 
  * [gpg] The option "-sat" is no longer an alias for --clearsign.
 
- * [gpgsm] Made --output option work with --export-secret-key-p12.
-
- * [gpg-connect-agent] Accept commands given as command line arguments.
-
  * [gpg] The option --fixed-list-mode is now implicitly used and obsolete.
 
  * [gpg] New control statement %ask-passphrase for the unattended key
    generation.
 
- * gpgsm now uses AES by default.
+ * [gpgsm] Now uses AES by default.
 
- * gpg-preset-passphrase works again.
+ * [gpgsm] Made --output option work with --export-secret-key-p12.
+
+ * [gpg-agent] Terminate process if the own listening socket is not
+   anymore served by ourself.
+
+ * [scdaemon] Made it more robust on W32.
+
+ * [gpg-connect-agent] Accept commands given as command line arguments.
+
+ * [w32] Initialized the socket subsystem for all keyserver helpers.
+
+ * [w32] The sysconf directory has been moved from a subdirectory of
+   the installation directory to %CSIDL_COMMON_APPDATA%/GNU/etc/gnupg.
+
+ * The gpg-preset-passphrase mechanism works again.
 
  * Admin PINs are cached again (bug in 2.0.9).
 
  * Support for version 2 OpenPGP cards.
 
- * [scdaemon] Made it more robust on W32.
-
  * Libgcrypt 1.4 is now required.
 
 
@@ -635,7 +639,8 @@ Noteworthy changes in version 1.9.0 (2003-08-05)
    development branch.
 
 
- Copyright 2002, 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+ Copyright 2002, 2003, 2004, 2005, 2006, 2007,
+           2008  Free Software Foundation, Inc.
 
  This file is free software; as a special exception the author gives
  unlimited permission to copy and/or distribute it, with or without

agent/ChangeLog

@@ -1,3 +1,16 @@
+2008-10-29  Werner Koch  <wk@g10code.com>
+
+	* gpg-agent.c (main): Move USE_STANDARD_SOCKET to the outer scope.
+	(create_socket_name): Remove arg USE_STANDARD_SOCKET.  Change all
+	callers.
+	(create_server_socket): Remove IS_STANDARD_NAME and replace it by
+	USE_STANDARD_SOCKET.  Change all callers.
+	(check_own_socket_running): New.
+	(check_own_socket, check_own_socket_thread): New.
+	(handle_tick): Check server socket once a minute.
+	(handle_connections): Remove the extra pth_wait in the shutdown
+	case.
+
 2008-10-20  Werner Koch  <wk@g10code.com>
 
 	* command.c (cmd_geteventcounter): Mark unused arg.

agent/gpg-agent.c

@@ -194,9 +194,14 @@ static ARGPARSE_OPTS opts[] = {
 #define TIMERTICK_INTERVAL    (2)    /* Seconds.  */
 #endif
 
-/* flag to indicate that a shutdown was requested */
+/* Flag to indicate that a shutdown was requested.  */
 static int shutdown_pending;
 
+/* Counter for the currently running own socket checks.  */
+static int check_own_socket_running;
+
+/* True if we are listening on the standard socket.  */
+static int use_standard_socket;
 
 /* It is possible that we are currently running under setuid permissions */
 static int maybe_setuid = 1;
@@ -241,10 +246,8 @@ static pid_t parent_pid = (pid_t)(-1);
    Local prototypes. 
  */
 
-static char *create_socket_name (int use_standard_socket,
+static char *create_socket_name (char *standard_name, char *template);
-                                 char *standard_name, char *template);
+static gnupg_fd_t create_server_socket (char *name, int is_ssh, 
-static gnupg_fd_t create_server_socket (int is_standard_name, char *name, 
-                                        int is_ssh, 
                                         assuan_sock_nonce_t *nonce);
 static void create_directories (void);
 
@@ -253,6 +256,7 @@ static void agent_deinit_default_ctrl (ctrl_t ctrl);
 
 static void handle_connections (gnupg_fd_t listen_fd,
                                 gnupg_fd_t listen_fd_ssh);
+static void check_own_socket (void);
 static int check_for_running_agent (int silent, int mode);
 
 /* Pth wrapper function definitions. */
@@ -494,7 +498,6 @@ main (int argc, char **argv )
   char *logfile = NULL;
   int debug_wait = 0;
   int gpgconf_list = 0;
-  int use_standard_socket = 0;
   gpg_error_t err;
   const char *env_file_name = NULL;
 
@@ -895,19 +898,15 @@ main (int argc, char **argv )
 
 
       /* Create the sockets.  */
-      socket_name = create_socket_name (use_standard_socket,
+      socket_name = create_socket_name 
-                                        "S.gpg-agent",
+        ("S.gpg-agent", "/tmp/gpg-XXXXXX/S.gpg-agent");
-                                        "/tmp/gpg-XXXXXX/S.gpg-agent");
       if (opt.ssh_support)
-	socket_name_ssh = create_socket_name (use_standard_socket, 
+	socket_name_ssh = create_socket_name 
-                                            "S.gpg-agent.ssh",
+          ("S.gpg-agent.ssh", "/tmp/gpg-XXXXXX/S.gpg-agent.ssh");
-                                            "/tmp/gpg-XXXXXX/S.gpg-agent.ssh");
 
-      fd = create_server_socket (use_standard_socket, socket_name, 0,
+      fd = create_server_socket (socket_name, 0, &socket_nonce);
-                                 &socket_nonce);
       if (opt.ssh_support)
-	fd_ssh = create_server_socket (use_standard_socket, socket_name_ssh, 1,
+	fd_ssh = create_server_socket (socket_name_ssh, 1, &socket_nonce_ssh);
-                                       &socket_nonce_ssh);
       else
 	fd_ssh = GNUPG_INVALID_FD;
 
@@ -1311,8 +1310,7 @@ get_agent_scd_notify_event (void)
    terminates the process in case of an error.  Returns: Pointer to an
    allocated string with the absolute name of the socket used.  */
 static char *
-create_socket_name (int use_standard_socket,
+create_socket_name (char *standard_name, char *template)
-		    char *standard_name, char *template)
 {
   char *name, *p;
 
@@ -1349,14 +1347,12 @@ create_socket_name (int use_standard_socket,
 
 
 
-/* Create a Unix domain socket with NAME.  IS_STANDARD_NAME indicates
+/* Create a Unix domain socket with NAME.  Returns the file descriptor
-   whether a non-random socket is used.  Returns the file descriptor
    or terminates the process in case of an error.  Not that this
-   function needs to be used for the regular socket first and only then
+   function needs to be used for the regular socket first and only
-   for the ssh socket.  */
+   then for the ssh socket.  */
 static gnupg_fd_t
-create_server_socket (int is_standard_name, char *name, int is_ssh,
+create_server_socket (char *name, int is_ssh, assuan_sock_nonce_t *nonce)
-                      assuan_sock_nonce_t *nonce)
 {
   struct sockaddr_un *serv_addr;
   socklen_t len;
@@ -1383,7 +1379,7 @@ create_server_socket (int is_standard_name, char *name, int is_ssh,
 	 + strlen (serv_addr->sun_path) + 1);
 
   rc = assuan_sock_bind (fd, (struct sockaddr*) serv_addr, len);
-  if (is_standard_name && rc == -1 && errno == EADDRINUSE)
+  if (use_standard_socket && rc == -1 && errno == EADDRINUSE)
     {
       /* Check whether a gpg-agent is already running on the standard
          socket.  We do this test only if this is not the ssh socket.
@@ -1416,7 +1412,7 @@ create_server_socket (int is_standard_name, char *name, int is_ssh,
                  gpg_strerror (gpg_error_from_errno (errno)));
       
       assuan_sock_close (fd);
-      if (is_standard_name)
+      if (use_standard_socket)
         *name = 0; /* Inhibit removal of the socket by cleanup(). */
       agent_exit (2);
     }
@@ -1530,6 +1526,11 @@ create_directories (void)
 static void
 handle_tick (void)
 {
+  static time_t last_minute;
+
+  if (!last_minute)
+    last_minute = time (NULL);
+
   /* Check whether the scdaemon has died and cleanup in this case. */
   agent_scd_check_aliveness ();
 
@@ -1548,6 +1549,14 @@ handle_tick (void)
         }
     }
 #endif /*HAVE_W32_SYSTEM*/
+  
+  /* Code to be run every minute.  */
+  if (last_minute + 60 <= time (NULL))
+    {
+      check_own_socket ();
+      last_minute = time (NULL);
+    }
+
 }
 
 
@@ -1755,13 +1764,9 @@ handle_connections (gnupg_fd_t listen_fd, gnupg_fd_t listen_fd_ssh)
           if (pth_ctrl (PTH_CTRL_GETTHREADS) == 1)
             break; /* ready */
 
-          /* Do not accept anymore connections and wait for existing
+          /* Do not accept new connections but keep on running the
-             connections to terminate */
+             loop to cope with the timer events.  */
-          signo = 0;
+          FD_ZERO (&fdset);
-          pth_wait (ev);
-          if (pth_event_occurred (ev) && signo)
-            handle_signal (signo);
-          continue;
 	}
 
       /* Create a timeout event if needed. */
@@ -1828,7 +1833,7 @@ handle_connections (gnupg_fd_t listen_fd, gnupg_fd_t listen_fd_ssh)
          new thread.  Thus we need to block those signals. */
       pth_sigmask (SIG_BLOCK, &sigs, &oldsigs);
 
-      if (FD_ISSET (FD2INT (listen_fd), &read_fdset))
+      if (!shutdown_pending && FD_ISSET (FD2INT (listen_fd), &read_fdset))
 	{
           ctrl_t ctrl;
 
@@ -1865,7 +1870,7 @@ handle_connections (gnupg_fd_t listen_fd, gnupg_fd_t listen_fd_ssh)
           fd = GNUPG_INVALID_FD;
 	}
 
-      if (listen_fd_ssh != GNUPG_INVALID_FD 
+      if (!shutdown_pending && listen_fd_ssh != GNUPG_INVALID_FD 
           && FD_ISSET ( FD2INT (listen_fd_ssh), &read_fdset))
 	{
           ctrl_t ctrl;
@@ -1917,6 +1922,111 @@ handle_connections (gnupg_fd_t listen_fd, gnupg_fd_t listen_fd_ssh)
 }
 
 
+
+/* Helper for check_own_socket.  */
+static int
+check_own_socket_pid_cb (void *opaque, const void *buffer, size_t length)
+{
+  membuf_t *mb = opaque;
+  put_membuf (mb, buffer, length);
+  return 0;
+}
+
+
+/* The thread running the actual check.  We need to run this in a
+   separate thread so that check_own_thread can be called from the
+   timer tick.  */
+static void *
+check_own_socket_thread (void *arg)
+{
+  int rc;
+  char *sockname = arg;
+  assuan_context_t ctx;
+  membuf_t mb;
+  char *buffer;
+
+  check_own_socket_running++;
+
+  rc = assuan_socket_connect (&ctx, sockname, (pid_t)(-1));
+  xfree (sockname);
+  if (rc)
+    {
+      log_error ("can't connect my own socket: %s\n", gpg_strerror (rc));
+      goto leave;
+    }
+ 
+  init_membuf (&mb, 100);
+  rc = assuan_transact (ctx, "GETINFO pid", check_own_socket_pid_cb, &mb,
+                        NULL, NULL, NULL, NULL);
+  put_membuf (&mb, "", 1);
+  buffer = get_membuf (&mb, NULL);
+  if (rc || !buffer)
+    {
+      log_error ("sending command \"%s\" to my own socket failed: %s\n", 
+                 "GETINFO pid", gpg_strerror (rc));
+      rc = 1;
+    }
+  else if ( (pid_t)strtoul (buffer, NULL, 10) != getpid ())
+    {
+      log_error ("socket is now serviced by another server\n");
+      rc = 1;
+    }
+  else if (opt.verbose)
+    log_error ("socket is still served by this server\n");
+    
+  xfree (buffer);
+  assuan_disconnect (ctx);
+
+ leave:
+  if (rc)
+    {
+      /* We may not remove the socket as it is now in use by another
+         server.  Setting the name to empty does this.  */
+      if (socket_name)
+        *socket_name = 0;
+      if (socket_name_ssh)
+        *socket_name_ssh = 0;
+      shutdown_pending = 2;
+      log_info ("this process is useless - shutting down\n");
+    }
+  check_own_socket_running--;
+  return NULL;
+}
+
+
+/* Check whether we are still listening on our own socket.  In case
+   another gpg-agent process started after us has taken ownership of
+   our socket, we woul linger around without any real taks.  Thus we
+   better check once in a while whether we are really needed.  */
+static void
+check_own_socket (void)
+{
+  char *sockname;
+  pth_attr_t tattr;
+
+  if (!use_standard_socket)
+    return; /* This check makes only sense in standard socket mode.  */
+
+  if (check_own_socket_running || shutdown_pending)
+    return;  /* Still running or already shutting down.  */
+
+  sockname = make_filename (opt.homedir, "S.gpg-agent", NULL);
+  if (!sockname)
+    return; /* Out of memory.  */
+
+  tattr = pth_attr_new();
+  pth_attr_set (tattr, PTH_ATTR_JOINABLE, 0);
+  pth_attr_set (tattr, PTH_ATTR_STACK_SIZE, 256*1024);
+  pth_attr_set (tattr, PTH_ATTR_NAME, "check-owb-socket");
+
+  if (!pth_spawn (tattr, check_own_socket_thread, sockname))
+      log_error ("error spawning check_own_socket_thread: %s\n",
+                 strerror (errno) );
+  pth_attr_destroy (tattr);
+}
+
+
+
 /* Figure out whether an agent is available and running. Prints an
    error if not.  If SILENT is true, no messages are printed.  Usually
    started with MODE 0.  Returns 0 if the agent is running. */

jnlib/ChangeLog

@@ -1,5 +1,10 @@
 2008-10-29  Werner Koch  <wk@g10code.com>
 
+	* stringhelp.c (make_filename): Implement using macros. Factor some
+	code out to ..
+	(change_slashes): New.
+	(make_filename_try): New.
+
 	* w32-gettext.c (gettext): Return if no domain is loaded.
 	Reported by Tom Pegios.
 

jnlib/stringhelp.c

@@ -1,6 +1,6 @@
 /* stringhelp.c -  standard string helper functions
  * Copyright (C) 1998, 1999, 2000, 2001, 2003, 2004, 2005,
- *               2006, 2007  Free Software Foundation, Inc.
+ *               2006, 2007, 2008  Free Software Foundation, Inc.
  *
  * This file is part of JNLIB.
  *
@@ -34,6 +34,28 @@
 
 #define tohex_lower(n) ((n) < 10 ? ((n) + '0') : (((n) - 10) + 'a'))
 
+/* Sometimes we want to avoid mixing slashes and backslashes on W32
+   and prefer backslashes.  There is usual no problem with mixing
+   them, however a very few W32 API calls can't grok plain slashes.
+   Printing filenames with mixed slashes also looks a bit strange.
+   This function has no effext on POSIX. */
+static inline char *
+change_slashes (char *name)
+{
+  char *p;
+
+#ifdef HAVE_DRIVE_LETTERS
+  if (strchr (name, '\\'))
+    {
+      for (p=name; *p; p++)
+        if (*p == '/')
+          *p = '\\';
+    }
+#endif /*HAVE_DRIVE_LETTERS*/
+  return name;
+}
+
+
 /*
  * Look for the substring SUB in buffer and return a pointer to that
  * substring in BUFFER or NULL if not found.
@@ -290,56 +312,65 @@ make_dirname(const char *filepath)
 }
 
 
-
+
-/****************
+/* Implementation of make_filename and make_filename_try.  We need to
- * Construct a filename from the NULL terminated list of parts.
+   use macros here toa void the use of the soemtimes problematic
- * Tilde expansion is done here.
+   va_copy fucntion which is not available on all systems.  */
- */
+#define MAKE_FILENAME_PART1                        \
-char *
+  va_list arg_ptr;                                 \
-make_filename( const char *first_part, ... )
+  size_t n;                                        \
-{
+  const char *s;                                   \
-  va_list arg_ptr ;
+  char *name, *home, *p;                           \
-  size_t n;
+                                                   \
-  const char *s;
+  va_start (arg_ptr, first_part);                  \
-  char *name, *home, *p;
+  n = strlen (first_part) + 1;                     \
-  
+  while ( (s = va_arg (arg_ptr, const char *)) )   \
-  va_start (arg_ptr, first_part);
+    n += strlen(s) + 1;                            \
-  n = strlen (first_part) + 1;
+  va_end(arg_ptr);                                 \
-  while ( (s = va_arg (arg_ptr, const char *)) )
+                                                   \
-    n += strlen(s) + 1;
+  home = NULL;                                     \
-  va_end(arg_ptr);
+  if ( *first_part == '~' && first_part[1] == '/'  \
-  
+       && (home = getenv("HOME")) && *home )       \
-  home = NULL;
-  if ( *first_part == '~' && first_part[1] == '/'
-       && (home = getenv("HOME")) && *home )
     n += strlen (home);                            
   
+#define MAKE_FILENAME_PART2                         \
+  p = (home                                         \
+       ? stpcpy (stpcpy (name,home), first_part + 1)\
+       : stpcpy(name, first_part));                 \
+                                                    \
+  va_start (arg_ptr, first_part);                   \
+  while ( (s = va_arg(arg_ptr, const char *)) )     \
+    p = stpcpy (stpcpy (p,"/"), s);                 \
+  va_end(arg_ptr);                                  \
+  return change_slashes (name);
+
+
+/* Construct a filename from the NULL terminated list of parts.  Tilde
+   expansion is done here.  This function will never fail. */
+char *
+make_filename (const char *first_part, ... )
+{
+  MAKE_FILENAME_PART1
   name = jnlib_xmalloc (n);
-  p = (home 
+  MAKE_FILENAME_PART2
-       ? stpcpy (stpcpy (name,home), first_part + 1)
-       : stpcpy(name, first_part));
-
-  va_start (arg_ptr, first_part) ;
-  while ( (s = va_arg(arg_ptr, const char *)) )
-    p = stpcpy (stpcpy (p,"/"), s);
-  va_end(arg_ptr);
-
-#ifdef HAVE_DRIVE_LETTERS
-  /* We better avoid mixing slashes and backslashes and prefer
-     backslashes.  There is usual no problem with mixing them, however
-     a very few W32 API calls can't grok plain slashes.  Printing
-     filenames with mixed slashes also looks a bit strange. */
-  if (strchr (name, '\\'))
-    {
-      for (p=name; *p; p++)
-        if (*p == '/')
-          *p = '\\';
-    }
-#endif /*HAVE_DRIVE_LETTERS*/
-  return name;
 }
 
+/* Construct a filename from the NULL terminated list of parts.  Tilde
+   expansion is done here.  This function may return NULL on error. */
+char *
+make_filename_try (const char *first_part, ... )
+{
+  MAKE_FILENAME_PART1
+  name = jnlib_xmalloc (n);
+  if (!name)
+    return NULL;
+  MAKE_FILENAME_PART2
+}
+#undef MAKE_FILENAME_PART1
+#undef MAKE_FILENAME_PART2
 
+
+
 /* Compare whether the filenames are identical.  This is a
    special version of strcmp() taking the semantics of filenames in
    account.  Note that this function works only on the supplied names

jnlib/stringhelp.h

@@ -38,6 +38,7 @@ size_t length_sans_trailing_ws (const unsigned char *line, size_t len);
 char *make_basename(const char *filepath, const char *inputpath);
 char *make_dirname(const char *filepath);
 char *make_filename( const char *first_part, ... );
+char *make_filename_try (const char *first_part, ... );
 int compare_filenames( const char *a, const char *b );
 
 int hextobyte (const char *s);