Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Thomas Graf]

On 10/08/15 at 08:20pm, Hannes Frederic Sowa wrote:
> Hi Alexei,
> 
> On Thu, Oct 8, 2015, at 07:23, Alexei Starovoitov wrote:
> > The feature is controlled by sysctl kernel.unprivileged_bpf_disabled.
> > This toggle defaults to off (0), but can be set true (1).  Once true,
> > bpf programs and maps cannot be accessed from unprivileged process,
> > and the toggle cannot be set back to false.
> 
> This approach seems fine to me.
> 
> I am wondering if it makes sense to somehow allow ebpf access per
> namespace? I currently have no idea how that could work and on which
> namespace type to depend or going with a prctl or even cgroup maybe. The
> rationale behind this is, that maybe some namespaces like openstack
> router namespaces could make usage of advanced ebpf capabilities in the
> kernel, while other namespaces, especially where untrusted third parties
> are hosted, shouldn't have access to those facilities.
> 
> In that way, hosters would be able to e.g. deploy more efficient
> performance monitoring container (which should still need not to run as
> root) while the majority of the users has no access to that. Or think
> about routing instances in some namespaces, etc. etc.

The standard way of granting privileges like this for containers is
through CAP_ which does seem like a good fit for this as well and would
also solve your mentioned openstack use case.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Hannes Frederic Sowa]

Hi,

Alexei Starovoitov  writes:

> On 10/8/15 11:20 AM, Hannes Frederic Sowa wrote:
>> Hi Alexei,
>>
>> On Thu, Oct 8, 2015, at 07:23, Alexei Starovoitov wrote:
>>> The feature is controlled by sysctl kernel.unprivileged_bpf_disabled.
>>> This toggle defaults to off (0), but can be set true (1).  Once true,
>>> bpf programs and maps cannot be accessed from unprivileged process,
>>> and the toggle cannot be set back to false.
>>
>> This approach seems fine to me.
>>
>> I am wondering if it makes sense to somehow allow ebpf access per
>> namespace? I currently have no idea how that could work and on which
>> namespace type to depend or going with a prctl or even cgroup maybe. The
>> rationale behind this is, that maybe some namespaces like openstack
>> router namespaces could make usage of advanced ebpf capabilities in the
>> kernel, while other namespaces, especially where untrusted third parties
>> are hosted, shouldn't have access to those facilities.
>>
>> In that way, hosters would be able to e.g. deploy more efficient
>> performance monitoring container (which should still need not to run as
>> root) while the majority of the users has no access to that. Or think
>> about routing instances in some namespaces, etc. etc.
>
> when we're talking about eBPF for networking or performance monitoring
> it's all going to be under root anyway.

I am not so sure, actually. Like PCP (Performance CoPilot), which does
long term collecting of performance data in the kernel and maybe sending
it over the network, it would be great if at least some capabilities
could be dropped after the bpf filedescriptor was allocated. But current
bpf syscall always checks capabilities on every call, which is actually
quite unusual for capabilities.

For networking the basic technique was also to drop capabilities sooner
than later.

Can we filter bpf syscall finegrained with selinux?

> The next question is
> how to let the programs run only for traffic or for applications within
> namespaces. Something gotta do this demux. It either can be in-kernel
> C code which is configured via some API that calls different eBPF
> programs based on cgroup or based on netns, or it can be another
> eBPF program that does demux on its own.

This sounds quite complex. Afaics this problem hasn't even be solved in
perf so far, tracepoints hit independent of the namespace currently.

> In case of tracing such 'demuxing' program can be attached to kernel
> events and call 'worker' programs via tail_call, so that 'worker'
> programs will have an illusion that they're working only with events
> that belong to their namespace.
> imo existing facilities already allow 'per namespace' eBPF, though
> the prog_array used to jump from 'demuxing' bpf into 'worker' bpf
> currently is a bit awkward to use (because of FD passing via daemon),
> but that will get solved soon.

Aha, so client namespaces hand over their fds to parent demuxer and it
sets up the necessary calls. Yeah, this seems to work.

> It feels that in-kernel C code doing filtering may be
> 'more robust' from namespace isolation point of view, but I don't
> think we have a concrete and tested proposal, so I would
> experiment with 'demuxing' bpf first.
> The programs in general don't have a notion of namespace. They
> need to be attached to veth via TC to get packets for
> particular namespace.

Okay.

For me namespacing of ebpf code is actually not that important, I would
much rather like to control which namespace is allowed to execute ebpf
in an unpriviledged manner. Like Thomas wrote, a capability was great
for that, but I don't know if any new capabilities will be added.

Thanks,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Daniel Borkmann]

On 10/09/2015 07:30 PM, Alexei Starovoitov wrote:
...

Openstack use case is different. There it will be prog_type_sched_cls
that can mangle packets, change skb metadata, etc under TC framework.
These are not suitable for all users and this patch leaves
them root-only. If you're proposing to add CAP_BPF_TC to let containers
use them without being CAP_SYS_ADMIN, then I agree, it is useful, but
needs a lot more safety analysis on tc side.


Well, I think if so, then this would need to be something generic for
tc instead of being specific to a single (out of various) entities
inside the tc framework, but I currently doubt that this makes much
sense. If we allow to operate already at that level, then restricting
to CAP_SYS_ADMIN makes more sense in that specific context/subsys to me.

Best,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Alexei Starovoitov]

On 10/9/15 4:45 AM, Hannes Frederic Sowa wrote:

Afaics this problem hasn't even be solved in
perf so far, tracepoints hit independent of the namespace currently.


yes and that's exactly what we're trying to solve.
The "demux+worker bpf programs" proposal is a work-in-progress solution
to get confidence how to actually separate tracepoint events into
namespaces before adding any new APIs to kernel.


For me namespacing of ebpf code is actually not that important, I would
much rather like to control which namespace is allowed to execute ebpf
in an unpriviledged manner. Like Thomas wrote, a capability was great
for that, but I don't know if any new capabilities will be added.


I think we're mixing too many things here.
First I believe eBPF 'socket filters' do not need any caps.
They're packet read-only and functionally very similar to classic with
a distinction that packet data can be aggregated into maps and programs
can be written in C. So I see no reason to restrict them per user or
per namespace.
Openstack use case is different. There it will be prog_type_sched_cls
that can mangle packets, change skb metadata, etc under TC framework.
These are not suitable for all users and this patch leaves
them root-only. If you're proposing to add CAP_BPF_TC to let containers
use them without being CAP_SYS_ADMIN, then I agree, it is useful, but
needs a lot more safety analysis on tc side.
Similar for prog_type_kprobe: we can add CAP_BPF_KPROBE to let
some trusted applications run unprivileged, but still being able
to do performance monitoring/analytics.
And we would need to carefully think about program restrictions,
since bpf_probe_read and kernel pointer walking is essential part
in tracing.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Alexei Starovoitov]

On 10/9/15 10:45 AM, Daniel Borkmann wrote:

On 10/09/2015 07:30 PM, Alexei Starovoitov wrote:
...

Openstack use case is different. There it will be prog_type_sched_cls
that can mangle packets, change skb metadata, etc under TC framework.
These are not suitable for all users and this patch leaves
them root-only. If you're proposing to add CAP_BPF_TC to let containers
use them without being CAP_SYS_ADMIN, then I agree, it is useful, but
needs a lot more safety analysis on tc side.


Well, I think if so, then this would need to be something generic for
tc instead of being specific to a single (out of various) entities
inside the tc framework, but I currently doubt that this makes much
sense. If we allow to operate already at that level, then restricting
to CAP_SYS_ADMIN makes more sense in that specific context/subsys to me.


Let me rephrase. I think it would be useful, but I have my doubts that
it's manageable, since analyzing dark corners of TC is not trivial.
Probably easier to allow prog_type_sched_cls/act under CAP_NET_ADMIN
and grant that to trusted apps. Though only tiny bit better than
requiring CAP_SYS_ADMIN.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Kees Cook]

On Wed, Oct 7, 2015 at 10:23 PM, Alexei Starovoitov  wrote:
> In order to let unprivileged users load and execute eBPF programs
> teach verifier to prevent pointer leaks.
> Verifier will prevent
> - any arithmetic on pointers
>   (except R10+Imm which is used to compute stack addresses)
> - comparison of pointers
>   (except if (map_value_ptr == 0) ... )
> - passing pointers to helper functions
> - indirectly passing pointers in stack to helper functions
> - returning pointer from bpf program
> - storing pointers into ctx or maps
>
> Spill/fill of pointers into stack is allowed, but mangling
> of pointers stored in the stack or reading them byte by byte is not.
>
> Within bpf programs the pointers do exist, since programs need to
> be able to access maps, pass skb pointer to LD_ABS insns, etc
> but programs cannot pass such pointer values to the outside
> or obfuscate them.
>
> Only allow BPF_PROG_TYPE_SOCKET_FILTER unprivileged programs,
> so that socket filters (tcpdump), af_packet (quic acceleration)
> and future kcm can use it.
> tracing and tc cls/act program types still require root permissions,
> since tracing actually needs to be able to see all kernel pointers
> and tc is for root only.
>
> For example, the following unprivileged socket filter program is allowed:
> int bpf_prog1(struct __sk_buff *skb)
> {
>   u32 index = load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol));
>   u64 *value = bpf_map_lookup_elem(_map, );
>
>   if (value)
> *value += skb->len;
>   return 0;
> }
>
> but the following program is not:
> int bpf_prog1(struct __sk_buff *skb)
> {
>   u32 index = load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol));
>   u64 *value = bpf_map_lookup_elem(_map, );
>
>   if (value)
> *value += (u64) skb;
>   return 0;
> }
> since it would leak the kernel address into the map.
>
> Unprivileged socket filter bpf programs have access to the
> following helper functions:
> - map lookup/update/delete (but they cannot store kernel pointers into them)
> - get_random (it's already exposed to unprivileged user space)
> - get_smp_processor_id
> - tail_call into another socket filter program
> - ktime_get_ns
>
> The feature is controlled by sysctl kernel.unprivileged_bpf_disabled.
> This toggle defaults to off (0), but can be set true (1).  Once true,
> bpf programs and maps cannot be accessed from unprivileged process,
> and the toggle cannot be set back to false.
>
> Signed-off-by: Alexei Starovoitov 

Reviewed-by: Kees Cook 

Thanks for making this safer! :)

-Kees

> ---
> v1->v2:
> - sysctl_unprivileged_bpf_disabled
> - drop bpf_trace_printk
> - split tests into separate patch to ease review
> ---
>  include/linux/bpf.h   |2 +
>  kernel/bpf/syscall.c  |   11 ++---
>  kernel/bpf/verifier.c |  106 
> -
>  kernel/sysctl.c   |   13 ++
>  net/core/filter.c |3 +-
>  5 files changed, 120 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 19b8a2081f88..e472b06df138 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -167,6 +167,8 @@ void bpf_prog_put_rcu(struct bpf_prog *prog);
>  struct bpf_map *bpf_map_get(struct fd f);
>  void bpf_map_put(struct bpf_map *map);
>
> +extern int sysctl_unprivileged_bpf_disabled;
> +
>  /* verify correctness of eBPF program */
>  int bpf_check(struct bpf_prog **fp, union bpf_attr *attr);
>  #else
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 5f35f420c12f..9f824b0f0f5f 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -18,6 +18,8 @@
>  #include 
>  #include 
>
> +int sysctl_unprivileged_bpf_disabled __read_mostly;
> +
>  static LIST_HEAD(bpf_map_types);
>
>  static struct bpf_map *find_and_alloc_map(union bpf_attr *attr)
> @@ -542,6 +544,9 @@ static int bpf_prog_load(union bpf_attr *attr)
> attr->kern_version != LINUX_VERSION_CODE)
> return -EINVAL;
>
> +   if (type != BPF_PROG_TYPE_SOCKET_FILTER && !capable(CAP_SYS_ADMIN))
> +   return -EPERM;
> +
> /* plain bpf_prog allocation */
> prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER);
> if (!prog)
> @@ -597,11 +602,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, 
> uattr, unsigned int, siz
> union bpf_attr attr = {};
> int err;
>
> -   /* the syscall is limited to root temporarily. This restriction will 
> be
> -* lifted when security audit is clean. Note that eBPF+tracing must 
> have
> -* this restriction, since it may pass kernel data to user space
> -*/
> -   if (!capable(CAP_SYS_ADMIN))
> +   if (!capable(CAP_SYS_ADMIN) && sysctl_unprivileged_bpf_disabled)
> return -EPERM;
>
> if (!access_ok(VERIFY_READ, uattr, 1))
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 

Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Alexei Starovoitov]

On 10/8/15 11:20 AM, Hannes Frederic Sowa wrote:

Hi Alexei,

On Thu, Oct 8, 2015, at 07:23, Alexei Starovoitov wrote:

The feature is controlled by sysctl kernel.unprivileged_bpf_disabled.
This toggle defaults to off (0), but can be set true (1).  Once true,
bpf programs and maps cannot be accessed from unprivileged process,
and the toggle cannot be set back to false.


This approach seems fine to me.

I am wondering if it makes sense to somehow allow ebpf access per
namespace? I currently have no idea how that could work and on which
namespace type to depend or going with a prctl or even cgroup maybe. The
rationale behind this is, that maybe some namespaces like openstack
router namespaces could make usage of advanced ebpf capabilities in the
kernel, while other namespaces, especially where untrusted third parties
are hosted, shouldn't have access to those facilities.

In that way, hosters would be able to e.g. deploy more efficient
performance monitoring container (which should still need not to run as
root) while the majority of the users has no access to that. Or think
about routing instances in some namespaces, etc. etc.


when we're talking about eBPF for networking or performance monitoring
it's all going to be under root anyway. The next question is
how to let the programs run only for traffic or for applications within
namespaces. Something gotta do this demux. It either can be in-kernel
C code which is configured via some API that calls different eBPF
programs based on cgroup or based on netns, or it can be another
eBPF program that does demux on its own.
In case of tracing such 'demuxing' program can be attached to kernel
events and call 'worker' programs via tail_call, so that 'worker'
programs will have an illusion that they're working only with events
that belong to their namespace.
imo existing facilities already allow 'per namespace' eBPF, though
the prog_array used to jump from 'demuxing' bpf into 'worker' bpf
currently is a bit awkward to use (because of FD passing via daemon),
but that will get solved soon.
It feels that in-kernel C code doing filtering may be
'more robust' from namespace isolation point of view, but I don't
think we have a concrete and tested proposal, so I would
experiment with 'demuxing' bpf first.
The programs in general don't have a notion of namespace. They
need to be attached to veth via TC to get packets for
particular namespace.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 net-next 1/3] bpf: enable non-root eBPF programs

[Hannes Frederic Sowa]

Hi Alexei,

On Thu, Oct 8, 2015, at 07:23, Alexei Starovoitov wrote:
> The feature is controlled by sysctl kernel.unprivileged_bpf_disabled.
> This toggle defaults to off (0), but can be set true (1).  Once true,
> bpf programs and maps cannot be accessed from unprivileged process,
> and the toggle cannot be set back to false.

This approach seems fine to me.

I am wondering if it makes sense to somehow allow ebpf access per
namespace? I currently have no idea how that could work and on which
namespace type to depend or going with a prctl or even cgroup maybe. The
rationale behind this is, that maybe some namespaces like openstack
router namespaces could make usage of advanced ebpf capabilities in the
kernel, while other namespaces, especially where untrusted third parties
are hosted, shouldn't have access to those facilities.

In that way, hosters would be able to e.g. deploy more efficient
performance monitoring container (which should still need not to run as
root) while the majority of the users has no access to that. Or think
about routing instances in some namespaces, etc. etc.

Thanks,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html