zeek / spicy-analyzers

Growing collection of Spicy-based protocol and file analyzers for Zeek
Other
32 stars 9 forks source link

Wireguard false positives #13

Closed keithjjones closed 3 years ago

keithjjones commented 3 years ago

I believe I'm seeing Wireguard false positives running this on a large university network (sorry I can't post their data). I think it's because protocolconfirmation is being called for every parsed type, so it's not seeing the full handshake before calling the first protocolconfirmation:

https://github.com/zeek/spicy-analyzers/blob/main/analyzer/protocol/wireguard/wireguard_zeek.spicy#L9

I'm seeing traffic identified as wireguard where there are a ton of packets in one direction but zero in the other direction, according to conn.log. This is likely a false positive. I'm making this issue to track work on fixing this. I think I can fix it, but I'm currently learning Spicy so it might not be right away.

0xxon commented 3 years ago

Yup, I actually worried a bit about this when writing the parser.

Could you check two things int he traffic for me:

I assume that it will do both - and the best fix might actually be to get spicy to error out when the first packet has extra data afterwards; however I am not sure one can do that with the primitives available at the moment.

0xxon commented 3 years ago

Looked over this with Keith - and at least with #14 there do no longer seem to be any (or at least no significant amounts) of false positives.

keithjjones commented 3 years ago

I recommend one more change to your dpd.sig:

signature wireguard_handshake_init {
  ip-proto == udp
  payload /^\x01\x00\x00\x00/
}

signature wireguard_handshake_resp {
  ip-proto == udp
  payload /^\x02\x00\x00\x00/
  requires-reverse-signature wireguard_handshake_init
  enable "wireguard"
}

I'm working on a binpac version of this analyzer and this dpd.sig kills the false positives we are seeing because it only activates on a full handshake. Without it the wireguard.log goes nuts from scans, or partial connections, or whatever I'm seeing on 3 big live networks. I think this is because it activates on any packet that looks like the first init via dpd, then the protocolconfirm happens right away in spicy. It's hard for me to tell what these weird not-full-handshake artifacts I'm seeing are because I can't pull a pcap, but the lines in wireguard.log don't look normal without looking for the full handshake. They are definitely noise to actual VPN connections as I'll see these easily 10-20:1 ratio on real networks without this dpd mod.

0xxon commented 3 years ago

I will follow up on this with you. At least from the data that you had yesterday, it seemed like the handshakes you saw were legitimate - they just did not get a response. And I think there is value in logging these.

keithjjones commented 3 years ago

Posting here for open discussion and because I won't be able to contribute to the spicy stuff for a while....

IMHO, I don't see the value in activating dpd on this liberal one sided signature we know up front will hit on more than just Wireguard:

https://github.com/zeek/spicy-analyzers/blob/topic/johanna/wg-misidentify/analyzer/protocol/wireguard/dpd.sig#L5

It will hit on tons of things that aren't wireguard on real networks. It's just four bytes, with no check in the reverse direction. That can be fixed with the example dpd.sig I posted above that at least ensures a handshake (even if the is_orig can't be checked, and is_orig is something I check in my binpac version in the wireguard-analyzer.pac file to reduce false positives). In the Spicy version, there are no checks other than the parse, four bytes up front, and size.

Then, once the spicy code parses one init message in either direction (no check on is_orig... we shouldn't see an init from !is_orig and we shouldn't see a response from is_orig during this handshake), it confirms the whole protocol: https://github.com/zeek/spicy-analyzers/blob/topic/johanna/wg-misidentify/analyzer/protocol/wireguard/wireguard_zeek.spicy#L9 We also shouldn't see a cookie or response before an init. The spicy code doesn't check for that either.

So this will call any single message parsed with those first four bytes with size of: https://github.com/zeek/spicy-analyzers/blob/topic/johanna/wg-misidentify/analyzer/protocol/wireguard/wireguard.spicy#L22 that is either an init, resp, or cookie on any udp port connection wireguard, which I think is premature and leading to the false positives I'm seeing on real networks. Even after adding the "nothing" fields in the "wg-misidentify" branch, which did help cut out the false positives dramatically that were the wrong size, I'm still seeing false positives (in my opinion) because the wiregaurd.log is filling up (a lot!) with stuff other than the wireguard inits, but looked like wireguard inits to this spicy code. It's not really wireguard unless we at least see a return response to the init in the correct directions. With my binpac code I'm seeing on real networks that this init/resp is sent over and over even for long idle connections on the same port combinations. It's a very unique thing to wireguard that can tell us this is really a wireguard connection and not a by-chance polyglot of some sort.

Then, since spicy is not full integrated in Zeek yet without installing extra tools, I had to move on to a non-spicy version of this analyzer for my research purposes. Unfortunately, I won't be able to quickly test or add back to this any time soon because I had to disable spicy from my real networks... but I am able and am happy to trouble shoot things through what I can learn from my binpac version on those networks. I will be continuing to play and tweak with that version and I'm happy to share info I learn from that process if you want to add it to this spicy version. I'll let you know when I can get some spicy testing on real networks again in the future, as I really enjoyed playing with this language and can't wait for it to be fully integrated into Zeek. Thanks!

0xxon commented 3 years ago

Talked a bit more to Keith - will leave like this for the moment until we have had a chance to actually test this on more traffic and see if it causes false positives.

I hope I will be able to do this within the next 2-3 weeks.

0xxon commented 3 years ago

After discussing this more with Keith, there actually do not seem to be significant false positives; instead there is just a lot of noise in the log because there is a significant number of hosts that never establish connections, blasting a UDP packet every few seconds. So far all of these were logged separately.

I just opened GH-33 which should address his. The wireguard log is changed to only have one line per UDP connection which counts the number of handshake packets encountered.

keithjjones commented 3 years ago

👍