Skip to content

Conversation

@jasmine-nahrain
Copy link
Collaborator

@jasmine-nahrain jasmine-nahrain commented Jan 8, 2026

Problem
OpenSSL provides SSL_client_hello_get0_ext(), SSL_client_hello_get0_ciphers() and SSL_client_hello_get1_extensions_present() to get client hello from an SSL object. BoringSSL doesn't have comparable functions. It requires the SSL_CLIENT_HELLO object via SSL_early_callback_ctx_extension_get(). Currently, there's no way to get the SSL_CLIENT_HELLO object in plugins, which causes friction when writing SSL-related plugins that need to work with both libraries.

Proposed Solution:

TSClientHello TSVConnClientHelloGet(TSVConn sslp);

This API provides access to the SSL_CLIENT_HELLO object within plugins and is usable during SSL hooks (TS_SSL_CLIENT_HELLO_HOOK, TS_SSL_SERVERNAME_HOOK).

Use Case: This enables plugins to access ClientHello data (cipher suites, extensions, SNI, ALPN, supported TLS versions) when using BoringSSL. Currently, the ja4_fingerprint plugin only works for openssl, this change allows us to add boringssl support.

Implementation Notes:

  • The SSL_CLIENT_HELLO is captured during the client hello callback and stored in TLSSNISupport
  • The data is valid during SSL handshake hooks
  • For OpenSSL, plugins can continue using existing TSSslConnectionGet() approach

This is a non-breaking addition. Existing OpenSSL-based plugins continue to work unchanged.

@jasmine-nahrain jasmine-nahrain self-assigned this Jan 8, 2026
@masaori335 masaori335 added the ja4_fingerprint Work related to JA4 fingerprinting label Jan 8, 2026
@masaori335 masaori335 added this to the 10.2.0 milestone Jan 8, 2026
@bryancall bryancall requested a review from bneradt January 12, 2026 22:55
@jasmine-nahrain
Copy link
Collaborator Author

[approve ci autest 1]

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

Thank you @jasmine-nahrain for exploring this. I think your patch here makes the plugin code look much better. I'm content with this idea as a compromise that addresses our variety of wants here (minimize noise in the ts.h namespace along with a clean boring+openssl interface for the plugin).

@maskit : I'm curious to hear your opinion on this as well. Are you content with this patch?

Comment on lines +1084 to +1093
struct tsapi_ssl_client_hello {
uint16_t version;
const uint8_t *cipher_suites;
size_t cipher_suites_len;
const uint8_t *extensions;
size_t extensions_len;
int *extension_ids;
size_t extension_ids_len;
void *ssl_ptr;
};
Copy link
Contributor

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.

Copy link
Member

@maskit maskit Jan 24, 2026

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_HELLO is only available during BoringSSL callback functions are called. So TSVConnClientHelloGet needs to be called on certain hooks (this should be documented). This plugin seems fine since TSVConnClientHelloGet and TSClientHelloDestroy are called on TS_SSL_CLIENT_HELLO_HOOK, but if another plugin needs information from Client Hello later on another hook, everything needs to be deep copied when TSVConnClientHelloGet is called.

Copy link
Member

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.

Copy link
Member

@maskit maskit left a comment

Choose a reason for hiding this comment

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

API documentation is mandatory.

The structure should probably be a class with public getters (and private members). Regardless, some of member variables should have const.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ja4_fingerprint Work related to JA4 fingerprinting Plugins

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants