From e4a1c308288d74a6ba7214f3d4909597906365f9 Mon Sep 17 00:00:00 2001
From: Tim Wojtulewicz <tim@corelight.com>
Date: Thu, 13 Apr 2023 15:48:06 -0700
Subject: [PATCH] Fix IEEE 802.11 analyzer to skip packets with the Protected
 bit enabled

---
 .../protocol/ieee802_11/IEEE802_11.cc         |  26 +++++++++---------
 .../btest/Traces/tunnels/gre-aruba-ccmp.pcap  | Bin 0 -> 692 bytes
 .../btest/core/tunnels/gre-aruba-ccmp.zeek    |   5 ++++
 3 files changed, 18 insertions(+), 13 deletions(-)
 create mode 100644 testing/btest/Traces/tunnels/gre-aruba-ccmp.pcap
 create mode 100644 testing/btest/core/tunnels/gre-aruba-ccmp.zeek

diff --git a/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc b/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc
index ad1ce796f2..edc04e08d0 100644
--- a/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc
+++ b/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc
@@ -26,19 +26,22 @@ bool IEEE802_11Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet*
 	if ( (fc_80211 >> 4) & 0x04 )
 		return false;
 
-	// 'To DS' and 'From DS' flags set indicate use of the 4th
-	// address field.
+	// 'To DS' and 'From DS' flags set indicate use of the 4th address field.
 	if ( (data[1] & 0x03) == 0x03 )
 		len_80211 += packet->L2_ADDR_LEN;
 
 	// Look for the QoS indicator bit.
 	if ( (fc_80211 >> 4) & 0x08 )
 		{
-		// Skip in case of A-MSDU subframes indicated by QoS
-		// control field.
+		// Skip in case of A-MSDU subframes indicated by QoS control field.
 		if ( data[len_80211] & 0x80 )
 			return false;
 
+		// Check for the protected bit. This means the data is encrypted and we can't
+		// do anything with it.
+		if ( data[1] & 0x40 )
+			return true;
+
 		len_80211 += 2;
 		}
 
@@ -48,8 +51,7 @@ bool IEEE802_11Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet*
 		return false;
 		}
 
-	// Determine link-layer addresses based
-	// on 'To DS' and 'From DS' flags
+	// Determine link-layer addresses based on 'To DS' and 'From DS' flags
 	switch ( data[1] & 0x03 )
 		{
 		case 0x00:
@@ -83,10 +85,9 @@ bool IEEE802_11Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet*
 		return false;
 		}
 
-	// Check that the DSAP and SSAP are both SNAP and that the control
-	// field indicates that this is an unnumbered frame.
-	// The organization code (24bits) needs to also be zero to
-	// indicate that this is encapsulated ethernet.
+	// Check that the DSAP and SSAP are both SNAP and that the control field indicates that this is
+	// an unnumbered frame.  The organization code (24bits) needs to also be zero to indicate that
+	// this is encapsulated ethernet.
 	if ( data[0] == 0xAA && data[1] == 0xAA && data[2] == 0x03 && data[3] == 0 && data[4] == 0 &&
 	     data[5] == 0 )
 		{
@@ -94,9 +95,8 @@ bool IEEE802_11Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet*
 		}
 	else
 		{
-		// If this is a logical link control frame without the
-		// possibility of having a protocol we care about, we'll
-		// just skip it for now.
+		// If this is a logical link control frame without the possibility of having a protocol we
+		// care about, we'll just skip it for now.
 		return false;
 		}
 
diff --git a/testing/btest/Traces/tunnels/gre-aruba-ccmp.pcap b/testing/btest/Traces/tunnels/gre-aruba-ccmp.pcap
new file mode 100644
index 0000000000000000000000000000000000000000..8a032644216bea34baccfd9279cfc39bcd18bead
GIT binary patch
literal 692
zcmca|c+)~A1{MYcU}0bca;zqOOX_vuV6X#nK$w9cj)9SpK}wG4--bp81~Co>R|ZBM
zkoZ6S`o~<4#5lRq1o9aeni)ErW=YLhpL(igzr*8eNk0yBJosF{?f^(-VgZ8!$f&|8
zhO51WUYOLJc+YQo%I)<giEifi=WDdY&hdoGSqOKC*Q8YEEOR$he8u9iDAuV=SxEBt
zqlXP&uDpJ`)BBh<<29+uKgT}!@fPsydGb?A{QHXPqvu5rd-mzErsz+f`{3b)^?mia
zU(Gb{e46jZbH`9$Uo6h9FTY}vxYE-^2`9E3C(Uc6om0bVwh1kd=)V5=amD_U30#_I
zSBGjGo$Qqx5GFqLtP|h<9qBUtkA8GTGEX~fmU%V9_vP_z-*;L2yl9jCBk8jF>WoQm
zx5(BdE!@!KHNB|rENgk~O`o*lo9T@?i?UiXKbbr7i$&UBarngMzb4`UC^Q!$g(fg$
Z&gu6+LZ*>{<3A!a|LQl4h9-f~1OPdrCN2N~

literal 0
HcmV?d00001

diff --git a/testing/btest/core/tunnels/gre-aruba-ccmp.zeek b/testing/btest/core/tunnels/gre-aruba-ccmp.zeek
new file mode 100644
index 0000000000..5ef3060638
--- /dev/null
+++ b/testing/btest/core/tunnels/gre-aruba-ccmp.zeek
@@ -0,0 +1,5 @@
+# @TEST-DOC: Tests a GRE ARUBA trace that contains IEEE 802.11 CCMP headers. This should report a weird about encrypted data.
+# @TEST-EXEC: zeek -C -b -r $TRACES/tunnels/gre-aruba-ccmp.pcap %INPUT
+# @TEST-EXEC: btest-diff weird.log
+
+@load base/frameworks/notice/weird