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 "Heroes of the Storm" video game signature detection. #1949

Merged
merged 28 commits into from
Apr 22, 2023

Conversation

nikitamishagin
Copy link
Contributor

In this change, I've added signature detection for the "Heroes of the Storm" video game. Many nested conditions help distinguish it from other similar games. Tested on six different traffic dumps.

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.

Please add one trace into tests/cfgs/default/pcaps/ with some examples of flows triggering the various patterns and then update the unit tests results

src/lib/ndpi_main.c Outdated Show resolved Hide resolved
src/lib/protocols/hots.c Outdated Show resolved Hide resolved
src/lib/protocols/hots.c Outdated Show resolved Hide resolved
src/lib/protocols/hots.c Outdated Show resolved Hide resolved
src/lib/protocols/hots.c Outdated Show resolved Hide resolved
src/lib/protocols/hots.c Outdated Show resolved Hide resolved
src/lib/protocols/hots.c Outdated Show resolved Hide resolved
@nikitamishagin
Copy link
Contributor Author

Please add one trace into tests/cfgs/default/pcaps/ with some examples of flows triggering the various patterns and then update the unit tests results

I'll try to add testing soon. But I'm not entirely clear what else needs to be done besides adding a traffic trace to /tests/cfgs/default/pcap/?

@IvanNardi
Copy link
Collaborator

Please add one trace into tests/cfgs/default/pcaps/ with some examples of flows triggering the various patterns and then update the unit tests results

I'll try to add testing soon. But I'm not entirely clear what else needs to be done besides adding a traffic trace to /tests/cfgs/default/pcap/?

Run the tests via ./tests/do.sh and update any results

@IvanNardi
Copy link
Collaborator

Please, note that you have to add hots.c to some list in windows/nDPI.vcxproj to fix the Windows CI job

@nikitamishagin
Copy link
Contributor Author

Trace added, results updated, hots.c added to windows/nDPI.vcxproj.

Signed-off-by: lns <[email protected]>
Signed-off-by: Toni Uhlig <[email protected]>
@IvanNardi
Copy link
Collaborator

IvanNardi commented Apr 21, 2023

Trace added, results updated, hots.c added to windows/nDPI.vcxproj.

One final step. As you can notice from CI results, you need to update some unit tests results. Everything will be fine when tests\do.sh will no complain anymore (on your machine)

EDIT: before that, please rebase

* try to get rid of some `printf(..)`s as they do not belong to a shared library
 * replaced all `exit(..)`s with `abort()`s to indicate an abnormal process termination

Signed-off-by: Toni Uhlig <[email protected]>
@nikitamishagin
Copy link
Contributor Author

One final step. As you can notice from CI results, you need to update some unit tests results. Everything will be fine when tests\do.sh will no complain anymore (on your machine)

EDIT: before that, please rebase

I see many errors in ci "Build/macos-12 x86_64 cc --with-pcre --with-maxminddb(pull_request)". But I can't understand why they appeared. Could you suggest what is the problem?

@IvanNardi
Copy link
Collaborator

One final step. As you can notice from CI results, you need to update some unit tests results. Everything will be fine when tests\do.sh will no complain anymore (on your machine)
EDIT: before that, please rebase

I see many errors in ci "Build/macos-12 x86_64 cc --with-pcre --with-maxminddb(pull_request)". But I can't understand why they appeared. Could you suggest what is the problem?

You need to update the tests results under tests/cfgs/*/results. At that point the fastest way is: remove all *.out files (from all the sub-folders) ; run tests/do.sh.

@IvanNardi
Copy link
Collaborator

You need to update the tests results under tests/cfgs/*/results. At that point the fastest way is: remove all *.out files (from all the sub-folders) ; run tests/do.sh.

@nikitamishagin, if you prefer, we can take care of that ourselves during the merge...

@nikitamishagin
Copy link
Contributor Author

nikitamishagin commented Apr 22, 2023

You need to update the tests results under tests/cfgs/*/results. At that point the fastest way is: remove all *.out files (from all the sub-folders) ; run tests/do.sh.

I did everything as you indicated. But there are 2 errors while reading the pcap file.

nikitamishagin, if you prefer, we can take care of that ourselves during the merge...

It is perfectly! Please correct me, because you definitely understand better than me what needs to be done)

@sonarcloud
Copy link

sonarcloud bot commented Apr 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@IvanNardi
Copy link
Collaborator

You need to update the tests results under tests/cfgs/*/results. At that point the fastest way is: remove all *.out files (from all the sub-folders) ; run tests/do.sh.

I did everything as you indicated. But there are 2 errors while reading the pcap file.

nikitamishagin, if you prefer, we can take care of that ourselves during the merge...

It is perfectly! Please correct me, because you definitely understand better than me what needs to be done)

For the next time, in the README file:

* Be sure to have nBPF support, cloning PF_RING in the same directory where you cloned nDPI: git clone https://github.com/ntop/PF_RING/ && cd PF_RING/userland/nbpf && ./configure && make
* From the nDPI root directory, ./autogen.sh --with-pcre (nBPF and PCRE are usually optional, but they are needed to run/update all the unit tests)

@IvanNardi IvanNardi merged commit d3e67fa into ntop:dev Apr 22, 2023
@IvanNardi
Copy link
Collaborator

@nikitamishagin , thanks for your contribution!

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