-
Notifications
You must be signed in to change notification settings - Fork 849
Add boringssl support to the ja4_fingerprint plugin #12790
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
Open
jasmine-nahrain
wants to merge
12
commits into
apache:master
Choose a base branch
from
jasmine-nahrain:bssl_ja4
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+225
−51
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
786fbaa
got client hello routed to plugins
jasmine-nahrain 3f4e5c4
Creates ja4 fingerprint with boringssl
jasmine-nahrain 5c8c8ed
cleanup a bit
jasmine-nahrain 0efeade
make ssl_client_hello const
jasmine-nahrain 9765eb8
spaces cleanup
jasmine-nahrain 2bc38d0
cleanup code
jasmine-nahrain fd03806
more cleanup
jasmine-nahrain 9b76bd0
Update plugin.cc
jasmine-nahrain 51ebe55
Update plugin.cc
jasmine-nahrain b57f338
Update ts.h
jasmine-nahrain 162af78
Update apidefs.h.in
jasmine-nahrain a42ae48
Update to make more clean
jasmine-nahrain File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's initialize these here (nullptr, 0).
Also: as a tweak on this, what do you think of making these private and adding public getters for these such that we can lazily load them as they are requested? Subsequent requests can then return the populated (cached) values if the same value is asked for twice. Currently, the caller has to pay for the population of all of these even though they might not need them all.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lazy load would be tricky. Best we can do is probably read and cache everything in
TSVConnClientHelloGet.As I commented on somewhere,
SSL_CLIENT_HELLOis only available during BoringSSL callback functions are called. SoTSVConnClientHelloGetneeds to be called on certain hooks (this should be documented). This plugin seems fine sinceTSVConnClientHelloGetandTSClientHelloDestroyare called onTS_SSL_CLIENT_HELLO_HOOK, but if another plugin needs information from Client Hello later on another hook, everything needs to be deep copied whenTSVConnClientHelloGetis called.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Public getters would be nice even if we don't do lazy land. Those would enable removing ifdef from the plugin code.