syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel]

[Dmitry Vyukov]

On Mon, Feb 17, 2020 at 8:24 PM Dmitry Vyukov <dvyukov at google.com> wrote:
>
> On Mon, Feb 17, 2020 at 4:42 PM Dmitry Vyukov <dvyukov at google.com> wrote:
> > > >
> > > > Observation:
> > > >
> > > > It seems to be starting to synthesize packets sent to the wireguard
> > > > socket. These aren't the proper handshake packets generated internally
> > > > by that triangle commit, but rather ones that syzkaller creates
> > > > itself. That's why we have coverage on wg_receive, which otherwise
> > > > wouldn't be called from a userspace process, since syzbot is sending
> > > > its own packets to that function.
> > > >
> > > > However, the packets it generates aren't getting very far, failing all
> > > > of the tests in validate_header_len. None of those checks are at all
> > > > cryptographic, which means it should be able to hit those eventually.
> > > > Anything we should be doing to help it out? After it gets past that
> > > > check, it'll wind up in the handshake queue or the data queue, and
> > > > then (in theory) it should be rejected on a cryptographic basis. But
> > > > maybe syzbot will figure out how to crash it instead :-P.
> > >
> > > Looking into this.
> > >
> > > Found the program that gives wg_receive coverage:
> > >
> > > r0 = openat$tun(0xffffffffffffff9c,
> > > &(0x7f0000000080)='/dev/net/tun\x00', 0x88002, 0x0)
> > > ioctl$TUNSETIFF(r0, 0x400454ca, &(0x7f00000000c0)={'syzkaller1\x00',
> > > 0x420000015001})
> > > r1 = socket$netlink(0x10, 0x3, 0x0)
> > > ioctl$sock_inet_SIOCSIFADDR(r1, 0x8914,
> > > &(0x7f0000000140)={'syzkaller1\x00', {0x7, 0x0, @empty}})
> > > write$tun(r0, &(0x7f00000002c0)={@void, @val, @ipv4=@udp={{0x5, 0x4,
> > > 0x0, 0x0, 0x1c, 0x0, 0x0, 0x0, 0x11, 0x0, @remote, @broadcast}, {0x0,
> > > 0x4e21, 0x8}}}, 0x26)
> > >
> > > Checked that doing SIOCSIFADDR is also required, otherwise the packet
> > > does not reach wg_receive.
> >
> >
> > All packets we inject with standard means (syz_emit_ethernet) get
> > rejected on the following check:
> >
> > static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
> > {
> > const struct iphdr *iph;
> > u32 len;
> >
> > /* When the interface is in promisc. mode, drop all the crap
> > * that it receives, do not try to analyse it.
> > */
> > if (skb->pkt_type == PACKET_OTHERHOST)
> > goto drop;
> >
> > Even if we drop IFF_NAPI_FRAGS which diverges packets who-knows-where.
> >
> > Somehow we need to get something other than PACKET_OTHERHOST...
> > Why is it dropping all remote packets?...
> > How do remote packets get into stack then?...
>
> I've managed to create a packet that reaches wg_receive, that is:
>
> syz_emit_ethernet(AUTO, &AUTO={@local, @empty, @void, {@ipv4={AUTO,
> @udp={{AUTO, AUTO, 0x0, 0x0, AUTO, 0x0, 0x0, 0x0, AUTO, 0x0, @empty,
> @empty, {[]}}, {0x0, 0x4e22, AUTO, 0x0, [], ""/10}}}}}, 0x0)
>
> Had to enumerate all possible combinations of local/remote mac,
> local/report ip, local/remote port.
>
> However, this is only without IFF_NAPI_FRAGS. With IFF_NAPI_FRAGS it
> reaches udp_gro_receive, but does not get past:
>
> if (!sk || NAPI_GRO_CB(skb)->encap_mark ||
>     (skb->ip_summed != CHECKSUM_PARTIAL &&
>      NAPI_GRO_CB(skb)->csum_cnt == 0 &&
>      !NAPI_GRO_CB(skb)->csum_valid) ||
>     !udp_sk(sk)->gro_receive)
>     goto out;


I've added descriptions for wireguard packets:
https://github.com/google/syzkaller/commit/012fbc3229ebef871a201ea431b16610e6e0d345
It gives all reachable coverage (without breaking crypto).

Strictly saying, for tcp we experimented with receiving ACKs back from
tun and exposing them to fuzzer to form proper SYNACKs:
https://github.com/google/syzkaller/blob/012fbc3229ebef871a201ea431b16610e6e0d345/executor/common_linux.h#L1390-L1441
https://github.com/google/syzkaller/blob/012fbc3229ebef871a201ea431b16610e6e0d345/sys/linux/vnet.txt#L24-L27

Theoretically, it could receive wireguard handshakes and form proper
replies with valid signatures and stuff.

I disabled IFF_NAPI_FRAGS entirely, it seems to prevent from getting
any meaningful coverage:
https://github.com/google/syzkaller/commit/39cd0f85a1ac60b88c793bd8f4a981227614da88