Make sni.yaml errors cause an unrecoverable TS crash. #8208
Conversation
iocore/net/SSLSNIConfig.cc
Outdated
| std::stringstream errMsg; | ||
| errMsg << zret; | ||
| Error("%s failed to load: %s", sni_filename, errMsg.str().c_str()); | ||
| Emergency("%s failed to load: %s", sni_filename, errMsg.str().c_str()); |
There was a problem hiding this comment.
Is this just the initial load(on startup) of the the sni.yaml?
There was a problem hiding this comment.
It looks like reloading the sni.yaml comes here for me...what I'm missing?
trafficserver/iocore/net/SSLSNIConfig.cc
Lines 196 to 201 in 11f987e
There was a problem hiding this comment.
@rob05c did you want it to die hard on a reload as well?
There was a problem hiding this comment.
Yeah, I guess I think not crashing on reload is fine. On startup, you're not breaking any running traffic. Reload is a harder decision: do we kill the app and break running traffic in the face of a potential security/access concern?
I guess not breaking running traffic is an acceptable choice here. @alficles ^
I tried to see what other configs like ip_allow.yaml do. It throws an exception and crashes ATS. But I'm not sure if that's intentional or desirable.
I also feel like a running config changing to be malformed is at least a little less likely. It seems highly likely to me that a user might create a file wrong and never notice the log error; it seems slightly less likely that a user would create a correct file, and later malform it.
| Emergency("%s failed to load: %s", sni_filename, errMsg.str().c_str()); | ||
| } else { | ||
| Error("%s failed to load: %s", sni_filename, errMsg.str().c_str()); | ||
| } |
There was a problem hiding this comment.
This logic may be common enough that we should add a new diag output call, perhaps InitEmergency(), that blocks startup if initializing, but simply logs an error otherwise.
| } | ||
| return 1; | ||
| } | ||
| Y_sni = std::move(Y_sni_tmp); |
There was a problem hiding this comment.
I added this logic to make sure the static data remained the coherent, old version if an invalid sni.yaml was reloaded.
|
[approve ci] |
|
[approve ci] |
(cherry picked from commit a0c5ac3)
|
Cherry-picked to v9.2.x |
* asf/9.2.x: (50 commits) Updated ChangeLog Reject Transfer-Encoding in pre-HTTP/1.1 requests (apache#8451) Better TLS Secrets Truncation. (apache#8489) ssl_secret debug printing: print only the first 50 bytes (apache#8483) Define TS_HTTP_VALUE_BROTLI and TS_HTTP_LEN_BROTLI (apache#8477) Fix case of brotli (apache#8476) TSSslSecretSet: Update SSL_CTX TLS Secrets (apache#8368) Adding doc/README.md (apache#8420) Doc: fix typos in Strategy documentation (apache#8408) Refactors and promotes the Txn Control mechanism with Get() and Set() (apache#8428) tests: Add shbang to python scripts with a main (apache#8430) Remove empty tests/unit_tests directoy+makefile (apache#8429) Adds new API: TSVConnSslSniGet (apache#8313) rate_limit: convert to using TSVConnSslSniGet (apache#8414) Update the Multiplexer Docs for Multplexed HTTPS Connections (apache#8440) bigobj: use automake to build test utilities (apache#8441) Make sni.yaml errors cause an unrecoverable TS crash at startup. (apache#8208) Fix timeout checks of NetHandler::manage_active_queue() (apache#8287) Fix Multiplexer POST/PUT Body Handling (apache#8439) Document proxy.config.memory.max_usage (apache#8450) ...
7 participants
No description provided.