fix: correct UFTP/BitTorrent nDPI misclassification + DPI accuracy disclaimer#89
Merged
NotYuSheng merged 8 commits intomainfrom Mar 28, 2026
Merged
Conversation
- Re-introduce correctMisclassification() in NdpiService to override nDPI's DPI result when port evidence is unambiguous: UDP 1044 (IANA UFTP) wrongly tagged BitTorrent, TCP 1720 (H.225) wrongly tagged Cassandra as a belt-and-suspenders guard - Fix hardcoded color: '#fff' on app badge in AnalysisOverview — use luminance-based getTextColor() for accessibility - Add info alert on overview explaining nDPI's probabilistic nature and advising users to cross-reference unexpected labels Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a backend correction mechanism for nDPI misclassifications and adds an informational alert in the frontend regarding DPI heuristics. It also enhances the UI by dynamically setting text colors for application badges. Feedback includes adding 'rel="noopener"' for security, improving null safety and maintainability in the backend logic, and optimizing frontend performance by caching color values.
Use SGDS OverlayTrigger + Popover (click-to-open, rootClose) instead of a persistent alert banner. Info icons sit inline next to the "Applications Detected" heading and the "Protocol Distribution" heading. Each popover explains the probabilistic nature of nDPI DPI and when to cross-reference results. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… DPI Protocol breakdown comes from IP packet headers (TCP/UDP/ICMP/ARP), not nDPI. Popover now accurately says it's deterministic and points users to the Applications section for nDPI-based identification. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Categories are derived from nDPI application detection and inherit the same probabilistic limitations. Popover explains this and mirrors the pattern used in Applications Detected and Protocol Distribution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each popover now fully explains its own detection method without assuming the user has read the others. Protocol popover no longer points to 'Applications Detected above'. Category popover spells out nDPI's probabilistic nature instead of saying 'same limitations'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nDPI 5.0.0 labels all H.323 suite flows as 'H323' without distinguishing sub-protocols. correctMisclassification() now splits them using IANA port assignments: - H323 + TCP + port 1720 → H225 (call signaling, IANA-registered) - H323 + TCP + any other port → H245 (media control, always dynamic) - H323 + UDP (RAS/gatekeeper on 1719) → left unchanged Pass transport protocol string to correctMisclassification() to prevent the H245 rule from firing on UDP flows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
'Cannot be misidentified' was too absolute. Tunnelled traffic (GRE, VXLAN, IP-in-IP) shows the outer transport, not the inner protocol. Reworded to 'accurately reflects what is declared in each packet's header' with an explicit tunnelling caveat. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to correct known nDPI misclassifications for protocols like UFTP and H.225, and adds informational UI components to explain the probabilistic nature of deep packet inspection. Review feedback highlights the need for stricter transport protocol checks in the correction logic, the removal of a placeholder URL in the documentation, and a minor performance optimization in the frontend color rendering.
- correctMisclassification: add transport checks to UFTP (UDP) and Cassandra→H225 (TCP) rules; extract PORT_UFTP/PORT_H225 constants; use Integer.equals() for null-safe port comparison - AnalysisOverview: cache getAppColor() result to avoid double call - Add rel="noopener noreferrer" to all target="_blank" links (AnalysisOverview, CategoryBreakdownChart) - Remove placeholder XXXX bug link from NdpiService Javadoc Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
correctMisclassification()inNdpiServiceto override nDPI's DPI result when port evidence is unambiguous: UDP 1044 (IANA-registered for UFTP) misclassified as BitTorrent, and TCP 1720 (H.225) misclassified as Cassandra as a belt-and-suspenders guardcolor: '#fff'on app name badges inAnalysisOverview— now usesgetTextColor()for luminance-based accessibility (follow-up from fix: replace hardcoded colour maps with color-hash for consistent, scalable colouring #87)Root cause (UFTP)
UFTP_v3_transfer.pcapng— flows carrying binary file-transfer payloads trigger nDPI's BitTorrent DPI heuristics, overriding port-based detection. UDP port 1044 is IANA-registered for UFTP. This is a known DPI false-positive pattern in nDPI 5.0.0 (upstream bug filed separately with ntop).Test plan
UFTP_v3_transfer.pcapng— conversations should now show UFTP, not BitTorrentdocker compose build && docker compose up -d— clean build🤖 Generated with Claude Code