On Fri, Jun 26, 2020 at 01:58:35PM +0900, Tetsuo Handa wrote:

On 2020/06/26 10:51, Alexei Starovoitov wrote:

quoted

On Thu, Jun 25, 2020 at 06:36:34PM -0700, Linus Torvalds wrote:

quoted

On Thu, Jun 25, 2020 at 12:34 PM David Miller [off-list ref] wrote:

quoted

It's kernel code executing in userspace.  If you don't trust the
signed code you don't trust the signed code.

Nothing is magic about a piece of code executing in userspace.

Well, there's one real issue: the most likely thing that code is going
to do is execute llvm to generate more code.

Wow! Are we going to allow execution of such complicated programs?

No. llvm was _never_ intended to be run from the blob.
bpfilter was envisioned as self contained binary. If it needed to do
optimizations on generated bpf code it would have to do them internally.

I was hoping that fork_usermode_blob() accepts only simple program
like the content of "hello64" generated by

pretty much. statically compiled elf that is self contained.

For example, a usermode process started by fork_usermode_blob() which was initially
containing

----------
while (read(0, &uid, sizeof(uid)) == sizeof(uid)) {
    if (uid == 0)
        write(1, "OK\n", 3);
    else
        write(1, "NG\n", 3);
}
----------

can be somehow tampered like

----------
while (read(0, &uid, sizeof(uid)) == sizeof(uid)) {
    if (uid != 0)
        write(1, "OK\n", 3);
    else
        write(1, "NG\n", 3);
}
----------

due to interference from the rest of the system, how can we say "we trust kernel
code executing in userspace" ?

I answered this already in the previous email.
Use security_ptrace_access_check() LSM hook to make sure that no other process
can tamper with blob's memory when it's running as user process.
In the future it would be trivial to add a new ptrace flag to
make sure that blob's memory is not ptraceable from the start.

My question is: how is the byte array (which was copied from kernel space) kept secure/intact
under "root can poke into kernel or any process memory." environment? It is obvious that
we can't say "we trust kernel code executing in userspace" without some mechanism.

Already answered.

Currently fork_usermode_blob() is not providing security context for the byte array to be
executed. We could modify fork_usermode_blob() to provide security context for LSMs, but
I'll be more happy if we can implement that mechanism without counting on in-tree LSMs, for
SELinux is too complicated to support.

I'm pretty sure it was answered in the upthread by selinux folks.
Quick recap: we can add security labels, sha, strings, you_name_it to the blob that
lsm hooks can track.
We can also add another LSM hook to fork_usermode_blob(), so if tomoyo is so worried
about blobs it would be able to reject all of them without too much work.

quoted

I think that's Tetsuo's point about lack of LSM hooks is kernel_sock_shutdown().
Obviously, kernel_sock_shutdown() can be called by kernel only.

I can't catch what you mean. The kernel code executing in userspace uses syscall
interface (e.g. SYSCALL_DEFINE2(shutdown, int, fd, int, how) path), doesn't it?

yes.

quoted

I suspect he's imaging a hypothetical situation where kernel bits of kernel module
interact with userblob bits of kernel module.
Then another root process tampers with memory of userblob.

Yes, how to protect the memory of userblob is a concern. The memory of userblob can
interfere (or can be interfered by) the rest of the system is a problem.

answered.

quoted

I think this is trivially enforceable without creating new features.
Existing security_ptrace_access_check() LSM hook can prevent tampering with
memory of userblob.

There is security_ptrace_access_check() LSM hook, but no zero-configuration
method is available.

huh?
tomoyo is not using that hook, but selinux and many other LSMs do.
please learn from others.

quoted

security label can carry that execution context.

If files get a chance to be associated with appropriate pathname and
security label.

I can easily add a fake pathname to the blob, but it won't help tomoyo.
That's what I was saying all along.
pathname based security provides false sense of security.

I'm pretty sure this old blog has been read by many folks who
are following this thread, but it's worth reminding again:
https://securityblog.org/2006/04/19/security-anti-pattern-path-based-access-control/
I cannot agree more with Joshua.
Here is a quote:
"The most obvious problem with this is that not all objects are files and thus do not have paths."

quoted

quoted

My personally strongest argument for remoiving this kernel code is
that it's been there for a couple of years now, and it has never
actually done anything useful, and there's no actual sign that it ever
will, or that there is a solid plan in place for it.

you probably missed the detailed plan:
https://lore.kernel.org/bpf/20200609235631.ukpm3xngbehfqthz@ast-mbp.dhcp.thefacebook.com/ (local)

The project #3 is the above is the one we're working on right now.
It should be ready to post in a week.

I got a question on project #3. Given that "cat /sys/fs/bpf/my_ipv6_route"
produces the same human output as "cat /proc/net/ipv6_route", how security
checks which are done for "cat /proc/net/ipv6_route" can be enforced for
"cat /sys/fs/bpf/my_ipv6_route" ? Unless same security checks (e.g. permission
to read /proc/net/ipv6_route ) is enforced, such bpf usage sounds like a method
for bypassing existing security mechanisms.

Standard file permissions. Nothing to do with bpf.