On 11/19/20 2:55 PM, John Baldwin wrote:
> On 11/9/20 1:49 PM, Michael Tuexen wrote:
>> Author: tuexen
>> Date: Mon Nov 9 21:49:40 2020
>> New Revision: 367530
>> URL: https://svnweb.freebsd.org/changeset/base/367530
>>
>> Log:
>> RFC 7323 specifies that:
>> * TCP segments without timestamps should be dropped when support for
>> the timestamp option has been negotiated.
>> * TCP segments with timestamps should be processed normally if support
>> for the timestamp option has not been negotiated.
>> This patch enforces the above.
>>
>> PR: 250499
>> Reviewed by: gnn, rrs
>> MFC after: 1 week
>> Sponsored by: Netflix, Inc
>> Differential Revision: https://reviews.freebsd.org/D27148
>>
>> Modified:
>> head/sys/netinet/tcp_input.c
>> head/sys/netinet/tcp_stacks/bbr.c
>> head/sys/netinet/tcp_stacks/rack.c
>> head/sys/netinet/tcp_syncache.c
>> head/sys/netinet/tcp_timewait.c
>>
>> Modified: head/sys/netinet/tcp_timewait.c
>> ==============================================================================
>> --- head/sys/netinet/tcp_timewait.c Mon Nov 9 21:19:17 2020
>> (r367529)
>> +++ head/sys/netinet/tcp_timewait.c Mon Nov 9 21:49:40 2020
>> (r367530)
>> @@ -376,7 +376,7 @@ tcp_twstart(struct tcpcb *tp)
>> * looking for a pcb in the listen state. Returns 0 otherwise.
>> */
>> int
>> -tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unused, struct tcphdr
>> *th,
>> +tcp_twcheck(struct inpcb *inp, struct tcpopt *to, struct tcphdr *th,
>> struct mbuf *m, int tlen)
>> {
>> struct tcptw *tw;
>> @@ -410,6 +410,16 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unu
>> */
>> if (thflags & TH_RST)
>> goto drop;
>> +
>> + /*
>> + * If timestamps were negotiated during SYN/ACK and a
>> + * segment without a timestamp is received, silently drop
>> + * the segment.
>> + * See section 3.2 of RFC 7323.
>> + */
>> + if (((to->to_flags & TOF_TS) == 0) && (tw->t_recent != 0)) {
>> + goto drop;
>> + }
>
> This causes an insta-panic with TOE because toe_4tuple_check() passes in a
> NULL
> pointer for 'to'. I'm working on a fix for that, but perhaps wait to MFC
> until
> the fix is ready so they can be merged together?
>
> That said, TOE only calls this in the case that it has gotten a new SYN, so I
> wonder if it makes sense to apply this check on a new SYN. For a new SYN,
> shouldn't we not care if the new connection is using a different timestamp
> option from the old connection? The language in RFC 7323 3.2 is all about
> segments on an existing connection, not segments from a new connection I
> think?
>
> That is, I think we should perhaps move this check after the TH_SYN check so
> that a mismatch doesn't prevent recycling?
Actually, we move the check below requiring TH_ACK, I think this would fix the
TOE
case and also DTRT for plain SYNs for non-TOE:
diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c
index c52eab956303..85f1ccbe40f9 100644
--- a/sys/netinet/tcp_timewait.c
+++ b/sys/netinet/tcp_timewait.c
@@ -411,16 +411,6 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to, struct
tcphdr *th,
if (thflags & TH_RST)
goto drop;
- /*
- * If timestamps were negotiated during SYN/ACK and a
- * segment without a timestamp is received, silently drop
- * the segment.
- * See section 3.2 of RFC 7323.
- */
- if (((to->to_flags & TOF_TS) == 0) && (tw->t_recent != 0)) {
- goto drop;
- }
-
#if 0
/* PAWS not needed at the moment */
/*
@@ -455,6 +445,16 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to, struct
tcphdr *th,
if ((thflags & TH_ACK) == 0)
goto drop;
+ /*
+ * If timestamps were negotiated during SYN/ACK and a
+ * segment without a timestamp is received, silently drop
+ * the segment.
+ * See section 3.2 of RFC 7323.
+ */
+ if (((to->to_flags & TOF_TS) == 0) && (tw->t_recent != 0)) {
+ goto drop;
+ }
+
/*
* Reset the 2MSL timer if this is a duplicate FIN.
*/
The commented out PAWS bits would also seem to not be relevant for SYN-only
packets? However, I'm less sure of if that bit should be moved later as
well. (Or perhaps it should just be removed. It has been #if 0'd since the
timewait structure was first added back in 2003 by jlemon@)
--
John Baldwin
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
