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 ja4 package #226

Merged
merged 1 commit into from
Nov 18, 2023
Merged

add ja4 package #226

merged 1 commit into from
Nov 18, 2023

Conversation

anthonykasza
Copy link

please include this new package in the zeek package source

@awelzel
Copy link
Contributor

awelzel commented Nov 17, 2023

Hey @anthonykasza - thanks for doing this. Just as it's not mentioned and the package name is overly generic, the repo contains code for ja4 fingerprinting and possibly ja4+ in the future (?).

Following just my own personal thoughts:

Given the licensing concerns (you even have CAUTION_LICENSING in zkg.meta as a tag), I'd think a two-package approach would be nicer (certainly depends on the viewpoint). A plain ja4 package that just does the basic "JA4 TLS Client Fingerprinting" and then a separate ja4-plus package or some such. The latter would be for those that 1) see value in the advanced fingerprints and 2) have a good understanding and can accommodate for whatever that means with respect to licensing in their environment.

That is just a thought, I think we can merge as is (mind fixing up the sorting in the .index file though as pointed out by the pre-commit hook?)

@anthonykasza
Copy link
Author

I have adjusted the ordering of the lines in zkg.index as requested.
Thank you for your thoughts on how to improve the organization of the package. I will take them into consideration.

@anthonykasza
Copy link
Author

@awelzel I have heeded your thoughts and am adding just the ja4 package in this PR. Thank you for your patience.

@anthonykasza anthonykasza changed the title add fingerprint package add ja4 package Nov 18, 2023
@awelzel awelzel merged commit 8f4e9f6 into zeek:master Nov 18, 2023
1 check passed
@awelzel
Copy link
Contributor

awelzel commented Nov 18, 2023

@awelzel I have heeded your thoughts and am adding just the ja4 package in this PR. Thank you for your patience.

Thanks, pulled.

The ja3 / ja3s columns in ssl.log always seemed like a good spot, I could see that be preferred over the separate ja4.log, but lets see where it goes :-)

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.

2 participants