crypto: fix native module compilation with FIPS #4023
Conversation
mscdex
added
crypto
|
/cc @nodejs/crypto |
|
Mmmm... @stefanmb why is FIPSLD needed for addons? They are shared modules after all. |
|
@indutny I don't think it is needed, but right now due to the way node-gyp works the addons will be compiled with the exact same settings as the Node executable you're running npm with. Meaning they get the special LD env variable which, as far as I can tell, is not needed and won't even work if you're missing the OpenSSL source. The purpose of my commit was to remove fipsld from the addons entirely. |
|
Aaaah, I get it now. Thought you were proposing to use fipsld in addons. Makes sense now! |
|
LGTM, but I haven't seen if it builds. Could somebody from @nodejs/crypto please confirm it? |
|
@stefanmb thank you for an awesome work! |
|
@indutny Yeah, it's a bit confusing. What I'm proposing may not be the best way to do it, but it's one way I found that works. I'm open to any and all suggestions. :) |
|
Issue was reported against 4.x so should consider for LTS |
|
LGTM to me as well provided these CI runs are ok: Run in regular mode: https://ci.nodejs.org/job/node-test-pull-request/852/ The second is the candidate subjob that I'm hoping to add to the regular regression runs. It builds/tests in FIPS capable mode. What it probably does not test is whether native modules will now build ok as it passed before, probably because I don't delete the src/build for the fipscanister in order to speed up the build time and or because native module compilation is not part of the standard tests. |
|
CI runs look clean to me (arm has one platform to complete but everything else is good). |
|
Is it always necessary to create |
Prevent OpenSSL's fipsld from being used to link native modules because this requires the original OpenSSL source to be available after Node's installation.
stefanmb
force-pushed
the
fips-module-compile-fix
branch
from
9ad8943 to
1fc485b
Compare
|
@shigeki Okay, I modified the Makefile and configure with your suggestions. Thanks! |
|
@shigeki Have you had a chance to review the updated version? Are there any other concerns? Thank you! |
|
@stefanmb Sorry, I missed your updates. LGTM. CI are running on https://ci.nodejs.org/job/node-test-commit-linux-fips/59/ and https://ci.nodejs.org/job/node-test-pull-request/906/ . |
Prevent OpenSSL's fipsld from being used to link native modules because this requires the original OpenSSL source to be available after Node's installation. Fixes: #3815 PR-URL: #4023 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
|
The results of CI are fine except on CentOS and some Windows. The faults are nothing to do with this. Landed in 181816e. Thanks. |
Prevent OpenSSL's fipsld from being used to link native modules because this requires the original OpenSSL source to be available after Node's installation. Fixes: #3815 PR-URL: #4023 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Prevent OpenSSL's fipsld from being used to link native modules because this requires the original OpenSSL source to be available after Node's installation. Fixes: #3815 PR-URL: #4023 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Prevent OpenSSL's fipsld from being used to link native modules because this requires the original OpenSSL source to be available after Node's installation. Fixes: #3815 PR-URL: #4023 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Prevent OpenSSL's fipsld from being used to link native modules because this requires the original OpenSSL source to be available after Node's installation. Fixes: nodejs#3815 PR-URL: nodejs#4023 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
When using a FIPS build of Node.js we cannot build and install native npm modules unless the original OpenSSL source is available at the same location where it was present during the Node executable's build. This issue occurs because OpenSSL's FIPS capsule requires linking with a special 'fipsld' utility. When configuring with --openssl-fips a global LD override is used that is then propagated to config.gypi. This file (config.gypi) is encoded into the Node executable itself (see node_natives.h) and is accessible via process.config. Node-gyp then appends all of process.config to each module's configuration (see configure.js).
There are multiple ways to fix this problem, I've opted to simply separate out the special FIPS link flag so it's only used during the Node.js executable's compilation, and not propagated to modules. While it is not strictly 'correct' to exclude the LD flag from process.config, doing so avoids having node-gyp be aware of special FIPS configuration exceptions.
Resolves #3815.