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

Draft: JA4 for TLS and QUIC -- v2 #9545

Closed
wants to merge 3 commits into from
Closed

Conversation

satta
Copy link
Contributor

@satta satta commented Oct 3, 2023

Previous PR: #9536

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

Changes to previous PR:

  • Rewrite JA4 calculation in Rust, eliminating the C implementation and re-enabling Rust unit tests.
  • Add C bindings for the Rust functions to be used in the TLS parser.
  • Add JA4 fields to schema.
SV_BRANCH=pr/1407

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 : pending
  • Code : doing right now
  • Commits segmentation : one commit will be enough in the end
  • Commit messages : Could you please put a reference to the ticket number in the commit message ?
  • Git ID set : looks fine for me
  • CLA : you already contributed
  • Doc update : should there be rules keywords for these ?
  • Redmine ticket : ok
  • Rustfmt : Could you please run it ? quic is supposed to be all rustfmted...
  • Tests : Suricata-verify tests look fine, some comments there
  • Dependencies added: none

@satta
Copy link
Contributor Author

satta commented Oct 3, 2023

Thanks for the work Sascha :-)

No problem, it's been a long weekend here and I appreciated the opportunity to dive into Rust again :)

  • Commits segmentation : one commit will be enough in the end

Totally.

  • Commit messages : Could you please put a reference to the ticket number in the commit message ?

Ah, the usual :P Sure.

  • Doc update : should there be rules keywords for these ?

Definitely, that's kind of the point. Will add these in the next iteration.

  • Rustfmt : Could you please run it ? quic is supposed to be all rustfmted...

Sure.

@satta
Copy link
Contributor Author

satta commented Oct 4, 2023

Next PR: #9553

@satta satta closed this Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants