dns: harden any-port UDP detection

[mmanoj]

Please sign (check) the below before submitting the Pull Request:

• I have signed the ntop Contributor License Agreement at https://github.com/ntop/legal/blob/main/individual-contributor-licence-agreement.md
• I have read the contributing guidelines at https://github.com/ntop/nDPI/blob/dev/CONTRIBUTING.md
• I have updated the documentation (in doc/) to reflect the changes made (if applicable)

Link to the related issue:

#1788

Describe changes:

Harden dns.custom_port=0 while preserving one-packet DNS classification.

• Classify valid arbitrary-port UDP DNS from the first packet.
• Preserve asymmetric and query-only traffic support.
• Do not require request/response correlation.
• Do not add candidate or transaction state to ndpi_flow_udp_struct.
• Validate arbitrary-port questions with stricter bounded geometry checks:
  • labels up to 63 bytes;
  • at most 32 labels;
  • encoded name length up to 253 bytes;
  • no compressed question names;
  • complete QTYPE/QCLASS fields;
  • nonzero QTYPE and QCLASS.
• Integrate validation into a dedicated strict question parser, avoiding an
  additional question-section traversal.
• Select strict parsing only for dns.custom_port=0.
• Keep the existing process_queries() path unchanged for DNS/53, mDNS,
  LLMNR, TCP DNS, and explicitly configured custom ports.
• Exclude invalid arbitrary-port candidates before classification without
  assigning a misleading DNS flow risk.
• Preserve existing optional asynchronous response processing after
  classification.

Validation:

• make -j$(nproc)
• ./tests/do-unit.sh
• NDPI_FAIL_FAST=1 ./tests/do.sh
• Query-only port-80 fixture: DNS with DPI packets: 1
• Existing any-port expected output restored
• Standard DNS and configured custom-port outputs unchanged

[IvanNardi]

I didn't look at the patch itself, but I really don't want to need 2 pkts for classification.
That, for a lot of reasons:

• performance
• asymmetric traffic support
• we can't write anymore on flow->protos.dns on request packet <-----

Can we harden the detection, keeping the current 1 pkt only logic?

[mmanoj]

Idea is to BASE-011: Neither PR #3232 nor PR #3233 implements request/response candidate correlation, stricter arbitrary-port question-section geometry masks, Mahalanobis, SPID/Markov, per-device windows, or DNS tunneling detection. Other Unique value of this proposal:

UNQ-001: Hardens the existing dns,custom_port,0 any-UDP-port mode rather than adding a second configuration mechanism.
UNQ-002: Adds stricter validation for arbitrary-port candidates than the current standard, positive custom-port, and any-UDP-port DNS paths.
UNQ-003: Requires request/response correlation before classification in any-UDP-port mode by default, reducing false positives from one DNS-shaped packet.
UNQ-004: Keeps PR #3232 as the preferred simple path for deployments that know their custom DNS port.
UNQ-005: Creates a clean boundary between DNS protocol discovery in Stage 1 and DNS tunneling suspicion in Stage 2/3.
UNQ-006: Gives upstream a concrete way to reduce the false-positive warning already documented for dns,custom_port,0. I will check as per your advice and update the way forward.

[mmanoj]

I will proceed as below if this approach is fine. @IvanNardi Please advice.

BASE-011:
Neither PR #3232 nor PR #3233 implements stricter arbitrary-port question-section geometry masks, Mahalanobis, SPID/Markov, per-device windows, or DNS tunneling detection. PR #3233 intentionally keeps one-packet any-UDP-port DNS classification.

UNQ-001: Hardens the existing dns,custom_port,0 any-UDP-port mode rather than adding a second configuration mechanism.
UNQ-002: Adds stricter validation for arbitrary-port candidates than the current standard, positive custom-port, and any-UDP-port DNS paths.

UNQ-003: Preserves nDPI’s current 1-packet DNS classification behavior, including asymmetric traffic support, while applying stricter stateless structural validation only to dns,custom_port,0.

UNQ-004: Keeps PR #3232 as the preferred simple path for deployments that know their custom DNS port.
UNQ-005: Creates a clean boundary between DNS protocol discovery in Stage 1 and DNS tunneling suspicion in Stage 2/3.
UNQ-006: Reduce false positives without changing the 1-packet model

Net effect:

• We lose the strongest FP-reduction lever: request/response correlation.
• We keep upstream alignment, performance simplicity, and asymmetric traffic support.
• Stage 2/3 can still later add tunnel suspicion using per-flow/per-host stats, but Stage 1 remains only “is this DNS-shaped enough to classify on arbitrary UDP port?”

[RajaMuhammadAwais]

@mmanoj I agree that arbitrary-port DNS detection should remain one-packet and stateless
However the proposed strict any-port path also has trade-offs It can introduce false negatives for valid DNS encodings such as compressed QNAMEs and the root QNAME while the current wire-length check may reject valid names near the RFC 1035 limit It also adds a second parsing path that must remain consistent with the existing DNS parser and increases maintenance and test complexity Most importantly stateless geometry checks reduce malformed-packet false positives but cannot provide the same confidence as request/response correlation DNS-shaped one-packet traffic can still be misclassified on arbitrary UDP ports
Therefore I suggest keeping PR #3232 as the deterministic solution for explicitly configured custom ports

[mmanoj]

@RajaMuhammadAwais

Thank you. I agree with the central limitation: no stateless, one-packet heuristic can distinguish DNS from an arbitrary UDP payload that is deliberately or accidentally DNS-shaped. PR #3246 does not claim to provide the confidence of request/response correlation.
The intended relationship between the changes is:

• PR dns: support an opt-in custom detection port #3232: deterministic detection when the deployment knows the custom DNS port.
• PR DNS: add (optional) detection over any port (on UDP) #3236: introduced the optional dns,custom_port,0 mode, which checks DNS on any UDP port and explicitly documents expected false positives.
• PR dns: harden any-port UDP detection #3246: hardens that existing any-port mode without changing its one-packet and stateless behavior.
  Therefore, PR dns: harden any-port UDP detection #3246 is not competing with PR dns: support an opt-in custom detection port #3232. PR dns: support an opt-in custom detection port #3232 remains the preferred and more reliable configuration. PR dns: harden any-port UDP detection #3246 addresses the separate case where users enable the already-supported custom_port=0 mode because the DNS port is unknown or variable.
  The concrete value of PR dns: harden any-port UDP detection #3246 is that arbitrary-port traffic no longer receives the same acceptance threshold as traffic with a strong port signal. Before classifying an arbitrary-port candidate, it now verifies the complete question geometry, label boundaries, QTYPE/QCLASS availability, count limits, transport restrictions, and conflicts with stateful UDP dissectors.
  It preserves the constraints requested during review:
• classification still happens from one packet;
• no request/response state is added;
• asymmetric and query-only captures remain supported;
• the standard DNS/53 path is unchanged;
• explicitly configured custom ports continue using the existing parser;
• the additional work runs only when the opt-in value custom_port=0 is selected;
• the strict question section is traversed once and its end offset and QTYPE are reused;
• rejected candidates do not modify flow->protos.dns.
  I agree that the current conservative checks expose standards-compliance trade-offs. The root QNAME rejection and 253-byte wire limit can be corrected to support the valid root encoding and the RFC 1035 255-byte encoded-name limit. Compressed QNAME handling should either be safely validated with bounded pointer traversal or be explicitly documented as an intentional conservative limitation of arbitrary-port detection.
  The separate parser is a maintenance cost, but it is deliberately isolated to avoid changing or slowing the DNS/53 hot path. It is not a second traversal of the packet. A shared side-effect-free question scanner could reduce duplication later, but that would create a larger and riskier change to the standard DNS path.
  In short, PR dns: harden any-port UDP detection #3246 provides risk reduction, not certainty:
• PR dns: support an opt-in custom detection port #3232 supplies deterministic port evidence.
• PR dns: harden any-port UDP detection #3246 supplies a stricter acceptance threshold when that evidence is unavailable.
• request/response correlation would provide stronger evidence but conflicts with the required one-packet and asymmetric behavior.
  If upstream’s decision is that arbitrary-port DNS should not be supported because one-packet confidence is inherently insufficient, then the consistent solution would be to remove or deprecate the custom_port=0 mode introduced by PR DNS: add (optional) detection over any port (on UDP) #3236. Retaining that mode while rejecting PR dns: harden any-port UDP detection #3246 would retain the same heuristic capability with weaker validation and more false-positive exposure.
  I can update PR dns: harden any-port UDP detection #3246 with the RFC boundary corrections, compressed-name policy, and focused positive/negative tests while preserving its current performance isolation.

[RajaMuhammadAwais]

@mmanoj Thanks for clarifying. I agree that #3232 is the deterministic solution for known custom ports, while custom_port=0 is only an opt-in heuristic. However, stricter geometry reduces malformed-packet false positives; it does not make arbitrary-port detection deterministic, since unrelated UDP data can still be DNS-shaped. The result should therefore be documented as DNS-shaped traffic on an arbitrary UDP port, not equivalent to #3232.
Before merging #3246, please add tests for root QNAME, compressed QNAME, the RFC 1035 255-byte wire limit, truncation, malformed labels, and DNS-shaped non-DNS payloads. Also, process_queries_strict() creates a second grammar: the same valid DNS packet may be accepted on port 53 but rejected on an arbitrary port. A shared bounds-checked scanner would avoid this semantic divergence; otherwise, the intentional limitations should be documented clearly.
its my POV

[IvanNardi]

add tests for root QNAME, compressed QNAME, the RFC 1035 255-byte wire limit, truncation, malformed labels, and DNS-shaped non-DNS payloads

+1

[mmanoj]

@RajaMuhammadAwais Appreciate your advice and valuable feedback to make this PR better.

@IvanNardi as advice, I will arrange the improvements asap. Thx for the support.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[mmanoj]

@IvanNardi please check the update commits

Key updates:

• Replaced process_queries_strict() with the shared bounds-checked DNS question scanner.
• Supports root QNAMEs, compressed QNAMEs, and the RFC 1035 255-byte expanded-name limit.
• Rejects truncation, malformed labels, invalid compression pointers, and zero QTYPE/QCLASS on the arbitrary-port path.
• Preserves one-packet, stateless classification.
• Keeps standard DNS ports on compatibility behavior to avoid regressions.
• Documents custom_port=0 as heuristic DNS-shaped traffic detection, including unavoidable false positives.
• Added a six-packet regression fixture covering every requested case.

Verification completed:

• Build passed.
• Unit tests passed.
• Full fail-fast regression suite passed.
• Standard DNS, TCP compression, and malformed DNS golden outputs remained unchanged.
• Randomized-port ASan/UBSan fuzzing completed 6,187 executions without findings.
• Merged latest upstream dev and resolved the flow-metadata API changes.