Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors

On 6/12/15 5:03 PM, Andy Lutomirski wrote:

On Fri, Jun 12, 2015 at 4:55 PM, Alexei Starovoitov [off-list ref] wrote:

quoted

On 6/12/15 4:47 PM, Andy Lutomirski wrote:

quoted

On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov [off-list ref]
wrote:

quoted

On 6/12/15 4:25 PM, Andy Lutomirski wrote:

quoted

It's a dangerous tool.  Also, shouldn't the returned uid match the
namespace of the task that installed the probe, not the task that's
being probed?

so leaking info to unprivileged apps is the concern?
The whole thing is for root only as you know.
The non-root is still far away. Today root needs to see the whole
kernel. That was the goal from the beginning.

This is more of a correctness issue than a security issue.  ISTM using
current_user_ns() in a kprobe is asking for trouble.  It certainly
allows any unprivilege user to show any uid it wants to the probe,
which is probably not what the installer of the probe expects.

probe doesn't expect anything. it doesn't make any decisions.
bpf is read only. it's _visibility_ into the kernel.
It's not used for security.
When we start connecting eBPF to seccomp I would agree that uid
handling needs to be done carefully, but we're not there yet.
I don't want to kill _visibility_ because in some distant future
bpf becomes a decision making tool in security area and
get_current_uid() will return numbers that shouldn't be blindly
used to reject/accept a user requesting something. That's far away.

All that is true, but the code that *installed* the bpf probe might
get might confused when it logs that uid 0 did such-and-such when
really some unprivileged userns root did it.

so what specifically you proposing?
Use from_kuid(&init_user_ns,...) instead?

Also, as you start calling more and more non-trivial functions from
bpf, you might need to start preventing bpf probe installations in
those functions.

yes. may be. I don't want to blacklist stuff yet, unless it
causes crashes. Recursive check is already there. Probably
something else will be needed.