Copilot commented on code in PR #13373:
URL: https://github.com/apache/trafficserver/pull/13373#discussion_r3574578006
##########
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:
`hasAnyCert` uses `lookup->count()` with the default context type, which
only counts `ssl_storage` entries. In BoringSSL builds, EC certificates are
stored in `ec_storage` and will not be counted, so an EC-only multicert config
would incorrectly be treated as `count()==0` and prevent partial commits even
though certs loaded successfully. Consider checking both RSA and EC counts (or
otherwise counting across storages).
--
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]
