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

[Soheil Hassas Yeganeh <soheil@google.com>]

On Mon, Nov 30, 2020 at 11:17 AM Victor Stewart <v@nametag.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@google.com> wrote:
> >
> > On Mon, Nov 30, 2020 at 10:05 AM Stefan Metzmacher <metze@samba.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
> > >
> > >
> > >