* gpgv.c, keydb.c (keydb_add_resource): Factored keyring creation out to
authorDavid Shaw <dshaw@jabberwocky.com>
Thu, 30 Sep 2004 15:00:58 +0000 (15:00 +0000)
committerDavid Shaw <dshaw@jabberwocky.com>
Thu, 30 Sep 2004 15:00:58 +0000 (15:00 +0000)
.. (maybe_create_keyring): .. new.  Make sure that we do the checks in a
locked state.  Problem reported by Stefan Haller.  Try to create the home
directory before acquiring a lock for the keyring. From Werner on stable
branch.

* g10.c (main): Blow up if we didn't lose setuid.  From Werner on stable
branch.

g10/ChangeLog
g10/g10.c
g10/gpgv.c
g10/keydb.c

index 0ab869c..bea26b8 100644 (file)
@@ -1,3 +1,15 @@
+2004-09-30  David Shaw  <dshaw@jabberwocky.com>
+
+       * gpgv.c, keydb.c (keydb_add_resource): Factored keyring creation
+       out to ..
+       (maybe_create_keyring): .. new.  Make sure that we do the checks
+       in a locked state.  Problem reported by Stefan Haller.  Try to
+       create the home directory before acquiring a lock for the keyring.
+       From Werner on stable branch.
+
+       * g10.c (main): Blow up if we didn't lose setuid.  From Werner on
+       stable branch.
+
 2004-09-29  David Shaw  <dshaw@jabberwocky.com>
 
        * keyedit.c, keylist.c, keyserver.c, mainproc.c: Reduce the many
index 66a1065..85c27d4 100644 (file)
--- a/g10/g10.c
+++ b/g10/g10.c
@@ -1685,6 +1685,13 @@ main( int argc, char **argv )
     maybe_setuid = 0;
     /* Okay, we are now working under our real uid */
 
+#if defined(HAVE_GETUID) && defined(HAVE_GETEUID)
+    /* There should be no way to get to this spot while still carrying
+       setuid privs.  Just in case, bomb out if we are. */
+    if(getuid()!=geteuid())
+      BUG();
+#endif
+
     set_native_charset (NULL); /* Try to auto set the character set */
 
     /* Try for a version specific config file first */
index a2d5ad3..ee340ce 100644 (file)
@@ -397,6 +397,7 @@ int tty_no_terminal(int onoff) {return 0;}
 /* We do not do any locking, so use these stubs here */
 void disable_dotlock(void) {}
 DOTLOCK create_dotlock( const char *file_to_lock ) { return NULL; }
+void destroy_dotlock (DOTLOCK h) {}
 int make_dotlock( DOTLOCK h, long timeout ) { return 0;}
 int release_dotlock( DOTLOCK h ) {return 0;}
 void remove_lockfiles(void) {}
index ab0c146..953bc58 100644 (file)
@@ -1,5 +1,5 @@
 /* keydb.c - key database dispatcher
- * Copyright (C) 2001, 2002, 2003 Free Software Foundation, Inc.
+ * Copyright (C) 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
  *
  * This file is part of GnuPG.
  *
@@ -70,6 +70,118 @@ static int lock_all (KEYDB_HANDLE hd);
 static void unlock_all (KEYDB_HANDLE hd);
 
 
+/* Handle the creation of a keyring if it does not yet exist.  Take
+   into acount that other processes might have the keyring alread
+   locked.  This lock check does not work if the directory itself is
+   not yet available. */
+static int
+maybe_create_keyring (char *filename, int force)
+{
+  DOTLOCK lockhd = NULL;
+  IOBUF iobuf;
+  int rc;
+  mode_t oldmask;
+  char *last_slash_in_filename;
+
+  /* A quick test whether the filename already exists. */
+  if (!access (filename, F_OK))
+    return 0;
+
+  /* If we don't want to create a new file at all, there is no need to
+     go any further - bail out right here.  */
+  if (!force) 
+    return G10ERR_OPEN_FILE;
+
+  /* First of all we try to create the home directory.  Note, that we
+     don't do any locking here because any sane application of gpg
+     would create the home directory by itself and not rely on gpg's
+     tricky auto-creation which is anyway only done for some home
+     directory name patterns. */
+  last_slash_in_filename = strrchr (filename, DIRSEP_C);
+  *last_slash_in_filename = 0;
+  if (access(filename, F_OK))
+    { 
+      static int tried;
+      
+      if (!tried)
+        {
+          tried = 1;
+          try_make_homedir (filename);
+        }
+      if (access (filename, F_OK))
+        {
+          rc = G10ERR_OPEN_FILE;
+          *last_slash_in_filename = DIRSEP_C;
+          goto leave;
+        }
+    }
+  *last_slash_in_filename = DIRSEP_C;
+
+
+  /* To avoid races with other instances of gpg trying to create or
+     update the keyring (it is removed during an update for a short
+     time), we do the next stuff in a locked state. */
+  lockhd = create_dotlock (filename);
+  if (!lockhd)
+    {
+      /* A reason for this to fail is that the directory is not
+         writable. However, this whole locking stuff does not make
+         sense if this is the case. An empty non-writable directory
+         with no keyring is not really useful at all. */
+      if (opt.verbose)
+        log_info ("can't allocate lock for `%s'\n", filename );
+
+      if (!force) 
+        return G10ERR_OPEN_FILE; 
+      else
+        return G10ERR_GENERAL;
+    }
+
+  if ( make_dotlock (lockhd, -1) )
+    {
+      /* This is something bad.  Probably a stale lockfile.  */
+      log_info ("can't lock `%s'\n", filename );
+      rc = G10ERR_GENERAL;
+      goto leave;
+    }
+
+  /* Now the real test while we are locked. */
+  if (!access(filename, F_OK))
+    {
+      rc = 0;  /* Okay, we may access the file now.  */
+      goto leave;
+    }
+
+  /* The file does not yet exist, create it now. */
+  oldmask = umask (077);
+  iobuf = iobuf_create (filename);
+  umask (oldmask);
+  if (!iobuf) 
+    {
+      log_error ( _("error creating keyring `%s': %s\n"),
+                  filename, strerror(errno));
+      rc = G10ERR_OPEN_FILE;
+      goto leave;
+    }
+
+  if (!opt.quiet)
+    log_info (_("keyring `%s' created\n"), filename);
+
+  iobuf_close (iobuf);
+  /* Must invalidate that ugly cache */
+  iobuf_ioctl (NULL, 2, 0, filename);
+  rc = 0;
+
+ leave:
+  if (lockhd)
+    {
+      release_dotlock (lockhd);
+      destroy_dotlock (lockhd);
+    }
+  return rc;
+}
+
+
 /*
  * Register a resource (which currently may only be a keyring file).
  * The first keyring which is added by this function is
@@ -84,7 +196,6 @@ keydb_add_resource (const char *url, int flags, int secret)
 {
     static int any_secret, any_public;
     const char *resname = url;
-    IOBUF iobuf = NULL;
     char *filename = NULL;
     int force=(flags&1);
     int rc = 0;
@@ -149,56 +260,9 @@ keydb_add_resource (const char *url, int flags, int secret)
        goto leave;
 
       case KEYDB_RESOURCE_TYPE_KEYRING:
-        if (access(filename, F_OK))
-          { /* file does not exist */
-           mode_t oldmask;
-           char *last_slash_in_filename;
-
-            if (!force) 
-              {
-                rc = G10ERR_OPEN_FILE;
-                goto leave;
-              }
-
-           last_slash_in_filename = strrchr (filename, DIRSEP_C);
-           *last_slash_in_filename = 0;
-           if (access(filename, F_OK))
-              { /* On the first time we try to create the default
-                  homedir and check again. */
-                static int tried;
-                
-                if (!tried)
-                  {
-                    tried = 1;
-                    try_make_homedir (filename);
-                  }
-               if (access (filename, F_OK))
-                  {
-                    rc = G10ERR_OPEN_FILE;
-                    *last_slash_in_filename = DIRSEP_C;
-                    goto leave;
-                  }
-              }
-           *last_slash_in_filename = DIRSEP_C;
-
-           oldmask=umask(077);
-           iobuf = iobuf_create (filename);
-           umask(oldmask);
-           if (!iobuf) 
-              {
-               log_error ( _("error creating keyring `%s': %s\n"),
-                            filename, strerror(errno));
-               rc = G10ERR_OPEN_FILE;
-               goto leave;
-              }
-
-            if (!opt.quiet)
-              log_info (_("keyring `%s' created\n"), filename);
-            iobuf_close (iobuf);
-            iobuf = NULL;
-            /* must invalidate that ugly cache */
-            iobuf_ioctl (NULL, 2, 0, (char*)filename);
-          } /* end file creation */
+        rc = maybe_create_keyring (filename, force);
+        if (rc)
+          goto leave;
 
         if(keyring_register_filename (filename, secret, &token))
          {