Fix TSSslClientCertUpdate updates

[bneradt]

TSSslClientCertUpdate cannot find normally configured outbound client
contexts because its lookup key differs from the key used by the context
map. The existing AuTest masked the failure with permissive gold-file
expressions.

This patch uses the stored certificate-path key for updates and makes
the AuTest explicitly require both successful update messages and the
expected client certificate subjects.

Fixes: #13575

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[bneradt]

Review notes

The InkAPI.cc fix is correct. SSLConfigParams::getCTX sets ctx_key = client_cert, and all three producers of that key — the conf-override path (SSLNetVConnection.cc:1215), params->clientCertPath, and sni.yaml (SSLSNIConfig.cc:185) — pass a path already resolved through Layout::relative_to. So both the new comment and key.assign(cert_path) are accurate.

Provenance worth adding to the commit message: this broke in 7dbb6cb ("Add hook for loading certificate and key data from plugin", #6609, 2021-06-25), which changed ctx_key from {cert}:{key} to client_cert and did not update InkAPI.cc. That makes this a backport candidate for 9.2.x/10.x — the Backport label isn't set.

The test changes, however, still don't assert anything.

---

1. Blocker — Streams.all is not an AuTest property, so the new assertions never run

Streams exposes All, not all. In autest/testenities/streams.py the properties are stdout, stderr, All, Warning, Error, Debug, Verbose, and neither Streams nor its base TestEntity overrides __setattr__. So s_server.Streams.all = <Tester> just plants a dead instance attribute and registers no tester.

Verified four ways against autest 1.10.6:

1. hasattr(Streams, 'All') == True, hasattr(Streams, 'all') == False.
2. docsrc/source/API/streams.rst documents exactly seven attributes; all is not among them.
3. AuTest's own test suite and docs use Streams.All 28 times and Streams.all zero times.
4. Direct experiment — a process printing alice.com with an assertion demanding bob.com:

# lowercase -> PASSES (assertion silently dropped)
tr.Processes.Default.Streams.all = Testers.ContainsExpression("bob.com", "demands bob.com")

Running Test probe: Passed
  Failed: 0
  Passed: 1

# capital -> FAILS, as it should
tr.Processes.Default.Streams.All = Testers.ContainsExpression("bob.com", "demands bob.com")

Process: Default: Failed
  Test : Checking that ReturnCode == 0 - Passed
  file .../stream.all.txt : demands bob.com - Failed
     Reason: Contents of .../stream.all.txt did not contains expression: "bob.com"

Consequences for this PR:

• Lines 122 and 149 register nothing. The two assertions that actually prove the fix — alice.com before the update, bob.com after — do not execute. This test passes with the InkAPI.cc change reverted.
• This is also the real reason the old test masked the bug. The PR description and TSSslClientCertUpdate cannot find configured contexts #13575 attribute it to "permissive gold-file expressions," but client-cert-after.gold (``bob.com``) was on .all too, so it was never evaluated at all. The permissive-gold explanation holds only for update.gold, where the duplicate Content = assignment overwrote the first tester.

Fix: Streams.all → Streams.All on both lines. There are 12 more dead Streams.all assignments elsewhere under tests/ (h3/h3_sni_check, timeout/quic_no_activity_timeout, timeout/default_inactivity_timeout, ip_allow/ip_allow, tls/tls_sni_groups, tls/allow-plain, tls/tls_hooks_client_verify); those are now handled in #13577 (marked WIP, since some may fail once their checks actually run).

2. Should fix — SSLConfig::acquire() is never released

InkAPI.cc:8236 acquires but never calls SSLConfig::release(params), on any of the three return paths. TSSslClientContextsNamesGet and TSSslClientContextFindByName immediately above both release correctly. Pre-existing, but until this patch the function bailed at ca_paths_key.empty() before doing any work; now that it actually runs, every plugin-driven cert rotation permanently pins an SSLConfigParams generation. Cheap to fix in a function you're already touching.

3. Only the first CA bucket is updated

The lookup loop breaks at the first top-level {ca_file}:{ca_path} bucket containing the cert. The same cert path can legitimately live under several buckets — sni.yaml always uses the global CA, while a conf-override with ssl_client_ca_cert_name produces a different top-level key for the same cert. Stale contexts survive under the others, and which bucket wins is unordered_map iteration order. Also newly reachable as of this patch.

4. A failed rebuild silently invalidates the working context

If SSLCreateClientContext returns nullptr, the code still stores the null shared_SSL_CTX into the map before returning TS_ERROR. getCTX lazy-reloads on null so it self-heals — same shape as clearCTX — but a failed update shouldn't quietly discard a good context. Worth an early bail, or a comment saying the null store is deliberate.

5. The API doc is now actively wrong

TSSslClientCertUpdate.en.rst:38 says cert_path "should be exact match as provided in configurations." It must be the path resolved against proxy.config.ssl.client.cert.path — this very test writes client_cert: "client1.pem" in sni.yaml but has to pass {SSLDir}/client1.pem. This PR is the right place to correct that sentence. Separately, TSSslClientContext.en.rst still describes the second-level key as "cert/key paths"; it has been cert-only since 7dbb6cb.

6. Minor

• The Content = → Content += change on traffic_out is a real improvement — the second = was replacing the first tester via TesterSet.Assign.
• Consider adding ExcludesExpression("Failed to update client cert"). That's the string that actually appeared in the failing sandbox, and it makes the regression bite even if a "Successfully" line shows up for an unrelated reason.
• std::string key{cert_path}; at the declaration reads better than declare-then-assign.
• The removed swoc::bwprint was nullptr-safe (bwf_base.h:1046 guards v != nullptr), so no crash was being fixed here — just a key that could never match.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[bneradt]

Thanks Claude: all points were valid and are addressed in 0dea1d8. The test now uses Streams.All and covers multiple CA buckets. The update retains the SSL configuration safely, updates every matching context, preserves existing contexts on rebuild failure, and documents the resolved certificate-path key.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.