Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for TCP Fast Open to prevent false negatives #1204

Closed
wants to merge 1 commit into from
Closed

Add support for TCP Fast Open to prevent false negatives #1204

wants to merge 1 commit into from

Conversation

cnotin
Copy link

@cnotin cnotin commented Apr 29, 2018

​Nmap currently discards SYN-ACK packets having the TCP Fast Open option (RFC 7413) set. Thus leading to false negative results.
This patch implements the support of this option, fixing the false negative.
I tested successfully this patch in the field, of course.

Full story:
During an internal pentest, I discovered a false negative on a TCP port that I knew for sure was open. SYN scans did not s​​ee it, whereas connect scans were good.

Running nmap with -ddd I saw that the SYN ACK packet was discarded:
Rejecting TCP packet because of bad TCP header

In Wireshark, I saw that the TCP Fast Open option was enabled.
Its kind is "34" which is not currently handled in the code. Therefore the code reads the next byte, which is the length of the option and incorrectly interprets it as a kind. According to the RFC, the length is a range from 6 to 18. In my case it was 8, so the code assumed it was a timestamp header and it was then incorrect.

I know that TCP Fast Open option should not appear in SYN-ACKs if the option was not present in the prior SYN. However, this is what I observed on my client's network...
This is also related to this message in the mailing list.

You can test it against the following scapy code, after preventing the default behavior of the kernel with iptables -A OUTPUT -p tcp --tcp-flags RST RST --sport 8000 -j DROP

#!/usr/bin/python
from scapy.all import *
   
a=sniff(count=1,filter="tcp and host 127.0.0.1 and port 8000")[0]

TCP_SYNACK=TCP(sport=8000, dport=a.sport, flags="SA", seq=a.seq, ack=a.seq+1, options=[(34, "ABCDEF"),('MSS', 1460)]) # triggers false negative

ANSWER=send(IP(src="127.0.0.1", dst="127.0.0.1")/TCP_SYNACK)

With the current nmap, you get:

# nmap -Pn -sS -p 8000 127.0.0.1 -ddd --reason
Starting Nmap 7.70 ( https://nmap.org ) at 2018-04-29 18:42 CEST
[...]
SENT (0.1109s) TCP [127.0.0.1:47933 > 127.0.0.1:8000 S seq=995761996 ack=0 off=6 res=0 win=1024 csum=0x6100 urp=0 <mss 1460>] IP [ver=4 ihl=5 tos=0x00 iplen=44 id=19678 foff=0 ttl=55 proto=6 csum=0x38ec]
[...]
Rejecting TCP packet because of bad TCP header
[...]
PORT     STATE    SERVICE  REASON
8000/tcp filtered http-alt no-response

And with the patch:

# ~/nmap/nmap -Pn -sS -p 8000 127.0.0.1 -ddd --reason
Starting Nmap 7.70SVN ( https://nmap.org ) at 2018-04-29 18:44 CEST
[...]
SENT (0.1204s) TCP [127.0.0.1:50033 > 127.0.0.1:8000 S seq=903515506 ack=0 off=6 res=0 win=1024 csum=0xF025 urp=0 <mss 1460>] IP [ver=4 ihl=5 tos=0x00 iplen=44 id=39884 foff=0 ttl=54 proto=6 csum=0xeafd]
[...]
RCVD (0.1878s) TCP [127.0.0.1:8000 > 127.0.0.1:50033 SA seq=903515506 ack=903515507 off=8 res=0 win=8192 csum=0x04EB urp=0] IP [ver=4 ihl=5 tos=0x00 iplen=52 id=1 foff=0 ttl=64 proto=6 csum=0x7cc1]
Found 127.0.0.1 in incomplete hosts list.
Discovered open port 8000/tcp on 127.0.0.1
[...]
PORT     STATE SERVICE  REASON
8000/tcp open  http-alt syn-ack ttl 64

@dmiller-nmap
Copy link

Thanks for this patch! I thought about this a while and decided that it's best that we do proper TLV checking for all TCP options except EOL and NOP (which are single-byte options). Doing it this way, there's no special case for Fast Open. The purpose of this function is to validate that known-length options have those lengths, and Fast Open is a variable-length option, so it can't really be checked in the same way. I just pushed an alternate change that does this, and I'll credit you with discovering the problem in the CHANGELOG.

nmap-bot pushed a commit that referenced this pull request May 1, 2018
nmap-bot pushed a commit that referenced this pull request May 1, 2018
@cnotin
Copy link
Author

cnotin commented May 1, 2018

Thanks @dmiller-nmap for your quick feedback and for crediting me!
FYI, I ran tests with your patch and it works great! It's also more generic.

@cnotin cnotin deleted the patch-1 branch February 6, 2019 23:16
@dmiller-nmap dmiller-nmap mentioned this pull request Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants