bryancall commented on code in PR #13373:
URL: https://github.com/apache/trafficserver/pull/13373#discussion_r3575055984
##########
src/iocore/net/SSLConfig.cc:
##########
@@ -703,8 +705,26 @@ SSLCertificateConfig::reconfigure(ConfigContext ctx)
// If the load succeeded, load it. If there is no current configuration,
load even a broken
// config so that a bad initial load doesn't completely disable TLS.
- if (retStatus || configid == 0) {
+ // If partial_reload is enabled and at least one cert was successfully
inserted, commit the
+ // partial lookup so that a single bad cert does not block all other certs
from being updated.
+ // lookup->count() is used rather than checking ssl_default, because
ssl_default is always
+ // populated with a bare bootstrap context (no X.509) for SNI-only
deployments that have no
+ // dest_ip: "*" entry, checking ssl_default would silently disable partial
reload for those.
+ // count() == 0 means all certs failed: hasAnyCert is false, partialCommit
is false,
+ // the lookup is discarded, and retStatus stays false so traffic_ctl reports
failure.
+ // When count() > 0 and partial_reload is on, retStatus is flipped to true
after the
+ // partial commit so that traffic_ctl reports the reload as successful.
+ const bool hasAnyCert = lookup->count() > 0;
+ const bool partialCommit = !retStatus && params->configPartialReload &&
hasAnyCert;
+ const bool initialLoad = (configid == 0);
Review Comment:
Agreeing with this, and it is my one blocker on the PR. `count()` defaults
to the GENERIC context and returns `ssl_storage->count()` only. On BoringSSL
builds `insert()` routes EC certificates into `ec_storage`, so an EC-only
multicert config that loads cleanly still reads `count() == 0`, and the partial
reload discards the new lookup and keeps the old config. The feature silently
does nothing for exactly the build and cert type many of us run. Counting
across both storages, with the EC term guarded for BoringSSL, plus an EC-cert
reload scenario, would close it.
--
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]
