Re: [kernel-hardening] [RFC] [PATCH 1/5] path_fchdir and path_fhandle LSM hooks

From: Jann Horn <jann@thejh.net>
To: "Reshetova, Elena" <elena.reshetova@intel.com>
Cc: "kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>,
	"linux-security-module@vger.kernel.org"
	<linux-security-module@vger.kernel.org>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"spender@grsecurity.net" <spender@grsecurity.net>,
	"jmorris@namei.org" <jmorris@namei.org>,
	"Schaufler, Casey" <casey.schaufler@intel.com>,
	"Leibowitz, Michael" <michael.leibowitz@intel.com>,
	"Roberts, William C" <william.c.roberts@intel.com>
Subject: Re: [kernel-hardening] [RFC] [PATCH 1/5] path_fchdir and path_fhandle LSM hooks
Date: Sun, 31 Jul 2016 14:02:55 +0200	[thread overview]
Message-ID: <20160731120255.GB14676@pc.thejh.net> (raw)
In-Reply-To: <2236FBA76BA1254E88B949DDB74E612B41B7029C@IRSMSX102.ger.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

On Sun, Jul 31, 2016 at 10:55:04AM +0000, Reshetova, Elena wrote:
> On Fri, Jul 29, 2016 at 10:34:36AM +0300, Elena Reshetova wrote:
> > This introduces two new LSM hooks operating on paths.
> > 
> >   - security_path_fchdir() checks for permission on
> >     changing working directory. It can be used by
> >     LSMs concerned on fchdir system call
> 
> >I don't think security_path_fchdir() is a good abstraction level. It
> neither covers the whole case of "cwd is changed" nor does it cover the
> whole case of "someone uses a file descriptor to a directory to look up
> stuff outside that directory".
> Do you have a suggestion on what can be a good place? 
> 
> >For example, security_path_fchdir() seems to be intended to prevent the use
> of a leaked file descriptor to the outside world for accessing other files
> in the outside world. 
> Yes, this was exactly the use case.
> 
> >But this is trivially bypassed by first using openat() directly instead of
> fchdir()+open() (something that used to work against grsecurity, but was
> fixed quite a while ago).
> The way it has been addressed in grsecurity is having a check inside
> filename_lookup() , but it doesn't look a very great place for putting a
> hook. I was thinking about it , but so far didn't find any other good
> alternatives. 

Yeah, if you want to have such a hook, I think it needs to be in
filename_lookup() or below - but that's a relatively hot function, so it
might have a measurable performance impact.

Alternatively, you could forbid double-chroots and use the LSM hooks for
file descriptor passing via unix domain sockets and binder to check
incoming file descriptors. As long as no directories are moved out of the
chroot directory by something outside the chroot (a process chrooted into
a parent directory or a process that isn't chrooted), that should
prooobably work?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]