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

JA4 for TLS and QUIC -- v6 #9634

Closed
wants to merge 1 commit into from
Closed

JA4 for TLS and QUIC -- v6 #9634

wants to merge 1 commit into from

Conversation

satta
Copy link
Contributor

@satta satta commented Oct 16, 2023

Previous PR: #9598

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/6379

Changes to previous PR:

  • Rebase.
  • Mention in documentation that JA4 hash is not derived from real data and does not match the other hashes.
  • Unify checks for whether to update the JA4 state to always check whether a) we have a JA4 object and b) we are in a client hello.
  • Update Redmine ticket link in commit message to correct format.
SV_BRANCH=pr/1426

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #9634 (6920b87) into master (1a132f4) will increase coverage by 0.10%.
The diff coverage is 92.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9634      +/-   ##
==========================================
+ Coverage   82.28%   82.39%   +0.10%     
==========================================
  Files         968      970       +2     
  Lines      274275   274720     +445     
==========================================
+ Hits       225700   226346     +646     
+ Misses      48575    48374     -201     
Flag Coverage Δ
fuzzcorpus 64.63% <65.09%> (+0.29%) ⬆️
suricata-verify 61.01% <85.64%> (+0.08%) ⬆️
unittests 62.88% <64.79%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

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

Thanks for the work Sascha :-)

  • CI : green
  • Code : ok for me
  • Commits segmentation : Ok
  • Commit messages : Ok, thanks
  • Git ID set : looks fine for me
  • CLA : you already contributed :-)
  • Doc update : Ok
  • Redmine ticket : ok
  • Rustfmt : ok
  • Tests : Suricata-verify tests still looks fine
  • Dependencies added: none

@jasonish
Copy link
Member

Just a note that there are questions with respect to the license and patent here. It should be made more clear if JA4 falls under patent restrictions soon: FoxIO-LLC/ja4#15 (or if its just JA4+).

@john-althouse
Copy link

Just a note that there are questions with respect to the license and patent here. It should be made more clear if JA4 falls under patent restrictions soon: FoxIO-LLC/ja4#15 (or if its just JA4+).

I've updated the thread and readme, there are no patent restrictions for JA4. Thanks for working on this!

@satta
Copy link
Contributor Author

satta commented Dec 23, 2023

New PR: #10095

@satta satta closed this Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Needs rebase to master
Development

Successfully merging this pull request may close these issues.

4 participants