On 03/20/2014 09:38 AM, [email protected] wrote:
> On Thu, Mar 20, 2014 at 04:48:30PM +0100, David Herrmann wrote:
>> On Thu, Mar 20, 2014 at 4:32 PM, <[email protected]> wrote:
>>> Why not make sealing an attribute of the "struct file", and enforce it
>>> at the VFS layer? That way all file system objects would have access
>>> to sealing interface, and for memfd_shmem, you can't get another
>>> struct file pointing at the object, the security properties would be
>>> identical.
>>
>> Sealing as introduced here is an inode-attribute, not "struct file".
>> This is intentional. For instance, a gfx-client can get a read-only FD
>> via /proc/self/fd/ and pass it to the compositor so it can never
>> overwrite the contents (unless the compositor has write-access to the
>> inode itself, in which case it can just re-open it read-write).
>
> Hmm, good point. I had forgotten about the /proc/self/fd hole.
> Hmm... what if we have a SEAL_PROC which forces the permissions of
> /proc/self/fd to be 000?
This is the second time in a week that someone has asked for a way to
have a struct file (or struct inode or whatever) that can't be reopened
through /proc/pid/fd. This should be quite easy to implement as a
separate feature.
Actually, that feature would solve a major pet peeve of mine, I think: I
want something like memfd that allows me to keep the thing read-write
but that whomever I pass the fd to can't change. With this feature, I
could do:
fd_rw = memfd_create (or O_TMPFILE or whatever)
fd_ro = open(/proc/self/fd/fd_ro, O_RDONLY);
fcntl(fd_ro, F_RESTRICT, F_RESTRICT_REOPEN);
send fd_ro via SCM_RIGHTS.
To really make this work well, I also want to SEAL_SHRINK the inode so
that the receiver can verify that I'm not going to truncate the file out
from under it.
Bingo, fast and secure one-way IPC.
--Andy
On Thu, Apr 10, 2014 at 12:14:27PM -0700, Andy Lutomirski wrote:
>
> This is the second time in a week that someone has asked for a way to
> have a struct file (or struct inode or whatever) that can't be reopened
> through /proc/pid/fd. This should be quite easy to implement as a
> separate feature.
What I suggested on a different thread was to add the following new
file descriptor flags, to join FD_CLOEXEC, which would be maniuplated
using the F_GETFD and F_SETFD fcntl commands:
FD_NOPROCFS disallow being able to open the inode via /proc/<pid>/fd
FD_NOPASSFD disallow being able to pass the fd via a unix domain socket
FD_LOCKFLAGS if this bit is set, disallow any further changes of FD_CLOEXEC,
FD_NOPROCFS, FD_NOPASSFD, and FD_LOCKFLAGS flags.
Regardless of what else we might need to meet the use case for the
proposed File Sealing API, I think this is a useful feature that could
be used in many other contexts besides just the proposed
memfd_create() use case.
Cheers,
- Ted
On Thu, Apr 10, 2014 at 1:32 PM, Theodore Ts'o <[email protected]> wrote:
> On Thu, Apr 10, 2014 at 12:14:27PM -0700, Andy Lutomirski wrote:
>>
>> This is the second time in a week that someone has asked for a way to
>> have a struct file (or struct inode or whatever) that can't be reopened
>> through /proc/pid/fd. This should be quite easy to implement as a
>> separate feature.
>
> What I suggested on a different thread was to add the following new
> file descriptor flags, to join FD_CLOEXEC, which would be maniuplated
> using the F_GETFD and F_SETFD fcntl commands:
>
> FD_NOPROCFS disallow being able to open the inode via /proc/<pid>/fd
>
> FD_NOPASSFD disallow being able to pass the fd via a unix domain socket
>
> FD_LOCKFLAGS if this bit is set, disallow any further changes of FD_CLOEXEC,
> FD_NOPROCFS, FD_NOPASSFD, and FD_LOCKFLAGS flags.
>
> Regardless of what else we might need to meet the use case for the
> proposed File Sealing API, I think this is a useful feature that could
> be used in many other contexts besides just the proposed
> memfd_create() use case.
It occurs to me that, before going nuts with these kinds of flags, it
may pay to just try to fix the /proc/self/fd issue for real -- we
could just make open("/proc/self/fd/3", O_RDWR) fail if fd 3 is
read-only. That may be enough for the file sealing thing.
--Andy
Hi
On Thu, Apr 10, 2014 at 10:37 PM, Andy Lutomirski <[email protected]> wrote:
> It occurs to me that, before going nuts with these kinds of flags, it
> may pay to just try to fix the /proc/self/fd issue for real -- we
> could just make open("/proc/self/fd/3", O_RDWR) fail if fd 3 is
> read-only. That may be enough for the file sealing thing.
For the sealing API, none of this is needed. As long as the inode is
owned by the uid who creates the memfd, you can pass it around and
no-one besides root and you can open /proc/self/fd/$fd (assuming chmod
700). If you share the fd with someone with the same uid as you,
you're screwed anyway. We don't protect users against themselves (I
mean, they can ptrace you, or kill()..). Therefore, I'm not really
convinced that we want this for memfd. At least no-one has provided a
_proper_ use-case for this so far.
Thanks
David
On Thu, Apr 10, 2014 at 1:49 PM, David Herrmann <[email protected]> wrote:
> Hi
>
> On Thu, Apr 10, 2014 at 10:37 PM, Andy Lutomirski <[email protected]> wrote:
>> It occurs to me that, before going nuts with these kinds of flags, it
>> may pay to just try to fix the /proc/self/fd issue for real -- we
>> could just make open("/proc/self/fd/3", O_RDWR) fail if fd 3 is
>> read-only. That may be enough for the file sealing thing.
>
> For the sealing API, none of this is needed. As long as the inode is
> owned by the uid who creates the memfd, you can pass it around and
> no-one besides root and you can open /proc/self/fd/$fd (assuming chmod
> 700). If you share the fd with someone with the same uid as you,
> you're screwed anyway. We don't protect users against themselves (I
> mean, they can ptrace you, or kill()..). Therefore, I'm not really
> convinced that we want this for memfd. At least no-one has provided a
> _proper_ use-case for this so far.
Hmm. Fair enough.
Would it make sense for the initial mode on a memfd inode to be 000?
Anyone who finds this to be problematic could use fchmod to fix it.
I might even go so far as to suggest that the default uid on the inode
should be 0 (i.e. global root), since there is the odd corner case of
root setting euid != 0, creating a memfd, and setting euid back to 0.
The latter might cause resource accounting issues, though.
--Andy
Hi
On Thu, Apr 10, 2014 at 11:16 PM, Andy Lutomirski <[email protected]> wrote:
> Would it make sense for the initial mode on a memfd inode to be 000?
> Anyone who finds this to be problematic could use fchmod to fix it.
memfd_create() should be subject to umask() just like anything else.
That should solve any possible race here, right?
Thanks
David
On Thu, Apr 10, 2014 at 3:57 PM, David Herrmann <[email protected]> wrote:
> Hi
>
> On Thu, Apr 10, 2014 at 11:16 PM, Andy Lutomirski <[email protected]> wrote:
>> Would it make sense for the initial mode on a memfd inode to be 000?
>> Anyone who finds this to be problematic could use fchmod to fix it.
>
> memfd_create() should be subject to umask() just like anything else.
> That should solve any possible race here, right?
Yes, but how many people will actually think about umask when doing
things that don't really look like creating files?
/proc/pid/fd is a really weird corner case in which the mode of an
inode that doesn't have a name matters. I suspect that almost no one
will ever want to open one of these things out of /proc/self/fd, and
those who do should be made to think about it.
It also avoids odd screwups where things are secure until someone runs
them with umask 000.
--Andy
Hi
On Fri, Apr 11, 2014 at 1:05 AM, Andy Lutomirski <[email protected]> wrote:
> /proc/pid/fd is a really weird corner case in which the mode of an
> inode that doesn't have a name matters. I suspect that almost no one
> will ever want to open one of these things out of /proc/self/fd, and
> those who do should be made to think about it.
I'm arguing in the context of memfd, and there's no security leak if
people get access to the underlying inode (at least I'm not aware of
any). As I said, context information is attached to the inode, not
file context, so I'm fine if people want to open multiple file
contexts via /proc. If someone wants to forbid open(), I want to hear
_why_. I assume the memfd object has uid==uid-of-creator and
mode==(777 & ~umask) (which usually results in X00, so no access for
non-owners). I cannot see how /proc is a security issue here.
Thanks
David
On Thu, Apr 10, 2014 at 4:16 PM, David Herrmann <[email protected]> wrote:
> Hi
>
> On Fri, Apr 11, 2014 at 1:05 AM, Andy Lutomirski <[email protected]> wrote:
>> /proc/pid/fd is a really weird corner case in which the mode of an
>> inode that doesn't have a name matters. I suspect that almost no one
>> will ever want to open one of these things out of /proc/self/fd, and
>> those who do should be made to think about it.
>
> I'm arguing in the context of memfd, and there's no security leak if
> people get access to the underlying inode (at least I'm not aware of
> any).
I'm not sure what you mean.
> As I said, context information is attached to the inode, not
> file context, so I'm fine if people want to open multiple file
> contexts via /proc. If someone wants to forbid open(), I want to hear
> _why_. I assume the memfd object has uid==uid-of-creator and
> mode==(777 & ~umask) (which usually results in X00, so no access for
> non-owners). I cannot see how /proc is a security issue here.
On further reflection, my argument for 000 is crap. As far as I can
see, the only time that the mode matters at all when playing with
/proc/pid/fd, and they only way to get a non-O_RDWR memfd is using
/proc/pid/fd, so I'll argue for 0600 instead.
Argument why 0600 is better than 0600 & ~umask: either callers don't
care because the inode mode simply doesn't matter or they're using
/proc/pid/fd to *reduce* permissions, in which case they'd probably
like to avoid having to play with umask or call fchmod.
Argument why 0600 is better than 0777 & ~umask: People /prod/pid/fd
are the only ones who care, in which case they probably prefer for the
permissions not be increased by other users if they give them a
reduced-permission fd.
Anyway, this is all mostly unimportant. Some text in the man page is
probably sufficient, but I still think that 0600 is trivial to
implement and a little bit more friendly.
--Andy
>
> Thanks
> David
--
Andy Lutomirski
AMA Capital Management, LLC
On Thu 2014-04-10 13:37:26, Andy Lutomirski wrote:
> On Thu, Apr 10, 2014 at 1:32 PM, Theodore Ts'o <[email protected]> wrote:
> > On Thu, Apr 10, 2014 at 12:14:27PM -0700, Andy Lutomirski wrote:
> >>
> >> This is the second time in a week that someone has asked for a way to
> >> have a struct file (or struct inode or whatever) that can't be reopened
> >> through /proc/pid/fd. This should be quite easy to implement as a
> >> separate feature.
> >
> > What I suggested on a different thread was to add the following new
> > file descriptor flags, to join FD_CLOEXEC, which would be maniuplated
> > using the F_GETFD and F_SETFD fcntl commands:
> >
> > FD_NOPROCFS disallow being able to open the inode via /proc/<pid>/fd
> >
> > FD_NOPASSFD disallow being able to pass the fd via a unix domain socket
> >
> > FD_LOCKFLAGS if this bit is set, disallow any further changes of FD_CLOEXEC,
> > FD_NOPROCFS, FD_NOPASSFD, and FD_LOCKFLAGS flags.
> >
> > Regardless of what else we might need to meet the use case for the
> > proposed File Sealing API, I think this is a useful feature that could
> > be used in many other contexts besides just the proposed
> > memfd_create() use case.
>
> It occurs to me that, before going nuts with these kinds of flags, it
> may pay to just try to fix the /proc/self/fd issue for real -- we
> could just make open("/proc/self/fd/3", O_RDWR) fail if fd 3 is
> read-only. That may be enough for the file sealing thing.
Yes please.
Current behaviour is very unexpected, and unexpected behaviour in
security area is normally called "security hole".
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On 04/10/2014 10:37 PM, Andy Lutomirski wrote:
> It occurs to me that, before going nuts with these kinds of flags, it
> may pay to just try to fix the /proc/self/fd issue for real -- we
> could just make open("/proc/self/fd/3", O_RDWR) fail if fd 3 is
> read-only. That may be enough for the file sealing thing.
Increasing privilege on O_PATH descriptors via access through
/proc/self/fd is part of the userspace API. The same thing might be
true for O_RDONLY descriptors, but it's a bit less likely that there are
any users out there. In any case, I'm not sure it makes sense to plug
the O_RDONLY hole while leaving the O_PATH hole open.
--
Florian Weimer / Red Hat Product Security Team