From owner-freebsd-audit Sun Jan 23 19:54:25 2000 Delivered-To: freebsd-audit@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 758) id A66C814F55; Sun, 23 Jan 2000 19:54:11 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id 998EE1CD67E; Sun, 23 Jan 2000 19:54:11 -0800 (PST) (envelope-from kris@hub.freebsd.org) Date: Sun, 23 Jan 2000 19:54:11 -0800 (PST) From: Kris Kennaway To: Peter Jeremy Cc: audit@FreeBSD.ORG Subject: Re: libc patch to warn about tempfiles In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sun, 16 Jan 2000, Kris Kennaway wrote: > > I think that changing the algorithm to use a denser encoding (eg > > encoding the PID in base-62 or more, rather than base 10) would be > > a better solution. This way you don't need to change the functions > > using mktemp() et al. > > Hmm..that's not a bad idea. With base-64 we'd have 36 bits, of which about > 17 would be taken up by the encoded PID, leaving about 520000 possible > tempfiles (a factor of 10^4 better than now). OTOH, using 10 X's in the > current system has a target space of size 52^6, or 1.97e+10 :-) Okay, here's a (lightly tested) patch which implements this. It base-64 encodes the PID into the first 3 characters (first 17 bits) + 1 random bit from arc4random() :) The remaining characters are taken from a larger set - basically all of the non-special characters which aren't likely to cause problems with code. With 6 X's and 76 characters in the second set this gives us 877952 possible tempfiles per PID, which is probably "good enough". Comments? Kris Index: stdio/mktemp.c =================================================================== RCS file: /home/ncvs/src/lib/libc/stdio/mktemp.c,v retrieving revision 1.18 diff -u -r1.18 mktemp.c --- stdio/mktemp.c 2000/01/12 09:23:41 1.18 +++ stdio/mktemp.c 2000/01/24 03:48:46 @@ -52,6 +52,11 @@ static int _gettemp __P((char *, int *, int, int)); +static unsigned char base64[] = + ".#0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; +static unsigned char padchar[] = + "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz!@#%^&-_=+:,.~"; + int mkstemps(path, slen) char *path; @@ -104,7 +109,8 @@ { register char *start, *trv, *suffp; struct stat sbuf; - int pid, rval; + int n, numx, pid, rval; + uint32_t in; if (doopen && domkdir) { errno = EINVAL; @@ -121,22 +127,29 @@ return (0); } pid = getpid(); - while (*trv == 'X' && pid != 0) { - *trv-- = (pid % 10) + '0'; - pid /= 10; + while (*trv == 'X') + trv--; + start = trv++; + numx = (suffp - trv); + if (numx < 3) { /* Not enough bits to base64-encode PID */ + errno = EINVAL; + return(0); } - while (*trv == 'X') { - char c; - - pid = (arc4random() & 0xffff) % (26+26); - if (pid < 26) - c = pid + 'A'; - else - c = (pid - 26) + 'a'; - *trv-- = c; + in = (arc4random() & 0x00007FFF) | (pid << 15); + n = 3; + /* Base-64 encode the PID */ + while (--n >= 0) { + *trv++ = base64[(in & 0xFC000000) >> 26]; + in <<= 6; } - start = trv + 1; + /* Pad out the remaining X's with random characters from our enlarged + set */ + while (trv < suffp) { + pid = (arc4random() % sizeof(padchar)); + *trv++ = padchar[pid]; + } + trv = start; /* * check the target directory; if you have six X's and it * doesn't exist this runs for a *very* long time. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sun Jan 23 20:52: 6 2000 Delivered-To: freebsd-audit@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 758) id 7443315726; Sun, 23 Jan 2000 20:52:05 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id 6628D1CD68B; Sun, 23 Jan 2000 20:52:05 -0800 (PST) (envelope-from kris@hub.freebsd.org) Date: Sun, 23 Jan 2000 20:52:05 -0800 (PST) From: Kris Kennaway To: Peter Jeremy Cc: audit@FreeBSD.ORG Subject: Re: libc patch to warn about tempfiles In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sun, 23 Jan 2000, Kris Kennaway wrote: > + while (trv < suffp) { > + pid = (arc4random() % sizeof(padchar)); Oops, this should of course be strlen(padchar). > + *trv++ = padchar[pid]; > + } > + trv = start; Kris ---- "How many roads must a man walk down, before you call him a man?" "Eight!" "That was a rhetorical question!" "Oh..then, seven!" -- Homer Simpson To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sun Jan 23 21:21:45 2000 Delivered-To: freebsd-audit@freebsd.org Received: from alcanet.com.au (mail.alcanet.com.au [203.62.196.10]) by hub.freebsd.org (Postfix) with ESMTP id 719C015608 for ; Sun, 23 Jan 2000 21:21:34 -0800 (PST) (envelope-from jeremyp@gsmx07.alcatel.com.au) Received: by border.alcanet.com.au id <115251>; Mon, 24 Jan 2000 16:21:58 +1100 Content-return: prohibited From: Peter Jeremy Subject: Re: libc patch to warn about tempfiles In-reply-to: ; from kris@hub.freebsd.org on Mon, Jan 24, 2000 at 02:55:15PM +1100 To: Kris Kennaway Cc: audit@FreeBSD.ORG Message-Id: <00Jan24.162158est.115251@border.alcanet.com.au> MIME-version: 1.0 X-Mailer: Mutt 1.0i Content-type: text/plain; charset=us-ascii References: Date: Mon, 24 Jan 2000 16:21:55 +1100 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On 2000-Jan-24 14:55:15 +1100, Kris Kennaway wrote: >On Sun, 16 Jan 2000, Kris Kennaway wrote: > >> > I think that changing the algorithm to use a denser encoding (eg >> > encoding the PID in base-62 or more, rather than base 10) would be >> > a better solution. This way you don't need to change the functions >> > using mktemp() et al. >> >> Hmm..that's not a bad idea. With base-64 we'd have 36 bits, of which about >> 17 would be taken up by the encoded PID, leaving about 520000 possible >> tempfiles (a factor of 10^4 better than now). OTOH, using 10 X's in the >> current system has a target space of size 52^6, or 1.97e+10 :-) > >Okay, here's a (lightly tested) patch which implements this. Adn here's a (non-tested) alternative. This differs from your patch as follows: - Add a few const's. - The program flow remains identical to the current code, just the modulus constants change. - I prefer (sizeof(padchar) - 1) to remove the need for a strlen inside a loop. - Correctly increment the random filename if the first choice exists. Index: mktemp.c =================================================================== RCS file: /home/CVSROOT/src/lib/libc/stdio/mktemp.c,v retrieving revision 1.18 diff -u -r1.18 mktemp.c --- mktemp.c 2000/01/12 09:23:41 1.18 +++ mktemp.c 2000/01/24 05:15:04 @@ -52,6 +52,11 @@ static int _gettemp __P((char *, int *, int, int)); +static unsigned char base64[] = + ".#0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; +static unsigned char padchar[] = +"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz!@#%^&-_=+:,.~"; + int mkstemps(path, slen) char *path; @@ -103,8 +108,10 @@ int slen; { register char *start, *trv, *suffp; + char *pad; struct stat sbuf; - int pid, rval; + uint32_t pid; + int rval, n; if (doopen && domkdir) { errno = EINVAL; @@ -120,20 +127,22 @@ errno = EINVAL; return (0); } - pid = getpid(); - while (*trv == 'X' && pid != 0) { - *trv-- = (pid % 10) + '0'; - pid /= 10; + + /* Encode the PID (with 1 bit of randomness) into 3 base-64 chars */ + pid = getpid() | (arc4random() & 0x00020000); + for (n = 0; *trv == 'X' && n < 3; n++) { + *trv-- = base64[pid & 0x3f]; + pid >>= 6; } - while (*trv == 'X') { - char c; + if (n < 3) { /* Not enough characters to encode PID */ + errno = EINVAL; + return(0); + } - pid = (arc4random() & 0xffff) % (26+26); - if (pid < 26) - c = pid + 'A'; - else - c = (pid - 26) + 'a'; - *trv-- = c; + /* Fill remaining space with random characters */ + while (*trv == 'X') { + pid = arc4random() % (sizeof(padchar) - 1); + *trv-- = padchar[pid]; } start = trv + 1; @@ -179,15 +188,11 @@ for (trv = start;;) { if (*trv == '\0' || trv == suffp) return(0); - if (*trv == 'Z') - *trv++ = 'a'; + pad = strchr(padchar, *trv); + if (pad == NULL || !*++pad) + *trv++ = padchar[0]; else { - if (isdigit((unsigned char)*trv)) - *trv = 'a'; - else if (*trv == 'z') /* inc from z to A */ - *trv = 'A'; - else - ++*trv; + *trv++ = *pad; break; } } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Mon Jan 24 0:39:10 2000 Delivered-To: freebsd-audit@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 758) id BA89F15111; Mon, 24 Jan 2000 00:39:09 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id A5BEA1CD5E6; Mon, 24 Jan 2000 00:39:09 -0800 (PST) (envelope-from kris@hub.freebsd.org) Date: Mon, 24 Jan 2000 00:39:09 -0800 (PST) From: Kris Kennaway To: Peter Jeremy Cc: audit@FreeBSD.ORG Subject: Re: libc patch to warn about tempfiles In-Reply-To: <00Jan24.162158est.115251@border.alcanet.com.au> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Mon, 24 Jan 2000, Peter Jeremy wrote: > Adn here's a (non-tested) alternative. This differs from your patch > as follows: Yeah, this is a better version :-) > - Add a few const's. Except you missed out these. I assume these were intended for base64[] and padchar[]? This patch looks good..any other comments from the audit gallery? :) Kris To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Mon Jan 24 11:15:26 2000 Delivered-To: freebsd-audit@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 758) id 1EED315A07; Mon, 24 Jan 2000 11:15:11 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id 0E4B81CD680 for ; Mon, 24 Jan 2000 11:15:11 -0800 (PST) (envelope-from kris@hub.freebsd.org) Date: Mon, 24 Jan 2000 11:15:11 -0800 (PST) From: Kris Kennaway To: audit@freebsd.org Subject: OPIE audit Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Hi guys, We need to fix up the OPIE utilities so they don't rely on a world-readable /etc/opiekeys (bad for dictionary attacks, like the recent w00w00 advisory points out). There are at least two ways to do this: 1) Audit the OPIE code for setuid rootness (this is the path which FreeBSD went with s/key a few years ago - dunno why opie wasn't done then too) - or setuid opieness (new uid). 2) Use a small setuid root helper app which does the authentication on behalf of the non-setuid program. Thoughts? Kris ---- "How many roads must a man walk down, before you call him a man?" "Eight!" "That was a rhetorical question!" "Oh..then, seven!" -- Homer Simpson To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Mon Jan 24 11:51:23 2000 Delivered-To: freebsd-audit@freebsd.org Received: from alcanet.com.au (mail.alcanet.com.au [203.62.196.10]) by hub.freebsd.org (Postfix) with ESMTP id 0BCCA14F7C for ; Mon, 24 Jan 2000 11:51:13 -0800 (PST) (envelope-from jeremyp@gsmx07.alcatel.com.au) Received: by border.alcanet.com.au id <115211>; Tue, 25 Jan 2000 06:51:42 +1100 Content-return: prohibited From: Peter Jeremy Subject: Re: libc patch to warn about tempfiles In-reply-to: ; from kris@hub.freebsd.org on Mon, Jan 24, 2000 at 07:39:58PM +1100 To: Kris Kennaway Cc: audit@FreeBSD.ORG Message-Id: <00Jan25.065142est.115211@border.alcanet.com.au> MIME-version: 1.0 X-Mailer: Mutt 1.0i Content-type: text/plain; charset=us-ascii References: <00Jan24.162158est.115251@border.alcanet.com.au> Date: Tue, 25 Jan 2000 06:51:42 +1100 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On 2000-Jan-24 19:39:58 +1100, Kris Kennaway wrote: >On Mon, 24 Jan 2000, Peter Jeremy wrote: >> - Add a few const's. > >Except you missed out these. I assume these were intended for base64[] and >padchar[]? Ooops, I posted the wrong version. I added the consts (and a #include to fix the compilation), but forgot to generate a new set of diffs :-(. Try this on (I've also fixed one style bug in my original): Index: mktemp.c =================================================================== RCS file: /home/CVSROOT/src/lib/libc/stdio/mktemp.c,v retrieving revision 1.18 diff -u -r1.18 mktemp.c --- mktemp.c 2000/01/12 09:23:41 1.18 +++ mktemp.c 2000/01/24 19:49:23 @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -52,6 +53,11 @@ static int _gettemp __P((char *, int *, int, int)); +static const unsigned char base64[] = + ".#0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; +static const unsigned char padchar[] = +"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz!@#%^&-_=+:,.~"; + int mkstemps(path, slen) char *path; @@ -103,8 +109,10 @@ int slen; { register char *start, *trv, *suffp; + const char *pad; struct stat sbuf; - int pid, rval; + uint32_t pid; + int rval, n; if (doopen && domkdir) { errno = EINVAL; @@ -120,20 +128,22 @@ errno = EINVAL; return (0); } - pid = getpid(); - while (*trv == 'X' && pid != 0) { - *trv-- = (pid % 10) + '0'; - pid /= 10; + + /* Encode the PID (with 1 bit of randomness) into 3 base-64 chars */ + pid = getpid() | (arc4random() & 0x00020000); + for (n = 0; *trv == 'X' && n < 3; n++) { + *trv-- = base64[pid & 0x3f]; + pid >>= 6; } - while (*trv == 'X') { - char c; + if (n < 3) { /* Not enough characters to encode PID */ + errno = EINVAL; + return(0); + } - pid = (arc4random() & 0xffff) % (26+26); - if (pid < 26) - c = pid + 'A'; - else - c = (pid - 26) + 'a'; - *trv-- = c; + /* Fill remaining space with random characters */ + while (*trv == 'X') { + pid = arc4random() % (sizeof(padchar) - 1); + *trv-- = padchar[pid]; } start = trv + 1; @@ -179,15 +189,11 @@ for (trv = start;;) { if (*trv == '\0' || trv == suffp) return(0); - if (*trv == 'Z') - *trv++ = 'a'; + pad = strchr(padchar, *trv); + if (pad == NULL || *++pad == '\0') + *trv++ = padchar[0]; else { - if (isdigit((unsigned char)*trv)) - *trv = 'a'; - else if (*trv == 'z') /* inc from z to A */ - *trv = 'A'; - else - ++*trv; + *trv++ = *pad; break; } } Peter To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Mon Jan 24 13: 0: 7 2000 Delivered-To: freebsd-audit@freebsd.org Received: from rover.village.org (rover.village.org [204.144.255.49]) by hub.freebsd.org (Postfix) with ESMTP id 0C21F159F3; Mon, 24 Jan 2000 12:59:36 -0800 (PST) (envelope-from imp@harmony.village.org) Received: from harmony.village.org (harmony.village.org [10.0.0.6]) by rover.village.org (8.9.3/8.9.3) with ESMTP id NAA02610; Mon, 24 Jan 2000 13:59:30 -0700 (MST) (envelope-from imp@harmony.village.org) Received: from harmony.village.org (localhost.village.org [127.0.0.1]) by harmony.village.org (8.9.3/8.8.3) with ESMTP id NAA06248; Mon, 24 Jan 2000 13:59:17 -0700 (MST) Message-Id: <200001242059.NAA06248@harmony.village.org> To: Kris Kennaway Subject: Re: OPIE audit Cc: audit@FreeBSD.ORG In-reply-to: Your message of "Mon, 24 Jan 2000 11:15:11 PST." References: Date: Mon, 24 Jan 2000 13:59:17 -0700 From: Warner Losh Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG In message Kris Kennaway writes: : We need to fix up the OPIE utilities so they don't rely on a : world-readable /etc/opiekeys (bad for dictionary attacks, like the recent : w00w00 advisory points out). There are at least two ways to do this: : : 1) Audit the OPIE code for setuid rootness (this is the path which FreeBSD : went with s/key a few years ago - dunno why opie wasn't done then too) - : or setuid opieness (new uid). : 2) Use a small setuid root helper app which does the authentication on : behalf of the non-setuid program. : : Thoughts? I like the idea of doing (1), but realize that (2) might be faster to produce. Warner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jan 26 13:37:49 2000 Delivered-To: freebsd-audit@freebsd.org Received: from MailAndNews.com (MailAndNews.com [199.29.68.160]) by hub.freebsd.org (Postfix) with ESMTP id 61530152CE for ; Wed, 26 Jan 2000 13:37:47 -0800 (PST) (envelope-from mheffner@mailandnews.com) Received: from muriel.penguinpowered.com [208.138.198.103] (mheffner@mailandnews.com); Wed, 26 Jan 2000 16:37:38 -0500 X-WM-Posted-At: MailAndNews.com; Wed, 26 Jan 00 16:37:38 -0500 Content-Length: 1190 Message-ID: X-Mailer: XFMail 1.4.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 Date: Wed, 26 Jan 2000 16:39:17 -0500 (EST) Reply-To: Mike Heffner From: Mike Heffner To: FreeBSD-audit Subject: lorder tempfiles Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG While doing a make world, lorder is run and creates a bunch of tempfiles like this: + -rw-r--r-- 1 root wheel 625 Jan 25 21:05 /tmp/_symbol_24973 + -rw-r--r-- 1 root wheel 842 Jan 25 21:05 /tmp/_reference_24973 - -rw-r--r-- 1 root wheel 842 Jan 25 21:05 /tmp/_reference_24973 - -rw-r--r-- 1 root wheel 625 Jan 25 21:05 /tmp/_symbol_24973 they're usually in that order. Could we use mktemp(1) to make it slightly more random? Here's a patch: Index: lorder.sh =================================================================== RCS file: /home/ncvs/src/usr.bin/lorder/lorder.sh,v retrieving revision 1.2 diff -u -r1.2 lorder.sh --- lorder.sh 1998/08/15 07:10:21 1.2 +++ lorder.sh 2000/01/26 21:36:19 @@ -45,8 +45,8 @@ esac # temporary files -R=/tmp/_reference_$$ -S=/tmp/_symbol_$$ +R=`mktemp /tmp/_reference_XXXXXXXXXX` +S=`mktemp /tmp/_symbol_XXXXXXXXXX` # remove temporary files on HUP, INT, QUIT, PIPE, TERM trap "rm -f $R $S; exit 1" 1 2 3 13 15 --------------------------------- Mike Heffner Fredericksburg, VA ICQ# 882073 Date: 26-Jan-2000 Time: 16:29:32 --------------------------------- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jan 26 21:28:27 2000 Delivered-To: freebsd-audit@freebsd.org Received: from MailAndNews.com (MailAndNews.com [199.29.68.160]) by hub.freebsd.org (Postfix) with ESMTP id 46510154A7 for ; Wed, 26 Jan 2000 21:28:24 -0800 (PST) (envelope-from mheffner@mailandnews.com) Received: from muriel.penguinpowered.com [208.138.198.103] (mheffner@mailandnews.com); Thu, 27 Jan 2000 00:28:21 -0500 X-WM-Posted-At: MailAndNews.com; Thu, 27 Jan 00 00:28:21 -0500 Content-Length: 1993 Message-ID: X-Mailer: XFMail 1.4.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 Date: Thu, 27 Jan 2000 00:30:05 -0500 (EST) Reply-To: Mike Heffner From: Mike Heffner To: FreeBSD-audit , To: FreeBSD-audit , Mike Haertel Subject: use mkstemp(3) for sort Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG This patch uses mkstemp(3), instead of a pid + sequence number(usually zero), for a tempfile name...reviewers? Index: sort.c =================================================================== RCS file: /home/ncvs/src/gnu/usr.bin/sort/sort.c,v retrieving revision 1.15 diff -u -r1.15 sort.c --- sort.c 1999/04/25 22:14:05 1.15 +++ sort.c 2000/01/27 05:17:26 @@ -276,6 +276,7 @@ static struct tempnode { char *name; + int fd; struct tempnode *next; } temphead; @@ -336,9 +337,15 @@ xtmpfopen (const char *file) { FILE *fp; - int fd; + int fd=-1; + struct tempnode *temp; - fd = open (file, O_EXCL | O_WRONLY | O_CREAT | O_TRUNC, 0600); + /* find the fd for file */ + for(temp = &temphead; temp->next; temp = temp->next) + if( strcmp(temp->next->name, file) == 0 ) + break; + if( temp->next ) + fd = temp->next->fd; if (fd < 0 || (fp = fdopen (fd, "w")) == NULL) { error (0, errno, "%s", file); @@ -418,23 +425,16 @@ static char * tempname (void) { - static unsigned int seq; int len = strlen (temp_file_prefix); - char *name = xmalloc (len + 1 + sizeof ("sort") - 1 + 5 + 5 + 1); + char *name; struct tempnode *node; node = (struct tempnode *) xmalloc (sizeof (struct tempnode)); - sprintf (name, - "%s%ssort%5.5d%5.5d", - temp_file_prefix, - (len && temp_file_prefix[len - 1] != '/') ? "/" : "", - (unsigned int) getpid () & 0xffff, seq); - - /* Make sure that SEQ's value fits in 5 digits. */ - ++seq; - if (seq >= 100000) - seq = 0; - + asprintf(&name, + "%s%ssortXXXXXXXXXX", + temp_file_prefix, + (len && temp_file_prefix[len - 1] != '/') ? "/" : ""); + node->fd = mkstemp(name); node->name = name; node->next = temphead.next; temphead.next = node; --------------------------------- Mike Heffner Fredericksburg, VA ICQ# 882073 Date: 27-Jan-2000 Time: 00:20:59 --------------------------------- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jan 26 22: 4: 5 2000 Delivered-To: freebsd-audit@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 758) id 8E0EB15401; Wed, 26 Jan 2000 22:04:04 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id 7ED1E1CD687; Wed, 26 Jan 2000 22:04:04 -0800 (PST) (envelope-from kris@hub.freebsd.org) Date: Wed, 26 Jan 2000 22:04:04 -0800 (PST) From: Kris Kennaway To: Mike Heffner Cc: FreeBSD-audit Subject: Re: lorder tempfiles In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Wed, 26 Jan 2000, Mike Heffner wrote: > While doing a make world, lorder is run and creates a bunch of tempfiles like > this: > + -rw-r--r-- 1 root wheel 625 Jan 25 21:05 /tmp/_symbol_24973 > + -rw-r--r-- 1 root wheel 842 Jan 25 21:05 /tmp/_reference_24973 > - -rw-r--r-- 1 root wheel 842 Jan 25 21:05 /tmp/_reference_24973 > - -rw-r--r-- 1 root wheel 625 Jan 25 21:05 /tmp/_symbol_24973 > > they're usually in that order. Could we use mktemp(1) to make it slightly more > random? Here's a patch: Ah, thats what that damn thing is from. I hadn't got around to tracking it down yet. Patch looks fine: I'll try and get this committed beforet he code freeze. Thanks, Kris ---- "How many roads must a man walk down, before you call him a man?" "Eight!" "That was a rhetorical question!" "Oh..then, seven!" -- Homer Simpson To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Jan 27 15: 6:54 2000 Delivered-To: freebsd-audit@freebsd.org Received: from alcanet.com.au (mail.alcanet.com.au [203.62.196.10]) by hub.freebsd.org (Postfix) with ESMTP id 6B07515835 for ; Thu, 27 Jan 2000 15:06:50 -0800 (PST) (envelope-from jeremyp@gsmx07.alcatel.com.au) Received: by border.alcanet.com.au id <115208>; Fri, 28 Jan 2000 10:07:14 +1100 Content-return: prohibited From: Peter Jeremy Subject: Re: use mkstemp(3) for sort In-reply-to: ; from mheffner@mailandnews.com on Thu, Jan 27, 2000 at 04:29:26PM +1100 To: Mike Heffner Cc: FreeBSD-audit , Mike Haertel Message-Id: <00Jan28.100714est.115208@border.alcanet.com.au> MIME-version: 1.0 X-Mailer: Mutt 1.0i Content-type: text/plain; charset=us-ascii References: Date: Fri, 28 Jan 2000 10:07:13 +1100 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On 2000-Jan-27 16:29:26 +1100, Mike Heffner wrote: >This patch uses mkstemp(3), instead of a pid + sequence number(usually zero), >for a tempfile name...reviewers? Overall, I like the idea of using mkstemp(), but a serious drawback (from the FSF's perspective) is the mkstemp() isn't widely implemented. There also appear to be problems related to the change from opening fd's as needed to keeping fd's open at all times: Whilst the sequence number might be zero for small files, I just tried sorting ~80MB of random text and wound up with 288 files - expecting an effectively unlimited number of FDs to be available is not (IMHO) acceptable. Also, the existing code appears to close FDs when it is (temporarily) finished with them. The patch does not change this close-when- finished behaviour, but does to assume that the FDs will always be open (note that fclose(fdopen(fd, ...)) will close fd). I believe the patch needs to be re-written to restrict the number of FD's required to sort an arbitrary amount of data. The simplest mechanism I can see for doing this would be to include a struct stat (or relevant parts thereof) within struct tempnode. tempname() would populate it by doing an fstat() on the fd returned from mkstemp and xtmpfopen() would do an fstat() on the file it opened and confirm that the file hadn't changed. (The comparison code can be gleaned from mkstemp()). Peter To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Jan 27 16:20:22 2000 Delivered-To: freebsd-audit@freebsd.org Received: from mail.rpi.edu (mail.rpi.edu [128.113.100.7]) by hub.freebsd.org (Postfix) with ESMTP id EDB6F159C2 for ; Thu, 27 Jan 2000 16:20:12 -0800 (PST) (envelope-from drosih@rpi.edu) Received: from [128.113.24.47] (gilead.acs.rpi.edu [128.113.24.47]) by mail.rpi.edu (8.9.3/8.9.3) with ESMTP id TAA37188; Thu, 27 Jan 2000 19:20:08 -0500 Mime-Version: 1.0 X-Sender: drosih@mail.rpi.edu Message-Id: In-Reply-To: References: Date: Thu, 27 Jan 2000 19:20:43 -0500 To: Mike Heffner , FreeBSD-audit From: Garance A Drosihn Subject: Re: use mkstemp(3) for sort Content-Type: text/plain; charset="us-ascii" ; format="flowed" Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG At 12:30 AM -0500 1/27/00, Mike Heffner wrote: >This patch uses mkstemp(3), instead of a pid + sequence number >(usually zero), for a tempfile name...reviewers? For something like this, I sometimes wonder if it would be better to have the program ('sort', in this case) to create a randomly- named directory in /tmp, make sure that directory is owned by the right user and is only readable by the user, and then create all if it's temporary files inside of that directory. While sort may "usually" only create one file, it's possible it will have to create lots (hundreds?) of files. If the above is done, not only will the security issues be addressed, but we'll have less locking and "general wear&tear" on the /tmp directory. Does that sound like a reasonable/worthwhile strategy? --- Garance Alistair Drosehn = gad@eclipse.acs.rpi.edu Senior Systems Programmer or drosih@rpi.edu Rensselaer Polytechnic Institute To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Jan 27 16:29:38 2000 Delivered-To: freebsd-audit@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 758) id C9F17158FA; Thu, 27 Jan 2000 16:29:36 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id A7F971CD6B9; Thu, 27 Jan 2000 16:29:36 -0800 (PST) (envelope-from kris@hub.freebsd.org) Date: Thu, 27 Jan 2000 16:29:36 -0800 (PST) From: Kris Kennaway To: Garance A Drosihn Cc: Mike Heffner , FreeBSD-audit Subject: Re: use mkstemp(3) for sort In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Thu, 27 Jan 2000, Garance A Drosihn wrote: > For something like this, I sometimes wonder if it would be better > to have the program ('sort', in this case) to create a randomly- > named directory in /tmp, make sure that directory is owned by the > right user and is only readable by the user, and then create all > if it's temporary files inside of that directory. This sounds like a better solution than making an invasive change which will have to be re-merged if we upgrade the code (assuming it's not taken up by the vendor). i.e. create the private directory securely with mkdtemp, and sort can be as insecure as it wants within it :-) Kris ---- "How many roads must a man walk down, before you call him a man?" "Eight!" "That was a rhetorical question!" "Oh..then, seven!" -- Homer Simpson To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Jan 27 17:38:39 2000 Delivered-To: freebsd-audit@freebsd.org Received: from MailAndNews.com (MailAndNews.com [199.29.68.160]) by hub.freebsd.org (Postfix) with ESMTP id C044314F43 for ; Thu, 27 Jan 2000 17:38:35 -0800 (PST) (envelope-from mheffner@mailandnews.com) Received: from muriel.penguinpowered.com [208.138.198.103] (mheffner@mailandnews.com); Thu, 27 Jan 2000 20:38:33 -0500 X-WM-Posted-At: MailAndNews.com; Thu, 27 Jan 00 20:38:33 -0500 Content-Length: 1078 Message-ID: X-Mailer: XFMail 1.4.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: Date: Thu, 27 Jan 2000 20:40:10 -0500 (EST) Reply-To: Mike Heffner From: Mike Heffner To: Kris Kennaway Subject: Re: use mkstemp(3) for sort Cc: FreeBSD-audit , Mike Heffner , Garance A Drosihn Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On 28-Jan-2000 Kris Kennaway wrote: | On Thu, 27 Jan 2000, Garance A Drosihn wrote: | |> For something like this, I sometimes wonder if it would be better |> to have the program ('sort', in this case) to create a randomly- |> named directory in /tmp, make sure that directory is owned by the |> right user and is only readable by the user, and then create all |> if it's temporary files inside of that directory. | | This sounds like a better solution than making an invasive change which | will have to be re-merged if we upgrade the code (assuming it's not taken | up by the vendor). | | i.e. create the private directory securely with mkdtemp, and sort can be | as insecure as it wants within it :-) Alright. That sounds a lot easier than what I did. I'll revisit it later this weekend, unless someone else makes that change before then. Thanks for reviewing it though. --------------------------------- Mike Heffner Fredericksburg, VA ICQ# 882073 Date: 27-Jan-2000 Time: 20:32:46 --------------------------------- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Fri Jan 28 16:23:36 2000 Delivered-To: freebsd-audit@freebsd.org Received: from MailAndNews.com (MailAndNews.com [199.29.68.160]) by hub.freebsd.org (Postfix) with ESMTP id 2B35B1503E for ; Fri, 28 Jan 2000 16:23:31 -0800 (PST) (envelope-from mheffner@mailandnews.com) Received: from muriel.penguinpowered.com [208.138.198.103] (mheffner@mailandnews.com); Fri, 28 Jan 2000 19:23:27 -0500 X-WM-Posted-At: MailAndNews.com; Fri, 28 Jan 00 19:23:27 -0500 Content-Length: 2309 Message-ID: X-Mailer: XFMail 1.4.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 Date: Fri, 28 Jan 2000 19:25:02 -0500 (EST) Reply-To: Mike Heffner From: Mike Heffner To: FreeBSD-audit , mike@gnu.ai.mit.edu Subject: sort, revised patch Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Alright, since it was our fourth snow day in a row =), I had time to redo this. Here's the revised patch, it keeps the original naming scheme for tempfiles, but puts them in a dir made with mkdtemp(3). Index: sort.c =================================================================== RCS file: /home/ncvs/src/gnu/usr.bin/sort/sort.c,v retrieving revision 1.15 diff -u -r1.15 sort.c --- sort.c 1999/04/25 22:14:05 1.15 +++ sort.c 2000/01/29 00:12:21 @@ -171,6 +171,8 @@ /* Prefix for temporary file names. */ static char *temp_file_prefix; +/* Temporary dir for temp files, *with* above prefix */ +static char *temp_dir = NULL; /* Flag to reverse the order of all comparisons. */ static int reverse; @@ -288,6 +290,9 @@ for (node = temphead.next; node; node = node->next) unlink (node->name); + if( temp_dir ) + rmdir(temp_dir); + } /* Allocate N bytes of memory dynamically, with error checking. */ @@ -413,6 +418,7 @@ } } +#define DIR_TEMPLATE "sortXXXXXXXXXX" /* Return a name for a temporary file. */ static char * @@ -420,14 +426,28 @@ { static unsigned int seq; int len = strlen (temp_file_prefix); - char *name = xmalloc (len + 1 + sizeof ("sort") - 1 + 5 + 5 + 1); + char *name; struct tempnode *node; node = (struct tempnode *) xmalloc (sizeof (struct tempnode)); + if( !temp_dir ) + { + temp_dir = xmalloc( len + 1 + strlen(DIR_TEMPLATE) + 1 ); + sprintf(temp_dir, + "%s%s%s", + temp_file_prefix, + (len && temp_file_prefix[len - 1] != '/') ? "/" : "", + DIR_TEMPLATE); + if( mkdtemp(temp_dir) == NULL ) + { + error(0, errno, _("can't make temp dir")); + exit(2); + } + } + name = xmalloc(strlen(temp_dir) + 1 + strlen("sort") + 5 + 5 + 1); sprintf (name, - "%s%ssort%5.5d%5.5d", - temp_file_prefix, - (len && temp_file_prefix[len - 1] != '/') ? "/" : "", + "%s/sort%5.5d%5.5d", + temp_dir, (unsigned int) getpid () & 0xffff, seq); /* Make sure that SEQ's value fits in 5 digits. */ --------------------------------- Mike Heffner Fredericksburg, VA ICQ# 882073 Date: 28-Jan-2000 Time: 19:15:54 --------------------------------- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Jan 29 14:38:32 2000 Delivered-To: freebsd-audit@freebsd.org Received: from MailAndNews.com (MailAndNews.com [199.29.68.160]) by hub.freebsd.org (Postfix) with ESMTP id 6BDCA14D6D for ; Sat, 29 Jan 2000 14:38:28 -0800 (PST) (envelope-from mheffner@mailandnews.com) Received: from muriel.penguinpowered.com [208.138.198.103] (mheffner@mailandnews.com); Sat, 29 Jan 2000 17:38:23 -0500 X-WM-Posted-At: MailAndNews.com; Sat, 29 Jan 00 17:38:23 -0500 Content-Length: 745 Message-ID: X-Mailer: XFMail 1.4.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 Date: Sat, 29 Jan 2000 17:40:08 -0500 (EST) Reply-To: Mike Heffner From: Mike Heffner To: FreeBSD-audit Subject: Some programs that could use some work Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Hi, I've compiled a basic list of some programs, that IMO, could do better tempfile handling. I did a basic grep of the usr.bin and quickly scanned some of the code, so the list might be slightly inaccurate. Anyone that is willing and has the time to, might want to look to see if there is something they could easily patch. I'll try to look at some of the programs over time and get some patches made, and I'll also go through the other dirs (gnu, usr.sbin, ...) and see if there are more programs to add. The list is at: http://my.ispchannel.com/~mheffner/freebsd/ Later, --------------------------------- Mike Heffner Fredericksburg, VA ICQ# 882073 Date: 29-Jan-2000 Time: 17:31:41 --------------------------------- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message