netdev - Re: [PATCH bpf-next v3 2/2] selftests/bpf: verify that rebinding to port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKH8qBsbHGgcVYhHf14gk13gxVc4VxzLhEbur7L_Z6uGbHzYHw@mail.gmail.com>
Date:   Tue, 26 Jan 2021 11:04:20 -0800
From:   Stanislav Fomichev <sdf@...gle.com>
To:     Martin KaFai Lau <kafai@...com>
Cc:     Netdev <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrey Ignatov <rdna@...com>
Subject: Re: [PATCH bpf-next v3 2/2] selftests/bpf: verify that rebinding to
 port < 1024 from BPF works

On Tue, Jan 26, 2021 at 10:18 AM Martin KaFai Lau <kafai@...com> wrote:
>
> On Tue, Jan 26, 2021 at 08:51:04AM -0800, Stanislav Fomichev wrote:
> > Return 3 to indicate that permission check for port 111
> > should be skipped.
> >
>
> [ ... ]
>
> > +void cap_net_bind_service(cap_flag_value_t flag)
> > +{
> > +     const cap_value_t cap_net_bind_service = CAP_NET_BIND_SERVICE;
> > +     cap_t caps;
> > +
> > +     caps = cap_get_proc();
> > +     if (CHECK(!caps, "cap_get_proc", "errno %d", errno))
> > +             goto free_caps;
> > +
> > +     if (CHECK(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_net_bind_service,
> > +                            flag),
> > +               "cap_set_flag", "errno %d", errno))
> > +             goto free_caps;
> > +
> > +     if (CHECK(cap_set_proc(caps), "cap_set_proc", "errno %d", errno))
> > +             goto free_caps;
> > +
> > +free_caps:
> > +     if (CHECK(cap_free(caps), "cap_free", "errno %d", errno))
> > +             goto free_caps;
> Also mentioned in v2, there is a loop.
Oops, missed that one, sorry.

> > +}
> > +
> > +void test_bind_perm(void)
> > +{
> > +     struct bind_perm *skel;
> > +     int cgroup_fd;
> > +
> > +     cgroup_fd = test__join_cgroup("/bind_perm");
> > +     if (CHECK(cgroup_fd < 0, "cg-join", "errno %d", errno))
> > +             return;
> > +
> > +     skel = bind_perm__open_and_load();
> > +     if (!ASSERT_OK_PTR(skel, "skel"))
> > +             goto close_cgroup_fd;
> > +
> > +     skel->links.bind_v4_prog = bpf_program__attach_cgroup(skel->progs.bind_v4_prog, cgroup_fd);
> > +     if (!ASSERT_OK_PTR(skel, "bind_v4_prog"))
> > +             goto close_skeleton;
> > +
> > +     cap_net_bind_service(CAP_CLEAR);
> > +     try_bind(110, EACCES);
> > +     try_bind(111, 0);
> > +     cap_net_bind_service(CAP_SET);
> Instead of always CAP_SET at the end of the test,
> it is better to do a cap_get_flag() to save the original value
> at the beginning of the test and restore it at the end
> of the test.
It might be easier to change cap_net_bind_service() to return a bool
which indicates that the flag was originally set.
If it wasn't, we can bypass cap_net_bind_service(CAP_SET).
Let me know if you strongly disagree, I'll try to play with this idea
and will send a v4 if it plays out nicely.