netdev - Re: [RFC 0/1] whitelisting UDP GSO and GRO cmsgs

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 30 Nov 2020 11:20:57 -0500
From:   Soheil Hassas Yeganeh <soheil@...gle.com>
To:     Victor Stewart <v@...etag.social>
Cc:     Stefan Metzmacher <metze@...ba.org>,
        io-uring <io-uring@...r.kernel.org>,
        Luke Hsiao <lukehsiao@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jann Horn <jannh@...gle.com>, Arjun Roy <arjunroy@...gle.com>,
        netdev <netdev@...r.kernel.org>, Jens Axboe <axboe@...nel.dk>
Subject: Re: [RFC 0/1] whitelisting UDP GSO and GRO cmsgs

On Mon, Nov 30, 2020 at 11:17 AM Victor Stewart <v@...etag.social> wrote:
>
> this being the list of UDP options.. i think we're good here? I'll put
> together a new patch.
>
> https://github.com/torvalds/linux/blob/b65054597872ce3aefbc6a666385eabdf9e288da/include/uapi/linux/udp.h#L30
>
> /* UDP socket options */
> #define UDP_CORK 1 /* Never send partially complete segments */
> #define UDP_ENCAP 100 /* Set the socket to accept encapsulated packets */
> #define UDP_NO_CHECK6_TX 101 /* Disable sending checksum for UDP6X */
> #define UDP_NO_CHECK6_RX 102 /* Disable accpeting checksum for UDP6 */
> #define UDP_SEGMENT 103 /* Set GSO segmentation size */
> #define UDP_GRO 104 /* This socket can receive UDP GRO packets */

That is not sufficient proof, because in udp_sendmsg() we also call
ip_cmsg_send() in udp_sendmsg(), and  ip_cmsg_recv_offset() in
udp_recvmsg().  That said, I have audited them and I think they are
sane.

Jann, what do you think?

> On Mon, Nov 30, 2020 at 3:15 PM Soheil Hassas Yeganeh <soheil@...gle.com> wrote:
> >
> > On Mon, Nov 30, 2020 at 10:05 AM Stefan Metzmacher <metze@...ba.org> wrote:
> > >
> > > Hi Soheil,
> > >
> > > > Thank you for CCing us.
> > > >
> > > > The reason for PROTO_CMSG_DATA_ONLY is explained in the paragraph
> > > > above in the commit message.  PROTO_CMSG_DATA_ONLY is basically to
> > > > allow-list a protocol that is guaranteed not to have the privilege
> > > > escalation in https://crbug.com/project-zero/1975.  TCP doesn't have
> > > > that issue, and I believe UDP doesn't have that issue either (but
> > > > please audit and confirm that with +Jann Horn).
> > > >
> > > > If you couldn't find any non-data CMSGs for UDP, you should just add
> > > > PROTO_CMSG_DATA_ONLY to inet dgram sockets instead of introducing
> > > > __sys_whitelisted_cmsghdrs as Stefan mentioned.
> > >
> > > Was there a specific reason why you only added the PROTO_CMSG_DATA_ONLY check
> > > in __sys_recvmsg_sock(), but not in __sys_sendmsg_sock()?
> >
> > We only needed this for recvmsg(MSG_ERRQUEUE) to support transmit
> > zerocopy.  So, we took a more conservative approach and didn't add it
> > for sendmsg().
> >
> > I believe it should be fine to add it for TCP sendmsg, because for
> > SO_MARK we check the user's capability:
> >
> > if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
> >           return -EPERM;
> >
> > I believe udp_sendmsg() is sane too and I cannot spot any issue there.
> >
> > > metze
> > >
> > >
> > >