On 8/17/23 9:09 AM, Yafang Shao wrote:
On Thu, Aug 17, 2023 at 11:31 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
On Wed, Aug 16, 2023 at 7:31 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
On Thu, Aug 17, 2023 at 9:54 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
On Mon, Aug 14, 2023 at 7:46 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
On Tue, Aug 15, 2023 at 8:28 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
On 8/14/23 7:33 AM, Yafang Shao wrote:
Add a new bpf_current_capable kfunc to check whether the current task
has a specific capability. In our use case, we will use it in a lsm bpf
program to help identify if the user operation is permitted.
Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
---
kernel/bpf/helpers.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index eb91cae..bbee7ea 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2429,6 +2429,11 @@ __bpf_kfunc void bpf_rcu_read_unlock(void)
rcu_read_unlock();
}
+__bpf_kfunc bool bpf_current_capable(int cap)
+{
+ return has_capability(current, cap);
+}
Since you are testing against 'current' capabilities, I assume
that the context should be process. Otherwise, you are testing
against random task which does not make much sense.
It is in the process context.
Since you are testing against 'current' cap, and if the capability
for that task is stable, you do not need this kfunc.
You can test cap in user space and pass it into the bpf program.
But if the cap for your process may change in the middle of
run, then you indeed need bpf prog to test capability in real time.
Is this your use case and could you describe in in more detail?
After we convert the capability of our networking bpf program from
CAP_SYS_ADMIN to CAP_BPF+CAP_NET_ADMIN to enhance the security, we
encountered the "pointer comparison prohibited" error, because
allow_ptr_leaks is enabled only when CAP_PERFMON is set. However, if
we enable the CAP_PERFMON for the networking bpf program, it can run
tracing bpf prog, perf_event bpf prog and etc, that is not expected by
us.
Hence we are planning to use a lsm bpf program to disallow it from
running other bpf programs. In our lsm bpf program we will check the
capability of processes, if the process has cap_net_admin, cap_bpf and
cap_perfmon but don't have cap_sys_admin we will refuse it to run
tracing and perf_event bpf program. While if a process has cap_bpf
and cap_perfmon but doesn't have cap_net_admin, that said it is a bpf
program which wants to run trace bpf, we will allow it.
We can't use lsm_cgroup because it is supported on cgroup2 only, while
we're still using cgroup1.
Another possible solution is enable allow_ptr_leaks for cap_net_admin
as well, but after I checked the commit which introduces the cap_bpf
and cap_perfmon [1], I think we wouldn't like to do it.
Sorry. None of these options are acceptable.
The idea of introducing a bpf_current_capable() kfunc just to work
around a deficiency in the verifier is not sound.
So what should we do then?
Just enabling the cap_perfmon for it? That does not sound as well ...
Yonghong already pointed out upthread that
comparison of two packet pointers is not a pointer leak.
See this code:
} else if (!try_match_pkt_pointers(insn, dst_reg, ®s[insn->src_reg],
this_branch, other_branch) &&
is_pointer_value(env, insn->dst_reg)) {
verbose(env, "R%d pointer comparison prohibited\n",
insn->dst_reg);
return -EACCES;
}
It's not clear why it doesn't address your case.
It can address the issue.
It seems we should do the code change below.
I presume in your actual program you also access the IP header (otherwise it
would be quite useless), and as you mentioned above, you would like this to be
accessible for just CAP_BPF + CAP_NET_ADMIN. The CAP_PERFMON restriction is
there for a reason, that is, to be on same cap level as tracing programs as you
could craft Spectre v1 style access. It's not a deficiency.
Thanks,
Daniel
