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: <CAHC9VhRcr03ZCURFi=EJyPvB3sgi44_aC5ixazC43Zs2bNJiDw@mail.gmail.com>
Date:   Fri, 7 Oct 2022 16:06:25 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.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 3:13 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
> 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.

Prior to the change it was impossible to call
sock_getsockopt(SO_PEERSEC) with a kernel address space pointer, now
with 4ff09db1b79b is it possible to call sk_getsockopt(SO_PEERSEC)
with a kernel address space pointer and cause problems.  Perhaps there
are no callers in the kernel that do such a thing at the moment, but
it seems like an easy mistake for someone to make, and the code to
catch it is both trivial and out of any critical path.

This is one of those cases where preventing a future problem is easy,
I think it would be foolish of us to ignore it.

--
paul-moore.com