1
0
mirror of git://git.gnupg.org/gnupg.git synced 2024-12-23 10:29:58 +01:00

New function to check the permissions of GNUPGHOME and the various files

that live there for safe permission/ownership (--no-permission-warning to
disable)
The newer glibcs print scary warnings about using mktemp().  The use here
was actually safe, but the warning was bound to confuse people, so here is
an arguably better tempname creator that pulls random bits from the pool.
This commit is contained in:
David Shaw 2001-12-20 05:02:30 +00:00
parent 4d6bda78c4
commit d5a695f198
8 changed files with 153 additions and 21 deletions

View File

@ -1,3 +1,27 @@
2001-12-19 David Shaw <dshaw@jabberwocky.com>
* 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 <wk@gnupg.org> 2001-12-19 Werner Koch <wk@gnupg.org>
* g10.c, passphrase.c [CYGWIN32]: Allow this as an alias for MINGW32. * g10.c, passphrase.c [CYGWIN32]: Allow this as an alias for MINGW32.

View File

@ -166,6 +166,7 @@ enum cmd_and_opt_values { aNull = 0,
oNoVerbose, oNoVerbose,
oTrustDBName, oTrustDBName,
oNoSecmemWarn, oNoSecmemWarn,
oNoPermissionWarn,
oNoArmor, oNoArmor,
oNoDefKeyring, oNoDefKeyring,
oNoGreeting, oNoGreeting,
@ -408,6 +409,7 @@ static ARGPARSE_OPTS opts[] = {
{ oNoVerbose, "no-verbose", 0, "@"}, { oNoVerbose, "no-verbose", 0, "@"},
{ oTrustDBName, "trustdb-name", 2, "@" }, { oTrustDBName, "trustdb-name", 2, "@" },
{ oNoSecmemWarn, "no-secmem-warning", 0, "@" }, /* used only by regression tests */ { oNoSecmemWarn, "no-secmem-warning", 0, "@" }, /* used only by regression tests */
{ oNoPermissionWarn, "no-permission-warning", 0, "@" },
{ oNoArmor, "no-armor", 0, "@"}, { oNoArmor, "no-armor", 0, "@"},
{ oNoArmor, "no-armour", 0, "@"}, { oNoArmor, "no-armour", 0, "@"},
{ oNoDefKeyring, "no-default-keyring", 0, "@" }, { oNoDefKeyring, "no-default-keyring", 0, "@" },
@ -682,6 +684,7 @@ main( int argc, char **argv )
char **orig_argv; char **orig_argv;
const char *fname; const char *fname;
char *username; char *username;
STRLIST unsafe_files=NULL;
int may_coredump; int may_coredump;
STRLIST sl, remusr= NULL, locusr=NULL; STRLIST sl, remusr= NULL, locusr=NULL;
STRLIST nrings=NULL, sec_nrings=NULL; STRLIST nrings=NULL, sec_nrings=NULL;
@ -815,6 +818,20 @@ main( int argc, char **argv )
pargs.flags= 1; /* do not remove the args */ pargs.flags= 1; /* do not remove the args */
next_pass: next_pass:
if( configname ) { 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; configlineno = 0;
configfp = fopen( configname, "r" ); configfp = fopen( configname, "r" );
if( !configfp ) { if( !configfp ) {
@ -988,6 +1005,8 @@ main( int argc, char **argv )
case oAlwaysTrust: opt.always_trust = 1; break; case oAlwaysTrust: opt.always_trust = 1; break;
case oLoadExtension: case oLoadExtension:
#ifndef __riscos__ #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, register_cipher_extension(orig_argc? *orig_argv:NULL,
pargs.r.ret_str); pargs.r.ret_str);
#else /* __riscos__ */ #else /* __riscos__ */
@ -1089,6 +1108,7 @@ main( int argc, char **argv )
case oCipherAlgo: def_cipher_string = m_strdup(pargs.r.ret_str); break; 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 oDigestAlgo: def_digest_string = m_strdup(pargs.r.ret_str); break;
case oNoSecmemWarn: secmem_set_flags( secmem_get_flags() | 1 ); break; case oNoSecmemWarn: secmem_set_flags( secmem_get_flags() | 1 ); break;
case oNoPermissionWarn: opt.no_perm_warn=1; break;
case oCharset: case oCharset:
if( set_native_charset( pargs.r.ret_str ) ) if( set_native_charset( pargs.r.ret_str ) )
log_error(_("%s is not a valid character set\n"), 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; default : pargs.err = configfp? 1:2; break;
} }
} }
if( configfp ) { if( configfp ) {
fclose( configfp ); fclose( configfp );
configfp = NULL; configfp = NULL;
@ -1187,6 +1208,18 @@ main( int argc, char **argv )
} }
#endif #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 ) if( may_coredump && !opt.quiet )
log_info(_("WARNING: program may create a core file!\n")); log_info(_("WARNING: program may create a core file!\n"));
@ -1334,6 +1367,7 @@ main( int argc, char **argv )
/* set the random seed file */ /* set the random seed file */
if( use_random_seed ) { if( use_random_seed ) {
char *p = make_filename(opt.homedir, "random_seed", NULL ); char *p = make_filename(opt.homedir, "random_seed", NULL );
check_permissions(p,0);
set_random_seed_file(p); set_random_seed_file(p);
m_free(p); m_free(p);
} }

View File

@ -114,6 +114,8 @@ keydb_add_resource (const char *url, int force, int secret)
else else
filename = m_strdup (resname); filename = m_strdup (resname);
check_permissions(filename,0);
if (!force) if (!force)
force = secret? !any_secret : !any_public; force = secret? !any_secret : !any_public;

View File

@ -34,6 +34,7 @@
#include "options.h" #include "options.h"
#include "memory.h" #include "memory.h"
#include "keydb.h" #include "keydb.h"
#include "cipher.h"
#include "status.h" #include "status.h"
#include "i18n.h" #include "i18n.h"
#include "util.h" #include "util.h"
@ -122,7 +123,7 @@ parse_keyserver_uri(char *uri)
opt.keyserver_port="0"; opt.keyserver_port="0";
else else
{ {
unsigned char *ch; char *ch;
/* Get the port */ /* Get the port */
opt.keyserver_port=strsep(&uri,"/"); opt.keyserver_port=strsep(&uri,"/");
@ -278,6 +279,14 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count)
BUG (); BUG ();
#endif #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 */ /* Build the filename for the helper to execute */
filename=m_alloc(strlen("gpgkeys_")+strlen(opt.keyserver_scheme)+1); filename=m_alloc(strlen("gpgkeys_")+strlen(opt.keyserver_scheme)+1);
@ -287,16 +296,38 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count)
if(opt.keyserver_options.use_temp_files) if(opt.keyserver_options.use_temp_files)
{ {
int attempts;
const char *tmp=get_temp_dir(); const char *tmp=get_temp_dir();
byte *randombits;
tempdir=m_alloc(strlen(tmp)+1+8+11+1); tempdir=m_alloc(strlen(tmp)+1+12+1);
sprintf(tempdir,"%s" DIRSEP_S "gpg-XXXXXX",tmp);
/* Yes, I'm using mktemp. No, this isn't automatically insecure /* Try 4 times to make the temp directory */
because of it. I am using it to make a temp dir, not a file, for(attempts=0;attempts<4;attempts++)
and I happily fail if it already exists. */ {
/* 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]);
m_free(randombits);
if(mkdir(tempdir,0700)==0)
{
madedir=1;
break;
}
}
if(!madedir)
{
log_error(_("%s: can't create temp directory after %d tries: %s\n"),
tempdir,attempts,strerror(errno));
goto fail;
}
tempfile_in=m_alloc(strlen(tempdir)+1+10+1); tempfile_in=m_alloc(strlen(tempdir)+1+10+1);
sprintf(tempfile_in,"%s" DIRSEP_S "ksrvin" EXTSEP_S "txt",tempdir); sprintf(tempfile_in,"%s" DIRSEP_S "ksrvin" EXTSEP_S "txt",tempdir);
@ -304,15 +335,6 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count)
tempfile_out=m_alloc(strlen(tempdir)+1+11+1); tempfile_out=m_alloc(strlen(tempdir)+1+11+1);
sprintf(tempfile_out,"%s" DIRSEP_S "ksrvout" EXTSEP_S "txt",tempdir); sprintf(tempfile_out,"%s" DIRSEP_S "ksrvout" EXTSEP_S "txt",tempdir);
if(mkdir(tempdir,0700)==-1)
{
log_error(_("%s: can't create directory: %s\n"),
tempdir,strerror(errno));
goto fail;
}
madedir=1;
tochild=fopen(tempfile_in,"w"); tochild=fopen(tempfile_in,"w");
if(tochild==NULL) if(tochild==NULL)
{ {

View File

@ -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_test_algo( int algo, unsigned int usage_flags );
int openpgp_pk_algo_usage ( int algo ); int openpgp_pk_algo_usage ( int algo );
int openpgp_md_test_algo( int algo ); int openpgp_md_test_algo( int algo );
int check_permissions(const char *path,int checkonly);
/*-- helptext.c --*/ /*-- helptext.c --*/
void display_online_help( const char *keyword ); void display_online_help( const char *keyword );

View File

@ -24,6 +24,9 @@
#include <string.h> #include <string.h>
#include <unistd.h> #include <unistd.h>
#include <errno.h> #include <errno.h>
#ifdef HAVE_STAT
#include <sys/stat.h>
#endif
#if defined(__linux__) && defined(__alpha__) && __GLIBC__ < 2 #if defined(__linux__) && defined(__alpha__) && __GLIBC__ < 2
#include <asm/sysinfo.h> #include <asm/sysinfo.h>
#include <asm/unistd.h> #include <asm/unistd.h>
@ -327,8 +330,6 @@ openpgp_pk_algo_usage ( int algo )
return use; return use;
} }
int int
openpgp_md_test_algo( int algo ) openpgp_md_test_algo( int algo )
{ {
@ -337,16 +338,60 @@ openpgp_md_test_algo( int algo )
return check_digest_algo(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;
}

View File

@ -104,6 +104,8 @@ struct {
int keep_temp_files:1; int keep_temp_files:1;
STRLIST other; STRLIST other;
} keyserver_options; } keyserver_options;
int keyserver_disable;
int no_perm_warn;
char *temp_dir; char *temp_dir;
int no_encrypt_to; int no_encrypt_to;
int interactive; int interactive;

View File

@ -444,6 +444,8 @@ tdbio_set_dbname( const char *new_dbname, int create )
: make_filename(opt.homedir, : make_filename(opt.homedir,
"trustdb" EXTSEP_S "gpg", NULL ); "trustdb" EXTSEP_S "gpg", NULL );
check_permissions(fname,0);
if( access( fname, R_OK ) ) { if( access( fname, R_OK ) ) {
if( errno != ENOENT ) { if( errno != ENOENT ) {
log_error( _("%s: can't access: %s\n"), fname, strerror(errno) ); log_error( _("%s: can't access: %s\n"), fname, strerror(errno) );