Don't retain TF_RCVD_TSTMP when TF_NOOPT prevents sending timestamps

[rscheff]

Summary

The strict adherance to RFC7323 (D27148) requires the server to take
into consideration, if it will be sending out a TSopt in the SYN,ACK.

Retaining the TF_RCVD_TSTMP would otherwise later indicate the
successful negotiation of the TSopt - even when the session was set up
without TS support.

See also bug 253576

MFC after: 3 days

Test Plan

ifconfig epair0 create
ifconfig epair0a 10.0.0.101 up
ifconfig epair0b 10.0.0.102 up
nc --no-tcpopt -l 10.0.0.101 12345 &
nc -s 10.0.0.102 10.0.0.101 12345 &

netstat -nap tcp

verify, that both half-connections are in the established state

[tuexen]

sys/netinet/tcp_input.c

1652–1653

We need to replace

if ((to.to_flags & TOF_SCALE) &&
    (tp->t_flags & TF_REQ_SCALE)) {

by

if ((to.to_flags & TOF_SCALE) &&
    (tp->t_flags & TF_REQ_SCALE) &&
    !(tp->t_flags & TF_NOOPT)) {

sys/netinet/tcp_syncache.c

1665

I guess the same argument applies to window scaling.
So I would suggest to replace

if (V_tcp_do_rfc1323) {

by

if (V_tcp_do_rfc1323 && !(ltflags & TF_NOOPT)) {

and remove the condition here.

[tuexen]

The above modifications are necessary in combination with D28656 to allow the following packetdrill script to run successfully:

 0.00 `sysctl -w net.inet.tcp.hostcache.purgenow=1`
+0.00 `sysctl -w net.inet.tcp.syncookies_only=0`
+0.00 `sysctl -w net.inet.tcp.syncookies=1`
+0.00 `sysctl -w net.inet.tcp.rfc1323=1`
+0.00 `sysctl -w net.inet.tcp.sack.enable=1`
+0.00 `sysctl -w net.inet.tcp.ecn.enable=2`

+0.00 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0.00 fcntl(3, F_GETFL) = 0x02 (flags O_RDWR)
+0.00 fcntl(3, F_SETFL, O_RDWR | O_NONBLOCK) = 0
+0.00 setsockopt(3, IPPROTO_TCP, TCP_NOOPT, [1], 4) = 0
+0.00 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
+0.00 > S    0:0(0)       win 65535
+0.05 < S    0:0(0)       win 65535 <mss 1460,nop,wscale 6,sackOK,TS val 100 ecr 0>
+0.00 > S.   0:0(0) ack 1 win 65535
+0.00 <  .   1:1(0) ack 1 win 65535
+0.00 < P.   1:2(1) ack 1 win 65535
+0.04 >  .   1:1(0) ack 2 win 65535

[rscheff]

• consider TF_NOOPT for all SYN options (simultaneous SYN)

[rscheff]

Generalizing, the other options need to be disabled too, in case of a simultaneous SYN. Disabling the request before that, during the tcp_newtcpcb() for an abundance of safety.

[rscheff]

• typo

[tuexen]

sys/netinet/tcp_subr.c

1766 ↗

(On Diff #83863)

I think this can't be true, since you just created the tcb and it is initialized with 0. So I would suggest to remove this change.

[rscheff]

Programmatically, correct. Which makes me wonder, shouldn't the TF_NOOPT (and other flags) be inherited from a listening socket, when you accept() an incoming connection?

E.g. if you want to check if specific options are set on an accepted socket (rather then the listening socket), it currently doesn't seem to reflect the settings actually in effect, but the system global defaults...

[rscheff]

• remove check in tcp_newtcpcb() again

[tuexen]

sys/netinet/tcp_input.c

1677

I don't think we should never be here with TCP_NOOPT.
Also disabling the path is not the correct action here.

[rscheff]

• moving the parallel SYN check further down, to gracefully disable FASTOPEN

[rscheff]

sys/netinet/tcp_input.c

1677

This also deals with the parallel open state: While we may not have sent TFO in the outgoing SYN, the incoming SYN could have a TFO option. I suggest to gracefully disable TFO in this case, instead of having one half-connecting in TFO state, and the other without TFO, as NOOPT prevents the transmission of TFO later in tcp_output().

[tuexen]

sys/netinet/tcp_input.c

1677

If you want to do a check here, do it above. The way you do it now results in disabling TFO with a reason for doing so.

1677

This also deals with the parallel open state: While we may not have sent TFO in the outgoing SYN, the incoming SYN could have a TFO option. I suggest to gracefully disable TFO in this case, instead of having one half-connecting in TFO state, and the other without TFO, as NOOPT prevents the transmission of TFO later in tcp_output().