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

Add extra entropy checks and more precise(?) analysis. #2383

Merged
merged 1 commit into from
May 9, 2024

Conversation

utoni
Copy link
Collaborator

@utoni utoni commented Apr 11, 2024

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

Describe changes:

This is more an idea on how entropy based categorization could give more details about the transmitted data. It's losely based on the Entropy Analysis paper, but needs some verification. Hopefully, someone may find this useful and may help me with it. :)
(not yet done reading the paper)

Also something to consider is if entropy calculation should be done per-packet instead per-flow..

@utoni utoni force-pushed the add/more-and-detailed-entropy-checks branch 2 times, most recently from 908f524 to 3fd0713 Compare April 17, 2024 15:49
@utoni utoni marked this pull request as ready for review April 17, 2024 15:53
@utoni utoni force-pushed the add/more-and-detailed-entropy-checks branch 3 times, most recently from 5510db2 to 2eca7c8 Compare April 18, 2024 10:44
@IvanNardi
Copy link
Collaborator

@utoni, do you have a copy of the original paper?

@utoni
Copy link
Collaborator Author

utoni commented Apr 19, 2024

Unfortunately not. :/

src/include/ndpi_main.h Show resolved Hide resolved
src/lib/ndpi_utils.c Outdated Show resolved Hide resolved
@utoni utoni force-pushed the add/more-and-detailed-entropy-checks branch from 2eca7c8 to 7083e0a Compare April 19, 2024 18:13
@IvanNardi IvanNardi requested a review from lucaderi April 20, 2024 15:21
@utoni utoni force-pushed the add/more-and-detailed-entropy-checks branch from 7083e0a to 374d056 Compare April 25, 2024 09:12
@utoni utoni requested a review from IvanNardi April 25, 2024 10:37
@utoni
Copy link
Collaborator Author

utoni commented Apr 26, 2024

icmp echo request @IvanNardi :)

@lucaderi
Copy link
Member

@utoni I am a bit sceptical about this PR. Entropy is a metric to measure chaos, and within specific boundaries you can find many different contents. So ndpi_entropy2str() for instance can IMHO be used as a hint but not for ground truth. So if you position it as hint I am happy, if you want to do more than that I am not convinced it's a good idea

@utoni
Copy link
Collaborator Author

utoni commented Apr 26, 2024

@lucaderi
I agree, there is still a high chance of false positives e.g. for video/audio/voip transfers as they may have a similar entropy as (compressed) executables.
What do you mean by "hint"? Not setting any risk and do what instead?

@lucaderi
Copy link
Member

I mean that "Compressed Executable" is not only this, but it's a possibility (or hint if you wish). So a broader set of possibilities (e.g. "Compressed Executable. or something else" or "Compressed Executable ?") can indicate that this is a hint and not a fact true 100%. More or less ad DPI confidence that @IvanNardi introduced in DPi classification some time ago.

@utoni
Copy link
Collaborator Author

utoni commented Apr 26, 2024

Ok, got it.

@IvanNardi
Copy link
Collaborator

@utoni, are you going to push a new version with updated labels/strings?

@utoni
Copy link
Collaborator Author

utoni commented May 7, 2024

Yea, ASAP :)

@utoni utoni force-pushed the add/more-and-detailed-entropy-checks branch from 374d056 to f8f669a Compare May 9, 2024 11:43
Copy link

sonarcloud bot commented May 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@utoni
Copy link
Collaborator Author

utoni commented May 9, 2024

done, I've also lowered the risk level from medium to low

Copy link
Collaborator

@IvanNardi IvanNardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@IvanNardi IvanNardi merged commit 18e03a2 into ntop:dev May 9, 2024
32 of 33 checks passed
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.

3 participants