diff options
| -rw-r--r-- | g10/ChangeLog | 16 | ||||
| -rw-r--r-- | g10/g10.c | 224 | ||||
| -rw-r--r-- | g10/keydb.c | 2 | ||||
| -rw-r--r-- | g10/keyedit.c | 5 | ||||
| -rw-r--r-- | g10/main.h | 1 | ||||
| -rw-r--r-- | g10/misc.c | 97 | ||||
| -rw-r--r-- | g10/tdbio.c | 2 |
7 files changed, 212 insertions, 135 deletions
diff --git a/g10/ChangeLog b/g10/ChangeLog index d9da39f43..3ab1403f9 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,19 @@ +2002-08-07 David Shaw <[email protected]> + + * keyedit.c (menu_revsig): Properly show a uid is revoked without + restarting gpg. This is Debian bug 124219, though their supplied + patch will not do the right thing. + + * main.h, tdbio.c (tdbio_set_dbname), misc.c (removed + check_permissions), keydb.c (keydb_add_resource), g10.c (main, + check_permissions): Significant reworking of the permission check + mechanism. The new behavior is to check everything in the homedir + by checking the homedir itself. If the user wants to put + (possibly shared) keyrings outside the homedir, they are not + checked. The options file and any extension files are checked + wherever they are, as well as their enclosing directories. This + is Debian bug 147760. + 2002-08-06 Stefan Bellon <[email protected]> * g10.c (main): Use of EXTSEP_S in new gpg.conf string. @@ -28,6 +28,9 @@ #ifdef HAVE_DOSISH_SYSTEM #include <fcntl.h> /* for setmode() */ #endif +#ifdef HAVE_STAT +#include <sys/stat.h> /* for stat() */ +#endif #define INCLUDED_BY_MAIN_MODULE 1 #include "packet.h" @@ -832,6 +835,180 @@ static void add_group(char *string) opt.grouplist=item; } +/* We need to check three things. + + 0) The homedir. It must be x00, a directory, and owned by the + user. + + 1) The options file. Okay unless it or its containing directory is + group or other writable or not owned by us. disable exec in this + case. + + 2) Extensions. Same as #2. + + Returns true if the item is unsafe. */ +static int +check_permissions(const char *path,int item) +{ +#if defined(HAVE_STAT) && !defined(HAVE_DOSISH_SYSTEM) + static int homedir_cache=-1; + char *tmppath,*isa,*dir; + struct stat statbuf,dirbuf; + int homedir=0,ret=0,checkonly=0; + int perm=0,own=0,enc_dir_perm=0,enc_dir_own=0; + + if(opt.no_perm_warn) + return 0; + + /* extensions may attach a path */ + if(item==2 && path[0]!=DIRSEP_C) + { + if(strchr(path,DIRSEP_C)) + tmppath=make_filename(path,NULL); + else + tmppath=make_filename(GNUPG_LIBDIR,path,NULL); + } + else + tmppath=m_strdup(path); + + /* If the item is located in the homedir, but isn't the homedir, + don't continue if we already checked the homedir itself. This is + to avoid user confusion with an extra options file warning which + could be rectified if the homedir itself had proper + permissions. */ + if(item!=0 && homedir_cache>-1 && + ascii_memcasecmp(opt.homedir,tmppath,strlen(opt.homedir))==0) + { + ret=homedir_cache; + goto end; + } + + /* It's okay if the file or directory doesn't exist */ + if(stat(tmppath,&statbuf)!=0) + { + ret=0; + goto end; + } + + /* Now check the enclosing directory. Theoretically, we could walk + this test up to the root directory /, but for the sake of sanity, + I'm stopping at one level down. */ + dir=make_dirname(tmppath); + + if(stat(dir,&dirbuf)!=0 || !S_ISDIR(dirbuf.st_mode)) + { + /* Weird error */ + ret=1; + goto end; + } + + m_free(dir); + + /* Assume failure */ + ret=1; + + if(item==0) + { + isa="homedir"; + + /* The homedir must be x00, a directory, and owned by the user. */ + + if(S_ISDIR(statbuf.st_mode)) + { + if(statbuf.st_uid==getuid()) + { + if((statbuf.st_mode & (S_IRWXG|S_IRWXO))==0) + ret=0; + else + perm=1; + } + else + own=1; + + homedir_cache=ret; + } + } + else if(item==1 || item==2) + { + if(item==1) + isa="configuration file"; + else + isa="extension"; + + /* The options or extension file. Okay unless it or its + containing directory is group or other writable or not owned + by us or root. */ + + if(S_ISREG(statbuf.st_mode)) + { + if(statbuf.st_uid==getuid() || statbuf.st_uid==0) + { + if((statbuf.st_mode & (S_IWGRP|S_IWOTH))==0) + { + /* it's not writable, so make sure the enclosing + directory is also not writable */ + if(dirbuf.st_uid==getuid() || dirbuf.st_uid==0) + { + if((dirbuf.st_mode & (S_IWGRP|S_IWOTH))==0) + ret=0; + else + enc_dir_perm=1; + } + else + enc_dir_own=1; + } + else + { + /* it's writable, so the enclosing directory had + better not let people get to it. */ + if(dirbuf.st_uid==getuid() || dirbuf.st_uid==0) + { + if((dirbuf.st_mode & (S_IRWXG|S_IRWXO))==0) + ret=0; + else + perm=enc_dir_perm=1; /* unclear which one to fix! */ + } + else + enc_dir_own=1; + } + } + else + own=1; + } + } + else + BUG(); + + if(!checkonly) + { + if(own) + log_info(_("WARNING: unsafe ownership on %s \"%s\"\n"), + isa,tmppath); + if(perm) + log_info(_("WARNING: unsafe permissions on %s \"%s\"\n"), + isa,tmppath); + if(enc_dir_own) + log_info(_("WARNING: unsafe enclosing directory " + "ownership on %s \"%s\"\n"), + isa,tmppath); + if(enc_dir_perm) + log_info(_("WARNING: unsafe enclosing directory " + "permissions on %s \"%s\"\n"), + isa,tmppath); + } + + end: + m_free(tmppath); + + if(homedir) + homedir_cache=ret; + + return ret; + +#endif /* HAVE_STAT && !HAVE_DOSISH_SYSTEM */ + + return 0; +} int main( int argc, char **argv ) @@ -843,8 +1020,6 @@ main( int argc, char **argv ) char **orig_argv; const char *fname; char *username; - STRLIST unsafe_files=NULL; - STRLIST extensions=NULL; int may_coredump; STRLIST sl, remusr= NULL, locusr=NULL; STRLIST nrings=NULL, sec_nrings=NULL; @@ -945,6 +1120,8 @@ main( int argc, char **argv ) default_config = 0; /* --no-options */ else if( pargs.r_opt == oHomedir ) opt.homedir = pargs.r.ret_str; + else if( pargs.r_opt == oNoPermissionWarn ) + opt.no_perm_warn=1; #ifdef USE_SHM_COPROCESSING else if( pargs.r_opt == oRunAsShmCP ) { /* does not make sense in a options file, we do it here, @@ -1001,13 +1178,14 @@ main( int argc, char **argv ) pargs.argc = &argc; pargs.argv = &argv; pargs.flags= 1; /* do not remove the args */ + + /* By this point we have a homedir, and cannot change it. */ + check_permissions(opt.homedir,0); + next_pass: if( configname ) { - - if(check_permissions(configname,0,1)) + if(check_permissions(configname,1)) { - add_to_strlist(&unsafe_files,configname); - /* If any options file is unsafe, then disable any external programs for keyserver calls or photo IDs. Since the external program to call is set in the options file, a @@ -1206,9 +1384,14 @@ main( int argc, char **argv ) case oAlwaysTrust: opt.always_trust = 1; break; case oLoadExtension: #ifndef __riscos__ - add_to_strlist(&extensions,pargs.r.ret_str); - register_cipher_extension(orig_argc? *orig_argv:NULL, - pargs.r.ret_str); +#ifdef USE_DYNAMIC_LINKING + if(check_permissions(pargs.r.ret_str,2)) + log_info(_("cipher extension \"%s\" not loaded due to " + "unsafe permissions\n"),pargs.r.ret_str); + else + register_cipher_extension(orig_argc? *orig_argv:NULL, + pargs.r.ret_str); +#endif #else /* __riscos__ */ not_implemented("load-extension"); #endif /* __riscos__ */ @@ -1496,28 +1679,6 @@ main( int argc, char **argv ) } #endif - check_permissions(opt.homedir,0,0); - - if(unsafe_files) - { - STRLIST tmp; - - for(tmp=unsafe_files;tmp;tmp=tmp->next) - check_permissions(tmp->d,0,0); - - free_strlist(unsafe_files); - } - - if(extensions) - { - STRLIST tmp; - - for(tmp=extensions;tmp;tmp=tmp->next) - check_permissions(tmp->d,1,0); - - free_strlist(extensions); - } - if( may_coredump && !opt.quiet ) log_info(_("WARNING: program may create a core file!\n")); @@ -1723,7 +1884,6 @@ main( int argc, char **argv ) /* set the random seed file */ if( use_random_seed ) { char *p = make_filename(opt.homedir, "random_seed", NULL ); - check_permissions(p,0,0); set_random_seed_file(p); m_free(p); } diff --git a/g10/keydb.c b/g10/keydb.c index 96d19105a..eb94ef363 100644 --- a/g10/keydb.c +++ b/g10/keydb.c @@ -114,8 +114,6 @@ keydb_add_resource (const char *url, int force, int secret) else filename = m_strdup (resname); - check_permissions(filename,0,0); - if (!force) force = secret? !any_secret : !any_public; diff --git a/g10/keyedit.c b/g10/keyedit.c index 1da939dfd..d9c3df09f 100644 --- a/g10/keyedit.c +++ b/g10/keyedit.c @@ -2996,7 +2996,10 @@ menu_revsig( KBNODE keyblock ) } changed = 1; /* we changed the keyblock */ update_trust = 1; - + /* Are we revoking our own uid? */ + if(primary_pk->keyid[0]==sig->keyid[0] && + primary_pk->keyid[1]==sig->keyid[1]) + unode->pkt->pkt.user_id->is_revoked=1; pkt = m_alloc_clear( sizeof *pkt ); pkt->pkttype = PKT_SIGNATURE; pkt->pkt.signature = sig; diff --git a/g10/main.h b/g10/main.h index 9edccaf21..09d4a93d8 100644 --- a/g10/main.h +++ b/g10/main.h @@ -71,7 +71,6 @@ int openpgp_cipher_test_algo( int algo ); int openpgp_pk_test_algo( int algo, unsigned int usage_flags ); int openpgp_pk_algo_usage ( int algo ); int openpgp_md_test_algo( int algo ); -int check_permissions(const char *path,int extension,int checkonly); void idea_cipher_warn( int show ); struct expando_args diff --git a/g10/misc.c b/g10/misc.c index 99c6076c5..b0e9543ab 100644 --- a/g10/misc.c +++ b/g10/misc.c @@ -24,9 +24,6 @@ #include <string.h> #include <unistd.h> #include <errno.h> -#ifdef HAVE_STAT -#include <sys/stat.h> -#endif #if defined(__linux__) && defined(__alpha__) && __GLIBC__ < 2 #include <asm/sysinfo.h> #include <asm/unistd.h> @@ -338,100 +335,6 @@ openpgp_md_test_algo( int algo ) return check_digest_algo(algo); } -int -check_permissions(const char *path,int extension,int checkonly) -{ -#if defined(HAVE_STAT) && !defined(HAVE_DOSISH_SYSTEM) - char *tmppath; - struct stat statbuf; - int ret=1; - int isdir=0; - - if(opt.no_perm_warn) - return 0; - - if(extension && path[0]!=DIRSEP_C) - { - if(strchr(path,DIRSEP_C)) - tmppath=make_filename(path,NULL); - else - tmppath=make_filename(GNUPG_LIBDIR,path,NULL); - } - else - tmppath=m_strdup(path); - - /* It's okay if the file doesn't exist */ - if(stat(tmppath,&statbuf)!=0) - { - ret=0; - goto end; - } - - isdir=S_ISDIR(statbuf.st_mode); - - /* We may have to revisit this if we start piping keyrings to gpg - over a named pipe or keyserver character device :) */ - if(!isdir && !S_ISREG(statbuf.st_mode)) - { - ret=0; - goto end; - } - - /* Per-user files must be owned by the user. Extensions must be - owned by the user or root. */ - if((!extension && statbuf.st_uid != getuid()) || - (extension && statbuf.st_uid!=0 && statbuf.st_uid!=getuid())) - { - if(!checkonly) - log_info(_("WARNING: unsafe ownership on %s \"%s\"\n"), - isdir?"directory":extension?"extension":"file",path); - goto end; - } - - /* This works for both directories and files - basically, we don't - care what the owner permissions are, so long as the group and - other permissions are 0 for per-user files, and non-writable for - extensions. */ - if((extension && (statbuf.st_mode & (S_IWGRP|S_IWOTH)) !=0) || - (!extension && (statbuf.st_mode & (S_IRWXG|S_IRWXO)) != 0)) - { - char *dir; - - /* However, if the directory the directory/file is in is owned - by the user and is 700, then this is not a problem. - Theoretically, we could walk this test up to the root - directory /, but for the sake of sanity, I'm stopping at one - level down. */ - - dir=make_dirname(tmppath); - if(stat(dir,&statbuf)==0 && statbuf.st_uid==getuid() && - S_ISDIR(statbuf.st_mode) && (statbuf.st_mode & (S_IRWXG|S_IRWXO))==0) - { - m_free(dir); - ret=0; - goto end; - } - - m_free(dir); - - if(!checkonly) - log_info(_("WARNING: unsafe permissions on %s \"%s\"\n"), - isdir?"directory":extension?"extension":"file",path); - goto end; - } - - ret=0; - - end: - m_free(tmppath); - - return ret; - -#endif /* HAVE_STAT && !HAVE_DOSISH_SYSTEM */ - - return 0; -} - /* Special warning for the IDEA cipher */ void idea_cipher_warn(int show) diff --git a/g10/tdbio.c b/g10/tdbio.c index 537e4c0d4..20b32afac 100644 --- a/g10/tdbio.c +++ b/g10/tdbio.c @@ -447,8 +447,6 @@ tdbio_set_dbname( const char *new_dbname, int create ) : make_filename(opt.homedir, "trustdb" EXTSEP_S "gpg", NULL ); - check_permissions(fname,0,0); - if( access( fname, R_OK ) ) { if( errno != ENOENT ) { log_error( _("%s: can't access: %s\n"), fname, strerror(errno) ); |