TCP Handshake failing

[FelixSchladt]

Hi All,

I am currently trying to port from the original PicoTCP to PicoTCP-NG and ecountered an issue.
As I was not yet able to find the source of the bug, I am kindly asking for your help/advice.

During the Handshake process, the following check in module/pico_tcp.c: tcp_parse_options fails:

static int tcp_parse_options(struct pico_frame *f)
{
    struct pico_socket_tcp *t = (struct pico_socket_tcp *)f->sock;
    uint8_t *opt = f->transport_hdr + PICO_SIZE_TCPHDR;
    uint32_t i = 0;
    f->timestamp = 0;

    if (f->buffer + f->buffer_len > f->transport_hdr + f->transport_len) //THIS CHECK FAILS
        return -1;

   [...]

I added some logging to the function and the output is as follows: (I cutted some output)

[...]
pico_tcp.c:2893: [sam] TCP> [tcp input] t_len: 40
pico_tcp.c:2894: [sam] TCP> flags = 0x02
pico_tcp.c:2895: [sam] TCP> s->state >> 8 = 2
pico_tcp.c:2896: [sam] TCP> [tcp input] socket: 0x1a7014 state: 2 <-- local port:5555 remote port: 59288 seq: 0x5e7f74ae ack: 0x00000000 flags: 0x02 t_len: 40, hdr: 40 payload: 0

pico_tcp.c:897: f->buffer + f->buffer_len (1733274) > f-transport_hdr + f->transport_len(1733270)
pico_tcp.c:898: f->buffer: 1733196 f->buffer_len: 78
pico_tcp.c:898: f-transport_hdr: 1733230 f->transport_len: 40
[...]

This log repeats till the test ist shutdown.

As the above check returns with -1 the frame is discarded and a handshake is never completed.

My understanding is, if the header len is 40, this means no options are present, therefore there are no options to parse.
Would it maybe correct if the return value of the check is instead 0? (in this case its working correct in my test)

I am not sure I 100% understand whats happening here, so I would be very glad if someone had some answers for me.

Kind Regards,
Felix

[danielinux]

Looks like a regression I've introduced in 4b9a167.
It looks like the check should have been inverted, like in pico_ipv4_process_in() (same commit).

My understanding is, if the header len is 40, this means no options are present, therefore there are no options to parse.

TCP Header with no option is 20B. A few options are commonly added all the time (timestamp+windows scaling) making a typical header around 40B.

Fixing...

[danielinux]

@FelixSchladt thanks for reporting this. please check if #20 solves the issue for you. I will merge in if confirmed.

[FelixSchladt]

@danielinux Thanks you so much for the quick fix and maintenance.

I tested it and it solves the Issue.

TCP Header with no option is 20B. A few options are commonly added all the time (timestamp+windows scaling) making a typical header around 40B.

I honestly didnt know that, so thanks for clarification 👍🏼

[danielinux]

Thanks for confirming! I'll merge in #20