net: tcp->recv_max_ack isn't used

[pfalcon]

We maintain tcp->recv_max_ack, but never really use. Let's decide if we drop it now or keep dragging along. (IIRC, RFC 793 uses it for something, but for what...).

(A note because I plan to a "more useful" field, it can be added instead, or in addition to an unused field like recv_max_ack).

[pfalcon]

@tbursztyka , @lpereira , @askawu : FYI

[lpereira]

It it's not being used, I'd say junk it -- if we end up implementing something that actually uses it, we can bring it back. Many of the timeout fields in the TCP context struct are a heritage from fnet, and in "zephyrfying" the code, some of them were ignored by our implementation.

[pfalcon]

Ok, so I spent time to wade thru this. First of all, tcp->recv_max_ack is updated like follows https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/net/ip/tcp.c#L525:

        if (net_tcp_seq_greater(tcp->send_seq, tcp->recv_max_ack)) {
                tcp->recv_max_ack = tcp->send_seq;
        }

So, the purpose of recv_max_ack is to track maximum value of the outgoing sequence number. But that doesn't make sense. Seqno monotonically increases, so the current value is the maximum.

Why would it be stored, anyway? Well, even the name recv_max_ack kinda hints that it may have something to do with checking received ackno. It's not immediately obvious that ackno should be validated (just as seqno), but kinda it is. For example, attacker may send an ACK with a high number, with a purpose to drop all packets from retransmission queue, so they weren't retransmitted to the legitimate receiver. Such an attack would be hard to perform in practice, because the first line of defense is seqno checking, but nonetheless.

And indeed, bookworming thru RFC 793, it has:

A new acknowledgment (called an "acceptable ack"), is one for which
the inequality below holds:

SND.UNA < SEG.ACK =< SND.NXT

We don't seem to verify this. Yet, RFC 793 even says:

Reset Generation
...
2. If the connection is in any non-synchronized state (LISTEN,
SYN-SENT, SYN-RECEIVED), and the incoming segment acknowledges
something not yet sent (the segment carries an unacceptable ACK)

(I.e. under some conditions, incorrect ackno should be a cause of connection reset).

[pfalcon]

Summing ip:

1. tcp->recv_max_ack is truly useless and should be removed.
2. But we should validate ackno of received packets.