Skip to content

Conversation

Gianla
Copy link

@Gianla Gianla commented Aug 10, 2025

Proof of concept for a patch implementing this feature.

This patch extended ndpi_mdns_rsp_entry with additional useful data and uses it to enhance ndpi_flow_struct.

Some points:

  1. I updated ndpiReader.c as well, though the formatting may not be ideal; I’m open to suggestions for improvements.
  2. Changes in dns.c could potentially apply to standard DNS, but for now all modifications are guarded by the proto->master_protocol == NDPI_PROTOCOL_MDNS condition;
  3. In particular, I added support for compressed names, as well as SRV and TXT records, which were not handled previously
  4. Right now I updated as many tests as I could, except for /tests/pcap/fuzz-2006-06-26-2594.pcap. Probably due to the ndpi_grab_dns_name() update, it still gives some problems with text format. After a lot of time debugging it, I can't understand what's wrong with it. Any suggestion about it is appreciated. DONE. Looks like opening it to update a test (n. 196) corrupted the entire file, so the diff never matched.
  5. Duplicate checks are currently not implemented, but they can be easily added above the new add_to_mdns_metadata() function if needed. DONE
  6. Some new checks can detect malformed packets. From my side, I did not feel fully confident to decide if and which set_risk should be used, so I have currently marked them as "TODO". I believe this is an important point that would benefit from further discussion with the core developers to determine the best approach.

If and when all changes for answer records will be approved, I'll be ready to extend it also for additional records since as the approach is basically the same.
If the format is okay, I will also update the serializer.

@Gianla Gianla marked this pull request as draft August 11, 2025 22:10
@IvanNardi
Copy link
Collaborator

@Gianla, I removed another include

@Gianla

This comment has been minimized.

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants