crypto: support FIPS mode of OpenSSL by indutny · Pull Request #1890 · nodejs/node

indutny · 2015-06-03T21:21:47Z

Support building and running with FIPS-compliant OpenSSL. The process is
following:

Download and verify openssl-fips-x.x.x.tar.gz from
https://www.openssl.org/source/
Extract source to openssl-fips folder
cd openssl-fips && ./config fipscanisterbuild --prefix=pwd/out
make -j && make install
Get into io.js checkout folder
./configure --openssl-fips=/path/to/openssl-fips/out
Build io.js with make -j

Fix: nodejs/node-v0.x-archive#25463

indutny · 2015-06-03T21:21:58Z

R= @nodejs/crypto

indutny · 2015-06-03T21:24:34Z

Aye, made a mistake in instructions. Force pushed!

bnoordhuis · 2015-06-03T21:38:24Z

configure

Using make_global_settings like that is a horrible and not very portable hack.

Ok, what would be your suggestion to override the linker command?

rvagg · 2015-06-03T23:32:27Z

@indutny I reckon those instructions should go in to the README rather than being lost in this issue, unless you can think of a better place for it?

indutny · 2015-06-04T00:00:39Z

Just figured out I could do it better and remove the need to patch fipsld. Will push an update soon.

indutny · 2015-06-04T00:00:42Z

@rvagg agree!

shigeki · 2015-06-04T02:43:44Z

The sources of openssl-fips-2.0.9 are about 8.4M bytes.
If it is really required, is it better to include all sources into the repo and build them within gyp?

thefourtheye · 2015-06-04T02:50:50Z

configure

It is not very pythonic. Recommended way is if not make_global_settings:

jbergstroem · 2015-06-04T06:18:49Z

I'm not very familiar to fips, but if it ends up being built as a library we should support building against a shared .so as well.

indutny · 2015-06-04T10:43:24Z

@shigeki sorry, but there are some complications with this FIPS thing. I'm not sure if it can be included in our source tree. Unless someone knows for sure.

shigeki · 2015-06-04T14:15:05Z

@indutny No, problem, you can take your choice.

indutny · 2015-06-04T20:36:08Z

Everything addressed, removed the need to patch OpenSSL FIPS checkout. PTAL (cc @bnoordhuis @rvagg @shigeki )

Support building and running with FIPS-compliant OpenSSL. The process is following: 1. Download and verify `openssl-fips-x.x.x.tar.gz` from https://www.openssl.org/source/ 2. Extract source to `openssl-fips` folder 3. `cd openssl-fips && ./config fipscanisterbuild --prefix=`pwd`/out` 4. `make -j && make install` 5. Get into io.js checkout folder 6. `./configure --openssl-fips=/path/to/openssl-fips/out` 7. Build io.js with `make -j` Fix: nodejs/node-v0.x-archive#25463

thefourtheye · 2015-06-04T21:08:18Z

src/node_crypto.cc

Why don't we directly do ERR_error_string(ERR_get_error(), NULL)?

It won't fit into 80 columns. Anyway, merged two previous lines into one. Thanks!

Oh, okay. I would have done

fprintf(stderr, "openssl fips failed: %s\n", ERR_error_string(ERR_get_error(), NULL));

Can we use that?

Yes, but what is the point? It is harder to read, and takes the same two lines.

True, I was thinking along the lines of saving some memory. But, as Donald Knuth said, "Premature optimization is the root of all evil". So, this should be fine I guess :-)

It won't save anything, in both JS and C/C++. Compiler are smart enough to figure it out ;)

@indutny Oh, I didn't know that even JS Engine could do that. Thanks :-)

rvagg · 2015-06-05T01:21:22Z

deps/openssl/doc/FIPS.md

could we have a link to this from the README? Perhaps at the bottom of the build section:

See also: [Building io.js with FIPS-compliant OpenSSL](./deps/openssl/doc/FIPS.md)

shigeki · 2015-06-05T09:02:41Z

There seems a fix needed to build on my Ubuntu because /bin/sh is not bash but dash.

diff --git a/deps/openssl/fips/fipsld b/deps/openssl/fips/fipsld
index 513982a..4c345cd 100755
--- a/deps/openssl/fips/fipsld
+++ b/deps/openssl/fips/fipsld
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/usr/bin/env bash

 # NOTE: Just a wrapper around normal fipsld
 FIPSLD=$1

And without CXX env, build was faild , so I set it manually.

I can't say whether this is really FIPS compliance or not because I've not read all of contents in OpenSSL handbook. I wonder if patched openssl can be allowed. Tests also show a lot of fips errors for their forbidden rules.

indutny · 2015-06-05T11:22:31Z

@shigeki I think this is quite unusual. Is there any reason to replace /bin/sh with different terminal, instead of using chsh?

indutny · 2015-06-05T11:22:40Z

@shigeki thanks for checking it out!

indutny · 2015-06-05T11:24:40Z

I'll read the FIPS guide to be sure, but here is an excerpt that I think should answer the question of compliance:

OpenSSL itself is not validated,and never will be. Instead a carefully defined software component
called the OpenSSL FIPS Object Module has been created.

indutny · 2015-06-05T17:15:40Z

I think it should be pretty much correct to use FIPS module with patched OpenSSL:

 The new
product may reference the FIPS 140-1 or FIPS 140-2 validated cryptographic module so
long as the new product does not alter the original validated cryptographic module.

and

 A
product which uses an embedded validated cryptographic module cannot claim itself to be
validated; only that it utilizes an embedded validated cryptographic module. There is no
assurance that a product is correctly utilizing an embedded validated cryptographic module
- this is outside the scope of the FIPS 140-1 or FIPS 140-2 validation.

IANAL, but OpenSSL was never FIPS compliant, and regardless of any patches it won't became one :)

shigeki · 2015-06-06T04:49:14Z

In Ubuntu, /bin/sh is linked to dash by default. https://wiki.ubuntu.com/DashAsBinSh

I understand OpenSSL was never FIPS compliant. It needs so much requirements. Thanks for clarification.

indutny · 2015-06-06T22:44:49Z

Gosh. Please forgive my ignorance, @shigeki . I was completely unaware of dash.

Maybe we can fix the script to make it POSIX compliant, instead of forcing the bash? I'll look into it tomorrow.

indutny · 2015-06-09T21:35:04Z

@shigeki please take another look, it still requires CXX, but should work with dash.

shigeki · 2015-06-10T08:17:17Z

deps/openssl/doc/FIPS.md

I think it is better to add some descriptions to the doc as

supported platform: Windows is not supported. Only build on MacOS and Linux are confirmed.

CXX env is needed.

confirmation of result of fips build as

$ ./iojs -e "console.log(process.versions.openssl)" 1.0.2a-fips

shigeki · 2015-06-10T08:18:05Z

Builds on my Ubuntu and MacOS are fine. Add some comments on the doc. LGTM.

indutny · 2015-06-10T10:46:47Z

@rvagg @shigeki all fixed, PTAL

rvagg · 2015-06-10T13:05:11Z

docs lgtm

thefourtheye · 2015-06-10T13:39:00Z

README.md

I don't have OS X, so I couldn't confirm this. Is ./Configure with a capital C, correct?

Yeah, I have OS X. So I can confirm it ;)

Cool, then :-) I just thought it was a typo.

jbergstroem · 2015-06-10T23:38:19Z

Since no one else has mentioned shared library support I'll just have a look at what can be done after this lands.

indutny · 2015-06-10T23:39:50Z

You mean shared OpenSSL? This will certainly work, except that it won't have patches that we have applied to it. Namely:

ae8831f

jbergstroem · 2015-06-10T23:42:38Z

@indutny great; I'll have a look at it later then (especially for the FIPS scenario).

indutny · 2015-06-10T23:48:07Z

Ok, cool. I tested it on OS X and Ubuntu, assuming that it is working and landing. Thanks everyone for the feedback!

indutny · 2015-06-10T23:50:57Z

Landed in 0f68377, thank you everyone!

Support building and running with FIPS-compliant OpenSSL. The process is following: 1. Download and verify `openssl-fips-x.x.x.tar.gz` from https://www.openssl.org/source/ 2. Extract source to `openssl-fips` folder 3. ``cd openssl-fips && ./config fipscanisterbuild --prefix=`pwd`/out`` (NOTE: On OS X, you may want to run ``./Configure darwin64-x86_64-cc --prefix=`pwd`/out`` if you are going to build x64-mode io.js) 4. `make -j && make install` 5. Get into io.js checkout folder 6. `./configure --openssl-fips=/path/to/openssl-fips/out` 7. Build io.js with `make -j` 8. Verify with `node -p "process.versions.openssl"` (`1.0.2a-fips`) Fix: nodejs/node-v0.x-archive#25463 PR-URL: #1890 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>

@rlidwka

Notable Changes: * libuv: Upgraded to 1.6.0 and 1.6.1, see full ChangeLog for details. (Saúl Ibarra Corretgé) #1905 #1889. Highlights include: - Fix TTY becoming blocked on OS X - Fix UDP send callbacks to not to be synchronous - Add uv_os_homedir() (exposed as os.homedir(), see below) * npm: See full release notes for details. (Kat Marchán) #1899. Highlight: - Use GIT_SSH_COMMAND (available as of Git 2.3) * openssl: - Upgrade to 1.0.2b and 1.0.2c, introduces DHE man-in-the-middle protection (Logjam) and fixes malformed ECParameters causing infinite loop (CVE-2015-1788). See the security advisory for full details. (Shigeki Ohtsu) #1950 #1958 - Support FIPS mode of OpenSSL, see README for instructions. (Fedor Indutny) #1890 * os: Add os.homedir() method. (Colin Ihrig) #1791 * smalloc: Deprecate whole module. (Vladimir Kurchatkin) #1822 * Add new collaborators: - Alex Kocharin (@rlidwka) - Christopher Monsanto (@monsanto) - Ali Ijaz Sheikh (@ofrobots) - Oleg Elifantiev (@Olegas) - Domenic Denicola (@domenic) - Rich Trott (@Trott)

indutny force-pushed the feature/fips-mode branch from c4b3183 to c133579 Compare June 3, 2015 21:24

indutny mentioned this pull request Jun 3, 2015

Enabling FIPS-compliant encryption nodejs/node-v0.x-archive#25463

Closed

mscdex added the crypto Issues and PRs related to the crypto subsystem. label Jun 3, 2015

bnoordhuis reviewed Jun 3, 2015
View reviewed changes

thefourtheye reviewed Jun 4, 2015
View reviewed changes

indutny force-pushed the feature/fips-mode branch 4 times, most recently from 9338069 to c9802dc Compare June 4, 2015 20:35

indutny force-pushed the feature/fips-mode branch from c9802dc to 282d4c9 Compare June 4, 2015 20:36

thefourtheye reviewed Jun 4, 2015
View reviewed changes

fix

c836aba

rvagg reviewed Jun 5, 2015
View reviewed changes

another fix

c425e47

shigeki reviewed Jun 10, 2015
View reviewed changes

indutny added 3 commits June 10, 2015 12:33

fixes

c5932ad

fix

d0bb37e

fix

8cfe4ab

thefourtheye reviewed Jun 10, 2015
View reviewed changes

fix

566a6f7

indutny closed this Jun 10, 2015

indutny deleted the feature/fips-mode branch June 10, 2015 23:51

rvagg mentioned this pull request Jun 11, 2015

Release proposal: 2.3.0 #1939

Closed

mhdawson mentioned this pull request Jul 24, 2015

FIPS build instructions #2242

Closed

stefanmb mentioned this pull request Nov 11, 2015

crypto: Resolve FIPS test failures and API issues #3760

Closed

Uh oh!

Conversation

indutny commented Jun 3, 2015

Uh oh!

indutny commented Jun 3, 2015

Uh oh!

indutny commented Jun 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rvagg commented Jun 3, 2015

Uh oh!

indutny commented Jun 4, 2015

Uh oh!

indutny commented Jun 4, 2015

Uh oh!

shigeki commented Jun 4, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbergstroem commented Jun 4, 2015

Uh oh!

indutny commented Jun 4, 2015

Uh oh!

shigeki commented Jun 4, 2015

Uh oh!

indutny commented Jun 4, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shigeki commented Jun 5, 2015

Uh oh!

indutny commented Jun 5, 2015

Uh oh!

indutny commented Jun 5, 2015

Uh oh!

indutny commented Jun 5, 2015

Uh oh!

indutny commented Jun 5, 2015

Uh oh!

shigeki commented Jun 6, 2015

Uh oh!

indutny commented Jun 6, 2015

Uh oh!

indutny commented Jun 9, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shigeki commented Jun 10, 2015

Uh oh!

indutny commented Jun 10, 2015

Uh oh!

rvagg commented Jun 10, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbergstroem commented Jun 10, 2015

Uh oh!