| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Review requested:
|
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #64949 +/- ##
==========================================
- Coverage 90.29% 90.28% -0.01%
==========================================
Files 759 759
Lines 248295 248295
Branches 46861 46858 -3
==========================================
- Hits 224205 224184 -21
+ Misses 15517 15515 -2
- Partials 8573 8596 +23 see 26 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
|
I'm not certain hiding an error reading something that may (or may not, i haven't done that due dilligence) be a convention and openssl default is a good idea. Making the error clearer I'd have no issue with. |
Sorry, something went wrong.
CONF_MFLAGS_IGNORE_MISSING_FILE is already set above, so a missing default config is fine. But OpenSSL only honours that flag for ENOENT/ENOTDIR - every other errno becomes BIO_R_SYS_LIB and stays fatal. Absent /etc/ssl/openssl.cnf works; the same file one chmod away kills startup. That asymmetry looks accidental, not deliberate. The branch only fires when neither OPENSSL_CONF nor --openssl-config was given, i.e. the compiled-in OPENSSLDIR path. Explicit configs and their .includes stay fatal, so the FIPS reasoning above is untouched. Does that address your concern? If you'd still rather not have it silent, I can print the error to stderr as a warning and continue. |
Sorry, something went wrong.
|
Setting OPENSSL_CONF= (empty) already skips config loading without touching the filesystem, so docker run -e OPENSSL_CONF= … works today. And I don't think the FIPS argument holds — the default openssl.cnf is where .include fipsmodule.cnf usually lives, which is what #38732 made fatal. With this, a FIPS host whose config becomes unreadable would just start without FIPS. I'd rather keep it fatal and mention OPENSSL_CONF in the error message, plus document that an empty value skips loading. |
Sorry, something went wrong.
|
You're right on both counts - OPENSSL_CONF= short-circuits before the file is opened, and the FIPS .include does live in the default config, so my argument missed the case that matters. Reworked: the error stays fatal, and now says to point OPENSSL_CONF or --openssl-config=file at another file, or set OPENSSL_CONF= (empty) to start without one. Documented that last bit too. PTAL. |
Sorry, something went wrong.
|
I'd drop the src/node.cc change entirely and land this as docs only. The OpenSSL error already names the file and the reason, and the message change fires for every config error, including parse failures where "start without one" is the wrong advice if the file sets up FIPS. If the variable is set to an empty value, Node.js starts without loading any
OpenSSL configuration file. This is a way past a default configuration file
that exists but cannot be read, for example when `/etc/ssl` is not accessible
to the user Node.js runs as, which is otherwise fatal at startup. No
configuration is applied in that case, including any [FIPS mode][] setup the
file would have performed.
If the [`--openssl-config`][] command-line option is used, the environment
variable is ignored, and an empty value has no effect.
|
Sorry, something went wrong.
A default OpenSSL configuration file that exists but cannot be read is fatal at startup: CONF_MFLAGS_IGNORE_MISSING_FILE only covers ENOENT and ENOTDIR, so a container that mounts /etc/ssl inaccessible to the user Node.js runs as cannot start at all. OpenSSL skips config loading entirely when OPENSSL_CONF is set to an empty value, which gets past this, but that was undocumented. Say so, including that no configuration is applied, FIPS setup included. Refs: nodejs#62230 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Orgad Shaneh <orgad.shaneh@audiocodes.com>
|
Done. Dropped the src/node.cc change and took your wording verbatim. I kept the test. It asserts only existing behaviour - unreadable default config is fatal, OPENSSL_CONF= starts - so it pins the escape hatch the docs now promise. Let me know if you'd rather this be strictly docs. |
Sorry, something went wrong.
Sorry, something went wrong.
A default OpenSSL configuration file that exists but cannot be read is fatal at startup: CONF_MFLAGS_IGNORE_MISSING_FILE only covers ENOENT and ENOTDIR, so a container that mounts /etc/ssl inaccessible to the user Node.js runs as cannot start at all. OpenSSL skips config loading entirely when OPENSSL_CONF is set to an empty value, which gets past this, but that was undocumented. Say so, including that no configuration is applied, FIPS setup included. Refs: #62230 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Orgad Shaneh <orgad.shaneh@audiocodes.com> PR-URL: #64949 Fixes: #62230 Reviewed-By: Filip Skokan <panva.ip@gmail.com>
A default OpenSSL configuration file that exists but cannot be read is fatal at startup: CONF_MFLAGS_IGNORE_MISSING_FILE only covers ENOENT and ENOTDIR, so a container that mounts /etc/ssl inaccessible to the user Node.js runs as cannot start at all. OpenSSL skips config loading entirely when OPENSSL_CONF is set to an empty value, which gets past this, but that was undocumented. Say so, including that no configuration is applied, FIPS setup included. Refs: #62230 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Orgad Shaneh <orgad.shaneh@audiocodes.com> PR-URL: #64949 Fixes: #62230 Reviewed-By: Filip Skokan <panva.ip@gmail.com>
| Back | FazBrowse Home | New Git URL |
OpenSSL is initialized with CONF_MFLAGS_IGNORE_MISSING_FILE, so a missing configuration file does not prevent Node.js from starting.
That flag only covers ENOENT and ENOTDIR though, so a file that exists but cannot be opened is still fatal. Running in a container where /etc/ssl is not accessible to the current user aborts startup with an error that gives no way out:
There is a way out: OpenSSL skips config loading entirely when OPENSSL_CONF is set to an empty value. Say so in the error message, along with the options that select a different file, and document the empty value.
Closes: #62230