From 9131996dd59eebff6ccb1a57d3ec3c032a521ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reis?= Date: Fri, 22 May 2015 14:16:07 +0100 Subject: [PATCH 1/7] test: running tls-server-verify clients in parallel OpenSSL s_client introduces some delay on Windows. With all clients running sequentially, this delay is big enough to break CI. This fix runs the clients in parallel (unless the test includes renegotiation), reducing the total run time. --- test/simple/test-tls-server-verify.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/simple/test-tls-server-verify.js b/test/simple/test-tls-server-verify.js index 085749af438..c8e9692717d 100644 --- a/test/simple/test-tls-server-verify.js +++ b/test/simple/test-tls-server-verify.js @@ -323,7 +323,21 @@ function runTest(testIndex) { if (tcase.debug) { console.error('TLS server running on port ' + common.PORT); } else { - runNextClient(0); + if (tcase.renegotiate) { + runNextClient(0); + } else { + var clientsCompleted = 0; + for (var i = 0; i < tcase.clients.length; i++) { + runClient(tcase.clients[i], function() { + clientsCompleted++; + if (clientsCompleted === tcase.clients.length) { + server.close(); + successfulTests++; + runTest(testIndex + 1); + } + }); + } + } } }); } From 28a6a19f70ed87b11a9f69b8b6d292784aca42d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reis?= Date: Fri, 22 May 2015 14:28:11 +0100 Subject: [PATCH 2/7] test: run tls-server-verify servers in parallel Different servers must use different ports. Since we can count only on common.PORT and common.PORT+1, run only 2 servers in parallel. --- test/simple/test-tls-server-verify.js | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/test/simple/test-tls-server-verify.js b/test/simple/test-tls-server-verify.js index c8e9692717d..a42359678e1 100644 --- a/test/simple/test-tls-server-verify.js +++ b/test/simple/test-tls-server-verify.js @@ -141,14 +141,14 @@ var serverKey = loadPEM('agent2-key'); var serverCert = loadPEM('agent2-cert'); -function runClient(options, cb) { +function runClient(port, options, cb) { // Client can connect in three ways: // - Self-signed cert // - Certificate, but not signed by CA. // - Certificate signed by CA. - var args = ['s_client', '-connect', '127.0.0.1:' + common.PORT]; + var args = ['s_client', '-connect', '127.0.0.1:' + port]; console.log(' connecting with', options.name); @@ -246,7 +246,7 @@ function runClient(options, cb) { // Run the tests var successfulTests = 0; -function runTest(testIndex) { +function runTest(port, testIndex) { var tcase = testCases[testIndex]; if (!tcase) return; @@ -309,31 +309,31 @@ function runTest(testIndex) { function runNextClient(clientIndex) { var options = tcase.clients[clientIndex]; if (options) { - runClient(options, function() { + runClient(port, options, function() { runNextClient(clientIndex + 1); }); } else { server.close(); successfulTests++; - runTest(testIndex + 1); + runTest(port, nextTest++); } } - server.listen(common.PORT, function() { + server.listen(port, function() { if (tcase.debug) { - console.error('TLS server running on port ' + common.PORT); + console.error('TLS server running on port ' + port); } else { if (tcase.renegotiate) { runNextClient(0); } else { var clientsCompleted = 0; for (var i = 0; i < tcase.clients.length; i++) { - runClient(tcase.clients[i], function() { + runClient(port, tcase.clients[i], function() { clientsCompleted++; if (clientsCompleted === tcase.clients.length) { server.close(); successfulTests++; - runTest(testIndex + 1); + runTest(port, nextTest++); } }); } @@ -343,7 +343,9 @@ function runTest(testIndex) { } -runTest(0); +var nextTest = 0; +runTest(common.PORT, nextTest++); +runTest(common.PORT + 1, nextTest++); process.on('exit', function() { From 7e1a2914d1de958f27606cbfd185d221c6c565fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reis?= Date: Fri, 22 May 2015 14:43:16 +0100 Subject: [PATCH 3/7] test: improve console output of tls-server-verify When running in parallel, it is not easy to identify what server and client failed when the test fails. This adds identifiers to all lines of console output. --- test/simple/test-tls-server-verify.js | 35 +++++++++++++++------------ 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/test/simple/test-tls-server-verify.js b/test/simple/test-tls-server-verify.js index a42359678e1..a4420c9a8fe 100644 --- a/test/simple/test-tls-server-verify.js +++ b/test/simple/test-tls-server-verify.js @@ -141,7 +141,7 @@ var serverKey = loadPEM('agent2-key'); var serverCert = loadPEM('agent2-cert'); -function runClient(port, options, cb) { +function runClient(prefix, port, options, cb) { // Client can connect in three ways: // - Self-signed cert @@ -151,7 +151,7 @@ function runClient(port, options, cb) { var args = ['s_client', '-connect', '127.0.0.1:' + port]; - console.log(' connecting with', options.name); + console.log(prefix + ' connecting with', options.name); switch (options.name) { case 'agent1': @@ -192,7 +192,7 @@ function runClient(port, options, cb) { break; default: - throw new Error('Unknown agent name'); + throw new Error(prefix + 'Unknown agent name'); } // To test use: openssl s_client -connect localhost:8000 @@ -209,7 +209,7 @@ function runClient(port, options, cb) { out += d; if (!goodbye && /_unauthed/g.test(out)) { - console.error(' * unauthed'); + console.error(prefix + ' * unauthed'); goodbye = true; client.stdin.end('goodbye\n'); authed = false; @@ -217,7 +217,7 @@ function runClient(port, options, cb) { } if (!goodbye && /_authed/g.test(out)) { - console.error(' * authed'); + console.error(prefix + ' * authed'); goodbye = true; client.stdin.end('goodbye\n'); authed = true; @@ -228,15 +228,17 @@ function runClient(port, options, cb) { //client.stdout.pipe(process.stdout); client.on('exit', function(code) { - //assert.equal(0, code, options.name + + //assert.equal(0, code, prefix + options.name + // ": s_client exited with error code " + code); if (options.shouldReject) { - assert.equal(true, rejected, options.name + + assert.equal(true, rejected, prefix + options.name + ' NOT rejected, but should have been'); } else { - assert.equal(false, rejected, options.name + + assert.equal(false, rejected, prefix + options.name + ' rejected, but should NOT have been'); - assert.equal(options.shouldAuth, authed); + assert.equal(options.shouldAuth, authed, prefix + + options.name + ' authed is ' + authed + + ' but should have been ' + options.shouldAuth); } cb(); @@ -247,10 +249,11 @@ function runClient(port, options, cb) { // Run the tests var successfulTests = 0; function runTest(port, testIndex) { + var prefix = testIndex + ' '; var tcase = testCases[testIndex]; if (!tcase) return; - console.error("Running '%s'", tcase.title); + console.error(prefix + "Running '%s'", tcase.title); var cas = tcase.CAs.map(loadPEM); @@ -281,7 +284,7 @@ function runTest(port, testIndex) { if (tcase.renegotiate && !renegotiated) { renegotiated = true; setTimeout(function() { - console.error('- connected, renegotiating'); + console.error(prefix + '- connected, renegotiating'); c.write('\n_renegotiating\n'); return c.renegotiate({ requestCert: true, @@ -297,11 +300,11 @@ function runTest(port, testIndex) { connections++; if (c.authorized) { - console.error('- authed connection: ' + + console.error(prefix + '- authed connection: ' + c.getPeerCertificate().subject.CN); c.write('\n_authed\n'); } else { - console.error('- unauthed connection: %s', c.authorizationError); + console.error(prefix + '- unauthed connection: %s', c.authorizationError); c.write('\n_unauthed\n'); } }); @@ -309,7 +312,7 @@ function runTest(port, testIndex) { function runNextClient(clientIndex) { var options = tcase.clients[clientIndex]; if (options) { - runClient(port, options, function() { + runClient(prefix + clientIndex + ' ', port, options, function() { runNextClient(clientIndex + 1); }); } else { @@ -321,14 +324,14 @@ function runTest(port, testIndex) { server.listen(port, function() { if (tcase.debug) { - console.error('TLS server running on port ' + port); + console.error(prefix + 'TLS server running on port ' + port); } else { if (tcase.renegotiate) { runNextClient(0); } else { var clientsCompleted = 0; for (var i = 0; i < tcase.clients.length; i++) { - runClient(port, tcase.clients[i], function() { + runClient(prefix + i + ' ', port, tcase.clients[i], function() { clientsCompleted++; if (clientsCompleted === tcase.clients.length) { server.close(); From d2f778c849eff9d68be9bb4f1bceac1e74fd36be Mon Sep 17 00:00:00 2001 From: Alexis Campailla Date: Fri, 22 May 2015 18:04:40 +0200 Subject: [PATCH 4/7] test,win: re-enable tls-server-verify in CI Now that the test is fixed, node-accept-pull-request should fail when the test fails. --- test/simple/simple.status | 1 - 1 file changed, 1 deletion(-) diff --git a/test/simple/simple.status b/test/simple/simple.status index 40d8208b142..4915a029b68 100644 --- a/test/simple/simple.status +++ b/test/simple/simple.status @@ -6,7 +6,6 @@ test-cluster-basic : PASS,FLAKY [$system==win32] test-timers-first-fire : PASS,FLAKY -test-tls-server-verify : PASS,FLAKY [$system==linux] test-fs-readfile-error : PASS,FLAKY From b5ee2f5d8ac772fa2d225bbb9622aa374057df32 Mon Sep 17 00:00:00 2001 From: Shigeki Ohtsu Date: Wed, 27 May 2015 10:16:51 +0900 Subject: [PATCH 5/7] test: kill child in tls-server-verify for speed up For better performance of the test, the parent kills child processes so as not to wait them to be ended. (cherry picked from commit 833b23636045f7afc929196139021630a390391a) --- test/simple/test-tls-server-verify.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/simple/test-tls-server-verify.js b/test/simple/test-tls-server-verify.js index a4420c9a8fe..aef24d7ae86 100644 --- a/test/simple/test-tls-server-verify.js +++ b/test/simple/test-tls-server-verify.js @@ -211,7 +211,7 @@ function runClient(prefix, port, options, cb) { if (!goodbye && /_unauthed/g.test(out)) { console.error(prefix + ' * unauthed'); goodbye = true; - client.stdin.end('goodbye\n'); + client.kill(); authed = false; rejected = false; } @@ -219,7 +219,7 @@ function runClient(prefix, port, options, cb) { if (!goodbye && /_authed/g.test(out)) { console.error(prefix + ' * authed'); goodbye = true; - client.stdin.end('goodbye\n'); + client.kill(); authed = true; rejected = false; } @@ -281,6 +281,12 @@ function runTest(port, testIndex) { var renegotiated = false; var server = tls.Server(serverOptions, function handleConnection(c) { + c.on('error', function(e) { + // child.kill() leads ECONNRESET errro in the TLS connection of + // openssl s_client via spawn(). A Test result is already + // checked by the data of client.stdout before child.kill() so + // these tls errors can be ignored. + }); if (tcase.renegotiate && !renegotiated) { renegotiated = true; setTimeout(function() { From 454d18525462d43ca94917995aaf0f05f14ec3f7 Mon Sep 17 00:00:00 2001 From: Shigeki Ohtsu Date: Wed, 27 May 2015 10:33:38 +0900 Subject: [PATCH 6/7] deps: add -no_rand_screen to openssl s_client In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. (cherry picked from commit 9f0f7c38e6df975dd39735d0e9ef968076369c74) --- deps/openssl/openssl/apps/app_rand.c | 14 ++++++++++---- deps/openssl/openssl/apps/s_client.c | 11 ++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/deps/openssl/openssl/apps/app_rand.c b/deps/openssl/openssl/apps/app_rand.c index 595fc7821c8..b6fe294a682 100644 --- a/deps/openssl/openssl/apps/app_rand.c +++ b/deps/openssl/openssl/apps/app_rand.c @@ -124,10 +124,16 @@ int app_RAND_load_file(const char *file, BIO *bio_e, int dont_warn) char buffer[200]; #ifdef OPENSSL_SYS_WINDOWS - BIO_printf(bio_e, "Loading 'screen' into random state -"); - BIO_flush(bio_e); - RAND_screen(); - BIO_printf(bio_e, " done\n"); + /* + * allocate 2 to dont_warn not to use RAND_screen() via + * -no_rand_screen option in s_client + */ + if (dont_warn != 2) { + BIO_printf(bio_e, "Loading 'screen' into random state -"); + BIO_flush(bio_e); + RAND_screen(); + BIO_printf(bio_e, " done\n"); + } #endif if (file == NULL) diff --git a/deps/openssl/openssl/apps/s_client.c b/deps/openssl/openssl/apps/s_client.c index b34d38afea5..2ec61facd20 100644 --- a/deps/openssl/openssl/apps/s_client.c +++ b/deps/openssl/openssl/apps/s_client.c @@ -233,6 +233,7 @@ static int ocsp_resp_cb(SSL *s, void *arg); static BIO *bio_c_out = NULL; static int c_quiet = 0; static int c_ign_eof = 0; +static int c_no_rand_screen = 0; #ifndef OPENSSL_NO_PSK /* Default PSK identity and key */ @@ -433,6 +434,10 @@ static void sc_usage(void) " -keymatexport label - Export keying material using label\n"); BIO_printf(bio_err, " -keymatexportlen len - Export len bytes of keying material (default 20)\n"); +#ifdef OPENSSL_SYS_WINDOWS + BIO_printf(bio_err, + " -no_rand_screen - Do not use RAND_screen() to initialize random state\n"); +#endif } #ifndef OPENSSL_NO_TLSEXT @@ -1009,6 +1014,10 @@ int MAIN(int argc, char **argv) keymatexportlen = atoi(*(++argv)); if (keymatexportlen == 0) goto bad; +#ifdef OPENSSL_SYS_WINDOWS + } else if (strcmp(*argv, "-no_rand_screen") == 0) { + c_no_rand_screen = 1; +#endif } else { BIO_printf(bio_err, "unknown option %s\n", *argv); badop = 1; @@ -1092,7 +1101,7 @@ int MAIN(int argc, char **argv) } } - if (!app_RAND_load_file(NULL, bio_err, 1) && inrand == NULL + if (!app_RAND_load_file(NULL, bio_err, ++c_no_rand_screen) && inrand == NULL && !RAND_status()) { BIO_printf(bio_err, "warning, not much extra random data, consider using the -rand option\n"); From c9718a4ff986f9ed76b0b36479f95ad49c8ea7ea Mon Sep 17 00:00:00 2001 From: Shigeki Ohtsu Date: Wed, 27 May 2015 10:41:43 +0900 Subject: [PATCH 7/7] test: add -no_rand_screen for tls-server-verify This improves the performance of openssl s_client on Windows and gains several seconds to finish test-tls-server-verify. (cherry picked from commit 2ff517e0e410ea33ba5a3d289a82fc315d120e8e) --- test/simple/test-tls-server-verify.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/simple/test-tls-server-verify.js b/test/simple/test-tls-server-verify.js index aef24d7ae86..b875a848bc5 100644 --- a/test/simple/test-tls-server-verify.js +++ b/test/simple/test-tls-server-verify.js @@ -150,6 +150,9 @@ function runClient(prefix, port, options, cb) { var args = ['s_client', '-connect', '127.0.0.1:' + port]; + // for the performance issue in s_client on Windows + if (process.platform === 'win32') + args.push('-no_rand_screen'); console.log(prefix + ' connecting with', options.name);