Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RESP protocol dissector implementation #2287

Closed
0xA50C1A1 opened this issue Jan 26, 2024 · 9 comments
Closed

RESP protocol dissector implementation #2287

0xA50C1A1 opened this issue Jan 26, 2024 · 9 comments
Labels

Comments

@0xA50C1A1
Copy link
Contributor

@IvanNardi @utoni hey guys. Atm I'm working on a RESP protocol dissector, but I've encountered some issues and I'm looking for expert advice.

RESP PDUs start with * followed by the field length and \r\n, so the first thing that came to mind was this check:

  if (packet->payload_packet_len > 10 && packet->payload[0] == '*' &&
      ndpi_strnstr((const char *)packet->payload, "\r\n", packet->payload_packet_len))
  {
    NDPI_LOG_INFO(ndpi_struct, "found RESP\n");
    ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_RESP,
                               NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
    return;
  }

But I suspect that such a simple check could potentially cause a lot of false positives - even if I haven't seen it on pcap examples from nDPI, it doesn't guarantee anything. Or maybe I'm worrying for nothing?

I thought about adding a check like if (flow->l4.tcp.resp_valid_packets > 2) {...} if(flow->packet_counter > 5) { NDPI_EXCLUDE_PROTO(ndpi_struct, flow); } , but that breaks the detection of some flows since * .. \r\n is often located not at the start of the segment.

@utoni
Copy link
Collaborator

utoni commented Jan 26, 2024

Is it possible to use the field length for a successful detection?

// EDIT: Did not read the protocol specs yet. 😄

@0xA50C1A1
Copy link
Contributor Author

Is it possible to use the field length for a successful detection?

// EDIT: Did not read the protocol specs yet. 😄

Well, I'm not sure about that. Btw, here are some pcap samples resp.zip

@IvanNardi
Copy link
Collaborator

Something like (in pseudo code)?

if(payload[0] == '*' &&
    (from offeset 1, one or more bytes are a number in ascii format) &&
    (after the ascii characters there is "\r\n") {
        FOUND
        return;
}
EXCLUDE

It should be enough...

@0xA50C1A1
Copy link
Contributor Author

Something like (in pseudo code)?

if(payload[0] == '*' &&
    (from offeset 1, one or more bytes are a number in ascii format) &&
    (after the ascii characters there is "\r\n") {
        FOUND
        return;
}
EXCLUDE

It should be enough...

So, you mean something like this?

if (packet->payload[0] == '*' && isascii(packet->payload[1]) && memcmp(&packet->payload[2], "\r\n", 2) == 0)
{
  FOUND;
  return;
}

@IvanNardi
Copy link
Collaborator

We can have a number bigger than 9... Just the idea (without all the necessary checks on payload length)

if (packet->payload[0] == '*') {
  num_digit = 0;
  while(isascii(packet->payload[1 + num_digit]) && num_digit++ < 3) { /* A reasonable upper bound ?*/
    ;
  if(num_digit > 0 && memcmp(&packet->payload[1+num_digit], "\r\n", 2)
    FOUND
}


```

@0xA50C1A1
Copy link
Contributor Author

Lol, I just noticed that nDPI has a RESP protocol dissector, but it's just called "Redis" for some reason. Ofc it doesn't work, so this question is still relevant.

@0xA50C1A1
Copy link
Contributor Author

@IvanNardi Thanks for your help, now I have figured out how to solve this issue.

@IvanNardi
Copy link
Collaborator

RESP = Redis serialization protocol

@0xA50C1A1
Copy link
Contributor Author

RESP = Redis serialization protocol

Yeah, I know. RESP sounds more correct than just Redis.

@0xA50C1A1 0xA50C1A1 mentioned this issue Jan 27, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants