TLS : OpenSSL Provider and Store API support to support HSMs #10622
Conversation
Any OpenSSL Provider configured in the OpenSSL configuration file will be able to be specified to be loaded and activated via configuration in Fluentbit Signed-off-by: Brent-Crawley-GGL <[email protected]>
Modified the TLS interface to be able to accept a Provider Query to enhance OpenSSL Provider support. Implemented the OpenSSL Store (via compile conditionals) to support loading TLS certificate and Key from store URIs - i.e support loading items from an HSM. Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Also ignore a few files present when using Visual Studio Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
…text Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Brent-Crawley-GGL
requested review from
PettitWesley,
edsiper,
niedbalski,
fujimotos,
patrick-stephens,
celalettin1286,
leonardo-albertovich and
koleini
as code owners
Signed-off-by: Brent-Crawley-GGL <[email protected]>
|
Sounds a good idea, one thing though is we'll definitely need some tests for it and docs - particularly for non-Linux platforms as that is often a nightmare to figure out. At the moment this is disabled for everything, I would like to see us prove it builds for all targets (that support it) by enabling it appropriately and adding any missing dependencies (presumably none). |
Guarded all the OpenSSL 3 functionality with defines to ensure compilation with older variants Signed-off-by: Brent-Crawley-GGL <[email protected]>
Brent-Crawley-GGL
changed the title
Signed-off-by: Brent-Crawley-GGL <[email protected]>
Signed-off-by: Brent-Crawley-GGL <[email protected]>
WalkthroughIntroduces OpenSSL provider/store support, adds TLS provider_query configuration and propagation, extends TLS APIs and backend signatures, updates plugins/tests to new TLS create signature, and wires global TLS configure/cleanup into runtime. Adds CMake option FLB_OPENSSL_STORE and ignores .vs/out in .gitignore. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Lib as flb_lib
participant TLS as flb_tls.c
participant OSSL as openssl.c
App->>Lib: do_start()
Lib->>TLS: flb_tls_configure(config)
TLS->>OSSL: tls_configure(config.openssl_providers)
OSSL-->>TLS: Providers loaded (if OpenSSL 3)
TLS-->>Lib: configured
App->>TLS: flb_tls_create(..., key_passwd, provider_query)
TLS->>OSSL: tls_context_create(..., provider_query)
alt provider_query/URI supported
OSSL->>OSSL: Load cert/key from provider/store
else fallback
OSSL->>OSSL: Load cert/key from files
end
OSSL-->>TLS: ctx*
TLS-->>App: flb_tls*
App-->>TLS: flb_tls_cleanup() on shutdown
TLS->>OSSL: tls_cleanup()
OSSL-->>TLS: providers reset
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
plugins/out_azure_blob/azure_blob_conf.c (1)
370-381: Enable TLS certificate verification and propagate provider_query in azure_blob_conf.cThe Azure Blob plugin currently creates its TLS context with verification turned off and omits the
tls_provider_query, which:
- Allows silent man-in-the-middle attacks against the remote configuration endpoint.
- Prevents users’ configured OpenSSL providers or HSM backends from being used.
Required changes:
• In plugins/out_azure_blob/azure_blob_conf.c (around lines 370–381), update the call to flb_tls_create() to enable verification and forward the plugin’s provider_query:
- tls_context = flb_tls_create(FLB_TLS_CLIENT_MODE, - FLB_FALSE, + tls_context = flb_tls_create(FLB_TLS_CLIENT_MODE, + FLB_TRUE, FLB_FALSE, host, NULL, NULL, NULL, NULL, - NULL, - NULL); + NULL, + context->ins != NULL + ? context->ins->tls_provider_query + : NULL);• No upstream I/O-flag change is needed when calling flb_upstream_create_url(): it already injects FLB_IO_TLS for https:// URLs.
Please apply this patch and verify that certificate validation succeeds against both Azure Blob default and custom endpoints.
plugins/out_azure_kusto/azure_kusto_conf.c (1)
88-93: Pass provider_query to upstream node and fix error-path leaksTwo points here:
- With the extended flb_upstream_node_create signature (new parameter before kv), this call should forward the output’s provider query instead of a bare NULL so TLS for Azure storage endpoints can also leverage Providers/HSMs.
- If node creation fails, sds_host and sds_port are leaked; destroy them in the failure path.
Apply something like the following (adjust param position if the signature differs, the intent is: last TLS-related argument before kv):
- node = flb_upstream_node_create( - NULL, sds_host, sds_port, FLB_TRUE, ctx->ins->tls->verify, - ctx->ins->tls->verify_hostname, - ctx->ins->tls->debug, ctx->ins->tls->vhost, NULL, NULL, NULL, - NULL, NULL, NULL, kv, config); + node = flb_upstream_node_create( + NULL, sds_host, sds_port, FLB_TRUE, + ctx->ins->tls->verify, + ctx->ins->tls->verify_hostname, + ctx->ins->tls->debug, + ctx->ins->tls->vhost, + NULL, /* ca_path */ + NULL, /* ca_file */ + NULL, /* crt_file */ + NULL, /* key_file */ + NULL, /* key_passwd */ + ctx->ins->tls_provider_query, /* provider_query (new param before kv) */ + kv, config);And ensure resources are reclaimed on failure:
- if (!node) { - flb_plg_error(ctx->ins, "error creating resource upstream node"); - } + if (!node) { + flb_plg_error(ctx->ins, "error creating resource upstream node"); + if (sds_host) { flb_sds_destroy(sds_host); sds_host = NULL; } + if (sds_port) { flb_sds_destroy(sds_port); sds_port = NULL; } + }plugins/filter_nightfall/nightfall.c (2)
428-433: Off-by-one when packing masked integer stringmsgpack_pack_str_with_body(new_rec_pk, "******", 7) packs 7 bytes, which includes the trailing NUL. Use the correct length.
- msgpack_pack_str_with_body(new_rec_pk, "******", 7); + msgpack_pack_str_with_body(new_rec_pk, "******", 6);
512-563: Leaks: per-record to_redact and msgpack_sbuffer not freed
- to_redact is allocated by scan_log per record; it is only freed once after the loop, leaking all but possibly the last allocation (or risking double-free in error paths).
- new_rec_sbuf allocated by redact_record for sensitive records is never destroyed after encoding, leaking per sensitive record.
Free both at the end of each iteration and drop the final free:
@@ - while ((ret = flb_log_event_decoder_next( + while ((ret = flb_log_event_decoder_next( &log_decoder, &log_event)) == FLB_EVENT_DECODER_SUCCESS) { ret = scan_log(ctx, log_event.body, &to_redact, &to_redact_size, &is_sensitive); @@ if (is_sensitive == FLB_TRUE) { ret = redact_record(log_event.body, &to_redact, &to_redact_size, tmp, &new_rec_sbuf); if (ret != 0) { flb_plg_error(ctx->ins, "redaction error"); flb_free(to_redact); msgpack_sbuffer_destroy(&new_rec_sbuf); flb_log_event_decoder_destroy(&log_decoder); flb_log_event_encoder_destroy(&log_encoder); return FLB_FILTER_NOTOUCH; } is_modified = FLB_TRUE; } if (is_modified) { ret = flb_log_event_encoder_begin_record(&log_encoder); @@ if (ret == FLB_EVENT_ENCODER_SUCCESS) { ret = flb_log_event_encoder_commit_record( &log_encoder); } + /* release per-record buffer */ + msgpack_sbuffer_destroy(&new_rec_sbuf); + is_modified = FLB_FALSE; } + + /* release per-record scan buffer regardless of sensitivity */ + if (to_redact) { + flb_free(to_redact); + to_redact = NULL; + to_redact_size = 0; + } } - flb_free(to_redact);
🧹 Nitpick comments (21)
.gitignore (1)
3-3: Good addition for Windows/VS developers; optional tweak for clarityAdding
.vsis helpful to keep local Visual Studio state out of the repo. As a minor nit, consider making it explicit that it’s a directory and (optionally) anchoring to repo root for parity with/out/.Suggested minimal change:
-.vs +/.vs/CMakeLists.txt (1)
199-200: Clarify option description and make dependency explicitConsider tightening the help text to make the TLS dependency and OpenSSL version expectation obvious at configure time.
Apply this small tweak:
-option(FLB_OPENSSL_STORE "Enable OpenSSL Store Support for TLS" No) +option(FLB_OPENSSL_STORE "Enable OpenSSL Store API support (requires FLB_TLS and OpenSSL >= 3.0)" No)Optionally, you could also gate visibility on FLB_TLS using cmake_dependent_option (if you choose to include CMake’s DependentOption module), but the clearer help string already helps users avoid misconfigurations.
include/fluent-bit/flb_output.h (1)
372-373: New tls_provider_query field: validate init/cleanup, config plumbing, and ABI notice
- Good placement under FLB_HAVE_TLS and next to other TLS strings. Please verify:
- flb_output_new initializes ins->tls_provider_query to NULL and flb_output_free_properties frees it.
- tls.provider_query is exposed in the output TLS config map and parsed by flb_output_set_property().
- flb_output_init_all (or the place that builds ins->tls) passes ins->tls_provider_query to flb_tls_create().
- Adding to struct flb_output_instance changes the ABI visible to out-of-tree plugins; call this out in release notes so external plugins rebuild against the new headers.
plugins/in_kubernetes_events/kubernetes_events_conf.c (1)
100-118: Optional: consider provider_query propagation for K8s eventsIf HSM-backed client identities are desired when contacting the API server, consider threading a provider_query setting into this input in the future (similar to outputs and upstream nodes). Safe to defer; just a potential enhancement.
include/fluent-bit/flb_input.h (1)
449-454: tls_provider_query init/free verified; please add upgrade noteInitialization and teardown of the new
tls_provider_queryfield have been confirmed, and configuration wiring is in place. To ensure external plugin authors are aware of the ABI change, please add a brief upgrade note.• Initialization
- In
src/flb_input.c,instance->tls_provider_queryis set toNULLalongside other TLS fields duringflb_input_new▶ initialization verified.
• Teardown- In
src/flb_input.c,flb_sds_destroy(ins->tls_provider_query)is invoked if non-NULL inflb_input_instance_exit▶ cleanup verified.
• Configuration mapping- Property key check for
"tls.provider_query"and corresponding call toflb_utils_set_plugin_string_property()exist in both input and output code paths.Action items:
- Add a note in the CHANGELOG or upgrade guide indicating that the public
flb_input_instance(and related structs) have grown by one pointer (tls_provider_query), which changes their ABI layout for out-of-tree plugins embedding or assuming struct layouts.plugins/out_azure_kusto/azure_kusto_conf.c (1)
66-87: Guard against NULL ‘port’ from flb_utils_url_splitAzure Storage SAS URLs typically omit an explicit port. If port is NULL, flb_sds_create(port) will fail. Default to “443” when absent.
You can defensively set a default:
- /* kv that will hold base uri, and sas token */ + /* kv that will hold base uri, and sas token */ kv = flb_hash_table_create(FLB_HASH_TABLE_EVICT_NONE, 2, 2); @@ - sds_host = flb_sds_create(host); - sds_port = flb_sds_create(port); + sds_host = flb_sds_create(host); + sds_port = port ? flb_sds_create(port) : flb_sds_create("443");tests/runtime/out_tcp.c (1)
322-324: Redundant flb_tls_init() in test now that lib init calls it once
- flb_init_env() (via pthread_once) already initializes TLS. This explicit flb_tls_init() in the test is no longer necessary and could complicate lifecycle if cleanup is adjusted later.
Apply this cleanup:
- ret = flb_tls_init(); - TEST_CHECK(ret == 0); -plugins/out_kinesis_streams/kinesis.c (1)
145-148: Remove duplicate retrieval of 'log_key'log_key is already read and assigned at Lines 97-100. This second block is redundant and risks future drift.
- tmp = flb_output_get_property("log_key", ins); - if (tmp) { - ctx->log_key = tmp; - } + /* log_key already retrieved earlier (see Lines 97-100) */tests/runtime/in_tcp.c (1)
368-377: Unit test updated to new flb_tls_create signature — OK; consider adding a provider-query testPassing an extra NULL before the final NULL matches the expanded TLS API. Consider adding a runtime test that sets a non-NULL provider query (behind the FLB_OPENSSL_STORE/feature guard) to exercise the new parameter end-to-end.
I can draft a minimal conditional test case that sets tls.provider_query in the input section and asserts the TLS context is created successfully.
include/fluent-bit/flb_config.h (1)
104-105: Pointer style consistency (char * vs char)*The file predominantly uses "char *name" spacing. Please align the new field with existing style.
- char* openssl_providers; + char *openssl_providers;src/flb_upstream_node.c (3)
126-130: Null check forflb_sds_createis insufficientThe
flb_sds_createfunction can return NULL even when the input string is non-NULL (based on the implementation insrc/flb_sds.c). The current check correctly handles allocation failure but doesn't validate that a valid provider query string was successfully duplicated.Consider adding a trace/debug log when provider_query is successfully set, similar to other TLS-related fields, to aid in troubleshooting HSM integration issues.
147-148: Inconsistent whitespace after commaThere's trailing whitespace after the comma on line 147.
Apply this diff to fix the formatting:
- tls_key_passwd, + tls_key_passwd,
41-41: Consider renaming parameter for clarityThe parameter name
tls_provider_queryis clear and descriptive. However, since this feature specifically supports OpenSSL providers and stores, consider documenting this in the function comment or renaming toopenssl_provider_queryfor absolute clarity about its scope.src/tls/flb_tls.c (2)
266-272: Add debug logging for provider configurationThe function correctly delegates to
tls_configurebut lacks visibility into what's being configured.Consider adding debug logging to track provider configuration:
void flb_tls_configure(struct flb_config* config) { if (!config) { return; } + if (config->openssl_providers) { + flb_debug("[tls] configuring OpenSSL providers: %s", config->openssl_providers); + } tls_configure(config); }
197-198: Parameter naming consistencyThe parameter
additional_datais generic while it specifically carries provider_query data. Consider using a more descriptive name.For better clarity, consider renaming
additional_datatoprovider_queryto match its actual usage:- const char *key_passwd, - const char *additional_data) + const char *key_passwd, + const char *provider_query)Then update the call to
tls_context_create:backend = tls_context_create(verify, debug, mode, vhost, ca_path, ca_file, - crt_file, key_file, key_passwd, - additional_data); + crt_file, key_file, key_passwd, + provider_query);include/fluent-bit/tls/flb_tls.h (1)
131-132: Consider documenting the additional_data parameterThe
additional_dataparameter would benefit from documentation to clarify it carries provider_query information.Add a comment above the function declaration:
+/* additional_data: OpenSSL provider query string (NULL if not using providers) */ struct flb_tls *flb_tls_create(int mode, int verify,src/tls/openssl.c (5)
105-114: Potential memory leak in provider resetThe function correctly unloads providers but doesn't check the return value of
OSSL_PROVIDER_unload.Consider checking return values and logging failures:
static void openssl_providers_reset(void) { flb_idebug("[tls] resetting providers"); for (int i = 0; i < MAX_OPENSSL_PROVIDERS; i++) if (openssl_providers.provider[i]) { - OSSL_PROVIDER_unload(openssl_providers.provider[i]); + if (!OSSL_PROVIDER_unload(openssl_providers.provider[i])) { + flb_warn("[tls] failed to unload provider at index %d", i); + } openssl_providers.provider[i] = NULL; } openssl_providers.current_provider = 0; }
744-780: Error handling in openssl_provider_get_itemThe function has good error handling but could benefit from more specific error messages.
Consider adding ERR_get_error() information to debug messages:
if (!store) { - flb_debug("[tls] Failed to load store %s", store_uri); + unsigned long err = ERR_get_error(); + char err_buf[256]; + ERR_error_string_n(err, err_buf, sizeof(err_buf)-1); + flb_debug("[tls] Failed to load store %s: %s", store_uri, err_buf); return false; }
806-818: Memory management in certificate handlingThe serialization and deserialization approach to remove provider query metadata is clever but could be documented better.
Add a comment explaining why the serialization is necessary:
- /* Serialise and De - Serialise to remove the provider query. Seems to interfere with - the signature later on */ + /* Serialize and deserialize to create a clean X509 object without provider-specific + metadata that could interfere with signature verification or certificate chain + validation in OpenSSL's standard verification paths */
993-993: Type cast inconsistencyThe cast to
(void*)removes const qualifier unnecessarily.The parameter is already const char*, no need to cast:
- SSL_CTX_set_default_passwd_cb_userdata(ssl_ctx, - (void*)key_passwd); + SSL_CTX_set_default_passwd_cb_userdata(ssl_ctx, key_passwd);
997-1021: Complex conditional logic could be simplifiedThe nested conditional compilation and if-else chains make this code hard to follow.
Consider extracting the store loading logic into a helper function to improve readability:
#if defined(FLB_USE_OPENSSL_STORE) && (OPENSSL_VERSION_NUMBER >= OPENSSL_3_0) static bool try_load_key_from_store(struct tls_context *ctx, const char *key_file, const char *provider_query, char *err_buf) { if (!openssl_provider_get_item((void**)&ctx->store_pkey, key_file, provider_query, openssl_provider_get_private_key)) { return false; } int ret = SSL_CTX_use_PrivateKey(ctx->ctx, ctx->store_pkey); if (ret != 1) { ERR_error_string_n(ERR_get_error(), err_buf, 255); flb_error("[tls] key_store '%s' %lu: %s", key_file, ERR_get_error(), err_buf); return false; } return true; } #endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (31)
.gitignore(2 hunks)CMakeLists.txt(2 hunks)include/fluent-bit/flb_config.h(2 hunks)include/fluent-bit/flb_input.h(1 hunks)include/fluent-bit/flb_output.h(1 hunks)include/fluent-bit/flb_upstream_node.h(2 hunks)include/fluent-bit/tls/flb_tls.h(2 hunks)plugins/filter_kubernetes/kube_meta.c(2 hunks)plugins/filter_nightfall/nightfall.c(1 hunks)plugins/in_kubernetes_events/kubernetes_events_conf.c(1 hunks)plugins/out_azure_blob/azure_blob_conf.c(1 hunks)plugins/out_azure_kusto/azure_kusto_conf.c(1 hunks)plugins/out_bigquery/bigquery.c(4 hunks)plugins/out_cloudwatch_logs/cloudwatch_logs.c(3 hunks)plugins/out_es/es_conf.c(2 hunks)plugins/out_kinesis_firehose/firehose.c(3 hunks)plugins/out_kinesis_streams/kinesis.c(3 hunks)plugins/out_opensearch/os_conf.c(2 hunks)plugins/out_s3/s3.c(3 hunks)src/aws/flb_aws_credentials.c(2 hunks)src/flb_config.c(3 hunks)src/flb_input.c(4 hunks)src/flb_lib.c(4 hunks)src/flb_oauth2.c(1 hunks)src/flb_output.c(4 hunks)src/flb_upstream_ha.c(6 hunks)src/flb_upstream_node.c(4 hunks)src/tls/flb_tls.c(5 hunks)src/tls/openssl.c(11 hunks)tests/runtime/in_tcp.c(1 hunks)tests/runtime/out_tcp.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
src/flb_config.c (1)
include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
src/flb_upstream_ha.c (2)
src/config_format/flb_config_format.c (1)
flb_cf_section_property_get_string(375-415)src/flb_sds.c (1)
flb_sds_destroy(389-399)
src/flb_lib.c (1)
src/tls/flb_tls.c (2)
flb_tls_configure(266-272)flb_tls_cleanup(295-298)
src/tls/flb_tls.c (1)
src/tls/openssl.c (2)
tls_configure(211-239)tls_cleanup(241-246)
src/flb_input.c (3)
src/flb_output.c (1)
prop_key_check(821-831)src/flb_utils.c (1)
flb_utils_set_plugin_string_property(1810-1834)src/flb_sds.c (1)
flb_sds_destroy(389-399)
src/flb_output.c (2)
src/flb_sds.c (1)
flb_sds_destroy(389-399)src/flb_utils.c (1)
flb_utils_set_plugin_string_property(1810-1834)
src/flb_upstream_node.c (1)
src/flb_sds.c (2)
flb_sds_create(78-90)flb_sds_destroy(389-399)
src/tls/openssl.c (1)
include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
include/fluent-bit/tls/flb_tls.h (1)
src/tls/flb_tls.c (3)
flb_tls_configure(266-272)flb_tls_cleanup(295-298)flb_tls_create(189-240)
🔇 Additional comments (46)
.gitignore (1)
45-45: No tracked files or scripts depend on top-levelout/–
git ls-filesconfirms no tracked paths underout/.
– Searches found only references toout/in third-party code underlib/and in test helpers (viaDPATH "/out/…"), neither of which affects our build/packaging.It’s safe to merge the new
/out/rule.include/fluent-bit/flb_upstream_node.h (1)
46-47: I’ve requested the implementations offlb_upstream_node_createandflb_upstream_node_destroy, and the Azure plugin call site to verify copy/free semantics and parameter coverage. [waiting for results]plugins/filter_kubernetes/kube_meta.c (2)
1738-1746: TLS create arity update looks correctCall site updated to pass the additional provider_query argument (NULL). No functional change otherwise.
1791-1799: TLS create arity update looks correctSecond call site also correctly passes the extra NULL for provider_query.
plugins/in_kubernetes_events/kubernetes_events_conf.c (1)
106-113: TLS create arity update looks correctThe extra provider_query parameter is passed as NULL; no behavioral change.
src/aws/flb_aws_credentials.c (2)
429-440: STS path also correctly threads provider_queryThe STS-specific TLS context now receives ins->tls_provider_query as well, aligning with the cred_tls change. Error handling paths still properly clean up on failure.
377-388: All AWS-relatedflb_tls_createcalls now includeins->tls_provider_query
Verified that every AWS provider/client in the following locations passesins->tls_provider_queryas the final argument toflb_tls_create:
src/aws/flb_aws_credentials.c(both IMDS and STS instantiations)plugins/out_s3/s3.c(client, provider, and STS contexts)plugins/out_opensearch/os_conf.c(AWS and STS contexts)plugins/out_kinesis_streams/kinesis.c(cred, client, and STS contexts)plugins/out_kinesis_firehose/firehose.c(cred, client, and STS contexts)plugins/out_es/es_conf.c(AWS and STS contexts)plugins/out_cloudwatch_logs/cloudwatch_logs.c(cred, client, and STS contexts)plugins/out_bigquery/bigquery.c(AWS IMDS, AWS STS, Google STS, Google IAM contexts)All other non–AWS-related
flb_tls_createcalls remain unchanged as intended.plugins/out_cloudwatch_logs/cloudwatch_logs.c (1)
245-256: Good: provider_query forwarded to all TLS contexts (cred, client, STS)All three TLS contexts now receive ins->tls_provider_query, which is critical for Provider/Store-backed keys. Error handling and cleanup remain intact.
Please verify builds with:
- FLB_HAVE_TLS enabled/disabled
- FLB_OPENSSL_STORE ON/OFF
- Non-Linux targets (Windows/macOS) to satisfy the reviewer concerns.
A fast sanity build sweep can be scripted in CI to flip FLB_OPENSSL_STORE and platform matrices.Also applies to: 262-272, 298-309
plugins/filter_nightfall/nightfall.c (1)
94-101: TLS api arity update handled; OK to pass NULLsThe extra trailing NULL for flb_tls_create aligns with the new API. Since Nightfall uses the default system trust and no client certs, NULLs are appropriate here.
src/flb_upstream_ha.c (3)
156-166: Wire tls.provider_query through HA node config — LGTM
- New field tls_provider_query, inclusion in known_keys, and overall plumbing look correct and consistent with other TLS fields.
239-240: Read + env-translate tls.provider_query — LGTM
- Property retrieval and environment-variable translation mirror existing TLS options and should behave as expected.
Also applies to: 250-251
325-330: Ownership oftls_provider_queryconfirmed
Theflb_upstream_node_createfunction duplicates thetls_provider_queryargument usingflb_sds_create(see src/flb_upstream_node.c:126). Therefore, destroying the localtls_provider_queryafter the call in src/flb_upstream_ha.c (lines 356–358) is safe and does not lead to use-after-free.src/flb_oauth2.c (1)
223-225: Update flb_tls_create arity — passing NULL provider query — LGTM
- Call site correctly passes the new trailing parameter. No behavioral change for OAuth2 (TLS provider query not needed here).
src/flb_lib.c (3)
123-136: Initialize TLS early in flb_init_env — LGTM
- Adding flb_tls_init() to the one-time environment initialization aligns with the new provider lifecycle.
138-142: Introduce flb_configure_env() to configure TLS per-config — LGTM
- The wrapper keeps TLS global init separate from per-config provider configuration, which is clean and extensible.
872-873: Configure TLS before engine start — LGTM
- Calling flb_configure_env(ctx->config) right before engine start ensures provider settings are applied after service properties are set.
plugins/out_bigquery/bigquery.c (1)
699-700: Propagate tls_provider_query to all TLS contexts — LGTM
- Passing ins->tls_provider_query into all flb_tls_create() call sites (IMDS, AWS STS, Google STS, Google IAM) is consistent and future-proofs HSM-backed TLS handshakes across these upstreams.
Also applies to: 739-740, 771-772, 800-801
tests/runtime/out_tcp.c (1)
333-335: Update flb_tls_create arity in test — LGTM
- Extra NULL argument for provider query matches the new signature.
plugins/out_kinesis_streams/kinesis.c (1)
164-174: All flb_tls_create() calls updated to the new 10-arg signature
Verified via script: everyflb_tls_create(...)invocation now has nine top-level commas (10 arguments), includingins->tls_provider_query. No further changes needed.plugins/out_opensearch/os_conf.c (1)
252-262: Provider query plumbed into AWS TLS contexts — LGTMBoth aws_tls and aws_sts_tls creation sites now forward ins->tls_provider_query to flb_tls_create(...). This is consistent with the updated TLS API and keeps the AWS auth path aligned with other outputs.
Also applies to: 314-324
include/fluent-bit/flb_config.h (1)
417-420: New service key for OpenSSL Providers — naming looks goodFLB_CONF_STR_OPENSSL_PROVIDERS set to "openssl.providers" is clear and consistent with existing service keys.
src/flb_config.c (3)
208-212: Wire-up of service config → config->openssl_providers looks correctThe service_configs entry maps "openssl.providers" into struct flb_config via offsetof. Good use of the existing STR handling path (env-var translation + flb_strdup + prior free).
316-316: Initialize openssl_providers to NULLExplicitly setting the default avoids accidental free() on uninitialized memory in exit paths. Looks good.
579-582: Free and Load OpenSSL Providers: Verified
- Confirmed that
config->openssl_providersis consumed in the OpenSSL TLS backend (src/tls/openssl.c, lines 224–232): the string is duplicated, tokenized, and each provider is loaded.- Verified that
flb_tls_configure(config)is called fromsrc/flb_lib.c(lines 140–141), ensuring the providers setting is applied at startup.- The free in
src/flb_config.c(lines 579–582) correctly releases the duplicated string, preventing leaks on shutdown/reload.LGTM.
plugins/out_es/es_conf.c (2)
447-449: Same propagation for STS provider TLS — consistent with the AWS pathMirrors the earlier change and keeps verification enabled; good parity between aws_tls and aws_sts_tls.
385-387: ❗ Manual verification required: ensureprovider_queryadded to allflb_tls_createcall sites
- In
plugins/out_es/es_conf.c, the callcorrectly propagates the newctx->aws_tls = flb_tls_create(FLB_TLS_CLIENT_MODE, ins->tls_verify, ins->tls_debug, ins->tls_provider_query);provider_queryparameter.- The heuristic search script only matches single-line invocations and did not account for multi-line calls (the vast majority of
flb_tls_createuses span multiple lines). As written, it cannot conclusively verify that every call site was updated.- Please manually review—or use a multi-line-capable search (for example:
or leveragerg -zP "flb_tls_create\\([^)]*\\)" | grep -C2 provider_queryast-grepto match C function calls—to confirm that no remainingflb_tls_createinvocations are missing the fourthprovider_queryargument.plugins/out_s3/s3.c (1)
789-790: S3 TLS contexts correctly include tls.provider_query; no issues foundAll three S3 TLS contexts (client_tls, provider_tls, sts_provider_tls) invoke flb_tls_create with ins->tls_provider_query as their final argument, and the ins->tls_verify flag continues to drive hostname verification for the client_tls call. A search for error/warning/info logs referencing “provider_query” returned no matches (only a debug‐level log in the OpenSSL backend, which is expected), so there’s no unintended exposure of the query string.
– plugins/out_s3/s3.c lines 785–795, 802–811, 833–844: verified provider_query argument
– flb_tls_create signature (null‐safe) confirmed
– No flb_error/flb_warn/flb_info prints of provider_query foundsrc/flb_input.c (3)
408-409: Initialize tls_provider_query to NULL — good defensive defaultKeeps instance state predictable when TLS is enabled later.
1296-1298: Pass provider_query to flb_tls_create during input TLS init
- Ordering matches the extended signature; safe even when NULL.
- Behavior for server vs client TLS is preserved via tls_session_mode.
876-879: Free tls_provider_query on destroy — lifecycle completeMatches how other tls.* strings are released. No leaks.
src/flb_output.c (4)
187-189: Release tls_provider_query in output properties destructorCompletes the output lifecycle; consistent with other tls.* fields.
772-773: Initialize output tls_provider_query to NULL — correct defaultKeeps behavior predictable before property parsing.
1361-1363: Output TLS creation now receives provider_queryCorrect parameter order; no change to verify/hostname semantics. Looks good.
1002-1004: tls.provider_query output property verified and included in help
- Confirmed that
tls.provider_queryis defined in the TLS config map insrc/tls/flb_tls.c(around lines 101–104) and thus will be included in the output plugin’s help via the call toflb_tls_get_config_mapinsrc/flb_output.c:1448.- The code at
src/flb_output.c:1002–1004correctly usesflb_utils_set_plugin_string_property("tls.provider_query", &ins->tls_provider_query, tmp);in line with other TLS properties.plugins/out_kinesis_firehose/firehose.c (1)
160-162: Firehose TLS contexts properly wired to tls_provider_query
- In plugins/out_kinesis_firehose/firehose.c, the calls to flb_tls_create for cred_tls (line 152), client_tls (line 168), and sts_tls (line 205) each pass ins->tls_provider_query as the final parameter, matching the intended change.
- A project-wide scan of flb_tls_create usages shows that AWS plugins for CloudWatch Logs, Kinesis Streams, and S3 follow the same pattern, ensuring consistency across all AWS outputs.
src/tls/flb_tls.c (2)
101-105: Good addition of provider_query configurationThe new
tls.provider_queryconfiguration option is well-documented and follows the existing pattern for TLS configuration options.
295-298: LGTM - Clean cleanup functionThe cleanup function correctly delegates to the backend-specific cleanup.
include/fluent-bit/tls/flb_tls.h (2)
69-73: API extension looks goodThe backend API is properly extended with an additional parameter for provider support.
122-123: New configuration lifecycle functions are well-placedThe new
flb_tls_configureandflb_tls_cleanupfunctions provide proper lifecycle management for TLS provider configuration.src/tls/openssl.c (7)
145-154: Memory management issue in openssl_providers_loadedThe function correctly duplicates the string to avoid modifying the original, but uses
flb_strdupwhich uses the custom allocator while freeing withflb_free.Good defensive programming with string duplication to avoid modifying the input.
709-736: Good URI detection logicThe
could_be_urifunction implements proper RFC 3986 scheme validation. This is a good security practice to differentiate between file paths and store URIs.
961-986: Good fallback logic for certificate loadingThe implementation correctly attempts to load from store first, then falls back to file loading. This provides good backward compatibility.
306-315: Memory cleanup in tls_context_destroy looks correctProper cleanup of store_cert and store_pkey with NULL checks.
66-71: Global state management for providersThe global
openssl_providersstructure is appropriate for provider management since OpenSSL providers are process-wide resources.
200-206: Provider state initializationGood defensive initialization of the provider array in
tls_init.
211-239: Thread Safety: Protect OpenSSL Provider ConfigurationThe new
tls_configure()implementation (and its public wrapperflb_tls_configure) unconditionally resets and reloads the global OpenSSL provider list without any serialization. From the grep output:
tls_configure()insrc/tls/openssl.cmanipulates OpenSSL’s provider state.flb_tls_configure()is defined atsrc/tls/flb_tls.c:266and invoked byflb_configure_env()insrc/flb_lib.c:140.If these functions ever run concurrently (e.g. during a live reconfiguration), they will race on the provider list. Please verify whether
flb_tls_configure()can be called from more than one thread. If so, either:
- Introduce a mutex (or other lock) around the calls to
openssl_providers_reset()/openssl_load_provider()intls_configure(), or- Document and enforce that provider configuration may only occur on a single, dedicated thread.
— ptf
| # Ensure OpenSSL Store support is available if TLS is enabled | ||
| if (FLB_OPENSSL_STORE) | ||
| if(NOT FLB_TLS) | ||
| message(FATAL_ERROR "FLB_ENABLE_OPENSSL_STORE requires FLB_TLS to be enabled.") | ||
| endif() | ||
| message(STATUS "OpenSSL Store defined") | ||
| add_definitions(-DFLB_USE_OPENSSL_STORE=1) | ||
| endif() | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Wrong option name in fatal error; define macro consistently; and gate after OpenSSL detection
- The fatal error message references a non-existent option “FLB_ENABLE_OPENSSL_STORE”; it should say “FLB_OPENSSL_STORE”.
- Prefer using the existing FLB_DEFINITION helper instead of add_definitions for consistency with the rest of the file.
- More importantly, the macro is defined before OpenSSL is discovered and without checking the OpenSSL version. If a builder sets FLB_OPENSSL_STORE=On with an OpenSSL < 3.0 (or no OpenSSL at all), configuration will appear to succeed here but compilation will likely fail elsewhere. Move the enablement to after find_package(OpenSSL) so you can fail fast (and clearly) when the version is insufficient.
Minimal immediate fix within this block:
-# Ensure OpenSSL Store support is available if TLS is enabled
+# Ensure TLS is enabled when OpenSSL Store support is requested
if (FLB_OPENSSL_STORE)
if(NOT FLB_TLS)
- message(FATAL_ERROR "FLB_ENABLE_OPENSSL_STORE requires FLB_TLS to be enabled.")
+ message(FATAL_ERROR "FLB_OPENSSL_STORE requires FLB_TLS to be enabled.")
endif()
- message(STATUS "OpenSSL Store defined")
- add_definitions(-DFLB_USE_OPENSSL_STORE=1)
+ message(STATUS "Configuring OpenSSL Store support")
+ # Note: macro definition is finalized after OpenSSL is found/version-checked
endif()Then, add the actual gating and macro definition right after OpenSSL discovery (near Lines 720–724). This makes the check accurate and the error actionable:
# Place this block immediately after setting FLB_HAVE_OPENSSL
if (FLB_OPENSSL_STORE)
if (NOT OPENSSL_FOUND)
message(FATAL_ERROR "FLB_OPENSSL_STORE requires OpenSSL to be available when FLB_TLS=On.")
endif()
# CMake's FindOpenSSL provides a version string; accommodate both legacy and modern variable names.
if (DEFINED OpenSSL_VERSION)
set(_FLB_OPENSSL_VER "${OpenSSL_VERSION}")
elseif(DEFINED OPENSSL_VERSION)
set(_FLB_OPENSSL_VER "${OPENSSL_VERSION}")
else()
set(_FLB_OPENSSL_VER "")
endif()
if (NOT _FLB_OPENSSL_VER STREQUAL "" AND _FLB_OPENSSL_VER VERSION_LESS "3.0.0")
message(FATAL_ERROR "FLB_OPENSSL_STORE requires OpenSSL >= 3.0. Detected: ${_FLB_OPENSSL_VER}")
endif()
message(STATUS "OpenSSL Store support: ENABLED${_FLB_OPENSSL_VER STREQUAL "" ? "" : " (OpenSSL "}${_FLB_OPENSSL_VER}${_FLB_OPENSSL_VER STREQUAL "" ? "" : ")"}")
FLB_DEFINITION(FLB_USE_OPENSSL_STORE)
endif()Notes:
- If you prefer to keep the early block, at minimum correct the option name and avoid defining FLB_USE_OPENSSL_STORE before verifying OpenSSL presence/version.
- The comment in Line 423 is misleading (“if TLS is enabled” vs the actual conditional on FLB_OPENSSL_STORE); the minimal fix above clarifies intent.
To ensure this behaves well across platforms (as requested by reviewers), please validate CMake configuration on at least Linux, macOS, and Windows with:
- OpenSSL 3.x installed (and on Windows confirm provider DSOs can be located at runtime).
- FLB_TLS=On and FLB_OPENSSL_STORE=On
Example configure invocations:
If you want, I can open a follow-up PR to wire a CMake configure-time summary that prints “OpenSSL Store: ENABLED/DISABLED (version)”.
🏁 Script executed:
# Linux/macOS (with pkg-config OpenSSL 3)
cmake -S . -B build -DFLB_TLS=On -DFLB_OPENSSL_STORE=On
# Windows (ensure OpenSSL 3 is on PATH or CMAKE_PREFIX_PATH points to it)
cmake -S . -B build -DFLB_TLS=On -DFLB_OPENSSL_STORE=On -DOPENSSL_USE_STATIC_LIBS=ONLength of output: 336
🏁 Script executed:
#!/bin/bash
# Locate OpenSSL discovery in CMakeLists.txt
grep -nP "find_package\\s*\\(\\s*OpenSSL" -n CMakeLists.txt | head -n 10
# Show existing block around line 423
sed -n '400,450p' CMakeLists.txt
# Show existing block around line 720
sed -n '700,750p' CMakeLists.txtLength of output: 2691
🏁 Script executed:
#!/bin/bash
# Search for all add_definitions usage in CMakeLists.txt
grep -n "add_definitions" CMakeLists.txt
# Search for FLB_DEFINITION usage in CMakeLists.txt (limit to first 20 occurrences)
grep -n "FLB_DEFINITION" CMakeLists.txt | head -n 20Length of output: 1528
Correct and relocate OpenSSL Store gating and definition
- Fix the fatal-error text to reference the real option
FLB_OPENSSL_STORE(notFLB_ENABLE_OPENSSL_STORE). - Replace the raw
add_definitions(-DFLB_USE_OPENSSL_STORE=1)with the project’sFLB_DEFINITION(FLB_USE_OPENSSL_STORE)helper for consistency. - Move the entire “OpenSSL Store” block so it runs after
find_package(OpenSSL)(around line 721) and:- Fails immediately if OpenSSL isn’t found.
- Checks that the discovered OpenSSL version is ≥ 3.0.0.
- Emits clear status messages including the detected version.
- Defines
FLB_USE_OPENSSL_STOREviaFLB_DEFINITION.
Minimal diff for the early block (around lines 423–431):
-# Ensure OpenSSL Store support is available if TLS is enabled
+# Ensure TLS is enabled when OpenSSL Store support is requested
if (FLB_OPENSSL_STORE)
if(NOT FLB_TLS)
- message(FATAL_ERROR "FLB_ENABLE_OPENSSL_STORE requires FLB_TLS to be enabled.")
+ message(FATAL_ERROR "FLB_OPENSSL_STORE requires FLB_TLS to be enabled.")
endif()
- message(STATUS "OpenSSL Store defined")
- add_definitions(-DFLB_USE_OPENSSL_STORE=1)
+ message(STATUS "OpenSSL Store support requested; will configure after OpenSSL discovery")
endif()Then, immediately after the existing TLS/OpenSSL discovery block (near line 721), add:
# === OpenSSL Store support ===
if (FLB_OPENSSL_STORE)
if (NOT OPENSSL_FOUND)
message(FATAL_ERROR "FLB_OPENSSL_STORE requires OpenSSL when FLB_TLS=On.")
endif()
# Determine OpenSSL version string
if (DEFINED OpenSSL_VERSION)
set(_FLB_OPENSSL_VER "${OpenSSL_VERSION}")
elseif (DEFINED OPENSSL_VERSION)
set(_FLB_OPENSSL_VER "${OPENSSL_VERSION}")
else()
set(_FLB_OPENSSL_VER "")
endif()
if (_FLB_OPENSSL_VER STREQUAL "" OR _FLB_OPENSSL_VER VERSION_LESS "3.0.0")
message(FATAL_ERROR
"FLB_OPENSSL_STORE requires OpenSSL ≥ 3.0. Detected: ${_FLB_OPENSSL_VER}")
endif()
message(STATUS
"OpenSSL Store support: ENABLED (OpenSSL ${_FLB_OPENSSL_VER})")
FLB_DEFINITION(FLB_USE_OPENSSL_STORE)
endif()This ensures immediate, clear failures on missing or too-old OpenSSL, keeps macro definitions consistent, and aligns with the rest of the project’s CMake conventions.
🤖 Prompt for AI Agents
In CMakeLists.txt lines ~423-431 and again near line ~721: the early OpenSSL
Store gating uses the wrong option name and raw add_definitions and must be
relocated; remove or replace the early block so it no longer emits a wrong fatal
message or uses add_definitions, and instead add the full "OpenSSL Store
support" block immediately after find_package(OpenSSL) (around line 721). In the
new block keep the if(FLB_OPENSSL_STORE) guard, fail immediately if NOT
OPENSSL_FOUND with a clear message referencing FLB_OPENSSL_STORE and FLB_TLS,
detect OpenSSL version by checking OpenSSL_VERSION or OPENSSL_VERSION into a
local _FLB_OPENSSL_VER, fail if that version is empty or VERSION_LESS "3.0.0"
with a clear fatal message including the detected version, emit a STATUS message
showing "OpenSSL Store support: ENABLED (OpenSSL <version>)", and define the
macro using FLB_DEFINITION(FLB_USE_OPENSSL_STORE) instead of add_definitions.
| else if (prop_key_check("tls.provider_query", k, len) == 0) { | ||
| flb_utils_set_plugin_string_property("tls.provider_query", &ins->tls_provider_query, tmp); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add tls.provider_query input property — parse path looks correct
- Uses flb_utils_set_plugin_string_property, matching other TLS string properties and preserving overwrite warnings.
- env var expansion applied earlier via flb_env_var_translate; good.
Please confirm the generic TLS config map/help includes tls.provider_query so users see it in -i -h output:
🏁 Script executed:
#!/bin/bash
# The TLS config map should declare tls.provider_query alongside other tls.* options
rg -nP --type=c --type=cpp -C3 'tls\.provider_query|provider_query' include fluent-bit srcLength of output: 16303
Add tls.provider_query to the generic TLS config map
The new tls.provider_query option is parsed and stored by the plugin, but it isn’t declared in the central TLS config map (src/tls/flb_tls.c), so it won’t show up in the -h help output. Please insert it into the tls_configmap array just before the terminating {0} entry:
diff --git a/src/tls/flb_tls.c b/src/tls/flb_tls.c
index 0123456..789abcd 100644
--- a/src/tls/flb_tls.c
+++ b/src/tls/flb_tls.c
@@ -77,6 +77,12 @@ struct flb_config_map tls_configmap[] = {
{
FLB_CONFIG_MAP_BOOL, "tls.verify_hostname", "off",
0, FLB_FALSE, 0,
"Enable or disable to verify hostname"
},
+ {
+ FLB_CONFIG_MAP_STR, "tls.provider_query", NULL,
+ 0, FLB_FALSE, 0,
+ "OpenSSL Provider Query to use for TLS operations"
+ },
/* EOF */
{0}
};This will ensure tls.provider_query appears in all plugin help listings under TLS options.
🤖 Prompt for AI Agents
In src/tls/flb_tls.c (add into the tls_configmap array just before the
terminating {0} entry), the tls.provider_query option is missing from the
central TLS config map; add an entry for "tls.provider_query" using the string
config map type (same pattern as other TLS string options) so it appears in
plugin help output and is parsed as a string in the generic TLS configuration.
|
|
||
| return true; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Check return value from OSSL_STORE_INFO_get1_PKEY
The function correctly uses get1 variant which increments reference count, but doesn't check for NULL return.
Add NULL check after getting the key:
*key = OSSL_STORE_INFO_get1_PKEY(store_info);
+ if (*key == NULL) {
+ flb_debug("[tls] Failed to extract private key from store info");
+ return false;
+ }
return true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return true; | |
| *key = OSSL_STORE_INFO_get1_PKEY(store_info); | |
| if (*key == NULL) { | |
| flb_debug("[tls] Failed to extract private key from store info"); | |
| return false; | |
| } | |
| return true; |
🤖 Prompt for AI Agents
In src/tls/openssl.c around lines 843-844, after calling
OSSL_STORE_INFO_get1_PKEY you must check its return for NULL; if it returns
NULL, free any owned OSSL_STORE_INFO and related resources, log or set the
appropriate error state, and return false (or propagate failure) instead of
proceeding. Update the code to test the returned EVP_PKEY* immediately, handle
the NULL case by cleaning up and returning failure, and only use the key when
the pointer is non-NULL.
Added support for loading OpenSSL Providers. Up to 4 Providers that are defined in the OpenSSL configuration file can be activated by specifying the name in the new configuration option. This isn't specific to TLS, and can be used in general to load Providers for specific functionality.
Added a new compile time option to support the OpenSSL Store API (FLB_OPENSSL_STORE). This will enable treating the TLS crt_file and key_file options as Store URIs. There is also the ability to define a Provider Query to further help with utilising Providers in this scenario. With this ability it becomes possible to load the TLS artifacts from an HSM.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
This was tested on an embedded device, running an i.mx8 Processor with an EdgeLock chip for Secure World access.
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
Configuration
Plugins
Chores