Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test/parallel/test-crypto-fips.js fails if node is built with --shared-openssl and --openssl-is-fips #42827

Closed
batrla opened this issue Apr 22, 2022 · 1 comment

Comments

@batrla
Copy link
Contributor

batrla commented Apr 22, 2022

Version

v18.0.0

Platform

Solaris 11.4

Subsystem

crypto FIPS

What steps will reproduce the bug?

  1. configure 'node' with shared OpenSSL 3.0 using following flags: ./configure --shared-openssl --openssl-is-fips ...
  2. run make
  3. run tests, test-crypto-fips.js fails

How often does it reproduce? Is there a required condition?

always reproduces

What is the expected behavior?

test/parallel/test-crypto-fips.js test PASSes

What do you see instead?

test/parallel/test-crypto-fips.js test FAILs

Additional information

The root cause is that if shared OpenSSL 3 is linked dynamically to node, then ./configure.py does not define OPENSSL_FIPS macro, which normally tells a test to expect FIPS is working. Since OPENSSL_FIPS is not defined, test expects FIPS is not working and is surprised when it actually works.

The problem starts here:

node/configure.py

Line 1538 in eeb27c2

if options.openssl_is_fips and not options.shared_openssl:

  if options.openssl_is_fips and not options.shared_openssl:

# even if --openssl-is-fips is passed, passing --shared-openssl causes the condition is not satisfied

    o['defines'] += ['OPENSSL_FIPS']

# as result, no -DOPENSSL_FIPS is passed

    variables['node_fipsinstall'] = b(True)

# node_fipsinstall is not needed, since OpenSSL 3 uses providers, there's no need to recompile OpenSSL library.

Because -DOPENSSL_FIPS is not defined in compilation process, following code in TestFipsCrypto() reduces to return false:

void TestFipsCrypto(const v8::FunctionCallbackInfo<v8::Value>& args) {

Finally test/parallel/test-crypto-fips.js calls TestFipsCrypto() and learns that FIPS should not be working. Test starts node in FIPS mode and is surprised that it works. Test reports failure.

Following patch makes the pass (all other tests pass too):

--- node-18.0.0.orig/configure.py       2022-04-18 04:29:22.000000000 +0000
+++ node-18.0.0/configure.py    2022-04-22 19:10:52.510174797 +0000
@@ -1535,8 +1535,10 @@
   if options.openssl_no_asm and options.shared_openssl:
     error('--openssl-no-asm is incompatible with --shared-openssl')
 
-  if options.openssl_is_fips and not options.shared_openssl:
+  if options.openssl_is_fips:
     o['defines'] += ['OPENSSL_FIPS']
+
+  if options.openssl_is_fips and not options.shared_openssl:
     variables['node_fipsinstall'] = b(True)
 
   if options.shared_openssl:
@bnoordhuis
Copy link
Member

Your analysis looks correct to me. Pull request welcome.

batrla pushed a commit to batrla/node that referenced this issue May 2, 2022
batrla pushed a commit to batrla/node that referenced this issue May 2, 2022
RafaelGSS pushed a commit that referenced this issue May 10, 2022
Fixes: #42827

PR-URL: #42947
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants