From 07ca62c090de5008779499710dc99ab93e427cfd Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 3 Apr 2018 10:35:45 +0200 Subject: [PATCH 1/2] test: set clientOpts.port property Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. --- test/parallel/test-tls-cnnic-whitelist.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-tls-cnnic-whitelist.js b/test/parallel/test-tls-cnnic-whitelist.js index 37a96e4eb7e0c0..2b036cb2a5583b 100644 --- a/test/parallel/test-tls-cnnic-whitelist.js +++ b/test/parallel/test-tls-cnnic-whitelist.js @@ -28,7 +28,7 @@ const testCases = [ rejectUnauthorized: true, ca: [loadPEM('fake-cnnic-root-cert')] }, - errorCode: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' + errorCode: 'CERT_HAS_EXPIRED' }, // Test 1: for the fix of node#2061 // agent6-cert.pem is signed by intermediate cert of ca3. @@ -58,7 +58,7 @@ function runTest(tindex) { const server = tls.createServer(tcase.serverOpts, (s) => { s.resume(); }).listen(0, common.mustCall(function() { - tcase.clientOpts = this.address().port; + tcase.clientOpts.port = this.address().port; const client = tls.connect(tcase.clientOpts); client.on('error', common.mustCall((e) => { assert.strictEqual(e.code, tcase.errorCode); From 5753046dc801b42b4fbc56fa28f0b0512c7db701 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Fri, 13 Apr 2018 15:11:38 +0200 Subject: [PATCH 2/2] test: remove test case 0 from tls-cnnic-whitelist I looks like this test has not worked as expected since commit 2bc7841d0fcdd066fe477873229125b6f003b693 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit dc875438a3953102febffa79b691317bb24ba2aa ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. --- test/parallel/test-tls-cnnic-whitelist.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/test/parallel/test-tls-cnnic-whitelist.js b/test/parallel/test-tls-cnnic-whitelist.js index 2b036cb2a5583b..e08e93013f6aca 100644 --- a/test/parallel/test-tls-cnnic-whitelist.js +++ b/test/parallel/test-tls-cnnic-whitelist.js @@ -14,22 +14,6 @@ function loadPEM(n) { } const testCases = [ - { // Test 0: for the check of a cert not 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. - serverOpts: { - key: loadPEM('agent7-key'), - cert: loadPEM('agent7-cert') - }, - clientOpts: { - port: undefined, - rejectUnauthorized: true, - ca: [loadPEM('fake-cnnic-root-cert')] - }, - errorCode: 'CERT_HAS_EXPIRED' - }, // Test 1: for the fix of node#2061 // agent6-cert.pem is signed by intermediate cert of ca3. // The server has a cert chain of agent6->ca3->ca1(root) but