| Message ID | 20200423160455.28509-1-quentin@isovalent.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] tools: bpftool: allow unprivileged users to probe features | expand |
On 4/23/20 6:04 PM, Quentin Monnet wrote: > There is demand for a way to identify what BPF helper functions are > available to unprivileged users. To do so, allow unprivileged users to > run "bpftool feature probe" to list BPF-related features. This will only > show features accessible to those users, and may not reflect the full > list of features available (to administrators) on the system. For > non-JSON output, print an informational message stating so at the top of > the list. > > Note that there is no particular reason why the probes were restricted > to root, other than the fact I did not need them for unprivileged and > did not bother with the additional checks at the time probes were added. > > Cc: Richard Palethorpe <rpalethorpe@suse.com> > Cc: Michael Kerrisk <mtk.manpages@gmail.com> > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- > .../bpftool/Documentation/bpftool-feature.rst | 4 +++ > tools/bpf/bpftool/feature.c | 32 +++++++++++++------ > 2 files changed, 26 insertions(+), 10 deletions(-) > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst > index b04156cfd7a3..313888e87249 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst > @@ -49,6 +49,10 @@ DESCRIPTION > Keyword **kernel** can be omitted. If no probe target is > specified, probing the kernel is the default behaviour. > > + Running this command as an unprivileged user will dump only > + the features available to the user, which usually represent a > + small subset of the parameters supported by the system. > + Looks good. I wonder whether the unprivileged should be gated behind an explicit subcommand e.g. `--unprivileged`. My main worry is that if there's a misconfiguration the emitted macro/ header file will suddenly contain a lot less defines and it might go unnoticed if some optimizations in the BPF code are then compiled out by accident. Maybe it would make sense to have a feature test for libcap and then also allow for root to check on features for unpriv this way? > **bpftool feature probe dev** *NAME* [**full**] [**macros** [**prefix** *PREFIX*]] > Probe network device for supported eBPF features and dump > results to the console. > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c > index 88718ee6a438..f455bc5fcc64 100644 > --- a/tools/bpf/bpftool/feature.c > +++ b/tools/bpf/bpftool/feature.c > @@ -471,6 +471,11 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types, > } > > res = bpf_probe_prog_type(prog_type, ifindex); > + /* Probe may succeed even if program load fails, for unprivileged users > + * check that we did not fail because of insufficient permissions > + */ > + if (geteuid() && errno == EPERM) > + res = false; > > supported_types[prog_type] |= res; >
2020-04-27 14:58 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net> > On 4/23/20 6:04 PM, Quentin Monnet wrote: >> There is demand for a way to identify what BPF helper functions are >> available to unprivileged users. To do so, allow unprivileged users to >> run "bpftool feature probe" to list BPF-related features. This will only >> show features accessible to those users, and may not reflect the full >> list of features available (to administrators) on the system. For >> non-JSON output, print an informational message stating so at the top of >> the list. >> >> Note that there is no particular reason why the probes were restricted >> to root, other than the fact I did not need them for unprivileged and >> did not bother with the additional checks at the time probes were added. >> >> Cc: Richard Palethorpe <rpalethorpe@suse.com> >> Cc: Michael Kerrisk <mtk.manpages@gmail.com> >> Signed-off-by: Quentin Monnet <quentin@isovalent.com> >> --- >> .../bpftool/Documentation/bpftool-feature.rst | 4 +++ >> tools/bpf/bpftool/feature.c | 32 +++++++++++++------ >> 2 files changed, 26 insertions(+), 10 deletions(-) >> >> diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst >> b/tools/bpf/bpftool/Documentation/bpftool-feature.rst >> index b04156cfd7a3..313888e87249 100644 >> --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst >> +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst >> @@ -49,6 +49,10 @@ DESCRIPTION >> Keyword **kernel** can be omitted. If no probe target is >> specified, probing the kernel is the default behaviour. >> + Running this command as an unprivileged user will dump only >> + the features available to the user, which usually represent a >> + small subset of the parameters supported by the system. >> + > > Looks good. I wonder whether the unprivileged should be gated behind an > explicit > subcommand e.g. `--unprivileged`. My main worry is that if there's a > misconfiguration > the emitted macro/ header file will suddenly contain a lot less defines > and it might > go unnoticed if some optimizations in the BPF code are then compiled out > by accident. > Maybe it would make sense to have a feature test for libcap and then > also allow for > root to check on features for unpriv this way? That's a valid concern, I'll rework the patch to add support for the explicit option on the command line as you suggest. Thanks for the review! Quentin
diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst index b04156cfd7a3..313888e87249 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst @@ -49,6 +49,10 @@ DESCRIPTION Keyword **kernel** can be omitted. If no probe target is specified, probing the kernel is the default behaviour. + Running this command as an unprivileged user will dump only + the features available to the user, which usually represent a + small subset of the parameters supported by the system. + **bpftool feature probe dev** *NAME* [**full**] [**macros** [**prefix** *PREFIX*]] Probe network device for supported eBPF features and dump results to the console. diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c index 88718ee6a438..f455bc5fcc64 100644 --- a/tools/bpf/bpftool/feature.c +++ b/tools/bpf/bpftool/feature.c @@ -471,6 +471,11 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types, } res = bpf_probe_prog_type(prog_type, ifindex); + /* Probe may succeed even if program load fails, for unprivileged users + * check that we did not fail because of insufficient permissions + */ + if (geteuid() && errno == EPERM) + res = false; supported_types[prog_type] |= res; @@ -499,6 +504,10 @@ probe_map_type(enum bpf_map_type map_type, const char *define_prefix, res = bpf_probe_map_type(map_type, ifindex); + /* Probe result depends on the success of map creation, no additional + * check required for unprivileged users + */ + maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1; if (strlen(map_type_name[map_type]) > maxlen) { p_info("map type name too long"); @@ -518,12 +527,17 @@ probe_helper_for_progtype(enum bpf_prog_type prog_type, bool supported_type, const char *define_prefix, unsigned int id, const char *ptype_name, __u32 ifindex) { - bool res; + bool res = false; - if (!supported_type) - res = false; - else + if (supported_type) { res = bpf_probe_helper(id, prog_type, ifindex); + /* Probe may succeed even if program load fails, for + * unprivileged users check that we did not fail because of + * insufficient permissions + */ + if (geteuid() && errno == EPERM) + res = false; + } if (json_output) { if (res) @@ -729,13 +743,11 @@ static int do_probe(int argc, char **argv) __u32 ifindex = 0; char *ifname; - /* Detection assumes user has sufficient privileges (CAP_SYS_ADMIN). - * Let's approximate, and restrict usage to root user only. + /* Full feature detection requires CAP_SYS_ADMIN privilege. + * Let's approximate, and warn if user is not root. */ - if (geteuid()) { - p_err("please run this command as root user"); - return -1; - } + if (geteuid()) + p_info("probing as unprivileged user, run as root to see all system features"); set_max_rlimit();
There is demand for a way to identify what BPF helper functions are available to unprivileged users. To do so, allow unprivileged users to run "bpftool feature probe" to list BPF-related features. This will only show features accessible to those users, and may not reflect the full list of features available (to administrators) on the system. For non-JSON output, print an informational message stating so at the top of the list. Note that there is no particular reason why the probes were restricted to root, other than the fact I did not need them for unprivileged and did not bother with the additional checks at the time probes were added. Cc: Richard Palethorpe <rpalethorpe@suse.com> Cc: Michael Kerrisk <mtk.manpages@gmail.com> Signed-off-by: Quentin Monnet <quentin@isovalent.com> --- .../bpftool/Documentation/bpftool-feature.rst | 4 +++ tools/bpf/bpftool/feature.c | 32 +++++++++++++------ 2 files changed, 26 insertions(+), 10 deletions(-)