netdev - Re: SO_PEERSEC protections in sk

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: <CAADnVQ+kRCfKn6MCvfYGhpHF0fUWBU-qJqvM=1YPfj02jM9zKw@mail.gmail.com>
Date:   Fri, 7 Oct 2022 12:12:50 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Martin KaFai Lau <martin.lau@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        LSM List <linux-security-module@...r.kernel.org>,
        selinux@...r.kernel.org
Subject: Re: SO_PEERSEC protections in sk_getsockopt()?

On Fri, Oct 7, 2022 at 10:43 AM Paul Moore <paul@...l-moore.com> wrote:
>
> On Wed, Oct 5, 2022 at 4:44 PM Paul Moore <paul@...l-moore.com> wrote:
> >
> > Hi Martin,
> >
> > In commit 4ff09db1b79b ("bpf: net: Change sk_getsockopt() to take the
> > sockptr_t argument") I see you wrapped the getsockopt value/len
> > pointers with sockptr_t and in the SO_PEERSEC case you pass the
> > sockptr_t:user field to avoid having to update the LSM hook and
> > implementations.  I think that's fine, especially as you note that
> > eBPF does not support fetching the SO_PEERSEC information, but I think
> > it would be good to harden this case to prevent someone from calling
> > sk_getsockopt(SO_PEERSEC) with kernel pointers.  What do you think of
> > something like this?
> >
> >   static int sk_getsockopt(...)
> >   {
> >     /* ... */
> >     case SO_PEERSEC:
> >       if (optval.is_kernel || optlen.is_kernel)
> >         return -EINVAL;
> >       return security_socket_getpeersec_stream(...);
> >     /* ... */
> >   }
>
> Any thoughts on this Martin, Alexei?  It would be nice to see this
> fixed soon ...

'fixed' ?
I don't see any bug.
Maybe WARN_ON_ONCE can be added as a precaution, but also dubious value.