From owner-freebsd-current@FreeBSD.ORG Wed Feb 29 19:32:40 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 58BC3106567A for ; Wed, 29 Feb 2012 19:32:40 +0000 (UTC) (envelope-from freebsd@damnhippie.dyndns.org) Received: from qmta09.emeryville.ca.mail.comcast.net (qmta09.emeryville.ca.mail.comcast.net [76.96.30.96]) by mx1.freebsd.org (Postfix) with ESMTP id 3804F8FC28 for ; Wed, 29 Feb 2012 19:32:40 +0000 (UTC) Received: from omta22.emeryville.ca.mail.comcast.net ([76.96.30.89]) by qmta09.emeryville.ca.mail.comcast.net with comcast id fvTZ1i0061vN32cA9vYgkJ; Wed, 29 Feb 2012 19:32:40 +0000 Received: from damnhippie.dyndns.org ([24.8.232.202]) by omta22.emeryville.ca.mail.comcast.net with comcast id fvYe1i00e4NgCEG8ivYfsT; Wed, 29 Feb 2012 19:32:39 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id q1TJWbta007780; Wed, 29 Feb 2012 12:32:37 -0700 (MST) (envelope-from freebsd@damnhippie.dyndns.org) From: Ian Lepore To: Jason Hellenthal In-Reply-To: <20120229180010.GA93342@DataIX.net> References: <20120228162447.GB58311@mech-cluster241.men.bris.ac.uk> <20120229072458.GA95427@DataIX.net> <20120229085716.GA66484@mech-cluster241.men.bris.ac.uk> <1330527621.1023.27.camel@revolution.hippie.lan> <20120229164115.GB64201@DataIX.net> <1330535893.1023.49.camel@revolution.hippie.lan> <20120229180010.GA93342@DataIX.net> Content-Type: text/plain; charset="us-ascii" Date: Wed, 29 Feb 2012 12:32:36 -0700 Message-ID: <1330543956.1023.72.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: freebsd-current@freebsd.org, jb Subject: Re: negative group permissions? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Feb 2012 19:32:40 -0000 On Wed, 2012-02-29 at 13:00 -0500, Jason Hellenthal wrote: > > On Wed, Feb 29, 2012 at 10:18:13AM -0700, Ian Lepore wrote: > > On Wed, 2012-02-29 at 11:41 -0500, Jason Hellenthal wrote: > > > > > > On Wed, Feb 29, 2012 at 04:18:45PM +0000, jb wrote: > > > > Ian Lepore damnhippie.dyndns.org> writes: > > > > > > > > > ... > > > > > It's not a > > > > > directory or executable file in the first place, so making it executable > > > > > for everyone except the owner and group is not some sort of subtle > > > > > security trick, it's just meaningless. > > > > > ... > > > > > > > > Is it meaningless ? > > > > > > > > Example: > > > > # cat /var/spool/output/lpd/.seq > > > > #! /usr/local/bin/bash > > > > touch /tmp/jb-test-`echo $$` > > > > > > > > # ls -al /var/spool/output/lpd/.seq > > > > -rw-r----x 1 root daemon 54 Feb 29 17:05 /var/spool/output/lpd/.seq > > > > # /var/spool/output/lpd/.seq > > > > # > > > > # ls /tmp/jb* > > > > /tmp/jb-test-61789 > > > > > > > > # chmod 0640 /var/spool/output/lpd/.seq > > > > # ls -al /var/spool/output/lpd/.seq > > > > -rw-r----- 1 root daemon 52 Feb 29 17:11 /var/spool/output/lpd/.seq > > > > # /var/spool/output/lpd/.seq > > > > su: /var/spool/output/lpd/.seq: Permission denied > > > > # > > > > > > > > > > Giving execute bit to others by security means to allow others to search > > > for that file and find it. If its not there then the process created by > > > current user will not be able to read the file since they are not part > > > of the daemon group. I would assume that sometimes the contents of .seq > > > was judged to be insecure for whatever reason but judged that a user > > > should be able to still in a sense read the file without reading its > > > contents. Negative perms are not harmful. > > > > > > I do suppose a 'daily_status_security_neggrpperm_dirs=' variable should > > > be added here to control which directories are being scanned much like > > > chknoid. > > > > > > > The exec bit's control over the ability to search applies to > > directories, not individual files. For example: > > > > revolution > whoami > > ilepore > > revolution > ll /tmp/test > > -rw-r----x 1 root daemon 0B Feb 29 07:37 /tmp/test* > > > > The file is 0641 and I'm not in the daemon group; I can list it. > > > > The issue is not with listing the file. Setting the execute bit on a > file where there is only a read bit higher up allows for the calling > process to read the contents and noone else. This is special and not a > flaw. > > > Again, the problem here seems to be the use of 0661 in the lpr program, > > not the idea of negative permissions, not the new scan for the use of > > negative permissions. It's just an old bug in an old program which used > > to be harmless and now is "mostly harmless". Instead of trying to "fix" > > it by causing the new scan to ignore it, why don't we fix it by fixing > > the program? (I'd submit a patch but it's a 1-character change -- it's > > not clear to me a patch would be easier for a commiter to handle than > > just finding and changing the only occurrance of "0661" in lpr.c.) > > > > It was intentional and not a flaw. This file should be readable by the > calling process and noone else. This is the way permissions work. > I'm sorry, but I can't make any sense of what you've said here. The file is already readable and writable by the process that creates it. On a second look just now I noticed the seteuid() calls in lpr before and after the file open/create. I thought that might be what you mean, that the process could lose access to the file with the seteuid() call after it's opened. I just tested that, and it doesn't seem to behave that way. I used simple test code similar to lpr.c but using mode 0660 instead of 0661: char buf[128]; int fd; int bytes; errno = 0; pid_t uid = getuid(); pid_t euid = geteuid(); printf("uid %d euid %d\n", uid, euid); seteuid(euid); fd = open("/tmp/test", O_RDWR|O_CREAT, 0660); flock(fd, LOCK_EX); printf("fd = %d errno = %d\n", fd, errno); seteuid(uid); bytes = read(fd, buf, sizeof(buf)); printf("read bytes = %d errno = %d\n", bytes, errno); sprintf(buf, "%03d", 1); bytes = write(fd, buf, strlen(buf)); printf("write bytes = %d errno = %d\n", bytes, errno); close(fd); revolution > ll tester -r-sr-sr-x 1 root daemon 7.4k Feb 29 12:03 tester* revolution > ./tester uid 902 euid 0 fd = 3 errno = 2 read bytes = 0 errno = 2 write bytes = 3 errno = 2 revolution > ll /tmp/test -rw-r----- 1 root wheel 3B Feb 29 12:04 /tmp/test revolution > cat /tmp/test 001 Could it be that once upon a time the second seteuid() call after the file open would have removed the read access for the process which opened the file, but there have been changes over the years that modifed that behavior and left the technique in lpr.c outdated and moot? -- Ian