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: stronger crypto in test fixtures #3759

Closed

Conversation

stefanmb
Copy link
Contributor

A number of pre-generated text fixtures are used in the crypto/tls tests, for example certificate files. The test fixtures in the test\fixtures\keys\ folder are generated by a Makefile, but those in the parent folder (test\fixtures) are not.

In many cases these test fixtures were generated using default (or deprecated) options with an old version of OpenSSL, which means that they use weak crypto (e.g. RC4). I have regenerated the test fixtures to be compatible with FIPS mode. The following are notes on the manual steps used to regenerate the fixtures, where applicable:

test/fixtures/test_cert.pfx

Problem:

openssl pkcs12 -info -in test/fixtures/test_cert.pfx
Enter Import Password:
MAC Iteration 2048
MAC verified OK
PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048

Solution:

Extract and decrypt the existing private key:
openssl pkcs12 -in test_cert.pfx -nocerts -out extracted_key.pem -nodes
Extract the certificate:
openssl pkcs12 -in test_cert.pfx -nokeys -out extracted_cert.pem
Package as PFX with 3DES encryption for the certificate (instead of 40bit RC2):
openssl pkcs12 -descert -export -out test_cert.pfx -inkey extracted_key.pem -in extracted_cert.pem
(n.b. password is 'sample', this is hardcoded in test-crypto.js)

test/fixtures/test_dsa(priv|pub)key.pem

Problem:

Bad length in test_dsa_privkey.pem:
0x0000000000b821c1 in fips_check_dsa_prng (dsa=0x1f28570, L=2048, N=160) at dsa_gen.c:426

It’s unclear how the initial test_dsa_privkey.pem was created, because it has a key of 2048 bits length generated using a prime number of 160 bits. This is not the default behaviour of openssl, which will use a 256 bit prime for keys of 2048 bits or more. See: https://www.openssl.org/docs/manmaster/crypto/DSA_generate_parameters_ex.html

Solution:

We regenerate the key as such:
openssl dsaparam 2048 -out params.pem
openssl gendsa -out test_dsa_privkey.pem params.pem
openssl dsa -in test_dsa_privkey.pem -pubout -out test_dsa_pubkey.pem

test/fixtures/test_rsa_privkey_encrypted.pem

Problem:

Path: parallel/test-crypto-rsa-dsa
crypto.js:324
return method(toBuf(key), buffer, padding, passphrase);
^
Error: error:060A80A3:digital envelope routines:FIPS_DIGESTINIT:disabled for fips
at Error (native)
at Object.privateDecrypt (crypto.js:324:12)
atnode/test/parallel/test-crypto-rsa-dsa.js:39:44

Solution:

Convert the private key to PKCS#8 using a PKCS#12 compatible algorithm (3DES).
openssl rsa -in test_rsa_privkey_encrypted.pem -out test_rsa_privkey_unencrypted.pem
openssl pkcs8 -in ./test_rsa_privkey_unencrypted.pem -topk8 -out ./test_rsa_privkey_encrypted.pem -v1 PBE-SHA1-3DES
N.B. password = ‘password’

test/fixtures/test_dsa_privkey_encrypted.pem

Problem:

Error: error:060A80A3:digital envelope routines:FIPS_DIGESTINIT:disabled for fips
at Error (native)
at Sign.sign (crypto.js:279:26)
at/node/test/parallel/test-crypto-rsa-dsa.js:247:22

Solution:

openssl pkcs8 -v1 PBE-SHA1-3DES -in test_dsa_privkey.pem -topk8 -out test_dsa_privkey_encrypted.pemtest_dsa_privkey_encrypted.pem
N.B. password = ‘password’

test/fixtures/pass-key.pem

Problem:

Path: parallel/test-tls-passphrase
_tls_common.js:85
c.context.setKey(options.key, options.passphrase);
^
Error: error:060A80A3:digital envelope routines:FIPS_DIGESTINIT:disabled for fips
at Error (native)
at Object.createSecureContext (_tls_common.js:85:19)
at new Server (_tls_wrap.js:718:25)
at Object.Server (_tls_wrap.js:709:41)
at Object. (node/test/parallel/test-tls-passphrase.js:17:18)
at Module._compile (module.js:434:26)
at Object.Module._extensions..js (module.js:452:10)
at Module.load (module.js:355:32)
at Function.Module._load (module.js:310:12)
at Function.Module.runMain (module.js:475:10)
Command: out/Debug/node
node.js/fips/node/test/parallel/test-tls-passphrase.js

Solution:

Resolve by converting to PCKS8.
openssl rsa -in pass-key.pem -out pass-key-unencrypted.pem
openssl pkcs8 -in pass-key-unencrypted.pem -topk8 -out pass-key.pem -v1 PBE-SHA1-3DES
N.B. password = ‘passphrase’

test/fixtures/keys/ca2*

Problem:

openssl crl -text -in ca2-crl.pem
Certificate Revocation List (CRL):
Version 1 (0x0)
Signature Algorithm: md5WithRSAEncryption
Issuer: /C=US/ST=CA/L=SF/O=Joyent/OU=Node.js/CN=ca2/emailAddress=ry@tinyclouds.org
Last Update: Aug 1 11:19:01 2013 GMT
Next Update: Apr 26 11:19:01 2016 GMT
Revoked Certificates:
Serial Number: EEBE2CE5211A12F7
Revocation Date: Aug 1 11:19:01 2013 GMT
Signature Algorithm: md5WithRSAEncryption

Solution:

Use sha512 message digest instead of md5.

@mscdex mscdex added tls Issues and PRs related to the tls subsystem. test Issues and PRs related to the tests. labels Nov 11, 2015
@@ -8,7 +8,7 @@ database = ca2-database.txt
name_opt = CA_default
cert_opt = CA_default
default_crl_days = 999
default_md = md5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move it one step further and use sha512.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sha256 or sha512 is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indutny @shigeki Fixed, thanks.

@jasnell
Copy link
Member

jasnell commented Nov 12, 2015

Love the detailed PR description, btw. Very nice.

@jasnell
Copy link
Member

jasnell commented Nov 12, 2015

LGTM

@shigeki
Copy link
Contributor

shigeki commented Nov 12, 2015

I found a typo in a commit message as RC40

@stefanmb stefanmb force-pushed the fips-cs8-strong-fixture-crypto branch from 2af2aa9 to 972f7db Compare November 12, 2015 21:51
@stefanmb
Copy link
Contributor Author

@shigeki Fixed typo. Thanks! Was thinking RC4 40 bits :)

Several test fixtures use use weak crypto (e.g. RC4 or MD5).
Rgenerated the test fixtures to be compatible with FIPS mode.
@stefanmb stefanmb force-pushed the fips-cs8-strong-fixture-crypto branch from 972f7db to 852becb Compare November 12, 2015 22:14
@stefanmb stefanmb changed the title test: Stronger crypto in test fixtures test: stronger crypto in test fixtures Nov 12, 2015
@indutny
Copy link
Member

indutny commented Nov 12, 2015

LGTM

@mhdawson
Copy link
Member

lgtm. Would just like lgtm from shigeki and then I'll go ahead and land unless somebody beats me to it.

@shigeki
Copy link
Contributor

shigeki commented Nov 13, 2015

LGTM

jasnell pushed a commit that referenced this pull request Nov 13, 2015
Several test fixtures use use weak crypto (e.g. RC4 or MD5).
Rgenerated the test fixtures to be compatible with FIPS mode.

PR-URL: #3759
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>
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Landed in 76f40f7

@jasnell jasnell closed this Nov 13, 2015
Fishrock123 pushed a commit that referenced this pull request Nov 17, 2015
Several test fixtures use use weak crypto (e.g. RC4 or MD5).
Rgenerated the test fixtures to be compatible with FIPS mode.

PR-URL: #3759
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>
MylesBorins pushed a commit that referenced this pull request Nov 30, 2015
Several test fixtures use use weak crypto (e.g. RC4 or MD5).
Rgenerated the test fixtures to be compatible with FIPS mode.

PR-URL: #3759
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>
rvagg pushed a commit that referenced this pull request Dec 4, 2015
Several test fixtures use use weak crypto (e.g. RC4 or MD5).
Rgenerated the test fixtures to be compatible with FIPS mode.

PR-URL: #3759
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>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
Several test fixtures use use weak crypto (e.g. RC4 or MD5).
Rgenerated the test fixtures to be compatible with FIPS mode.

PR-URL: #3759
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>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
Several test fixtures use use weak crypto (e.g. RC4 or MD5).
Rgenerated the test fixtures to be compatible with FIPS mode.

PR-URL: #3759
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants