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: refactor and fix test-crypto #9807

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 44 additions & 45 deletions test/parallel/test-crypto.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,33 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var util = require('util');
const common = require('../common');

if (!common.hasCrypto) {
common.skip('missing crypto');
return;
}
var crypto = require('crypto');

crypto.DEFAULT_ENCODING = 'buffer';
const assert = require('assert');
const crypto = require('crypto');
const fs = require('fs');
const tls = require('tls');

var fs = require('fs');
crypto.DEFAULT_ENCODING = 'buffer';

// Test Certificates
var caPem = fs.readFileSync(common.fixturesDir + '/test_ca.pem', 'ascii');
var certPem = fs.readFileSync(common.fixturesDir + '/test_cert.pem', 'ascii');
var certPfx = fs.readFileSync(common.fixturesDir + '/test_cert.pfx');
var keyPem = fs.readFileSync(common.fixturesDir + '/test_key.pem', 'ascii');
var tls = require('tls');
const caPem = fs.readFileSync(common.fixturesDir + '/test_ca.pem', 'ascii');
const certPem = fs.readFileSync(common.fixturesDir + '/test_cert.pem', 'ascii');
const certPfx = fs.readFileSync(common.fixturesDir + '/test_cert.pfx');
const keyPem = fs.readFileSync(common.fixturesDir + '/test_key.pem', 'ascii');

// 'this' safety
// https://github.com/joyent/node/issues/6690
assert.throws(function() {
var options = {key: keyPem, cert: certPem, ca: caPem};
var credentials = crypto.createCredentials(options);
var context = credentials.context;
var notcontext = { setOptions: context.setOptions, setKey: context.setKey };
crypto.createCredentials({ secureOptions: 1 }, notcontext);
}, TypeError);
const options = {key: keyPem, cert: certPem, ca: caPem};
const credentials = tls.createSecureContext(options);
const context = credentials.context;
const notcontext = { setOptions: context.setOptions, setKey: context.setKey };
tls.createSecureContext({ secureOptions: 1 }, notcontext);
}, /^TypeError: Illegal invocation$/);

// PFX tests
assert.doesNotThrow(function() {
Expand All @@ -37,55 +36,55 @@ assert.doesNotThrow(function() {

assert.throws(function() {
tls.createSecureContext({pfx: certPfx});
}, 'mac verify failure');
}, /^Error: mac verify failure$/);

assert.throws(function() {
tls.createSecureContext({pfx: certPfx, passphrase: 'test'});
}, 'mac verify failure');
}, /^Error: mac verify failure$/);

assert.throws(function() {
tls.createSecureContext({pfx: 'sample', passphrase: 'test'});
}, 'not enough data');
}, /^Error: not enough data$/);


// update() should only take buffers / strings
assert.throws(function() {
crypto.createHash('sha1').update({foo: 'bar'});
}, /buffer/);
}, /^TypeError: Data must be a string or a buffer$/);


function assertSorted(list) {
// Array#sort() modifies the list in place so make a copy.
var sorted = util._extend([], list).sort();
const sorted = list.slice().sort();
assert.deepStrictEqual(list, sorted);
}

// Assume that we have at least AES-128-CBC.
assert.notEqual(0, crypto.getCiphers().length);
assert.notEqual(-1, crypto.getCiphers().indexOf('aes-128-cbc'));
assert.equal(-1, crypto.getCiphers().indexOf('AES-128-CBC'));
assert.notStrictEqual(0, crypto.getCiphers().length);
assert(crypto.getCiphers().includes('aes-128-cbc'));
assert(!crypto.getCiphers().includes('AES-128-CBC'));
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if these are easier to read if written like this:

assert.strictEqual(crypto.getCiphers().includes('AES-128-CBC'), false);

assertSorted(crypto.getCiphers());

// Assume that we have at least AES256-SHA.
assert.notEqual(0, tls.getCiphers().length);
assert.notEqual(-1, tls.getCiphers().indexOf('aes256-sha'));
assert.equal(-1, tls.getCiphers().indexOf('AES256-SHA'));
assert.notStrictEqual(0, tls.getCiphers().length);
assert(tls.getCiphers().includes('aes256-sha'));
assert(!tls.getCiphers().includes('AES256-SHA'));
assertSorted(tls.getCiphers());

// Assert that we have sha and sha1 but not SHA and SHA1.
assert.notEqual(0, crypto.getHashes().length);
assert.notEqual(-1, crypto.getHashes().indexOf('sha1'));
assert.notEqual(-1, crypto.getHashes().indexOf('sha'));
assert.equal(-1, crypto.getHashes().indexOf('SHA1'));
assert.equal(-1, crypto.getHashes().indexOf('SHA'));
assert.notEqual(-1, crypto.getHashes().indexOf('RSA-SHA1'));
assert.equal(-1, crypto.getHashes().indexOf('rsa-sha1'));
assert.notStrictEqual(0, crypto.getHashes().length);
assert(crypto.getHashes().includes('sha1'));
assert(crypto.getHashes().includes('sha'));
assert(!crypto.getHashes().includes('SHA1'));
assert(!crypto.getHashes().includes('SHA'));
assert(crypto.getHashes().includes('RSA-SHA1'));
assert(!crypto.getHashes().includes('rsa-sha1'));
assertSorted(crypto.getHashes());

// Assume that we have at least secp384r1.
assert.notEqual(0, crypto.getCurves().length);
assert.notEqual(-1, crypto.getCurves().indexOf('secp384r1'));
assert.equal(-1, crypto.getCurves().indexOf('SECP384R1'));
assert.notStrictEqual(0, crypto.getCurves().length);
assert(crypto.getCurves().includes('secp384r1'));
assert(!crypto.getCurves().includes('SECP384R1'));
assertSorted(crypto.getCurves());

// Regression tests for #5725: hex input that's not a power of two should
Expand All @@ -100,18 +99,18 @@ assert.throws(function() {

assert.throws(function() {
crypto.createHash('sha1').update('0', 'hex');
}, /Bad input string/);
}, /^TypeError: Bad input string$/);

assert.throws(function() {
crypto.createSign('RSA-SHA1').update('0', 'hex');
}, /Bad input string/);
}, /^TypeError: Bad input string$/);

assert.throws(function() {
crypto.createVerify('RSA-SHA1').update('0', 'hex');
}, /Bad input string/);
}, /^TypeError: Bad input string$/);

assert.throws(function() {
var priv = [
const priv = [
'-----BEGIN RSA PRIVATE KEY-----',
'MIGrAgEAAiEA+3z+1QNF2/unumadiwEr+C5vfhezsb3hp4jAnCNRpPcCAwEAAQIgQNriSQK4',
'EFwczDhMZp2dvbcz7OUUyt36z3S4usFPHSECEQD/41K7SujrstBfoCPzwC1xAhEA+5kt4BJy',
Expand All @@ -121,7 +120,7 @@ assert.throws(function() {
''
].join('\n');
crypto.createSign('RSA-SHA256').update('test').sign(priv);
}, /digest too big for rsa key/);
}, /digest too big for rsa key$/);

assert.throws(function() {
// The correct header inside `test_bad_rsa_privkey.pem` should have been
Expand All @@ -133,7 +132,7 @@ assert.throws(function() {
// $ openssl pkcs8 -topk8 -inform PEM -outform PEM -in mykey.pem \
// -out private_key.pem -nocrypt;
// Then open private_key.pem and change its header and footer.
var sha1_privateKey = fs.readFileSync(common.fixturesDir +
const sha1_privateKey = fs.readFileSync(common.fixturesDir +
'/test_bad_rsa_privkey.pem', 'ascii');
// this would inject errors onto OpenSSL's error stack
crypto.createSign('sha1').sign(sha1_privateKey);
Expand All @@ -144,4 +143,4 @@ console.log(crypto.randomBytes(16));

assert.throws(function() {
tls.createSecureContext({ crl: 'not a CRL' });
}, '/Failed to parse CRL/');
}, /^Error: Failed to parse CRL$/);