Date:	Fri, 13 Jul 2007 04:42:06 +0200
From:	Zeus Gómez Marmolejo <zeus@...zina.org>
To:	casey@...aufler-ca.com
CC:	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: New LSM security operation

> 
> An application that is careful not to destroy existing information
> would not be able to prevent itself from doing so if the file is
> hidden. Its pretty important to avoid defeating programs that are
> trying to behave properly.
> 

It's true what you are saying... But I think the best way to know if 
there is an existing file with that name is by the stat() syscall, not 
reading the contents of the directory, by the way is very inefficient if 
there are a lot of files in that directory.

For example, the editor "vi" when editing a file "p1.c" and saving with 
":w p2.c" it searches first if this file exists with:

stat64("p2.c", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0

(... just verified it with strace). I think doing this is the best way, 
and most programs do that in this way.

On the other hand, I'm only talking about modifying the: "sys_readdir", 
"sys_getdents" and "sys_getdents64" system calls, all implemented in the 
"fs/readdir.c" file.

In fact, I've tried to code it, and I think it's best done in the 
"filldir()" callback, called each time that an entry in the directory is 
found by the underlying filesystem walk. Just check if this entry is 
permitted to list and then return without adding it into the buffer. I 
tried this (in fs/readdir.c, filldir64()):

static int filldir64(void * __buf, const char * name, int namlen,
                      loff_t offset, u64 ino, unsigned int d_type)
{
   struct linux_dirent64 __user *dirent;
   struct getdents_callback64 * buf = (struct getdents_callback64 *) __buf;
   int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 1, sizeof(u64));
+ int res;
+
+ res = security_inode_list( buf->i_dir, iget(buf->sb, ino) );
+ if (res)
+   return 0;

   buf->error = -EINVAL;   /* only used if we fail.. */
   ...
}

Adding two members in the callback structure (sb and i_dir) with the 
superblock and inode of the parent directory.

This works in all filesystems with a superblock, but for example in 
sysfs, the operation iget() segfaults because can't search for an inode 
using the superblock operations, because it doesn't have the 
read_inode() operation.

Is there any other way to obtain the "struct inode *" from the inode 
number, without using the superblock operations!?

> 
> In the open-for-read case returning an error other than ENOENT gives
> away the existance of the file. Returning ENOENT is a lie, so is just
> wrong.
> 

OK. But maybe is what the administrator wants to return to the user. If 
the file is not readable or writable and not listable by some user, then 
for him simply doesn't exist... But, I don't care... if we are 
consistent with what we are doing, the correct thing is to return an 
EACCES because of the forbidding read.

> 
> Sounds to me like the problem you're out to solve is specific to
> ls under SELinux. Fixing* ls is straitforward, don't say anything
> about directory entries you don't have access to rather than the
> current annoying error messages. Fixing* SELinux may be an option,
> you may even be able to achieve what you're after using the current
> mechanisms. The SELinux mailing list might be worth cross-posting.
> 


I don't think this is a "ls" problem. The fact is if some user wants to 
know the pid names or some other directory entries, he just have to 
compile his version of "ls" and would get all this names because the 
getdents() system call is showing them.

I think they are different policies, one is getting the attributes of a 
given file (that is what SELinux can forbid) and then this is why "ls 
-l" complains about showing them. But the other policy is just not 
showing the file, but you may get the permissions if you really know the 
name: "ls -l /etc/abadbad", this is not possible with SELinux nor with 
other security module because the Linux SM framework just lack the hook 
to avoid doing this listing.

Thank you for your comments!!


Zeus.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists