From: Tom Yu Date: Sat, 27 Jan 2001 04:41:32 +0000 (+0000) Subject: * dest_tkt.c: Clean up uid handling. Fix stat checks X-Git-Tag: krb5-1.3-alpha1~1682 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=4b462941d797202fa58d0afedce083cba15b6d89;p=krb5.git * dest_tkt.c: Clean up uid handling. Fix stat checks * in_tkt.c: Clean up uid handling. Fix stat checks. * tf_util.c: Clean up uid handling. Fix stat checks. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@12954 dc483132-0cff-0310-8789-dd5450dbe970 --- diff --git a/src/lib/krb4/ChangeLog b/src/lib/krb4/ChangeLog index 830498f29..f1a4fbf59 100644 --- a/src/lib/krb4/ChangeLog +++ b/src/lib/krb4/ChangeLog @@ -1,3 +1,11 @@ +2001-01-26 Tom Yu + + * dest_tkt.c: Clean up uid handling. Fix stat checks. + + * in_tkt.c: Clean up uid handling. Fix stat checks. + + * tf_util.c: Clean up uid handling. Fix stat checks. + 2001-01-25 Tom Yu * Makefile.in (OBJS, SRCS): Add prot_client.o, prot_client.c. diff --git a/src/lib/krb4/dest_tkt.c b/src/lib/krb4/dest_tkt.c index 50c1352b2..78878228a 100644 --- a/src/lib/krb4/dest_tkt.c +++ b/src/lib/krb4/dest_tkt.c @@ -1,14 +1,29 @@ /* - * dest_tkt.c + * lib/krb4/dest_tkt.c * - * Copyright 1985, 1986, 1987, 1988 by the Massachusetts Institute - * of Technology. + * Copyright 1985, 1986, 1987, 1988, 2000, 2001 by the Massachusetts + * Institute of Technology. All Rights Reserved. * - * For copying and distribution information, please see the file - * . + * Export of this software from the United States of America may + * require a specific license from the United States Government. + * It is the responsibility of any person or organization contemplating + * export to obtain such a license before exporting. + * + * WITHIN THAT CONSTRAINT, permission to use, copy, modify, and + * distribute this software and its documentation for any purpose and + * without fee is hereby granted, provided that the above copyright + * notice appear in all copies and that both that copyright notice and + * this permission notice appear in supporting documentation, and that + * the name of M.I.T. not be used in advertising or publicity pertaining + * to distribution of the software without specific, written prior + * permission. Furthermore if you modify this software you must label + * your software as modified software and not distribute it in such a + * fashion that it might be confused with the original M.I.T. software. + * M.I.T. makes no representations about the suitability of + * this software for any purpose. It is provided "as is" without express + * or implied warranty. */ -#include "mit-copyright.h" #include "krb.h" #include #include @@ -17,12 +32,29 @@ #ifdef TKT_SHMEM #include #endif +#ifdef HAVE_UNISTD_H +#include +#endif #include #ifndef O_SYNC #define O_SYNC 0 #endif +#ifdef HAVE_SETEUID +#define do_seteuid(e) seteuid((e)) +#else +#ifdef HAVE_SETRESUID +#define do_seteuid(e) setresuid(-1, (e), -1) +#else +#ifdef HAVE_SETREUID +#define do_seteuid(e) setreuid(geteuid(), (e)) +#else +#define do_seteuid(e) (errno = EPERM, -1) +#endif +#endif +#endif + /* * dest_tkt() is used to destroy the ticket store upon logout. * If the ticket file does not exist, dest_tkt() returns RET_TKFIL. @@ -38,10 +70,13 @@ dest_tkt() char *file = TKT_FILE; int i,fd; extern int errno; - struct stat statb; + int ret; + struct stat statpre, statpost; char buf[BUFSIZ]; + uid_t me, metoo; #ifdef TKT_SHMEM char shmidname[MAXPATHLEN]; + size_t shmidlen; #endif /* TKT_SHMEM */ /* If ticket cache selector is null, use default cache. */ @@ -49,22 +84,56 @@ dest_tkt() file = tkt_string(); errno = 0; - if (lstat(file,&statb) < 0) - goto out; + ret = KSUCCESS; + me = getuid(); + metoo = geteuid(); - if (!(statb.st_mode & S_IFREG) -#ifdef notdef - || statb.st_mode & 077 -#endif - ) + if (lstat(file, &statpre) < 0) + return (errno == ENOENT) ? RET_TKFIL : KFAILURE; + /* + * This does not guard against certain cases that are vulnerable + * to race conditions, such as world-writable or group-writable + * directories that are not stickybitted, or untrusted path + * components. In all other cases, the following checks should be + * sufficient. It is assumed that the aforementioned certain + * vulnerable cases are unlikely to arise on a well-administered + * system where the user is not deliberately being stupid. + */ + if (!(statpre.st_mode & S_IFREG) || me != statpre.st_uid + || statpre.st_nlink != 1) + return KFAILURE; + /* + * Yes, we do uid twiddling here. It's not optimal, but some + * applications may expect that the ruid is what should really own + * the ticket file, e.g. setuid applications. + */ + if (me != metoo && do_seteuid(me) < 0) + return KFAILURE; + if ((fd = open(file, O_RDWR|O_SYNC, 0)) < 0) { + ret = (errno == ENOENT) ? RET_TKFIL : KFAILURE; goto out; - - if ((fd = open(file, O_RDWR|O_SYNC, 0)) < 0) + } + /* + * Do some additional paranoid things. The worst-case situation + * is that a user may be fooled into opening a non-regular file + * briefly if the file is in a directory with improper + * permissions. + */ + if (fstat(fd, &statpost) < 0) { + (void)close(fd); + ret = KFAILURE; + goto out; + } + if (statpre.st_dev != statpost.st_dev + || statpre.st_ino != statpost.st_ino) { + (void)close(fd); + errno = 0; + ret = KFAILURE; goto out; + } memset(buf, 0, BUFSIZ); - - for (i = 0; i < statb.st_size; i += BUFSIZ) + for (i = 0; i < statpost.st_size; i += BUFSIZ) if (write(fd, buf, BUFSIZ) != BUFSIZ) { #ifndef NO_FSYNC (void) fsync(fd); @@ -81,17 +150,22 @@ dest_tkt() (void) unlink(file); out: - if (errno == ENOENT) return RET_TKFIL; - else if (errno != 0) return KFAILURE; + if (me != metoo && do_seteuid(metoo) < 0) + return KFAILURE; + if (ret != KSUCCESS) + return ret; + #ifdef TKT_SHMEM /* * handle the shared memory case */ - (void) strncpy(shmidname, file, sizeof(shmidname) - 1); - shmidname[sizeof(shmidname) - 1] = '\0'; - (void) strcat(shmidname, ".shm", sizeof(shmidname) - 1 - strlen(shmidname)); - if ((i = krb_shm_dest(shmidname)) != KSUCCESS) - return(i); -#endif /* TKT_SHMEM */ - return(KSUCCESS); + shmidlen = strlen(file) + sizeof(".shm"); + if (shmidlen > sizeof(shmidname)) + return RET_TKFIL; + (void)strcpy(shmidname, file); + (void)strcat(shmidname, ".shm"); + return krb_shm_dest(shmidname); +#else /* !TKT_SHMEM */ + return KSUCCESS; +#endif /* !TKT_SHMEM */ } diff --git a/src/lib/krb4/in_tkt.c b/src/lib/krb4/in_tkt.c index 07e56d0b9..3fc6c558e 100644 --- a/src/lib/krb4/in_tkt.c +++ b/src/lib/krb4/in_tkt.c @@ -1,14 +1,29 @@ /* - * in_tkt.c + * lib/krb4/in_tkt.c * - * Copyright 1985, 1986, 1987, 1988 by the Massachusetts Institute - * of Technology. + * Copyright 1985, 1986, 1987, 1988, 2000, 2001 by the Massachusetts + * Institute of Technology. All Rights Reserved. * - * For copying and distribution information, please see the file - * . + * Export of this software from the United States of America may + * require a specific license from the United States Government. + * It is the responsibility of any person or organization contemplating + * export to obtain such a license before exporting. + * + * WITHIN THAT CONSTRAINT, permission to use, copy, modify, and + * distribute this software and its documentation for any purpose and + * without fee is hereby granted, provided that the above copyright + * notice appear in all copies and that both that copyright notice and + * this permission notice appear in supporting documentation, and that + * the name of M.I.T. not be used in advertising or publicity pertaining + * to distribution of the software without specific, written prior + * permission. Furthermore if you modify this software you must label + * your software as modified software and not distribute it in such a + * fashion that it might be confused with the original M.I.T. software. + * M.I.T. makes no representations about the suitability of + * this software for any purpose. It is provided "as is" without express + * or implied warranty. */ -#include "mit-copyright.h" #include #include #include "krb.h" @@ -34,7 +49,7 @@ extern int krb_debug; #define do_seteuid(e) seteuid((e)) #else #ifdef HAVE_SETRESUID -#define do_seteuid(e) setresuid(getuid(), (e), geteuid()) +#define do_seteuid(e) setresuid(-1, (e), -1) #else #ifdef HAVE_SETREUID #define do_seteuid(e) setreuid(geteuid(), (e)) @@ -55,7 +70,7 @@ in_tkt(pname,pinst) { int tktfile; uid_t me, metoo, getuid(), geteuid(); - struct stat buf; + struct stat statpre, statpost; int count; char *file = TKT_FILE; int fd; @@ -72,20 +87,49 @@ in_tkt(pname,pinst) me = getuid (); metoo = geteuid(); - if (lstat(file,&buf) == 0) { - if (buf.st_uid != me || !(buf.st_mode & S_IFREG) || - buf.st_mode & 077) { + if (lstat(file, &statpre) == 0) { + if (statpre.st_uid != me || !(statpre.st_mode & S_IFREG) + || statpre.st_nlink != 1 || statpre.st_mode & 077) { if (krb_debug) fprintf(stderr,"Error initializing %s",file); return(KFAILURE); } + /* + * Yes, we do uid twiddling here. It's not optimal, but some + * applications may expect that the ruid is what should really + * own the ticket file, e.g. setuid applications. + */ + if (me != metoo && do_seteuid(me) < 0) + return KFAILURE; /* file already exists, and permissions appear ok, so nuke it */ - if ((fd = open(file, O_RDWR|O_SYNC, 0)) < 0) + fd = open(file, O_RDWR|O_SYNC, 0); + (void)unlink(file); + if (me != metoo && do_seteuid(metoo) < 0) + return KFAILURE; + if (fd < 0) { goto out; /* can't zero it, but we can still try truncating it */ + } + + /* + * Do some additional paranoid things. The worst-case + * situation is that a user may be fooled into opening a + * non-regular file briefly if the file is in a directory with + * improper permissions. + */ + if (fstat(fd, &statpost) < 0) { + (void)close(fd); + goto out; + } + if (statpre.st_dev != statpost.st_dev + || statpre.st_ino != statpost.st_ino) { + (void)close(fd); + errno = 0; + goto out; + } memset(charbuf, 0, sizeof(charbuf)); - for (i = 0; i < buf.st_size; i += sizeof(charbuf)) + for (i = 0; i < statpost.st_size; i += sizeof(charbuf)) if (write(fd, charbuf, sizeof(charbuf)) != sizeof(charbuf)) { #ifndef NO_FSYNC (void) fsync(fd); @@ -117,12 +161,7 @@ in_tkt(pname,pinst) /* Set umask to ensure that we have write access on the created ticket file. */ mask = umask(077); - if ((tktfile = creat(file,0600)) < 0) { - umask(mask); - if (krb_debug) - fprintf(stderr,"Error initializing %s",TKT_FILE); - return(KFAILURE); - } + tktfile = open(file, O_RDWR|O_SYNC|O_CREAT|O_EXCL, 0600); umask(mask); if (me != metoo) { if (do_seteuid(metoo) < 0) { @@ -134,19 +173,11 @@ in_tkt(pname,pinst) if (krb_debug) printf("swapped UID's %d and %d\n",me,metoo); } - if (lstat(file,&buf) < 0) { + if (tktfile < 0) { if (krb_debug) fprintf(stderr,"Error initializing %s",TKT_FILE); return(KFAILURE); } - - if (buf.st_uid != me || !(buf.st_mode & S_IFREG) || - buf.st_mode & 077) { - if (krb_debug) - fprintf(stderr,"Error initializing %s",TKT_FILE); - return(KFAILURE); - } - count = strlen(pname)+1; if (write(tktfile,pname,count) != count) { (void) close(tktfile); diff --git a/src/lib/krb4/tf_util.c b/src/lib/krb4/tf_util.c index f77a2994a..71706c0e1 100644 --- a/src/lib/krb4/tf_util.c +++ b/src/lib/krb4/tf_util.c @@ -1,20 +1,38 @@ /* - * tf_util.c + * lib/krb4/tf_util.c * - * Copyright 1987, 1988 by the Massachusetts Institute of Technology. + * Copyright 1985, 1986, 1987, 1988, 2000, 2001 by the Massachusetts + * Institute of Technology. All Rights Reserved. * - * For copying and distribution information, please see the file - * . + * Export of this software from the United States of America may + * require a specific license from the United States Government. + * It is the responsibility of any person or organization contemplating + * export to obtain such a license before exporting. + * + * WITHIN THAT CONSTRAINT, permission to use, copy, modify, and + * distribute this software and its documentation for any purpose and + * without fee is hereby granted, provided that the above copyright + * notice appear in all copies and that both that copyright notice and + * this permission notice appear in supporting documentation, and that + * the name of M.I.T. not be used in advertising or publicity pertaining + * to distribution of the software without specific, written prior + * permission. Furthermore if you modify this software you must label + * your software as modified software and not distribute it in such a + * fashion that it might be confused with the original M.I.T. software. + * M.I.T. makes no representations about the suitability of + * this software for any purpose. It is provided "as is" without express + * or implied warranty. */ -#include "mit-copyright.h" - #include "krb.h" #include "k5-int.h" #include #include #include +#ifdef HAVE_UNISTD_H +#include +#endif #include #include @@ -44,7 +62,6 @@ char *shmat(); #ifdef NEED_UTIMES #include -#include #ifdef __SCO__ #include #endif @@ -62,6 +79,20 @@ int utimes(path, times) } #endif +#ifdef HAVE_SETEUID +#define do_seteuid(e) seteuid((e)) +#else +#ifdef HAVE_SETRESUID +#define do_seteuid(e) setresuid(-1, (e), -1) +#else +#ifdef HAVE_SETREUID +#define do_seteuid(e) setreuid(geteuid(), (e)) +#else +#define do_seteuid(e) (errno = EPERM, -1) +#endif +#endif +#endif + /* * fd must be initialized to something that won't ever occur as a real * file descriptor. Since open(2) returns only non-negative numbers as @@ -149,7 +180,7 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) int rw; { int wflag; - uid_t me= getuid(); + uid_t me, metoo; struct stat stat_buf, stat_buffd; #ifdef TKT_SHMEM char shmidname[MAXPATHLEN]; @@ -163,6 +194,7 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) } me = getuid(); + metoo = geteuid(); switch (rw) { case R_TKT_FIL: @@ -196,8 +228,30 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) curpos = sizeof(tfbfr); #ifdef TKT_SHMEM + if (lstat(shmidname, &stat_buf) < 0) { + switch (errno) { + case ENOENT: + return NO_TKT_FIL; + default: + return TKT_FIL_ACC; + } + } + if (stat_buf.st_uid != me || !(stat_buf.st_mode & S_IFREG) + || stat_buf.st_nlink != 1 || stat_buf.st_mode & 077) { + return TKT_FIL_ACC; + } + + /* + * Yes, we do uid twiddling here. It's not optimal, but some + * applications may expect that the ruid is what should really own + * the ticket file, e.g. setuid applications. + */ + if (me != metoo && do_seteuid(me) < 0) + return KFAILURE; sfp = fopen(shmidname, "r"); /* only need read/write on the actual tickets */ + if (me != metoo && do_seteuid(metoo) < 0) + return KFAILURE; if (sfp == 0) { switch(errno) { case ENOENT: @@ -207,10 +261,11 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) } } - /* lstat() and fstat() the file to check that the file we opened is the * - * one we think it is, and to check ownership. */ - if ((fstat(sfp->_file, &stat_buffd) < 0) || - (lstat(shmidname, &stat_buf) < 0)) { + /* + * fstat() the file to check that the file we opened is the one we + * think it is. + */ + if (fstat(fileno(sfp), &stat_buffd) < 0) { (void) close(fd); fd = -1; switch(errno) { @@ -271,8 +326,25 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) tmp_shm_addr = krb_shm_addr; #endif /* TKT_SHMEM */ + if (lstat(tf_name, &stat_buf) < 0) { + switch (errno) { + case ENOENT: + return NO_TKT_FIL; + default: + return TKT_FIL_ACC; + } + } + if (stat_buf.st_uid != me || !(stat_buf.st_mode & S_IFREG) + || stat_buf.st_nlink != 1 || stat_buf.st_mode & 077) { + return TKT_FIL_ACC; + } + if (wflag) { + if (me != metoo && do_seteuid(me) < 0) + return KFAILURE; fd = open(tf_name, O_RDWR, 0600); + if (me != metoo && do_seteuid(metoo) < 0) + return KFAILURE; if (fd < 0) { switch(errno) { case ENOENT: @@ -281,10 +353,11 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) return TKT_FIL_ACC; } } - /* lstat() and fstat() the file to check that the file we opened is the * - * one we think it is, and to check ownership. */ - if ((fstat(fd, &stat_buffd) < 0) || - (lstat(tf_name, &stat_buf) < 0)) { + /* + * fstat() the file to check that the file we opened is the + * one we think it is, and to check ownership. + */ + if (fstat(fd, &stat_buffd) < 0) { (void) close(fd); fd = -1; switch(errno) { @@ -327,7 +400,11 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) * for read-only operations and locked for shared access. */ + if (me != metoo && do_seteuid(me) < 0) + return KFAILURE; fd = open(tf_name, O_RDONLY, 0600); + if (me != metoo && do_seteuid(metoo) < 0) + return KFAILURE; if (fd < 0) { switch(errno) { case ENOENT: @@ -336,10 +413,11 @@ KRB5_DLLIMP int KRB5_CALLCONV tf_init(tf_name, rw) return TKT_FIL_ACC; } } - /* lstat() and fstat() the file to check that the file we opened is the * - * one we think it is, and to check ownership. */ - if ((fstat(fd, &stat_buffd) < 0) || - (lstat(tf_name, &stat_buf) < 0)) { + /* + * fstat() the file to check that the file we opened is the one we + * think it is, and to check ownership. + */ + if (fstat(fd, &stat_buffd) < 0) { (void) close(fd); fd = -1; switch(errno) {