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

build: introduce --openssl-is-fips flag #25412

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jan 9, 2019

This commit introduces a new configuration flag named
--openssl-is-fips which is intended to be used when linking against
an OpenSSL library that is FIPS compatible.

The motivation for this is that Red Hat Enterprise Linux 8 (RHEL8)
comes with OpenSSL 1.1.1 and includes FIPS support, and we would
like to be able to dynamically link against this version and also have
FIPS features enabled in node, like would be done when statically
linking and using the --openssl-fips flag.

The suggestion here is to introduce a new flag:

$ ./configure --help
...
--openssl-is-fips specifies that the shared OpenSSL library is FIPS
                  compatible

This flag could be used in combination with the --shared-openssl flag:

$ ./configure --shared-openssl --openssl-is-fips

This will enable FIPS support in node and the runtime flags will be
availalbe to enable FIPS (--enable-fips, --force-fips).

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@danbev danbev added the wip Issues and PRs that are still a work in progress. label Jan 9, 2019
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jan 9, 2019
@mhdawson
Copy link
Member

mhdawson commented Jan 9, 2019

Do you have any more details on the FIPs certification that RedHat got for OpenSSL 1.1.1 that you can share?

@sam-github
Copy link
Contributor

@danbev sorry to bike-shed on the config name, but I was a bit confused by why it had to exist until I read the configure code.

The problem is that we only specify FIPS build flags ATM when linking against a FIPS canister (--openssl-fips=canister.o).

Can we call the new flag --openssl-is-fips, and describe it as "specifies that the OpenSSL library is FIPS compatible"? This would sort it right next to --openssl-fips in the help output (I wish it had been called --openssl-fips-container=), and not in the shared section, where the flags all have a very nice predictable form and meaning ATM:

    --shared-cares      link to a shared cares DLL instead of static linking
    --shared-cares-includes=SHARED_LIBCARES_INCLUDES
    --shared-cares-libname=SHARED_LIBCARES_LIBNAME
    --shared-cares-libpath=SHARED_LIBCARES_LIBPATH
... etc for all the deps where shared is supported

@danbev
Copy link
Contributor Author

danbev commented Jan 9, 2019

Do you have any more details on the FIPs certification that RedHat got for OpenSSL 1.1.1 that you can share?

I'll try to find some information about this that I can share.

@sam-github I like the suggestion. I'll follow up on this on Friday (I'm not working tomorrow). Thanks

@danbev danbev changed the title build: introduce --shared-openssl-fips flag build: introduce --openssl-is-fips flag Jan 11, 2019
@danbev danbev removed the wip Issues and PRs that are still a work in progress. label Jan 11, 2019
@danbev
Copy link
Contributor Author

danbev commented Jan 11, 2019

@danbev
Copy link
Contributor Author

danbev commented Jan 14, 2019

Re-run of failing node-test-commit-windows-fanned
Re-run of failing node-test-commit-arm-fanned

@addaleax
Copy link
Member

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 14, 2019
@danbev
Copy link
Contributor Author

danbev commented Jan 15, 2019

Re-run of failing node-test-commit-arm

This commit introduces a new configuration flag named
--openssl-is-fips which is intended to be used when linking against
an OpenSSL library that is FIPS compatible.

The motivation for this is that Red Hat Enterprise Linux 8 (RHEL8)
comes with OpenSSL 1.1.1 and includes FIPS support, and we would
like to be able to dynamically link against this version and also have
FIPS features enabled in node, like would be done when statically
linking and using the --openssl-fips flag.

The suggestion here is to introduce a new flag:
$ ./configure --help
...
--openssl-is-fips specifies that the shared OpenSSL version is FIPS
                  compatible

This flag could be used in combination with the shared-openssl flag:
$ ./configure --shared-openssl ---openssl-is-fips

This will enable FIPS support in node and the runtime flags will be
availalbe to enable FIPS (--enable-fips, --force-fips).
Currently, while FIPS is not supported yet for this release there might
be an option to dynamically link against a FIPS compatible OpenSSL
version.

This commit fixes the compiler errors.
@danbev
Copy link
Contributor Author

danbev commented Jan 16, 2019

@danbev
Copy link
Contributor Author

danbev commented Jan 17, 2019

Re-run of failing node-test-commit-arm-fanned ✔️

@danbev
Copy link
Contributor Author

danbev commented Jan 17, 2019

Landed in 7e7266a, and 228a3f8.

@danbev danbev closed this Jan 17, 2019
@danbev danbev deleted the crypto_dynlink_fips branch January 17, 2019 04:32
danbev added a commit that referenced this pull request Jan 17, 2019
This commit introduces a new configuration flag named
--openssl-is-fips which is intended to be used when linking against
an OpenSSL library that is FIPS compatible.

The motivation for this is that Red Hat Enterprise Linux 8 (RHEL8)
comes with OpenSSL 1.1.1 and includes FIPS support, and we would
like to be able to dynamically link against this version and also have
FIPS features enabled in node, like would be done when statically
linking and using the --openssl-fips flag.

The suggestion here is to introduce a new flag:
$ ./configure --help
...
--openssl-is-fips specifies that the shared OpenSSL version is FIPS
                  compatible

This flag could be used in combination with the shared-openssl flag:
$ ./configure --shared-openssl ---openssl-is-fips

This will enable FIPS support in node and the runtime flags will be
availalbe to enable FIPS (--enable-fips, --force-fips).

PR-URL: #25412
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danbev added a commit that referenced this pull request Jan 17, 2019
Currently, while FIPS is not supported yet for this release there might
be an option to dynamically link against a FIPS compatible OpenSSL
version.

This commit fixes the compiler errors.

PR-URL: #25412
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jan 23, 2019
This commit introduces a new configuration flag named
--openssl-is-fips which is intended to be used when linking against
an OpenSSL library that is FIPS compatible.

The motivation for this is that Red Hat Enterprise Linux 8 (RHEL8)
comes with OpenSSL 1.1.1 and includes FIPS support, and we would
like to be able to dynamically link against this version and also have
FIPS features enabled in node, like would be done when statically
linking and using the --openssl-fips flag.

The suggestion here is to introduce a new flag:
$ ./configure --help
...
--openssl-is-fips specifies that the shared OpenSSL version is FIPS
                  compatible

This flag could be used in combination with the shared-openssl flag:
$ ./configure --shared-openssl ---openssl-is-fips

This will enable FIPS support in node and the runtime flags will be
availalbe to enable FIPS (--enable-fips, --force-fips).

PR-URL: #25412
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jan 23, 2019
Currently, while FIPS is not supported yet for this release there might
be an option to dynamically link against a FIPS compatible OpenSSL
version.

This commit fixes the compiler errors.

PR-URL: #25412
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants