Hi all
Either parse byte by byte, or use htonl(). Not both, so only:
tsval = (tcp_getoptbyte() << 24);
tsval |= (tcp_getoptbyte() << 16);
tsval |= (tcp_getoptbyte() << 8);
tsval |= tcp_getoptbyte();
And you should be finished.
Stian Skjelstad
Hello Tsachi, all,
On Wed, May 7, 2025 at 10:44 AM Tsachi Kimeldorfer <t_kimel@hotmail.com> wrote:
>
> In the current lwIP implementation, the TCP Timestamp option (TSval) is parsed by sequentially reading 4 bytes using tcp_getoptbyte() and combining them with shifts.
> However, the assembly of these bytes assumes little-endian order, which causes incorrect results on big-endian systems (such as MIPS), where lwip_ntohl() is defined as a no-op.
> This results in pcb->ts_recent containing an incorrectly parsed value, which is then echoed back in TSecr, violating the TCP Timestamp Echo Reply behavior defined in RFC 7323.
> ________________________________
> Affected Code from tcp_in.c function tcp_parseopt():
>
> tsval = tcp_getoptbyte();
> tsval |= (tcp_getoptbyte() << 8);
> tsval |= (tcp_getoptbyte() << 16);
> tsval |= (tcp_getoptbyte() << 24);
> tsval = lwip_ntohl(tsval); // NO-OP on big-endian
>
Suspicious...
> This code assembles the TSval in little-endian byte order,
Yes, but this seems wrong.
> but since the value is received in network byte order (big-endian),
Yes.
> the lwip_ntohl() macro must correct it.
No.
> However, on big-endian systems lwip_ntohl(x) is defined as (x), resulting in no correction, and thus a corrupted value.
Correct, ntohl(x) should be NO-OP on big-endian systems.
The bug seems in the assembly of the 32-bit variable from the option bytes:
I would expect indeed that the TSval field is big-endian inside the
network packet (like all TCP/IP header fields) and thus the 32-bit
tsval variable must be assembled from bytes, assuming the bytes are
big-endian ordered.
I would expect this to work:
tsval = (tcp_getoptbyte() << 24);
tsval |= (tcp_getoptbyte() << 16);
tsval |= (tcp_getoptbyte() << 8);
tsval |= tcp_getoptbyte();
tsval = lwip_ntohl(tsval); // NO-OP on big-endian <- correct
Regards,
Leon.
_______________________________________________
lwip-devel mailing list
lwip-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/lwip-devel