* syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] [not found] ` <CACT4Y+awD47=Q3taT_-yQPfQ4uyW-DRpeWBbSHcG6_=b20PPwg@mail.gmail.com> @ 2020-02-04 21:39 ` Jason A. Donenfeld 2020-02-17 11:20 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Jason A. Donenfeld @ 2020-02-04 21:39 UTC (permalink / raw) To: Dmitry Vyukov; +Cc: netdev, syzbot, WireGuard mailing list Hey Dmitry, I see you got wireguard's netlink stuff hooked up to syzkaller. Excellent work, and thanks! It's already finding bugs. Right now it seems to know about 5 different keys you've come up with, and not much in the way of endpoints. I think we can improve this. For keys, there are a few cases we care about: 1) Low order keys 2) Negative keys 3) Normal keys 4) Keys that correspond to other keys (private ==> public) For this last point, if we just have a few with that correspondance quality in there, syzkaller will eventually wind up configuring two interfaces that can talk to each other, which is good. Here's a collection of keys you can use, in base64, that will cover those cases, if you want to add these instead of the current ones in there: 1) AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= 4Ot6fDtBuK4WVuP68Z/EatoJjeucMrH9hmIFFl9JuAA= 2) 2/////////////////////////////////////////8= TJyVvKNQjCSx0LFVnIPvWwREXMRYHI6G2CJO3dCfEdc= 3,4) oFyoT2ycjjhT4v16cK4Psg+hUmAMsAhFF08IB2+NeEM= l1ydgcmDyCCe54ElS4mfjtklrp8JI8I8YvU8V82/aRw= sIBz6NROkePakiwiQ4JEu4hcaeJpyOnYNbEUKTpN3G4= 0XMomfYRzYmUA01/QT3JV2MOVJPChaykAGXLYxG+aWs= oMuHmkf1vGRMDmk/ptAxx0oVU7bpAbn/L1GMeAQvtUI= 9E2jZ6iO5lZPAgIRRWcnCC9c6+6LG/Xrczc0G0WbOSI= That's 10 keys total, which should be a decent collection to replace your current set of hard coded keys in there. You can unbase64 these into C format with commands like: $ echo '2/////////////////////////////////////////8=' | base64 -d | xxd -i 0xdb, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff The second thing is getting two wireguard interfaces to talk to each other. This probably should happen over localhost. That means the listen port of one should be the endpoint of the other. So maybe you can get away fuzzing these with: Listen ports: 51820 51821 51822 [randomly selected] and Endpoints: 127.0.0.1:51820 127.0.0.1:51821 127.0.0.1:51822 [::1]:51820 [::1]:51821 [::1]:51822 [randomly selected] Finally the "allowed ips" for a peer, the routing table entry that points to wireguard, and the packet that's being sent, should all somehow correspond. But probably an allowed ips of 0.0.0.0/0 will eventually be fuzzed to, which covers everything for the first part, so let's see if the rest falls into place on its own. What do you think of all that? Jason _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-04 21:39 ` syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] Jason A. Donenfeld @ 2020-02-17 11:20 ` Dmitry Vyukov 2020-02-17 11:31 ` Jason A. Donenfeld 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2020-02-17 11:20 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: netdev, syzbot, WireGuard mailing list On Tue, Feb 4, 2020 at 10:39 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hey Dmitry, > > I see you got wireguard's netlink stuff hooked up to syzkaller. > Excellent work, and thanks! It's already finding bugs. > > Right now it seems to know about 5 different keys you've come up with, > and not much in the way of endpoints. I think we can improve this. > > For keys, there are a few cases we care about: > > 1) Low order keys > 2) Negative keys > 3) Normal keys > 4) Keys that correspond to other keys (private ==> public) > > For this last point, if we just have a few with that correspondance > quality in there, syzkaller will eventually wind up configuring two > interfaces that can talk to each other, which is good. Here's a > collection of keys you can use, in base64, that will cover those > cases, if you want to add these instead of the current ones in there: > > 1) > AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= > 4Ot6fDtBuK4WVuP68Z/EatoJjeucMrH9hmIFFl9JuAA= > > 2) > 2/////////////////////////////////////////8= > TJyVvKNQjCSx0LFVnIPvWwREXMRYHI6G2CJO3dCfEdc= > > 3,4) > oFyoT2ycjjhT4v16cK4Psg+hUmAMsAhFF08IB2+NeEM= > l1ydgcmDyCCe54ElS4mfjtklrp8JI8I8YvU8V82/aRw= > sIBz6NROkePakiwiQ4JEu4hcaeJpyOnYNbEUKTpN3G4= > 0XMomfYRzYmUA01/QT3JV2MOVJPChaykAGXLYxG+aWs= > oMuHmkf1vGRMDmk/ptAxx0oVU7bpAbn/L1GMeAQvtUI= > 9E2jZ6iO5lZPAgIRRWcnCC9c6+6LG/Xrczc0G0WbOSI= > > That's 10 keys total, which should be a decent collection to replace > your current set of hard coded keys in there. You can unbase64 these > into C format with commands like: > > $ echo '2/////////////////////////////////////////8=' | base64 -d | xxd -i > 0xdb, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff > > The second thing is getting two wireguard interfaces to talk to each > other. This probably should happen over localhost. That means the > listen port of one should be the endpoint of the other. So maybe you > can get away fuzzing these with: > > Listen ports: > 51820 > 51821 > 51822 > [randomly selected] > > and > > Endpoints: > 127.0.0.1:51820 > 127.0.0.1:51821 > 127.0.0.1:51822 > [::1]:51820 > [::1]:51821 > [::1]:51822 > [randomly selected] > > Finally the "allowed ips" for a peer, the routing table entry that > points to wireguard, and the packet that's being sent, should all > somehow correspond. But probably an allowed ips of 0.0.0.0/0 will > eventually be fuzzed to, which covers everything for the first part, > so let's see if the rest falls into place on its own. > > What do you think of all that? > > Jason Hi Jason, [getting through backlog after a tip...] I think you addressed all of this by now, right? And we got decent coverage of wireguard. Anything else low hanging left? https://github.com/google/syzkaller/commit/2c71f1a9122cc3cb0abacbbec6359c40db02be35 https://github.com/google/syzkaller/commit/4d1ab643be2091f794ec55d83ec8acf7b0a60be3 https://github.com/google/syzkaller/commit/c5ed587f4af5e639f7373d8ebf10ac049cb9c71b Thanks! _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-17 11:20 ` Dmitry Vyukov @ 2020-02-17 11:31 ` Jason A. Donenfeld 2020-02-17 11:44 ` Jason A. Donenfeld 0 siblings, 1 reply; 13+ messages in thread From: Jason A. Donenfeld @ 2020-02-17 11:31 UTC (permalink / raw) To: Dmitry Vyukov; +Cc: netdev, syzbot, WireGuard mailing list Hey Dmitry, Yes! Our side discussions wound up getting everything pretty squared away, and coverage on syzkaller looks pretty good to me. By inference, I think we're hitting most code paths in WireGuard. Syzkaller, though, is missing non-userspace-process coverage from: - workqueues - napi callback - timer callback - udp tunnel callback Seems like there might be some future research to be done on how we can track these. But tracking it or not, the fact that packets are flowing on some path implies that other code paths are being hit. So I feel pretty good about syzkaller's ability to dig up nice wireguard bugs. Jason _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-17 11:31 ` Jason A. Donenfeld @ 2020-02-17 11:44 ` Jason A. Donenfeld 2020-02-17 15:19 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Jason A. Donenfeld @ 2020-02-17 11:44 UTC (permalink / raw) To: Dmitry Vyukov; +Cc: netdev, syzbot, WireGuard mailing list 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. _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-17 11:44 ` Jason A. Donenfeld @ 2020-02-17 15:19 ` Dmitry Vyukov 2020-02-17 15:42 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2020-02-17 15:19 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: netdev, syzbot, WireGuard mailing list On Mon, Feb 17, 2020 at 12:44 PM Jason A. Donenfeld <Jason@zx2c4.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. _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-17 15:19 ` Dmitry Vyukov @ 2020-02-17 15:42 ` Dmitry Vyukov 2020-02-17 19:24 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2020-02-17 15:42 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: netdev, syzbot, WireGuard mailing list On Mon, Feb 17, 2020 at 4:19 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Mon, Feb 17, 2020 at 12:44 PM Jason A. Donenfeld <Jason@zx2c4.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?... _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-17 15:42 ` Dmitry Vyukov @ 2020-02-17 19:24 ` Dmitry Vyukov 2020-02-18 10:00 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2020-02-17 19:24 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: netdev, syzbot, WireGuard mailing list On Mon, Feb 17, 2020 at 4:42 PM Dmitry Vyukov <dvyukov@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; _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-17 19:24 ` Dmitry Vyukov @ 2020-02-18 10:00 ` Dmitry Vyukov 2020-02-19 10:22 ` Jason A. Donenfeld 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2020-02-18 10:00 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: netdev, syzbot, WireGuard mailing list On Mon, Feb 17, 2020 at 8:24 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Mon, Feb 17, 2020 at 4:42 PM Dmitry Vyukov <dvyukov@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 _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-18 10:00 ` Dmitry Vyukov @ 2020-02-19 10:22 ` Jason A. Donenfeld 2020-02-20 16:14 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Jason A. Donenfeld @ 2020-02-19 10:22 UTC (permalink / raw) To: Dmitry Vyukov; +Cc: netdev, syzbot, WireGuard mailing list On Tue, Feb 18, 2020 at 11:00 AM Dmitry Vyukov <dvyukov@google.com> wrote: > I've added descriptions for wireguard packets: > https://github.com/google/syzkaller/commit/012fbc3229ebef871a201ea431b16610e6e0d345 > It gives all reachable coverage (without breaking crypto). Oh, great, that looks really good. It now fails at the index match, which is basically trying to brute force a 32-bit integer that's changing every 3 minutes, which syzkaller is probably too slow to do on it's own. But that's fine. Your commit has some questions in it, like "# Not clear if these indexes are also generated randomly and we need to guess them or not.". Here's what's up with those indices: Message message_handshake_initiation: the sender picks a random 32-bit number, and places it in the "sender_index" field. Message message_handshake_response: the sender picks a random 32-bit number, and places it in the "sender_index" field. It places the 32-bit number that it received from message_handshake_initiation into the "receiver_index" field. Message message_handshake_cookie: the sender places the 32-bit number that it received from message_handshake_initiation or message_handshake_response into the "receive_index" field. Message message_data: the sender places the 32-bit number that it picked for the message_handshake_initiation or message_handshake_response into the "key_idx" field. I'm not sure it'll be too feasible to correlate these relations via fuzzing. And either way, I think we've got decent enough non-crypto coverage now on the receive path. I noticed a small seemingly insignificant function with low coverage that's on the rtnl path that might benefit from some attention and also help find bugs in other devices: wg_netdevice_notification. This triggers on various things, but the only case it really cares about is NETDEV_REGISTER, which happens when the interface changes network namespaces. WireGuard holds a reference to its underlying creating namespace, and in order to avoid circular reference counting or UaF it needs to either relinquish or get a reference. There are other drivers with similar type of reference counting management, I would assume. This sort of thing seems up the ally of the types of bugs and races syzkaller likes to find. The way to trigger it is with `ip link set dev wg0 netns blah`, and then back to its original netns. That's this code in net/core/rtnetlink.c: if (tb[IFLA_NET_NS_PID] || tb[IFLA_NET_NS_FD] || tb[IFLA_TARGET_NETNSID]) { struct net *net = rtnl_link_get_net_capable(skb, dev_net(dev), tb, CAP_NET_ADMIN); if (IS_ERR(net)) { err = PTR_ERR(net); goto errout; } err = dev_change_net_namespace(dev, net, ifname); put_net(net); if (err) goto errout; status |= DO_SETLINK_MODIFIED; } That seems to have decent coverage, but not over wireguard. Is that just a result of the syzkaller @devname not yet being expanded to wg{0,1,2}, and it'll take a few more weeks for it to learn that? @netns_id seems probably good, being restricted to 0:4; is it possible these weren't created though a priori? _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-19 10:22 ` Jason A. Donenfeld @ 2020-02-20 16:14 ` Dmitry Vyukov 2020-02-20 16:33 ` Jason A. Donenfeld 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2020-02-20 16:14 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: netdev, syzbot, WireGuard mailing list On Wed, Feb 19, 2020 at 11:23 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > On Tue, Feb 18, 2020 at 11:00 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > I've added descriptions for wireguard packets: > > https://github.com/google/syzkaller/commit/012fbc3229ebef871a201ea431b16610e6e0d345 > > It gives all reachable coverage (without breaking crypto). > > Oh, great, that looks really good. It now fails at the index match, > which is basically trying to brute force a 32-bit integer that's > changing every 3 minutes, which syzkaller is probably too slow to do > on it's own. But that's fine. > > Your commit has some questions in it, like "# Not clear if these > indexes are also generated randomly and we need to guess them or > not.". Here's what's up with those indices: > > Message message_handshake_initiation: the sender picks a random 32-bit > number, and places it in the "sender_index" field. > Message message_handshake_response: the sender picks a random 32-bit > number, and places it in the "sender_index" field. It places the > 32-bit number that it received from message_handshake_initiation into > the "receiver_index" field. > Message message_handshake_cookie: the sender places the 32-bit number > that it received from message_handshake_initiation or > message_handshake_response into the "receive_index" field. > Message message_data: the sender places the 32-bit number that it > picked for the message_handshake_initiation or > message_handshake_response into the "key_idx" field. > > I'm not sure it'll be too feasible to correlate these relations via > fuzzing. And either way, I think we've got decent enough non-crypto > coverage now on the receive path. > > I noticed a small seemingly insignificant function with low coverage > that's on the rtnl path that might benefit from some attention and > also help find bugs in other devices: wg_netdevice_notification. This > triggers on various things, but the only case it really cares about is > NETDEV_REGISTER, which happens when the interface changes network > namespaces. WireGuard holds a reference to its underlying creating > namespace, and in order to avoid circular reference counting or UaF it > needs to either relinquish or get a reference. There are other drivers > with similar type of reference counting management, I would assume. > This sort of thing seems up the ally of the types of bugs and races > syzkaller likes to find. The way to trigger it is with `ip link set > dev wg0 netns blah`, and then back to its original netns. That's this > code in net/core/rtnetlink.c: > > if (tb[IFLA_NET_NS_PID] || tb[IFLA_NET_NS_FD] || > tb[IFLA_TARGET_NETNSID]) { > struct net *net = rtnl_link_get_net_capable(skb, dev_net(dev), > tb, CAP_NET_ADMIN); > if (IS_ERR(net)) { > err = PTR_ERR(net); > goto errout; > } > > err = dev_change_net_namespace(dev, net, ifname); > put_net(net); > if (err) > goto errout; > status |= DO_SETLINK_MODIFIED; > } > > That seems to have decent coverage, but not over wireguard. Is that > just a result of the syzkaller @devname not yet being expanded to > wg{0,1,2}, and it'll take a few more weeks for it to learn that? > @netns_id seems probably good, being restricted to 0:4; is it possible > these weren't created though a priori? Running an instance aimed at these functions: "enable_syscalls": [ "socket$nl_generic", "syz_genetlink_get_family_id$wireguard", "sendmsg$WG_CMD*", "ioctl$ifreq_SIOCGIFINDEX_wireguard*", "socket$nl_route", "sendmsg$nl_route", "setsockopt", "syz_emit_ethernet", "syz_extract_tcp_res", "socket$inet*", "accept4$inet*", "bind$inet*", "connect$inet*", "sendto$inet*", "recvfrom$inet*", "syz_genetlink_get_family_id$fou", "sendmsg$FOU*", "unshare", "setns", "socket", "socketpair", "getuid", "getgid", "bpf", "setsockopt", "getsockopt", "ioctl", "accept4", "connect", "bind", "listen", "sendmsg", "recvmsg", "syz_emit_ethernet", "syz_extract_tcp_res", "write$tun", "syz_init_net_socket", "syz_genetlink_get_family_id", "syz_open_procfs$namespace", "ioctl$NS_GET*", "ioctl$TUN*", "openat$tun", "getpid", "mmap" ] I got some coverage in wg_netdevice_notification: https://imgur.com/a/1sJZKtp Or you mean the parts that are still red? I think theoretically these parts should be reachable too because syzkaller can do unshare and obtain net ns fd's. It's quite hard to test because it just crashes all the time on known bugs. So maybe the most profitable way to get more coverage throughout the networking subsystem now is to fix the top layer of crashers ;) 2020/02/20 16:29:08 vm-9: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 16:32:25 vm-6: crash: kernel BUG at net/core/skbuff.c:LINE! 2020/02/20 16:33:13 vm-16: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:33:46 vm-5: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:34:22 vm-2: crash: WARNING in restore_regulatory_settings 2020/02/20 16:34:27 vm-4: crash: unregister_netdevice: waiting for DEV to become free 2020/02/20 16:35:04 vm-26: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 16:35:17 vm-34: crash: no output from test machine 2020/02/20 16:35:20 vm-0: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:36:42 vm-3: crash: WARNING in restore_regulatory_settings 2020/02/20 16:37:47 vm-29: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:38:54 vm-31: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:39:00 vm-9: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:41:41 vm-4: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 16:43:00 vm-23: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 16:44:01 vm-2: crash: possible deadlock in htab_lru_map_delete_node 2020/02/20 16:44:06 vm-38: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:47:13 vm-8: crash: unregister_netdevice: waiting for DEV to become free 2020/02/20 16:47:20 vm-25: crash: unregister_netdevice: waiting for DEV to become free 2020/02/20 16:47:33 vm-18: crash: BUG: using smp_processor_id() in preemptible [ADDR] code: syz-executor 2020/02/20 16:49:37 vm-37: crash: unregister_netdevice: waiting for DEV to become free 2020/02/20 16:51:51 vm-12: crash: WARNING in print_bfs_bug 2020/02/20 16:53:28 vm-4: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 16:55:24 vm-17: crash: INFO: task hung in register_netdevice_notifier 2020/02/20 16:56:26 vm-34: crash: BUG: MAX_LOCKDEP_KEYS too low! 2020/02/20 16:57:11 vm-3: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 16:57:44 vm-31: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 16:58:13 vm-33: crash: BUG: MAX_LOCKDEP_KEYS too low! 2020/02/20 16:58:35 vm-32: crash: BUG: using smp_processor_id() in preemptible [ADDR] code: syz-executor 2020/02/20 17:03:12 vm-9: crash: INFO: task hung in raw_release 2020/02/20 17:03:20 vm-3: crash: unregister_netdevice: waiting for DEV to become free 2020/02/20 17:03:26 vm-6: crash: BUG: using smp_processor_id() in preemptible [ADDR] code: syz-executor 2020/02/20 17:05:08 vm-31: crash: INFO: task hung in lock_sock_nested 2020/02/20 17:06:01 vm-16: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 17:06:24 vm-26: crash: INFO: task hung in bcm_release 2020/02/20 17:07:20 vm-27: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 17:08:03 vm-11: crash: BUG: MAX_LOCKDEP_ENTRIES too low! 2020/02/20 17:08:05 vm-17: crash: WARNING in hsr_addr_subst_dest 2020/02/20 17:09:12 vm-22: crash: BUG: MAX_LOCKDEP_ENTRIES too low! _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-20 16:14 ` Dmitry Vyukov @ 2020-02-20 16:33 ` Jason A. Donenfeld 2020-02-20 16:44 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Jason A. Donenfeld @ 2020-02-20 16:33 UTC (permalink / raw) To: Dmitry Vyukov; +Cc: netdev, syzbot, WireGuard mailing list Hi Dmitry, On Thu, Feb 20, 2020 at 5:14 PM Dmitry Vyukov <dvyukov@google.com> wrote: > I got some coverage in wg_netdevice_notification: > https://imgur.com/a/1sJZKtp > > Or you mean the parts that are still red? Yes, it's the red parts that interest me. Intermixing those with various wireguard-specific netlink calls and setting devices up and down and putting traffic through those sockets, in weird ways, could dig up bugs. > I think theoretically these parts should be reachable too because > syzkaller can do unshare and obtain net ns fd's. > > It's quite hard to test because it just crashes all the time on known bugs. > So maybe the most profitable way to get more coverage throughout the > networking subsystem now is to fix the top layer of crashers ;) Ahhh, interesting, so the issue is that syzkaller is finding too many other networking stack bugs before it gets to being able to play with wireguard. Shucks. Jason _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-20 16:33 ` Jason A. Donenfeld @ 2020-02-20 16:44 ` Dmitry Vyukov 2020-02-20 16:59 ` Jason A. Donenfeld 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2020-02-20 16:44 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: netdev, syzbot, WireGuard mailing list On Thu, Feb 20, 2020 at 5:34 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Dmitry, > > On Thu, Feb 20, 2020 at 5:14 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > I got some coverage in wg_netdevice_notification: > > https://imgur.com/a/1sJZKtp > > > > Or you mean the parts that are still red? > > Yes, it's the red parts that interest me. Intermixing those with > various wireguard-specific netlink calls and setting devices up and > down and putting traffic through those sockets, in weird ways, could > dig up bugs. > > > I think theoretically these parts should be reachable too because > > syzkaller can do unshare and obtain net ns fd's. > > > > It's quite hard to test because it just crashes all the time on known bugs. > > So maybe the most profitable way to get more coverage throughout the > > networking subsystem now is to fix the top layer of crashers ;) > > Ahhh, interesting, so the issue is that syzkaller is finding too many > other networking stack bugs before it gets to being able to play with > wireguard. Shucks. If it's aimed only at, say, wireguard netlink interface, then it's not distracted by bugs in other parts. But as you add some ipv4/6 tcp/udp sockets, more netlink to change these net namespaces, namespaces related syscalls, packet injection, etc, in the end it covers quite a significant part of kernel. You know how fuzzing works, right. You really need to fix the current layer of bugs to get to the next one. And we accumulated 600+ open bugs. It still finds some new ones, but I guess these are really primitive ones (as compared to its full bug finding potential). _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] 2020-02-20 16:44 ` Dmitry Vyukov @ 2020-02-20 16:59 ` Jason A. Donenfeld 0 siblings, 0 replies; 13+ messages in thread From: Jason A. Donenfeld @ 2020-02-20 16:59 UTC (permalink / raw) To: Dmitry Vyukov; +Cc: netdev, syzbot, WireGuard mailing list On Thu, Feb 20, 2020 at 5:45 PM Dmitry Vyukov <dvyukov@google.com> wrote: > If it's aimed only at, say, wireguard netlink interface, then it's not > distracted by bugs in other parts. But as you add some ipv4/6 tcp/udp > sockets, more netlink to change these net namespaces, namespaces > related syscalls, packet injection, etc, in the end it covers quite a > significant part of kernel. You know how fuzzing works, right. You > really need to fix the current layer of bugs to get to the next one. > And we accumulated 600+ open bugs. It still finds some new ones, but I > guess these are really primitive ones (as compared to its full bug > finding potential). Yea, seems reasonable. I need to get a local syzkaller instance set up for customization and then start patching the things that seem to be standing in the way. Either way, so long as there isn't some implementation issue or logical problem getting in the way of calling that codepath, I'm satisfied in knowing that syzkaller will get there eventually. _______________________________________________ WireGuard mailing list WireGuard@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/wireguard ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-02-20 16:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20191208232734.225161-1-Jason@zx2c4.com> [not found] ` <CACT4Y+bsJVmgbD-WogwU=LfWiPN1JgjBrwx4s8Y14hDd7vqqhQ@mail.gmail.com> [not found] ` <CAHmME9o0AparjaaOSoZD14RAW8_AJTfKfcx3Y2ndDAPFNC-MeQ@mail.gmail.com> [not found] ` <CACT4Y+Zssd6OZ2-U4kjw18mNthQyzPWZV_gkH3uATnSv1SVDfA@mail.gmail.com> [not found] ` <CAHmME9oM=YHMZyg23WEzmZAof=7iv-A01VazB3ihhR99f6X1cg@mail.gmail.com> [not found] ` <CACT4Y+aCEZm_BA5mmVTnK2cR8CQUky5w1qvmb2KpSR4-Pzp4Ow@mail.gmail.com> [not found] ` <CAHmME9rYstVLCBOgdMLqMeVDrX1V-f92vRKDqWsREROWdPbb6g@mail.gmail.com> [not found] ` <CAHmME9qUWr69o0r+Mtm8tRSeQq3P780DhWAhpJkNWBfZ+J5OYA@mail.gmail.com> [not found] ` <CACT4Y+YfBDvQHdK24ybyyy5p07MXNMnLA7+gq9axq-EizN6jhA@mail.gmail.com> [not found] ` <CAHmME9qcv5izLz-_Z2fQefhgxDKwgVU=MkkJmAkAn3O_dXs5fA@mail.gmail.com> [not found] ` <CACT4Y+arVNCYpJZsY7vMhBEKQsaig_o6j7E=ib4tF5d25c-cjw@mail.gmail.com> [not found] ` <CAHmME9ofmwig2=G+8vc1fbOCawuRzv+CcAE=85spadtbneqGag@mail.gmail.com> [not found] ` <CACT4Y+awD47=Q3taT_-yQPfQ4uyW-DRpeWBbSHcG6_=b20PPwg@mail.gmail.com> 2020-02-04 21:39 ` syzkaller wireguard key situation [was: Re: [PATCH net-next v2] net: WireGuard secure network tunnel] Jason A. Donenfeld 2020-02-17 11:20 ` Dmitry Vyukov 2020-02-17 11:31 ` Jason A. Donenfeld 2020-02-17 11:44 ` Jason A. Donenfeld 2020-02-17 15:19 ` Dmitry Vyukov 2020-02-17 15:42 ` Dmitry Vyukov 2020-02-17 19:24 ` Dmitry Vyukov 2020-02-18 10:00 ` Dmitry Vyukov 2020-02-19 10:22 ` Jason A. Donenfeld 2020-02-20 16:14 ` Dmitry Vyukov 2020-02-20 16:33 ` Jason A. Donenfeld 2020-02-20 16:44 ` Dmitry Vyukov 2020-02-20 16:59 ` Jason A. Donenfeld
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).