doc: document that an empty OPENSSL_CONF skips config loading - #64949
Conversation
|
Review requested:
|
dccd16f to
1480dd6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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 🚀 New features to boost your workflow:
|
1480dd6 to
e9e2c31
Compare
|
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. |
The branch only fires when neither 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. |
|
Setting And I don't think the FIPS argument holds — the default I'd rather keep it fatal and mention |
e9e2c31 to
d76097d
Compare
d76097d to
0d8303f
Compare
|
You're right on both counts - Reworked: the error stays fatal, and now says to point |
|
I'd drop the |
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>
0d8303f to
096ad5b
Compare
|
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, |
This comment was marked as outdated.
This comment was marked as outdated.
|
Landed in a0c366c |
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>
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>
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>
Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com>
Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com>
Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com>
Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: remove upstreamed patch * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: remove upstreamed patch * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
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