Issue with checking for only SYN and SYN-ACK flags to be set

[reethikar]

Found a 🐞 with the function that checks for SYN, SYN/ACK packets. (First reported in #4(comment))

In tcp_handshake.go, L289:L305 in isSynSegment() and isSynAckSegment(): I found the following legitimate case where the ECN and CWR flags are being set by the VPN, presumably to indicate the capability for handling Explicit Congestion Notifications:

87  30.043184   <VPN_IP>    <SERVER_IP> TCP 78  60707 → 443 [SYN, ECN, CWR] Seq=0 Win=65535 Len=0 MSS=1340 WS=64 TSval=1754955918 TSecr=0 SACK_PERM=1
88  30.043226   <SERVER_IP> <VPN_IP>    TCP 74  443 → 60707 [SYN, ACK, ECN] Seq=0 Ack=1 Win=26847 Len=0 MSS=8961 SACK_PERM=1 TSval=2960511171 TSecr=1754955918 WS=128
89  30.403246   <VPN_IP>    <SERVER_IP> TCP 66  60707 → 443 [ACK] Seq=1 Ack=1 Win=131456 Len=0 TSval=1754956384 TSecr=2960511171

And the webserver responds appropriately with the ECN flag set in the SYN-ACK packet. I think we should account for this, and accommodate it.

I tried simply removing those flags from the checks on lines 289 and 305 but that doesn't do the trick. @NullHypothesis Would you be able to take a look into this? 🙏

For the runs already captured, where this could have been an issue, we have the pcaps, plus since I logged the FourTuple, we know exactly which sets of packets SYN-ACK and ACK packets to extract this info from the pcap.

[NullHypothesis]

Perhaps we need to also remove the check for the tcp.NS flag...?

[reethikar]

Closed by #7