bneradt commented on code in PR #13576:
URL: https://github.com/apache/trafficserver/pull/13576#discussion_r3856732846
##########
src/api/InkAPI.cc:
##########
@@ -8232,59 +8233,86 @@ TSSslClientCertUpdate(const char *cert_path, const char
*key_path)
return TS_ERROR;
}
- std::string key;
- shared_SSL_CTX client_ctx = nullptr;
- SSLConfigParams *params = SSLConfig::acquire();
+ // --- Pin the active SSL configuration ---
+ //
+ // Keep this configuration generation alive across every early return and
+ // release it automatically when the update finishes.
+ std::string key{cert_path};
+ SSLConfig::scoped_config params;
- // Generate second level key for client context lookup
- swoc::bwprint(key, "{}:{}", cert_path, key_path);
+ // The client context map is keyed by the resolved certificate path.
Dbg(dbg_ctl_ssl_cert_update, "TSSslClientCertUpdate(): Use %.*s as key for
lookup", static_cast<int>(key.size()), key.data());
- if (nullptr != params) {
- // Try to update client contexts maps
- auto &ca_paths_map = params->top_level_ctx_map;
- auto &map_lock = params->ctxMapLock;
- std::string ca_paths_key;
- // First try to locate the client context and its CA path (by top level)
- ink_mutex_acquire(&map_lock);
- for (auto &ca_paths_pair : ca_paths_map) {
- auto &ctx_map = ca_paths_pair.second;
- auto iter = ctx_map.find(key);
- if (iter != ctx_map.end() && iter->second != nullptr) {
- ca_paths_key = ca_paths_pair.first;
- break;
- }
+ if (!params) {
+ return TS_ERROR;
+ }
+
+ auto &ca_paths_map = params->top_level_ctx_map;
+ auto &map_lock = params->ctxMapLock;
+ std::vector<std::string> ca_paths_keys;
+
+ // --- Find every matching CA bucket ---
+ //
+ // A certificate can be used with more than one CA configuration. Snapshot
+ // all matching bucket keys while holding the map lock, then release it
+ // before performing the expensive context construction.
+ ink_mutex_acquire(&map_lock);
+ for (auto const &[ca_paths_key, ctx_map] : ca_paths_map) {
+ if (ctx_map.contains(key)) {
+ ca_paths_keys.push_back(ca_paths_key);
}
- ink_mutex_release(&map_lock);
+ }
+ ink_mutex_release(&map_lock);
+
+ if (ca_paths_keys.empty()) {
+ return TS_ERROR;
+ }
- // Only update on existing
- if (ca_paths_key.empty()) {
+ std::vector<std::pair<std::string, shared_SSL_CTX>> client_contexts;
+
+ // --- Build every replacement context ---
+ //
+ // Build all replacements before changing the live map. If any construction
+ // fails, the existing working contexts remain installed.
+ client_contexts.reserve(ca_paths_keys.size());
+ for (auto const &ca_paths_key : ca_paths_keys) {
+ size_t sep = ca_paths_key.find(':');
+ std::string ca_bundle_file = ca_paths_key.substr(0, sep);
+ std::string ca_bundle_path = ca_paths_key.substr(sep + 1);
+ shared_SSL_CTX client_ctx(SSLCreateClientContext(params,
ca_bundle_path.empty() ? nullptr : ca_bundle_path.c_str(),
+ ca_bundle_file.empty() ?
nullptr : ca_bundle_file.c_str(), cert_path,
+ key_path),
Review Comment:
Fixed—the declaration, definition, and caller now consistently use CA file
followed by CA path.
##########
tests/gold_tests/pluginTest/cert_update/cert_update.test.py:
##########
@@ -116,9 +122,11 @@
"s_server", "openssl s_server -www -key {0}/server1.pem -cert
{0}/server1.pem -accept {1} -Verify 1 -msg".format(
ts.Variables.SSLDir, ts.Variables.s_server_port))
s_server.Ready = When.PortReady(ts.Variables.s_server_port)
-tr.MakeCurlCommand('--verbose --insecure --ipv4 --header "Host: foo.com"
https://localhost:{}'.format(ts.Variables.ssl_port), ts=ts)
+tr.MakeCurlCommand(
+ '--verbose --insecure --ipv4 --header "Host: foo.com"
https://localhost:{}/override-ca'.format(ts.Variables.ssl_port), ts=ts)
tr.Processes.Default.StartBefore(s_server)
-s_server.Streams.all = "gold/client-cert-pre.gold"
+s_server.Streams.All = Testers.ContainsExpression(
+ "alice.com", "The initial outbound connection should use the original
client certificate")
Review Comment:
Added a normal pre-update request. The test now explicitly verifies both CA
contexts use the original certificate before rotation and the replacement
afterward.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
