Re: negative group permissions?

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 <freebsd <at> 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