Marinetti / Bugs / #41 TCP connection RST fails to use correct IP

Logged In: YES
user_id=63328

I've had a little closer look at this, and it's a fair amount of
work to fix.

The TCPSENDDGM routine is used all throughout I.TCP, and
is the single send point for all outgoing TCP datagrams.
Problem is that it uses several variables from the user record,
such as destination IP, source port, destination port and TCP
receive window size. When in the CLOSED state, all these
variables in the user record are zeroed.

I actually changed the routine to be TCPSENDDGMTOIP,
where all the variables (including the new ones) were passed
in on the stack, then created a new TCPSENDDGM which
simply pushes the required user record variables on before
calling TCPSENDDGMTOIP. Then I changed the CLOSED
state call to push the new vars from the incoming datagram
and not the user record.

But then I found two more issues, so I backed everything out.
Someone needs to first verify what the receive window should
be in a CLOSED non-initiated state. Also, the user record
gets sent to SENDIPHEADER to generate the IP header, so
at this point I'm not sure whether these need to be primed as
well, or whether they're actually valid.

I think the solution I mentioned above is the best way to fix it,
but someone needs to spend the time to double check the
window size and the IP variables, before they make any
changes.

This routine is one of the most critical in Marinetti. and speed
also needs to be taken into consideration, whether that many
vars on the stack is worth it.

In something I've been working on recently, I've run into an instance of this issue with more problematic effects. In brief, when Marinetti has one or more active TCP connections but it processes an incoming TCP packet that is not from any of those connections, it will generally send out a TCP RST packet to one of the active connections, which may result in terminating that connection. This can also happen when packets come in or are processed from a previous valid connection that has been aborted with TCPIPAbortTCP.

The attached packet trace shows an example of this happening. (This is actually a session using the GSplus emulator; I’ve seen what I’m pretty sure is the same thing on a real GS but can’t easily record packet traces there.) In this case, my program on the GS (192.168.1.116) was communicating with a web server. The web server decides to terminate the TCP connection (due to inactivity), so it sends a FIN packet (packet 1 in the trace). Because the GS is busy processing and doesn't respond to this for a few seconds, it is then retransmitted several times (packets 2-5).

When Marinetti next has a chance to process packets, it sees the first FIN packet and ACKs it (packet 6). It also (in response to a retransmitted FIN packet?) sends a duplicate ACK (packet 7). Then my code sees that the connection has been closed by the server, and calls TCPIPAbortTCP to quickly close the connection on the GS side. This causes Marinetti to send a TCP RST (packet 8).

My code now tries to start a new connection to fetch more data from the server, causing Marinetti to send a SYN packet (packet 9). Immediately after that, however, it sends a TCP RST packet (packet 10). This is actually in response to one of the packets from the old connection that is only now being processed, but it is sent on the new connection (source port 1042). Note that the sequence number is in line with the old connection, not the new one.

The server accepts the new connection request by sending a SYN+ACK packet (packet 11). Immediately after that, however, it sends an ICMP port unreachable message (packet 12). This is in response to the erroneous RST packet 10, and I believe it indicates that that packet is being rejected because it has an out-of-range sequence number. Marinetti, however, interprets it as rejecting the new connection. The server hasn't actually rejected the connection and continues resending SYN+ACK packets, but Marinetti ignores this.

The key problem here is that Marinetti erroneously sends an RST packet on an active connection in response to an unclaimed packet that is not part of that connection. This occurs because TCPTOSSUNCLAIMED calls TCPSENDDGM without setting USERPTR, so it still contains the last value it had. This will usually be the user record for one of the active connections, if there are any. In the case shown, it’s the record for the new connection that was just started. The RST packet therefore uses the destination IP and source and destination ports from that record, rather than getting them from the packet that it is actually responding to.

The correct fix for this would to be send out RST packets with the right IP and port values, but as Richard described that might require non-trivial code changes. If this issue can't be properly fixed in the near term, I would suggest just disabling TCPTOSSUNCLAIMED, e.g. by making it simply return immediately. I tried patching it in memory to do that (changing the initial PHB instruction to RTL), and it seemed to avoid the problem: unclaimed packets are just ignored, and erroneous RST packets are not generated, so the connection is not terminated. AFAIK this has no downside, since it doesn't work correctly anyway.

Another way of preventing the connection from being erroneously terminated in this case would have been if Marinetti implemented TCP sequence number checking for ICMP messages, as described in RFC 5927 Section 4.1. It could have seen that the ICMP message related to something out of sequence with the real connection (in this case, the erroneous RST, but potentially also a malicious packet) and ignored it, rather than terminating the connection. This is a much less critical issue, but would also be nice to have.

Bad RST.png

Wireshark trace.pcapng

TCP connection RST fails to use correct IP

Group

Searches

Help

#41 TCP connection RST fails to use correct IP

Discussion