Commit 51ab40dc authored by David S. Miller's avatar David S. Miller

Code (and commentary) in SYN-RECEIVED processing

assumes that it cannot be reached in the crossed SYN case.
This is wrong if the original SYNs came from a malicious packet
generator third party.  This can result in a 4 minute ACK
fight if the sequence numbers are correct.

The fix is the verify the ACK before we do anything else, which
should cover all cases.

This bug was discovered by Casper Dik.
parent aecec5f9
...@@ -849,8 +849,38 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb, ...@@ -849,8 +849,38 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb,
/* Further reproduces section "SEGMENT ARRIVES" /* Further reproduces section "SEGMENT ARRIVES"
for state SYN-RECEIVED of RFC793. for state SYN-RECEIVED of RFC793.
It is broken, however, it does not work only It is broken, however, it does not work only
when SYNs are crossed, which is impossible in our when SYNs are crossed.
case.
You would think that SYN crossing is impossible here, since
we should have a SYN_SENT socket (from connect()) on our end,
but this is not true if the crossed SYNs were sent to both
ends by a malicious third party. We must defend against this,
and to do that we first verify the ACK (as per RFC793, page
36) and reset if it is invalid. Is this a true full defense?
To convince ourselves, let us consider a way in which the ACK
test can still pass in this 'malicious crossed SYNs' case.
Malicious sender sends identical SYNs (and thus identical sequence
numbers) to both A and B:
A: gets SYN, seq=7
B: gets SYN, seq=7
By our good fortune, both A and B select the same initial
send sequence number of seven :-)
A: sends SYN|ACK, seq=7, ack_seq=8
B: sends SYN|ACK, seq=7, ack_seq=8
So we are now A eating this SYN|ACK, ACK test passes. So
does sequence test, SYN is truncated, and thus we consider
it a bare ACK.
If tp->defer_accept, we silently drop this bare ACK. Otherwise,
we create an established connection. Both ends (listening sockets)
accept the new incoming connection and try to talk to each other. 8-)
Note: This case is both harmless, and rare. Possibility is about the
same as us discovering intelligent life on another plant tomorrow.
But generally, we should (RFC lies!) to accept ACK But generally, we should (RFC lies!) to accept ACK
from SYNACK both here and in tcp_rcv_state_process(). from SYNACK both here and in tcp_rcv_state_process().
...@@ -862,6 +892,22 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb, ...@@ -862,6 +892,22 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb,
before attempt to create socket. before attempt to create socket.
*/ */
/* RFC793 page 36: "If the connection is in any non-synchronized state ...
* and the incoming segment acknowledges something not yet
* sent (the segment carries an unaccaptable ACK) ...
* a reset is sent."
*/
if (!(flg & TCP_FLAG_ACK))
return NULL;
/* Invalid ACK: reset will be sent by listening socket */
if (TCP_SKB_CB(skb)->ack_seq != req->snt_isn+1)
return sk;
/* Also, it would be not so bad idea to check rcv_tsecr, which
* is essentially ACK extension and too early or too late values
* should cause reset in unsynchronized states.
*/
/* RFC793: "first check sequence number". */ /* RFC793: "first check sequence number". */
if (paws_reject || !tcp_in_window(TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq, if (paws_reject || !tcp_in_window(TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq,
...@@ -891,19 +937,6 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb, ...@@ -891,19 +937,6 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb,
if (flg & (TCP_FLAG_RST|TCP_FLAG_SYN)) if (flg & (TCP_FLAG_RST|TCP_FLAG_SYN))
goto embryonic_reset; goto embryonic_reset;
/* RFC793: "fifth check the ACK field" */
if (!(flg & TCP_FLAG_ACK))
return NULL;
/* Invalid ACK: reset will be sent by listening socket */
if (TCP_SKB_CB(skb)->ack_seq != req->snt_isn+1)
return sk;
/* Also, it would be not so bad idea to check rcv_tsecr, which
* is essentially ACK extension and too early or too late values
* should cause reset in unsynchronized states.
*/
/* If TCP_DEFER_ACCEPT is set, drop bare ACK. */ /* If TCP_DEFER_ACCEPT is set, drop bare ACK. */
if (tp->defer_accept && TCP_SKB_CB(skb)->end_seq == req->rcv_isn+1) { if (tp->defer_accept && TCP_SKB_CB(skb)->end_seq == req->rcv_isn+1) {
req->acked = 1; req->acked = 1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment