linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruen@suse.de>
To: Al Viro <viro@ftp.linux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	David Howells <dhowells@redhat.com>,
	sds@tycho.nsa.gov, casey@schaufler-ca.com,
	linux-fsdevel@vger.kernel.org, nfsv4@linux-nfs.org,
	linux-kernel@vger.kernel.org, selinux@tycho.nsa.gov,
	linux-security-module@vger.kernel.org
Subject: Re: Adding a security parameter to VFS functions
Date: Fri, 17 Aug 2007 20:05:06 +0200	[thread overview]
Message-ID: <200708172005.06731.agruen@suse.de> (raw)
In-Reply-To: <20070816233412.GO21089@ftp.linux.org.uk>

On Friday 17 August 2007 01:34, Al Viro wrote:
> On Thu, Aug 16, 2007 at 03:57:24PM -0700, Linus Torvalds wrote:
> > I personally consider this an affront to everythign that is decent.
> > 
> > Why the *hell* would mkdir() be so magical as to need something like that?
> > 
> > Make it something sane, like a "struct nameidata" instead, and make it at 
> > least try to look like the path creation that is done by "open()".  Or 
> > create a "struct file *" or something.
> 
> No.  I agree that mkdir/mknod/etc. should all take the same thing.
> *However*, it should not be nameidata or file.  Why?  Because it
> should not contain vfsmount.  Object creation should not *care*
> where fs is mounted.  At all. The same goes for open(), BTW - letting it see
> the reference to vfsmount via nameidata is bloody wrong and I really
> couldn't care less what kinds of perversions apparmour crowd might like -
> pathnames simply do not belong there.

At the filesystem layer I mostly agree with you -- ordinary filesystems don't 
have a real business with vfsmounts. At the vfs / lsm layer it's a different 
story though. The vfs is not only about physical object creation alone, but 
also about lookups and security checks (lookup related or not). Consider an 
example:

The same filesystem is bind mounted at /foo/mnt and /bar/mnt. A process tries 
to create /foo/mnt/file or /bar/mnt/file. Depending on the permissions 
of /foo and /bar, the operations may succeed or fail independently. Both 
paths point at the same file, but they may grant different permissions (and 
nobody in their right mind would require them to be equivalent).

The checks we are doing in LSM hooks like security_inode_mknod are pretty 
similar -- we are taking the entire paths to objects into account, and so 
different paths to the same object may result in different permissions. The 
major difference is this:

 * The vfs performs lookups in forward direction, and incrementally. Arbitrary
   amounts of time can pass from when a lookup starts at the root to when it
   reaches a final object. (Think of things like mkdirat, but also the pwd,
   which is basically the same as a directory file handle). The vfs doesn't
   care when directories above the current lookup position become
   inaccessible, renamed, or moved.

 * We need to determine whether an object may be accessed at the time of the
   access, based on the location of the object in the filesystem namespace
   (i.e., dentry and vfsmount). And *this* is why we need the vfsmounts
   in the LSM hooks (and also in the vfs_* functions which are calling the
   hooks) [*].

We are not arguing to pass the vfsmount down the inode operations. At least
some of the LSM hooks are already being passed the vfsmounts, so we are not
even asking for something entirely new and unheard of.

By not letting the LSM hooks know about the vfsmounts you are effectively 
making pathname-based access control impossible. I would expect arguments 
more technically sound than "bloody wrong" and "couldn't care less" for 
killing an entire category of LSMs that lots of people find rather useful.


Thanks,
Andreas


[*] In the current implementation we are computing the entire paths to objects
    with d_path(). We cannot do this incrementally during lookups because
   directories along the path to an object can be renamed or moved around
   any time before the actual access. It doesn't seem hard to add caching so
   that we'll have to do this much less frequently, and maybe we can sometimes
   determine that the pwd hasn't changed and start checking from there, but
   all of this wouldn't change the fundamental model or the slow path / worst
   case.

-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE Linux Products GmbH

  reply	other threads:[~2007-08-17 18:05 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-15 11:40 Adding a security parameter to VFS functions David Howells
2007-08-15 16:23 ` Casey Schaufler
2007-08-15 16:52   ` David Howells
2007-08-16 22:20   ` Andreas Gruenbacher
2007-08-16 22:36 ` Andreas Gruenbacher
2007-08-16 22:57 ` Linus Torvalds
2007-08-16 23:30   ` Kyle Moffett
2007-08-16 23:34   ` Al Viro
2007-08-17 18:05     ` Andreas Gruenbacher [this message]
2007-08-20 12:09 ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200708172005.06731.agruen@suse.de \
    --to=agruen@suse.de \
    --cc=casey@schaufler-ca.com \
    --cc=dhowells@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=nfsv4@linux-nfs.org \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@tycho.nsa.gov \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ftp.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).