Re: [PATCH v1] tcp: enhancing timestamps random algo to address issues arising from NAT mapping

[Florian Westphal <fw@strlen.de>]

George Guo <guodongtai@kylinos.cn> wrote:
> Tsval=tsoffset+local_clock, here tsoffset is randomized with saddr and daddr parameters in func
> secure_tcp_ts_off. Most of time it is OK except for NAT mapping to the same port and daddr.
> Consider the following scenario:
> 	ns1:                ns2:
> 	+-----------+        +-----------+
> 	|           |        |           |
> 	|           |        |           |
> 	|           |        |           |
> 	| veth1     |        | vethb     |
> 	|192.168.1.1|        |192.168.1.2|
> 	+----+------+        +-----+-----+
> 	     |                     |
> 	     |                     |
> 	     | br0:192.168.1.254   |
> 	     +----------+----------+
> 	 veth0          |     vetha
> 	 192.168.1.3    |    192.168.1.4
> 	                |
> 	               nat(192.168.1.x -->172.30.60.199)
> 	                |
> 	                V
> 	               eth0
> 	         172.30.60.199
> 	               |
> 	               |
> 	               +----> ... ...    ---->server: 172.30.60.191
> 
> Let's say ns1 (192.168.1.1) generates a timestamp ts1, and ns2 (192.168.1.2) generates a timestamp
> ts2, with ts1 > ts2.
> 
> If ns1 initiates a connection to a server, and then the server actively closes the connection,
> entering the TIME_WAIT state, and ns2 attempts to connect to the server while port reuse is in
> progress, due to the presence of NAT, the server sees both connections as originating from the
> same IP address (e.g., 172.30.60.199) and port. However, since ts2 is smaller than ts1, the server
> will respond with the acknowledgment (ACK) for the fourth handshake.
> 
>        SERVER                                               	CLIENT
> 
>    1.  ESTABLISHED                                          	ESTABLISHED
> 
>        (Close)
>    2.  FIN-WAIT-1  --> <SEQ=100><ACK=300><TSval=20><CTL=FIN,ACK>  --> CLOSE-WAIT
> 
>    3.  FIN-WAIT-2  <-- <SEQ=300><ACK=101><TSval=40><CTL=ACK>      <-- CLOSE-WAIT
> 
>                                                             (Close)
>    4.  TIME-WAIT   <-- <SEQ=300><ACK=101><TSval=41><CTL=FIN,ACK>  <-- LAST-ACK
> 
>    5.  TIME-WAIT   --> <SEQ=101><ACK=301><TSval=25><CTL=ACK>      --> CLOSED
> 
>   - - - - - - - - - - - - - port reused - - - - - - - - - - - - - - -
> 
>    5.1. TIME-WAIT   <-- <SEQ=255><TSval=30><CTL=SYN>             <-- SYN-SENT
> 
>    5.2. TIME-WAIT   --> <SEQ=101><ACK=301><TSval=35><CTL=ACK>    --> SYN-SENT
> 
>    5.3. CLOSED      <-- <SEQ=301><CTL=RST>             		 <-- SYN-SENT
> 
>    6.  SYN-RECV    <-- <SEQ=255><TSval=34><CTL=SYN>              <-- SYN-SENT
> 
>    7.  SYN-RECV    --> <SEQ=400><ACK=301><TSval=40><CTL=SYN,ACK> --> ESTABLISHED
> 
>    1.  ESTABLISH   <-- <SEQ=301><ACK=401><TSval=55><CTL=ACK>     <-- ESTABLISHED
> 
> This enhancement uses sport and daddr rather than saddr and daddr, which keep the timestamp
> monotonically increasing in the situation described above. Then the port reuse is like this:

We used to have per-connection timestamps, i.e. hash used to include
port numbers as well.

Unfortunately there were problem reports, too many devices expect
monotonically increasing ts from the same address.

See 28ee1b746f49 ("secure_seq: downgrade to per-host timestamp offsets")

So, I don't think we can safely substitute saddr with sport.