LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@fys.uio.no>
To: Andreas Gruenbacher <agruen@suse.de>
Cc: Christoph Hellwig <hch@infradead.org>, Tony Jones <tonyj@suse.de>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	chrisw@sous-sol.org, linux-security-module@vger.kernel.org,
	viro@zeniv.linux.org.uk
Subject: Re: [RFC 0/28] Patches to pass vfsmount to LSM inode security hooks
Date: Tue, 06 Feb 2007 00:51:52 -0800	[thread overview]
Message-ID: <1170751912.6242.30.camel@lade.trondhjem.org> (raw)
In-Reply-To: <200702051920.36057.agruen@suse.de>

On Mon, 2007-02-05 at 19:20 -0800, Andreas Gruenbacher wrote:
> On Monday 05 February 2007 11:02, Christoph Hellwig wrote:
> > On Mon, Feb 05, 2007 at 10:58:26AM -0800, Trond Myklebust wrote:
> > > On Mon, 2007-02-05 at 18:44 +0000, Christoph Hellwig wrote:
> > > > Just FYI:  Al was very opposed to the idea of passing the vfsmount to
> > > > the vfs_ helpers, so you should discuss this with him.
> > > >
> > > > Looking at the actual patches I see you're lazy in a lot of places.
> > > > Please make sure that when you introduce a vfsmount argument somewhere
> > > > that it is _always_ passed and not just when it's conveniant.  Yes,
> > > > that's more work, but then again if you're not consistant anyone
> > > > half-serious will laught at a security model using this infrasturcture.
> > >
> > > nfsd in particular tends to be a bit lazy about passing around vfsmount
> > > info. Forcing it to do so should not be hard since the vfsmount is
> > > already cached in the "struct export" (which can be found using the
> > > filehandle). It will take a bit of re-engineering in order to pass that
> > > information around inside the nfsd code, though.
> >
> > I actually have a patch to fix that.  It's part of a bigger series
> > that's not quite ready, but I hope to finish all of it this month.
> 
> It's actually not hard to "fix", and nfsd would look a little less weird. But 
> what would this add, what do pathnames mean in the context of nfsd, and would 
> nfsd actually become less weird?
> 
> On the wire, nfs transmits file handles, not filenames. The file handle 
> usually contains the inode numbers of the file and the containing directory.
> 
> The code in fs/exportfs/expfs.c:find_exported_dentry() shows that fh_verify() 
> should return a dentry that may be connected or not, depending on the export 
> options and whether it's a file or directory. Together with the vfsmount from 
> the svc_export, we could compute a pathname.
> 
> But there is no way to tell different hardlinks to the same inode in the same 
> directory from each other (both the file and directory inode are the same), 
> and depending on the export options, we may or may not be able to distinguish 
> different hardlinks across directories.

Who cares? There is no way to export a partial directory, and in any
case the subtree_check crap is borken beyond repair (see cross-directory
renames which lead to actual changes to the filehandle - broken, broken,
broken!!!!).

> If the nohide or crossmnt export options are used, we might run into similar 
> aliasing issues with mounts (I'm not sure about this).

Wrong. Those create new export points since you are crossing into a
different filesystem.

> So we could compute pathnames, but they wouldn't be very meaningful. Instead 
> of going that way, it seems more reasonable to not do pathname based access 
> control for the kernel kernel nfsd at all. Passing NULL as the vfsmounts does 
> that. With a properly configured nfsd, the areas that remote users could 
> access would still be well confined.

Huh? Even if you don't pass in a vfsmount, you _still_ need to pass in a
super_block. Inodes are only unique per filesystem.
In fact, on an ideal NFS export, there is no ambiguity between the two
(see above comment about subtree_check) since the entire filesystem will
be exported.

> The focus with this whole pathname based security stuff really is to be able 
> to better confine local processes. The kernel nfsd is a kernel thread, and as 
> such, confining it from a security point of view cannot really work, anyway.
> 
> > > Note also that it might be nice to enforce the vfsmount argument by
> > > replacing the existing dentry parameters with a struct path instead of
> > > adding an extra reference to the vfsmount to existing functions.
> >
> > That definitly sounds like a good idea, independent of whether we want
> > to pass the vfsmount in more places or not.
> 
> Note that you can still pass a NULL vfsmount in a struct path.

...but we will have Dick Cheney track you down and shoot you if you do.
The point is that you only have to check _one_ argument (the
initialisation of the struct path) instead of having a check for every
function argument in the vfs.

Trond


  reply	other threads:[~2007-02-06  8:52 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-05 18:22 Tony Jones
2007-02-05 18:22 ` [RFC 1/28] Pass struct vfsmount to the inode_create LSM hook Tony Jones
2007-02-05 18:22 ` [RFC 2/28] Remove redundant check from proc_setattr() Tony Jones
2007-02-05 19:16   ` Chris Wright
2007-02-05 18:22 ` [RFC 3/28] Pass struct file down to remove_suid and children Tony Jones
2007-02-05 18:22 ` [RFC 4/28] Add a vfsmount parameter to notify_change() Tony Jones
2007-02-05 18:23 ` [RFC 5/28] Pass struct vfsmount to the inode_setattr LSM hook Tony Jones
2007-02-05 18:23 ` [RFC 6/28] Add struct vfsmount parameter to vfs_mkdir() Tony Jones
2007-02-05 18:23 ` [RFC 7/28] Pass struct vfsmount to the inode_mkdir LSM hook Tony Jones
2007-02-05 18:23 ` [RFC 8/28] Add a struct vfsmount parameter to vfs_mknod() Tony Jones
2007-02-05 18:23 ` [RFC 9/28] Pass struct vfsmount to the inode_mknod LSM hook Tony Jones
2007-02-05 18:23 ` [RFC 10/28] Add a struct vfsmount parameter to vfs_symlink() Tony Jones
2007-02-05 18:23 ` [RFC 11/28] Pass struct vfsmount to the inode_symlink LSM hook Tony Jones
2007-02-05 18:24 ` [RFC 12/28] Pass struct vfsmount to the inode_readlink " Tony Jones
2007-02-05 18:24 ` [RFC 13/28] Add struct vfsmount parameters to vfs_link() Tony Jones
2007-02-05 18:24 ` [RFC 14/28] Pass struct vfsmount to the inode_link LSM hook Tony Jones
2007-02-05 18:24 ` [RFC 15/28] Add a struct vfsmount parameter to vfs_rmdir() Tony Jones
2007-02-05 18:24 ` [RFC 16/28] Pass struct vfsmount to the inode_rmdir LSM hook Tony Jones
2007-02-05 18:24 ` [RFC 17/28] Add a struct vfsmount parameter to vfs_unlink() Tony Jones
2007-02-05 18:25 ` [RFC 18/28] Pass struct vfsmount to the inode_unlink LSM hook Tony Jones
2007-02-05 18:25 ` [RFC 19/28] Add struct vfsmount parameters to vfs_rename() Tony Jones
2007-02-05 18:25 ` [RFC 20/28] Pass struct vfsmount to the inode_rename LSM hook Tony Jones
2007-02-05 18:25 ` [RFC 21/28] Add a struct vfsmount parameter to vfs_setxattr() Tony Jones
2007-02-05 18:25 ` [RFC 22/28] Pass struct vfsmount to the inode_setxattr LSM hook Tony Jones
2007-02-05 18:25 ` [RFC 23/28] Add a struct vfsmount parameter to vfs_getxattr() Tony Jones
2007-02-05 18:25 ` [RFC 24/28] Pass struct vfsmount to the inode_getxattr LSM hook Tony Jones
2007-02-05 18:26 ` [RFC 25/28] Add a struct vfsmount parameter to vfs_listxattr() Tony Jones
2007-02-05 18:26 ` [RFC 26/28] Pass struct vfsmount to the inode_listxattr LSM hook Tony Jones
2007-02-05 18:26 ` [RFC 27/28] Add a struct vfsmount parameter to vfs_removexattr() Tony Jones
2007-02-05 18:26 ` [RFC 28/28] Pass struct vfsmount to the inode_removexattr LSM hook Tony Jones
2007-02-05 18:44 ` [RFC 0/28] Patches to pass vfsmount to LSM inode security hooks Christoph Hellwig
2007-02-05 18:58   ` Trond Myklebust
2007-02-05 19:02     ` Christoph Hellwig
2007-02-06  3:20       ` Andreas Gruenbacher
2007-02-06  8:51         ` Trond Myklebust [this message]
2007-02-06  9:48           ` Christoph Hellwig
2007-02-06 10:31             ` Neil Brown
2007-02-07  9:25           ` Andreas Gruenbacher
2007-02-06  9:47         ` Christoph Hellwig
2007-02-06 10:26           ` Neil Brown
2007-02-06 10:37             ` Christoph Hellwig
2007-02-12 18:32               ` J. Bruce Fields
2007-02-07  9:58           ` Andreas Gruenbacher
2007-02-07 12:11             ` Christoph Hellwig
2007-02-05 19:15     ` Chris Wright
2007-02-06  0:44   ` Andreas Gruenbacher
2007-02-06  2:13   ` Andreas Gruenbacher
2007-02-06  9:52     ` Christoph Hellwig
2007-02-07  9:04       ` Andreas Gruenbacher
2007-02-06 12:55     ` Stephen Smalley
2007-02-07  8:55       ` Andreas Gruenbacher
2007-02-07 15:43         ` Chris Wright
2007-02-07 16:06           ` Stephen Smalley
2007-02-07 16:25           ` Jeff Mahoney
2007-02-07 19:55             ` Andreas Gruenbacher
2007-02-05 19:26 ` Casey Schaufler
2007-02-05 19:39   ` Arjan van de Ven
2007-02-05 19:50   ` Chris Wright
2007-02-05 20:23     ` Casey Schaufler
2007-02-06  2:30     ` Andreas Gruenbacher
2007-02-06 14:20     ` Tetsuo Handa

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=1170751912.6242.30.camel@lade.trondhjem.org \
    --to=trond.myklebust@fys.uio.no \
    --cc=agruen@suse.de \
    --cc=chrisw@sous-sol.org \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=tonyj@suse.de \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [RFC 0/28] Patches to pass vfsmount to LSM inode security hooks' \
    /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

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).