linux-kernel - Re: [PATCH 24/32] vfs: syscall: Add fsopen() to prepare for superblock creation [ver #9]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFw7ACqm4fzG_52xTR3B-MqB0hLvcprgjm3jayPDDn6=-A@mail.gmail.com>
Date:   Thu, 12 Jul 2018 14:40:49 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     David Howells <dhowells@...hat.com>
Cc:     Andrew Lutomirski <luto@...nel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Linux API <linux-api@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jann Horn <jannh@...gle.com>
Subject: Re: [PATCH 24/32] vfs: syscall: Add fsopen() to prepare for
 superblock creation [ver #9]

On Thu, Jul 12, 2018 at 2:26 PM David Howells <dhowells@...hat.com> wrote:
>
> The problem is that there's more than one actual "open" involved.

No. The problem is "write()".

This is not about open, about fsopen, or about anything at all.

This is about the fact that "write()" by definition can happen in a
different - and unexpected - context. Whether that be due to suid or
due to splice, or due to any other random issue is entirely
immaterial.

(The same is true of "read()" too, but very few people try to make
"read()" have side effects, so it's less of an issue. It does happen,
though).

But once you have another interface than "read/write()", the issues go
away. Those other interfaces are synchronous, and now you can decide
"ok, I'll just use current creds".

>  (1) Pass the creds from ->get_tree() all the way down into pathwalk and make
>      sure *every* check that pathwalk does uses it.

No. See above.

If your write() does anything but buffering data, it's not getting merged.

>  (2) When do_the_create_thing() is invoked, it wraps the call to ->get_tree()
>      with override_creds(file->f_cred).

No.

We do not wrap creds in any case. It's just asking for *another* kind
of security issue, where you fool some higher-security thing into
giving you access because it wrapped the higher-security case instead.

>  (3) Forget using an fd to refer to the context.  fsopen() takes absolutely
>      everything, perhaps as a kv array and spits out an O_PATH fd.

That works.

Or you know - do what I told you to do ALL THE TIME, which was to not
use write(), or to only buffer things with write().

But yes, any option that simply avoids read and write is fine.

You can even have a file descriptor. We already have file descriptors
that cannot be read from or written to. It's quite common for special
devices, the whole "open /dev/floppy with O_NONBLOCK only to be able
to do control operations with it" goes back to pretty much day #1.

More recently, we have the whole "FMODE_PATH" kind of file descriptor,
which works as a directory entry, but not for read and write.

So file descriptors can have very useful properties.

But no. We do not use "write()" to implement actions. If you think you
need to check permissions and think you need a "cred", then you're not
using write(). It really is that simple.

Not using write just avouds *all* the problems. If you can fool a suid
application to do arbitrary system calls for you, then it's not the
system call that is the security problem.

                Linus