Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1164430 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid,  gid, comm accessors Alexei Starovoitov <ast@plumgrid.com> - 2015-06-13 01:30 +0200
    Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid,  comm accessors Andy Lutomirski <luto@amacapital.net> - 2015-06-13 01:30 +0200
      Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid,  gid, comm accessors Alexei Starovoitov <ast@plumgrid.com> - 2015-06-13 01:40 +0200
        Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid,  comm accessors Andy Lutomirski <luto@amacapital.net> - 2015-06-13 01:50 +0200
          Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid,  gid, comm accessors Alexei Starovoitov <ast@plumgrid.com> - 2015-06-13 02:00 +0200
            Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid,  comm accessors Andy Lutomirski <luto@amacapital.net> - 2015-06-13 02:10 +0200
              Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid,  gid, comm accessors Alexei Starovoitov <ast@plumgrid.com> - 2015-06-13 02:20 +0200
                Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid,  gid, comm accessors Alexei Starovoitov <ast@plumgrid.com> - 2015-06-13 02:30 +0200
                Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid,  comm accessors Andy Lutomirski <luto@amacapital.net> - 2015-06-13 02:30 +0200

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

On 6/12/15 3:54 PM, Andy Lutomirski wrote:
> On Fri, Jun 12, 2015 at 3:44 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On 6/12/15 3:08 PM, Andy Lutomirski wrote:
>>>
>>> On Fri, Jun 12, 2015 at 2:40 PM, Alexei Starovoitov <ast@plumgrid.com>
>>> wrote:
>>>>
>>>> eBPF programs attached to kprobes need to filter based on
>>>> current->pid, uid and other fields, so introduce helper functions:
>>>>
>>>> u64 bpf_get_current_pid_tgid(void)
>>>> Return: current->tgid << 32 | current->pid
>>>>
>>>> u64 bpf_get_current_uid_gid(void)
>>>> Return: current_gid << 32 | current_uid
>>>
>>>
>>> How does this work wrt namespaces,
>>
>>
>> from_kuid(current_user_ns(), uid)
>>
>
> Is current_user_ns() well defined in the context of an eBPF program?

What do you mean 'well defined'?
Semantically same as 'current'. Depending on where particular
kprobe is placed, 'current' is either meaningful or not. Program
author needs to know what he's doing. It's a tool.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

On Fri, Jun 12, 2015 at 4:23 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 3:54 PM, Andy Lutomirski wrote:
>>
>> On Fri, Jun 12, 2015 at 3:44 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>> On 6/12/15 3:08 PM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> On Fri, Jun 12, 2015 at 2:40 PM, Alexei Starovoitov <ast@plumgrid.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> eBPF programs attached to kprobes need to filter based on
>>>>> current->pid, uid and other fields, so introduce helper functions:
>>>>>
>>>>> u64 bpf_get_current_pid_tgid(void)
>>>>> Return: current->tgid << 32 | current->pid
>>>>>
>>>>> u64 bpf_get_current_uid_gid(void)
>>>>> Return: current_gid << 32 | current_uid
>>>>
>>>>
>>>>
>>>> How does this work wrt namespaces,
>>>
>>>
>>>
>>> from_kuid(current_user_ns(), uid)
>>>
>>
>> Is current_user_ns() well defined in the context of an eBPF program?
>
>
> What do you mean 'well defined'?
> Semantically same as 'current'. Depending on where particular
> kprobe is placed, 'current' is either meaningful or not. Program
> author needs to know what he's doing. It's a tool.
>

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?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1164437

On 6/12/15 4:25 PM, Andy Lutomirski wrote:
> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>
>> 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.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1164480

On 6/12/15 4:47 PM, Andy Lutomirski wrote:
> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>
>>> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Fri, Jun 12, 2015 at 4:55 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 4:47 PM, Andy Lutomirski wrote:
>>
>> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> 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.

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.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1164491

On 6/12/15 5:03 PM, Andy Lutomirski wrote:
> On Fri, Jun 12, 2015 at 4:55 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On 6/12/15 4:47 PM, Andy Lutomirski wrote:
>>>
>>> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com>
>>> wrote:
>>>>
>>>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>>>
>>>>>
>>>>> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1164493

On 6/12/15 5:24 PM, Andy Lutomirski wrote:
>> >so what specifically you proposing?
>> >Use from_kuid(&init_user_ns,...) instead?
> That seems reasonable to me.  After all, you can't install one of
> these probes from a non-init userns.

ok. will respin with that change.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Fri, Jun 12, 2015 at 5:15 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 5:03 PM, Andy Lutomirski wrote:
>>
>> On Fri, Jun 12, 2015 at 4:55 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>> On 6/12/15 4:47 PM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> 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?

That seems reasonable to me.  After all, you can't install one of
these probes from a non-init userns.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web