Closed awelzel closed 1 year ago
Yeah, the problem here is that the Teredo analyzer flags anything starting with 0x00 0x00
or 0x00 0x01
as being a Teredo packet. That's going to hit an awful lot of other protocols that it shouldn't.
For another reference, here's how Wireshark is handling the heuristic for Teredo: https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-teredo.c#L237-302. It doesn't properly handle our teredo-udp-in-udp.pcap capture, and only shows the outer UDP frame with a data hunk inside it.
A different angle, @0xxon today said that analyzer's shouldn't raise violations if they weren't confirmed and given that we have a session with Teredo, maybe that's an okay fix:
--- a/src/packet_analysis/protocol/teredo/Teredo.cc
+++ b/src/packet_analysis/protocol/teredo/Teredo.cc
@@ -184,7 +184,8 @@ bool TeredoAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack
detail::TeredoEncapsulation te(this);
if ( ! te.Parse(data, len) )
{
- AnalyzerViolation("Bad Teredo encapsulation", conn, (const char*)data, len);
+ if AnalyzerConfirmed(packet->session)
+ AnalyzerViolation("Bad Teredo encapsulation", conn, (const char*)data, len);
return false;
}
It doesn't solve the fact that the analyzer is even considered, but it does stop raising analyzer_violation / analyzer_violation_info events if we're not even sure yet this is Teredo.
It's interesting: The Teredo::Weird()
implementation basically does that, so seems fine for AnalyzerViolation()
It's interesting: The Teredo::Weird() implementation basically does that, so seems fine for AnalyzerViolation()
I can confirm that this does indeed work. I have a NetBIOS pcap here, and the analyzer violations go away with that patch. I agree that I wish it could just avoid going into the analyzer at all, but I also tried reimplementing Wireshark's heuristic and Zeek fails to find the tunnels in the same ways that Wireshark itself does. I'll open a quick PR with this.
I haven't looked closer, but in the course of #2657 , it popped out that the MDNS packets from external pcaps or wikipedia.trace is causing analyzer violations for Teredo.
In the zeek-testing-private repository for short/medium there are also a few hits on non MDNS traffic.
If it's actually any MDNS packet, maybe we can improve the
DoDetect()
method of Teredo, or remove it and rely on the well-known port, or skip*:5353 -> (224.0.0.251|ff02::fb):5353
endpoints.