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

crypto: fix VerifyCallback in case of verify error #2064

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2310,7 +2310,7 @@ inline CheckResult CheckWhitelistedServerCert(X509_STORE_CTX* ctx) {
inline int VerifyCallback(int preverify_ok, X509_STORE_CTX* ctx) {
// Failure on verification of the cert is handled in
// Connection::VerifyError.
if (preverify_ok == 0)
if (preverify_ok == 0 || X509_STORE_CTX_get_error(ctx) != X509_V_OK)
return 1;

// Server does not need to check the whitelist.
Expand Down
89 changes: 65 additions & 24 deletions test/parallel/test-tls-cnnic-whitelist.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,74 @@ if (!common.hasCrypto) {
var tls = require('tls');
var fs = require('fs');
var path = require('path');
var finished = 0;

var error = false;

// agent7-cert.pem is issued by the fake CNNIC root CA so that its
// hash is not listed in the whitelist.
var options = {
key: fs.readFileSync(path.join(common.fixturesDir, 'keys/agent7-key.pem')),
cert: fs.readFileSync(path.join(common.fixturesDir, 'keys/agent7-cert.pem'))
};

var server = tls.createServer(options, function(s) {
s.resume();
}).listen(common.PORT, function() {
var client = tls.connect({
port: common.PORT,
rejectUnauthorized: true,
function filenamePEM(n) {
return path.join(common.fixturesDir, 'keys', n + '.pem');
}

function loadPEM(n) {
return fs.readFileSync(filenamePEM(n));
}

var testCases = [
{ // Test 0: for the check of a cert not existed in the whitelist.
// agent7-cert.pem is issued by the fake CNNIC root CA so that its
// hash is not listed in the whitelist.
// fake-cnnic-root-cert has the same subject name as the original
// rootCA.
ca: [fs.readFileSync(path.join(common.fixturesDir,
'keys/fake-cnnic-root-cert.pem'))]
});
client.on('error', function(e) {
assert.strictEqual(e.code, 'CERT_REVOKED');
error = true;
server.close();
serverOpts: {
key: loadPEM('agent7-key'),
cert: loadPEM('agent7-cert')
},
clientOpts: {
port: common.PORT,
rejectUnauthorized: true,
ca: [loadPEM('fake-cnnic-root-cert')]
},
errorCode: 'CERT_REVOKED'
},
// Test 1: for the fix of iojs#2061
// agent6-cert.pem is signed by intermidate cert of ca3.
// The server has a cert chain of agent6->ca3->ca1(root) but
// tls.connect should be failed with an error of
// UNABLE_TO_GET_ISSUER_CERT_LOCALLY since the root CA of ca1 is not
// installed locally.
{
serverOpts: {
ca: loadPEM('ca3-key'),
key: loadPEM('agent6-key'),
cert: loadPEM('agent6-cert')
},
clientOpts: {
port: common.PORT,
rejectUnauthorized: true
},
errorCode: 'UNABLE_TO_GET_ISSUER_CERT_LOCALLY'
}
];

function runTest(tindex) {
var tcase = testCases[tindex];

if (!tcase) return;

var server = tls.createServer(tcase.serverOpts, function(s) {
s.resume();
}).listen(common.PORT, function() {
var client = tls.connect(tcase.clientOpts);
client.on('error', function(e) {
assert.strictEqual(e.code, tcase.errorCode);
server.close(function() {
finished++;
runTest(tindex + 1);
});
});
});
});
}

runTest(0);

process.on('exit', function() {
assert(error);
assert.equal(finished, testCases.length);
});