Re: SELinux metadata protection

From: Serge E. Hallyn (serue@private)
Date: Sun Jan 01 2006 - 07:38:56 PST


Quoting KaiGai Kohei (kaigai@private):
> It seems a bit curious behavior for me. Why can an unauthorized process
> be allowed to know whether the file exists or not ?
> I think it's worthwhile to conceal the existence of files from unauthorized
> processes.

This behavior is also necessary for meeting the new medium robustness
protection profiles.

> Thanks, any comments please, and have a good year :)

Haven't looked at the patch in detail, but the following stood out to me:

> --- linux-2.6.14.5-selinux/fs/readdir.c	2005-12-26 19:26:33.000000000 -0500
> +++ linux-2.6.14.5-selinux.mp/fs/readdir.c	2005-12-29 20:26:55.000000000 -0500
> @@ -33,7 +33,11 @@ int vfs_readdir(struct file *file, filld
>  	down(&inode->i_sem);
>  	res = -ENOENT;
>  	if (!IS_DEADDIR(inode)) {
> -		res = file->f_op->readdir(file, buf, filler);
> +		/* NOTE:
> +		   When LSM was not enable, security_file_readdir()
> +		   is same as 'file->f_op->readdir()'. 
> +		*/
> +		res = security_file_readdir(file, buf, filler);
>  		file_accessed(file);

I don't like this - the dir->f_op->readdir should not be done inside
a function which claims to be a security check.  Plus the added code
doesn't have a return value of it's own.  So why not stay closer to usual
linux code and do something like

	security_prep_readdir();
	res = file->f_op->readdir(file, buf, filler);

inside vfs_readdir(), where security_prep_readdir() is defined away
in the non-LSM case, and is 

> +static inline void security_file_readdir (struct file *dir, void *buffer, filldir_t filler)
> +{
> +	security_filldir_t private;
> +
> +	private.dir = dir;
> +	private.buffer = buffer;
> +	private.filler = filler;

in the LSM case?

thanks,
-serge



This archive was generated by hypermail 2.1.3 : Sun Jan 01 2006 - 15:30:32 PST