diff options
Diffstat (limited to 'g10')
| -rw-r--r-- | g10/ChangeLog | 24 | ||||
| -rw-r--r-- | g10/g10.c | 34 | ||||
| -rw-r--r-- | g10/keydb.c | 2 | ||||
| -rw-r--r-- | g10/keyserver.c | 52 | ||||
| -rw-r--r-- | g10/main.h | 1 | ||||
| -rw-r--r-- | g10/misc.c | 59 | ||||
| -rw-r--r-- | g10/options.h | 2 | ||||
| -rw-r--r-- | g10/tdbio.c | 2 |
8 files changed, 154 insertions, 22 deletions
diff --git a/g10/ChangeLog b/g10/ChangeLog index 5ff54e138..94d34c13e 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,27 @@ +2001-12-19 David Shaw <[email protected]> + + * misc.c (check_permissions): New function to stat() and ensure + the permissions of GNUPGHOME and the files have safe permissions. + + * keydb.c (keydb_add_resource): Check keyring permissions. + + * tdbio.c (tdbio_set_dbname): Check permissions of trustdb.gpg + + * keyserver.c (keyserver_spawn): Disable keyserver schemes that + involve running external programs if the options file has unsafe + permissions or ownership. + + * g10.c, options.h: New option --no-permission-warning to disable + the permission warning message(s). This also permits use of the + keyserver if it had been disabled (see above). Also check the + permissions/ownership of random_seed. + + * keyserver.c (keyserver_spawn): The new glibc prints a warning + when using mktemp() (the code was already secure, but the warning + was bound to cause confusion). Use a different implementation + based on get_random_bits() instead. Also try a few times to get + the temp dir before giving up. + 2001-12-19 Werner Koch <[email protected]> * g10.c, passphrase.c [CYGWIN32]: Allow this as an alias for MINGW32. @@ -166,6 +166,7 @@ enum cmd_and_opt_values { aNull = 0, oNoVerbose, oTrustDBName, oNoSecmemWarn, + oNoPermissionWarn, oNoArmor, oNoDefKeyring, oNoGreeting, @@ -408,6 +409,7 @@ static ARGPARSE_OPTS opts[] = { { oNoVerbose, "no-verbose", 0, "@"}, { oTrustDBName, "trustdb-name", 2, "@" }, { oNoSecmemWarn, "no-secmem-warning", 0, "@" }, /* used only by regression tests */ + { oNoPermissionWarn, "no-permission-warning", 0, "@" }, { oNoArmor, "no-armor", 0, "@"}, { oNoArmor, "no-armour", 0, "@"}, { oNoDefKeyring, "no-default-keyring", 0, "@" }, @@ -682,6 +684,7 @@ main( int argc, char **argv ) char **orig_argv; const char *fname; char *username; + STRLIST unsafe_files=NULL; int may_coredump; STRLIST sl, remusr= NULL, locusr=NULL; STRLIST nrings=NULL, sec_nrings=NULL; @@ -815,6 +818,20 @@ main( int argc, char **argv ) pargs.flags= 1; /* do not remove the args */ next_pass: if( configname ) { + + if(check_permissions(configname,1)) + { + add_to_strlist(&unsafe_files,configname); + + /* If any options file is unsafe, then disable the keyserver + code. Since the keyserver code can call an external + program, and the external program to call is set in the + options file, a unsafe options file can lead to an + arbitrary program being run. */ + + opt.keyserver_disable=1; + } + configlineno = 0; configfp = fopen( configname, "r" ); if( !configfp ) { @@ -988,6 +1005,8 @@ main( int argc, char **argv ) case oAlwaysTrust: opt.always_trust = 1; break; case oLoadExtension: #ifndef __riscos__ + if(check_permissions(pargs.r.ret_str,1)) + add_to_strlist(&unsafe_files,pargs.r.ret_str); register_cipher_extension(orig_argc? *orig_argv:NULL, pargs.r.ret_str); #else /* __riscos__ */ @@ -1089,6 +1108,7 @@ main( int argc, char **argv ) case oCipherAlgo: def_cipher_string = m_strdup(pargs.r.ret_str); break; case oDigestAlgo: def_digest_string = m_strdup(pargs.r.ret_str); break; case oNoSecmemWarn: secmem_set_flags( secmem_get_flags() | 1 ); break; + case oNoPermissionWarn: opt.no_perm_warn=1; break; case oCharset: if( set_native_charset( pargs.r.ret_str ) ) log_error(_("%s is not a valid character set\n"), @@ -1162,6 +1182,7 @@ main( int argc, char **argv ) default : pargs.err = configfp? 1:2; break; } } + if( configfp ) { fclose( configfp ); configfp = NULL; @@ -1187,6 +1208,18 @@ main( int argc, char **argv ) } #endif + check_permissions(opt.homedir,0); + + if(unsafe_files) + { + STRLIST tmp; + + for(tmp=unsafe_files;tmp;tmp=tmp->next) + check_permissions(tmp->d,0); + + free_strlist(unsafe_files); + } + if( may_coredump && !opt.quiet ) log_info(_("WARNING: program may create a core file!\n")); @@ -1334,6 +1367,7 @@ 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); set_random_seed_file(p); m_free(p); } diff --git a/g10/keydb.c b/g10/keydb.c index 8fb6b115a..f4888bf3a 100644 --- a/g10/keydb.c +++ b/g10/keydb.c @@ -114,6 +114,8 @@ keydb_add_resource (const char *url, int force, int secret) else filename = m_strdup (resname); + check_permissions(filename,0); + if (!force) force = secret? !any_secret : !any_public; diff --git a/g10/keyserver.c b/g10/keyserver.c index 1f9cf2100..908f510b5 100644 --- a/g10/keyserver.c +++ b/g10/keyserver.c @@ -34,6 +34,7 @@ #include "options.h" #include "memory.h" #include "keydb.h" +#include "cipher.h" #include "status.h" #include "i18n.h" #include "util.h" @@ -122,7 +123,7 @@ parse_keyserver_uri(char *uri) opt.keyserver_port="0"; else { - unsigned char *ch; + char *ch; /* Get the port */ opt.keyserver_port=strsep(&uri,"/"); @@ -278,6 +279,14 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count) BUG (); #endif + if(opt.keyserver_disable && !opt.no_perm_warn) + { + log_info(_("keyserver scheme \"%s\" disabled due to unsafe " + "options file permissions\n"),opt.keyserver_scheme); + + return KEYSERVER_SCHEME_NOT_FOUND; + } + /* Build the filename for the helper to execute */ filename=m_alloc(strlen("gpgkeys_")+strlen(opt.keyserver_scheme)+1); @@ -287,31 +296,44 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count) if(opt.keyserver_options.use_temp_files) { + int attempts; const char *tmp=get_temp_dir(); + byte *randombits; - tempdir=m_alloc(strlen(tmp)+1+8+11+1); - sprintf(tempdir,"%s" DIRSEP_S "gpg-XXXXXX",tmp); + tempdir=m_alloc(strlen(tmp)+1+12+1); - /* Yes, I'm using mktemp. No, this isn't automatically insecure - because of it. I am using it to make a temp dir, not a file, - and I happily fail if it already exists. */ + /* Try 4 times to make the temp directory */ + for(attempts=0;attempts<4;attempts++) + { + /* Using really random bits is probably overkill here. The + worst thing that can happen with a directory name collision + is that the user will get an error message. */ + randombits=get_random_bits(8*4,0,0); - mktemp(tempdir); + sprintf(tempdir,"%s" DIRSEP_S "gpg-%02X%02X%02X%02X",tmp, + randombits[0],randombits[1],randombits[2],randombits[3]); - tempfile_in=m_alloc(strlen(tempdir)+1+10+1); - sprintf(tempfile_in,"%s" DIRSEP_S "ksrvin" EXTSEP_S "txt",tempdir); + m_free(randombits); - tempfile_out=m_alloc(strlen(tempdir)+1+11+1); - sprintf(tempfile_out,"%s" DIRSEP_S "ksrvout" EXTSEP_S "txt",tempdir); + if(mkdir(tempdir,0700)==0) + { + madedir=1; + break; + } + } - if(mkdir(tempdir,0700)==-1) + if(!madedir) { - log_error(_("%s: can't create directory: %s\n"), - tempdir,strerror(errno)); + log_error(_("%s: can't create temp directory after %d tries: %s\n"), + tempdir,attempts,strerror(errno)); goto fail; } - madedir=1; + tempfile_in=m_alloc(strlen(tempdir)+1+10+1); + sprintf(tempfile_in,"%s" DIRSEP_S "ksrvin" EXTSEP_S "txt",tempdir); + + tempfile_out=m_alloc(strlen(tempdir)+1+11+1); + sprintf(tempfile_out,"%s" DIRSEP_S "ksrvout" EXTSEP_S "txt",tempdir); tochild=fopen(tempfile_in,"w"); if(tochild==NULL) diff --git a/g10/main.h b/g10/main.h index 32ed0b3e8..4a94bdeb9 100644 --- a/g10/main.h +++ b/g10/main.h @@ -67,6 +67,7 @@ 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 checkonly); /*-- helptext.c --*/ void display_online_help( const char *keyword ); diff --git a/g10/misc.c b/g10/misc.c index e75c712da..d1ff14c4b 100644 --- a/g10/misc.c +++ b/g10/misc.c @@ -24,6 +24,9 @@ #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> @@ -327,8 +330,6 @@ openpgp_pk_algo_usage ( int algo ) return use; } - - int openpgp_md_test_algo( int algo ) { @@ -337,16 +338,60 @@ openpgp_md_test_algo( int algo ) return check_digest_algo(algo); } +int +check_permissions(const char *path,int checkonly) +{ +#ifdef HAVE_STAT + struct stat statbuf; + int isdir=0; + if(opt.no_perm_warn) + return 0; + /* It's okay if the file doesn't exist */ + if(stat(path,&statbuf)!=0) + return 0; + isdir=S_ISDIR(statbuf.st_mode); + /* The user doesn't own the file */ + if(statbuf.st_uid != getuid()) + { + if(!checkonly) + log_info(_("Warning: unsafe ownership on %s \"%s\"\n"), + isdir?"directory":"file",path); + return 1; + } + /* 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. */ + if((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(path); + 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); + return 0; + } + m_free(dir); + if(!checkonly) + log_info(_("Warning: unsafe permissions on %s \"%s\"\n"), + isdir?"directory":"file",path); + return 1; + } +#endif - - - - - + return 0; +} diff --git a/g10/options.h b/g10/options.h index 4f4eca4bf..6b54a8708 100644 --- a/g10/options.h +++ b/g10/options.h @@ -104,6 +104,8 @@ struct { int keep_temp_files:1; STRLIST other; } keyserver_options; + int keyserver_disable; + int no_perm_warn; char *temp_dir; int no_encrypt_to; int interactive; diff --git a/g10/tdbio.c b/g10/tdbio.c index 50064cf59..0c8f84246 100644 --- a/g10/tdbio.c +++ b/g10/tdbio.c @@ -444,6 +444,8 @@ tdbio_set_dbname( const char *new_dbname, int create ) : make_filename(opt.homedir, "trustdb" EXTSEP_S "gpg", NULL ); + check_permissions(fname,0); + if( access( fname, R_OK ) ) { if( errno != ENOENT ) { log_error( _("%s: can't access: %s\n"), fname, strerror(errno) ); |