From 85f975596887930f8488f9008367d052b9aa660d Mon Sep 17 00:00:00 2001 From: David Shaw Date: Thu, 30 Sep 2004 15:00:58 +0000 Subject: [PATCH] * 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. --- g10/ChangeLog | 12 ++++ g10/g10.c | 7 +++ g10/gpgv.c | 1 + g10/keydb.c | 168 ++++++++++++++++++++++++++++++++++---------------- 4 files changed, 136 insertions(+), 52 deletions(-) diff --git a/g10/ChangeLog b/g10/ChangeLog index 0ab869cf1..bea26b8a6 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,15 @@ +2004-09-30 David Shaw + + * 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 * keyedit.c, keylist.c, keyserver.c, mainproc.c: Reduce the many diff --git a/g10/g10.c b/g10/g10.c index 66a106526..85c27d424 100644 --- 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 */ diff --git a/g10/gpgv.c b/g10/gpgv.c index a2d5ad391..ee340cefd 100644 --- a/g10/gpgv.c +++ b/g10/gpgv.c @@ -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) {} diff --git a/g10/keydb.c b/g10/keydb.c index ab0c1463e..953bc5853 100644 --- a/g10/keydb.c +++ b/g10/keydb.c @@ -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)) {