On Tue, Nov 2, 2021 at 3:03 AM Leonard Crestez cdleonard@gmail.com wrote:
On 11/1/21 9:22 PM, Francesco Ruggeri wrote:
+/* Compute SNE for a specific packet (by seq). */ +static int compute_packet_sne(struct sock *sk, struct tcp_authopt_info *info,
u32 seq, bool input, __be32 *sne)+{
u32 rcv_nxt, snd_nxt;// We can't use normal SNE computation before reaching TCP_ESTABLISHED// For TCP_SYN_SENT the dst_isn field is initialized only after we// validate the remote SYN/ACK// For TCP_NEW_SYN_RECV there is no tcp_authopt_info at allif (sk->sk_state == TCP_SYN_SENT ||sk->sk_state == TCP_NEW_SYN_RECV ||sk->sk_state == TCP_LISTEN)return 0;In case of TCP_NEW_SYN_RECV, if our SYNACK had sequence number 0xffffffff, we will receive an ACK sequence number of 0, which should have sne = 1.
In a somewhat similar corner case, when we receive a SYNACK to our SYN in tcp_rcv_synsent_state_process, if the SYNACK has sequence number 0xffffffff, we set tp->rcv_nxt to 0, and we should set sne to 1.
There may be more similar corner cases related to a wraparound during the handshake.
Since as you pointed out all we need is "recent" valid <sne, seq> pairs as reference, rather than relying on rcv_sne being paired with tp->rcv_nxt (and similarly for snd_sne and tp->snd_nxt), would it be easier to maintain reference <sne, seq> pairs for send and receive in tcp_authopt_info, appropriately handle the different handshake cases and initialize the pairs, and only then track them in tcp_rcv_nxt_update and tcp_rcv_snd_update?
For TCP_NEW_SYN_RECV there is no struct tcp_authopt_info, only a request minisock. I think those are deliberately kept small save resources on SYN floods so I'd rather not increase their size.
For all the handshake cases we can just rely on SNE=0 for ISN and we already need to keep track of ISNs because they're part of the signature.
Exactly. But the current code, when setting rcv_sne and snd_sne, always compares the sequence number with the <info->rcv_sne, tp->rcv_nxt> (or <info->snd_sne, tp->snd_nxt>) pair, where info->rcv_sne and info->snd_sne are initialized to 0 at the time of info creation. In other words, the code assumes that rcv_sne always corresponds to tp->rcv_nxt, and snd_sne to tp->snd_nxt. But that may not be true when info is created, on account of rollovers during a handshake. So it is not just a matter of what to use for SNE before info is created and used, but also how SNEs are initialized in info. That is why I was suggesting of saving valid <sne, seq> pairs (initialized with <0, ISN>) in tcp_authopt_info rather than just SNEs, and then always compare seq to those pairs if info is available. The pairs could then be updated in tcp_rcv_nxt_update and tcp_snd_una_update.
Regards, Francesco