From e23f2b9cef8428bda51b413642d9720ba5c590d5 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 4 Oct 1996 09:31:07 +0000 Subject: - changed the umask handling. We now set the umask to 0 and explicitly set the mode on all created files. I think this is a better policy. - change the debug levels on some items - fix a charset handling bug which affected foreign and extended charset users - no longer switch back to the original directory when idle, instead switch to / as the original directory may not be readable by ordinary users. - fix some bugs where the create mode of files was not being explicitly set (it was relying on the umask and using fopen). Not a big bug as it only affected obscure commands like the messaging ops. - got rid of the lock code in the lpq cache as its no longer needed - rewrote smbrun to be faster and to remove the security hole. We now don't actually need a external smbrun binary, its all done by smbd. - add a more explicit warning about uids and gids of -1 or 65535 (This used to be commit 5aa735c940ccdb6acae5f28449d484181c912e49) --- source3/client/client.c | 4 +- source3/include/local.h | 4 ++ source3/include/proto.h | 2 +- source3/lib/charset.c | 2 + source3/lib/util.c | 8 ++- source3/locking/shmem.c | 6 +- source3/passdb/smbpass.c | 2 +- source3/printing/printing.c | 29 +++------ source3/smbd/message.c | 28 ++++----- source3/smbd/server.c | 18 +++--- source3/smbd/uid.c | 146 ++++++++++++++++++++++++++++++++++++++------ source3/utils/testprns.c | 5 +- 12 files changed, 180 insertions(+), 74 deletions(-) diff --git a/source3/client/client.c b/source3/client/client.c index cdf33a14b3..596d6a9677 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -1521,7 +1521,7 @@ static void do_get(char *rname,char *lname,file_info *finfo1) get_total_time_ms += this_time; get_total_size += finfo.size; - DEBUG(2,("(%g kb/s) (average %g kb/s)\n", + DEBUG(1,("(%g kb/s) (average %g kb/s)\n", finfo.size / (1.024*this_time + 1.0e-4), get_total_size / (1.024*get_total_time_ms))); } @@ -2022,7 +2022,7 @@ static void do_put(char *rname,char *lname,file_info *finfo) put_total_time_ms += this_time; put_total_size += finfo->size; - DEBUG(2,("(%g kb/s) (average %g kb/s)\n", + DEBUG(1,("(%g kb/s) (average %g kb/s)\n", finfo->size / (1.024*this_time + 1.0e-4), put_total_size / (1.024*put_total_time_ms))); } diff --git a/source3/include/local.h b/source3/include/local.h index 3f8572e73d..115617094c 100644 --- a/source3/include/local.h +++ b/source3/include/local.h @@ -154,4 +154,8 @@ #define LONG_CONNECT_TIMEOUT 30 #define SHORT_CONNECT_TIMEOUT 5 + +/* the directory to sit in when idle */ +#define IDLE_DIR "/" + #endif diff --git a/source3/include/proto.h b/source3/include/proto.h index 1b76591abd..8b26aa062f 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -805,7 +805,7 @@ void init_uid(void); BOOL become_guest(void); BOOL become_user(int cnum, int uid); BOOL unbecome_user(void ); -int smbrun(char *cmd,char *outfile); +int smbrun(char *cmd,char *outfile,BOOL shared); /*The following definitions come from username.c */ diff --git a/source3/lib/charset.c b/source3/lib/charset.c index ada3ef790a..cd8c7577c9 100644 --- a/source3/lib/charset.c +++ b/source3/lib/charset.c @@ -40,6 +40,8 @@ static void add_dos_char(int lower, int upper) if (lower && upper) { lower_char_map[(char)upper] = (char)lower; upper_char_map[(char)lower] = (char)upper; + lower_char_map[(char)lower] = (char)lower; + upper_char_map[(char)upper] = (char)upper; } } diff --git a/source3/lib/util.c b/source3/lib/util.c index c6a808ce83..efe91a5046 100644 --- a/source3/lib/util.c +++ b/source3/lib/util.c @@ -123,6 +123,7 @@ void reopen_logs(void) if (!strcsequal(fname,debugf) || !dbf || !file_exist(debugf,NULL)) { + int oldumask = umask(022); strcpy(debugf,fname); if (dbf) fclose(dbf); if (append_log) @@ -130,6 +131,7 @@ void reopen_logs(void) else dbf = fopen(debugf,"w"); if (dbf) setbuf(dbf,NULL); + umask(oldumask); } } else @@ -205,7 +207,9 @@ va_dcl { if (!dbf) { + int oldumask = umask(022); dbf = fopen(debugf,"w"); + umask(oldumask); if (dbf) setbuf(dbf,NULL); else @@ -2883,7 +2887,7 @@ connect_again: } if (ret < 0 && (errno == EINPROGRESS || errno == EALREADY)) { - DEBUG(2,("timeout connecting to %s:%d\n",inet_ntoa(*addr),port)); + DEBUG(1,("timeout connecting to %s:%d\n",inet_ntoa(*addr),port)); close(res); return -1; } @@ -2896,7 +2900,7 @@ connect_again: #endif if (ret < 0) { - DEBUG(2,("error connecting to %s:%d (%s)\n", + DEBUG(1,("error connecting to %s:%d (%s)\n", inet_ntoa(*addr),port,strerror(errno))); return -1; } diff --git a/source3/locking/shmem.c b/source3/locking/shmem.c index f68059afa2..e17cf1aa8d 100644 --- a/source3/locking/shmem.c +++ b/source3/locking/shmem.c @@ -75,17 +75,13 @@ static int shm_times_locked = 0; static BOOL shm_register_process(char *processreg_file, pid_t pid, BOOL *other_processes) { - int old_umask; int shm_processes_fd = -1; int nb_read; pid_t other_pid; int free_slot = -1; - int erased_slot; - + int erased_slot; - old_umask = umask(0); shm_processes_fd = open(processreg_file, O_RDWR | O_CREAT, 0666); - umask(old_umask); if ( shm_processes_fd < 0 ) { DEBUG(0,("ERROR shm_register_process : processreg_file open failed with code %d\n",errno)); diff --git a/source3/passdb/smbpass.c b/source3/passdb/smbpass.c index cd4a7ccf66..275ad5e353 100644 --- a/source3/passdb/smbpass.c +++ b/source3/passdb/smbpass.c @@ -60,7 +60,7 @@ do_pw_lock(int fd, int waitsecs, int type) int pw_file_lock(char *name, int type, int secs) { - int fd = open(name, O_RDWR | O_CREAT, 0666); + int fd = open(name, O_RDWR | O_CREAT, 0600); if (fd < 0) return (-1); if (do_pw_lock(fd, secs, type)) { diff --git a/source3/printing/printing.c b/source3/printing/printing.c index ad840d7f51..87552ab3ff 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -130,7 +130,7 @@ void print_file(int fnum) tempstr = build_print_command(cnum, PRINTCOMMAND(snum), syscmd, Files[fnum].name); if (tempstr != NULL) { - int ret = smbrun(syscmd,NULL); + int ret = smbrun(syscmd,NULL,False); DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret)); } else @@ -923,7 +923,6 @@ int get_printqueue(int snum,int cnum,print_queue_struct **queue, struct stat sbuf; BOOL dorun=True; int cachetime = lp_lpqcachetime(); - int lfd = -1; *line = 0; check_lpq_cache(snum); @@ -954,20 +953,10 @@ int get_printqueue(int snum,int cnum,print_queue_struct **queue, DEBUG(3,("Using cached lpq output\n")); dorun = False; } - - if (dorun) { - lfd = file_lock(outfile,LPQ_LOCK_TIMEOUT); - if (lfd<0 || - (!fstat(lfd,&sbuf) && (time(NULL) - sbuf.st_mtime)= 0) file_unlock(lfd); return(0); } @@ -1006,12 +994,13 @@ int get_printqueue(int snum,int cnum,print_queue_struct **queue, fclose(f); - if (lfd >= 0) file_unlock(lfd); - - if (!cachetime) + if (!cachetime) { unlink(outfile); - else + } else { + /* we only expect this to succeed on trapdoor systems, on normal systems + the file is owned by root */ chmod(outfile,0666); + } return(count); } @@ -1047,7 +1036,7 @@ void del_printqueue(int cnum,int snum,int jobid) string_sub(syscmd,"%j",jobstr); standard_sub(cnum,syscmd); - ret = smbrun(syscmd,NULL); + ret = smbrun(syscmd,NULL,False); DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret)); lpq_reset(snum); /* queue has changed */ } @@ -1085,7 +1074,7 @@ void status_printjob(int cnum,int snum,int jobid,int status) string_sub(syscmd,"%j",jobstr); standard_sub(cnum,syscmd); - ret = smbrun(syscmd,NULL); + ret = smbrun(syscmd,NULL,False); DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret)); lpq_reset(snum); /* queue has changed */ } diff --git a/source3/smbd/message.c b/source3/smbd/message.c index b26a6605ed..22523aad3b 100644 --- a/source3/smbd/message.c +++ b/source3/smbd/message.c @@ -42,8 +42,8 @@ static void msg_deliver(void) { pstring s; fstring name; - FILE *f; int i; + int fd; if (! (*lp_msg_command())) { @@ -56,21 +56,19 @@ static void msg_deliver(void) sprintf(s,"/tmp/msg.XXXXXX"); strcpy(name,(char *)mktemp(s)); - f = fopen(name,"w"); - if (!f) - { - DEBUG(1,("can't open message file %s\n",name)); - return; - } + fd = open(name,O_WRONLY|O_CREAT|O_TRUNC|O_EXCL,0600); + if (fd == -1) { + DEBUG(1,("can't open message file %s\n",name)); + return; + } - for (i=0;iuid)) @@ -2149,7 +2149,7 @@ int make_connection(char *service,char *user,char *password, int pwlen, char *de pstring cmd; strcpy(cmd,lp_preexec(SNUM(cnum))); standard_sub(cnum,cmd); - smbrun(cmd,NULL); + smbrun(cmd,NULL,False); } /* we've finished with the sensitive stuff */ @@ -2629,7 +2629,7 @@ void close_cnum(int cnum, int uid) pstring cmd; strcpy(cmd,lp_postexec(SNUM(cnum))); standard_sub(cnum,cmd); - smbrun(cmd,NULL); + smbrun(cmd,NULL,False); unbecome_user(); } @@ -2640,7 +2640,7 @@ void close_cnum(int cnum, int uid) pstring cmd; strcpy(cmd,lp_rootpostexec(SNUM(cnum))); standard_sub(cnum,cmd); - smbrun(cmd,NULL); + smbrun(cmd,NULL,False); } Connections[cnum].open = False; @@ -2764,8 +2764,10 @@ BOOL claim_connection(int cnum,char *name,int max_connections,BOOL Clear) if (!file_exist(fname,NULL)) { + int oldmask = umask(022); f = fopen(fname,"w"); if (f) fclose(f); + umask(oldmask); } total_recs = file_size(fname) / sizeof(crec); @@ -3617,7 +3619,9 @@ static void usage(char *pname) fault_setup(exit_server); - umask(0777 & ~DEF_CREATE_MASK); + /* we want total control over the permissions on created files, + so set our umask to 0 */ + umask(0); init_uid(); diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index 555cd457e7..7274c18478 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -27,9 +27,6 @@ extern connection_struct Connections[]; static int initial_uid; static int initial_gid; -static int old_umask = 022; - -static pstring OriginalDir; /* what user is current? */ struct current_user current_user; @@ -57,7 +54,7 @@ void init_uid(void) current_user.cnum = -1; - GetWd(OriginalDir); + ChDir(IDLE_DIR); } @@ -69,6 +66,10 @@ static BOOL become_uid(int uid) if (initial_uid != 0) return(True); + if (uid == -1 || uid == 65535) { + DEBUG(1,("WARNING: using uid %d is a security risk\n",uid)); + } + #ifdef AIX { /* AIX 3 stuff - inspired by a code fragment in wu-ftpd */ @@ -118,6 +119,10 @@ static BOOL become_gid(int gid) { if (initial_uid != 0) return(True); + + if (gid == -1 || gid == 65535) { + DEBUG(1,("WARNING: using gid %d is a security risk\n",gid)); + } #ifdef USE_SETRES if (setresgid(-1,gid,-1) != 0) @@ -199,7 +204,6 @@ static BOOL check_user_ok(int cnum,user_struct *vuser,int snum) ****************************************************************************/ BOOL become_user(int cnum, int uid) { - int new_umask; user_struct *vuser; int snum,gid; int id = uid; @@ -259,14 +263,11 @@ BOOL become_user(int cnum, int uid) return(False); } - new_umask = 0777 & ~CREATE_MODE(cnum); - old_umask = umask(new_umask); - current_user.cnum = cnum; current_user.id = id; - DEBUG(5,("become_user uid=(%d,%d) gid=(%d,%d) new_umask=0%o\n", - getuid(),geteuid(),getgid(),getegid(),new_umask)); + DEBUG(5,("become_user uid=(%d,%d) gid=(%d,%d)\n", + getuid(),geteuid(),getgid(),getegid())); return(True); } @@ -279,9 +280,7 @@ BOOL unbecome_user(void ) if (current_user.cnum == -1) return(False); - ChDir(OriginalDir); - - umask(old_umask); + ChDir(IDLE_DIR); if (initial_uid == 0) { @@ -318,9 +317,9 @@ BOOL unbecome_user(void ) current_user.uid = initial_uid; current_user.gid = initial_gid; - if (ChDir(OriginalDir) != 0) + if (ChDir(IDLE_DIR) != 0) DEBUG(0,("%s chdir(%s) failed in unbecome_user\n", - timestring(),OriginalDir)); + timestring(),IDLE_DIR)); DEBUG(5,("unbecome_user now uid=(%d,%d) gid=(%d,%d)\n", getuid(),geteuid(),getgid(),getegid())); @@ -332,14 +331,69 @@ BOOL unbecome_user(void ) /**************************************************************************** -run a command via system() using smbrun, being careful about uid/gid handling +This is a utility function of smbrun(). It must be called only from +the child as it may leave the caller in a privilaged state. ****************************************************************************/ -int smbrun(char *cmd,char *outfile) +static BOOL setup_stdout_file(char *outfile,BOOL shared) +{ + int fd; + mode_t mode = S_IWUSR|S_IRUSR|S_IRGRP|S_IROTH; + + close(1); + + if (shared) { + /* become root - unprivilaged users can't delete these files */ +#ifdef USE_SETRES + setresgid(0,0,0); + setresuid(0,0,0); +#else + setuid(0); + seteuid(0); +#endif + } + + /* now create the file with O_EXCL set */ + unlink(outfile); + fd = open(outfile,O_RDWR|O_CREAT|O_TRUNC|O_EXCL,mode); + + if (fd == -1) return False; + + if (fd != 1) { + if (dup2(fd,1) != 0) { + DEBUG(2,("Failed to create stdout file descriptor\n")); + close(fd); + return False; + } + close(fd); + } + return True; +} + + +/**************************************************************************** +run a command being careful about uid/gid handling and putting the output in +outfile (or discard it if outfile is NULL). + +if shared is True then ensure the file will be writeable by all users +but created such that its owned by root. This overcomes a security hole. + +if shared is not set then open the file with O_EXCL set +****************************************************************************/ +int smbrun(char *cmd,char *outfile,BOOL shared) { + int fd,pid; + int uid = current_user.uid; + int gid = current_user.gid; + +#if USE_SYSTEM int ret; pstring syscmd; char *path = lp_smbrun(); + /* in the old method we use system() to execute smbrun which then + executes the command (using system() again!). This involves lots + of shell launches and is very slow. It also suffers from a + potential security hole */ if (!file_exist(path,NULL)) { DEBUG(0,("SMBRUN ERROR: Can't find %s. Installation problem?\n",path)); @@ -347,13 +401,69 @@ int smbrun(char *cmd,char *outfile) } sprintf(syscmd,"%s %d %d \"(%s 2>&1) > %s\"", - path,current_user.uid,current_user.gid,cmd, + path,uid,gid,cmd, outfile?outfile:"/dev/null"); DEBUG(5,("smbrun - running %s ",syscmd)); ret = system(syscmd); DEBUG(5,("gave %d\n",ret)); return(ret); +#else + /* in this newer method we will exec /bin/sh with the correct + arguments, after first setting stdout to point at the file */ + + if ((pid=fork())) { + int status=0; + /* the parent just waits for the child to exit */ + if (waitpid(pid,&status,0) != pid) { + DEBUG(2,("waitpid(%d) : %s\n",pid,strerror(errno))); + return -1; + } + return status; + } + + + /* we are in the child. we exec /bin/sh to do the work for us. we + don't directly exec the command we want because it may be a + pipeline or anything else the config file specifies */ + + /* point our stdout at the file we want output to go into */ + if (outfile && !setup_stdout_file(outfile,shared)) { + exit(80); + } + + /* now completely lose our privilages. This is a fairly paranoid + way of doing it, but it does work on all systems that I know of */ +#ifdef USE_SETRES + setresgid(0,0,0); + setresuid(0,0,0); + setresgid(gid,gid,gid); + setresuid(uid,uid,uid); +#else + setuid(0); + seteuid(0); + setgid(gid); + setegid(gid); + setuid(uid); + seteuid(uid); +#endif + + if (getuid() != uid || geteuid() != uid || + getgid() != gid || getegid() != gid) { + /* we failed to lose our privilages - do not execute the command */ + exit(81); /* we can't print stuff at this stage, instead use exit codes + for debugging */ + } + + /* close all other file descriptors, leaving only 0, 1 and 2. 0 and + 2 point to /dev/null from the startup code */ + for (fd=3;fd<256;fd++) close(fd); + + execl("/bin/sh","sh","-c",cmd,NULL); + + /* not reached */ + exit(82); +#endif } diff --git a/source3/utils/testprns.c b/source3/utils/testprns.c index b41b4a4c42..4709eb0691 100644 --- a/source3/utils/testprns.c +++ b/source3/utils/testprns.c @@ -53,10 +53,9 @@ int main(int argc, char *argv[]) else { dbf = fopen("test.log", "w"); - if (dbf == NULL) + if (dbf == NULL) { printf("Unable to open logfile.\n"); - else - { + } else { DEBUGLEVEL = 3; pszTemp = (argc < 3) ? PRINTCAP_NAME : argv[2]; printf("Looking for printer %s in printcap file %s\n", -- cgit