From a1b114ec471959a8da3533e554ec5ba2e7255850 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 9 Jan 2017 08:38:31 -0500 Subject: [PATCH 1/7] tls: do not crash on STARTTLS when OCSP requested `TLSSocket` should not have a hard dependency on `tls.Server`, since it may be running without it in cases like `STARTTLS`. Fix: #10704 --- lib/_tls_wrap.js | 6 +++ test/parallel/test-tls-starttls-server.js | 50 +++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 test/parallel/test-tls-starttls-server.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 434384cec81595..66e373e55a6392 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -114,6 +114,12 @@ function requestOCSP(self, hello, ctx, cb) { if (!ctx) ctx = self.server._sharedCreds; + + // Running on non-TLS server + if (!ctx) + return cb(null); + + // TODO(indutny): eventually disallow raw `SecureContext` if (ctx.context) ctx = ctx.context; diff --git a/test/parallel/test-tls-starttls-server.js b/test/parallel/test-tls-starttls-server.js new file mode 100644 index 00000000000000..5093ed843598a4 --- /dev/null +++ b/test/parallel/test-tls-starttls-server.js @@ -0,0 +1,50 @@ +'use strict'; +const common = require('../common'); + +if (!common.hasCrypto) { + common.skip('missing crypto'); + return; +} + +const assert = require('assert'); +const net = require('net'); +const tls = require('tls'); +const fs = require('fs'); + +const key = fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'); +const cert = fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'); + +const server = net.createServer(common.mustCall((s) => { + const tlsSocket = new tls.TLSSocket(s, { + isServer: true, + server: server, + + secureContext: tls.createSecureContext({ + key: key, + cert: cert + }), + + SNICallback: common.mustCall((hostname, callback) => { + assert.equal(hostname, 'test.test'); + + callback(null, null); + }) + }); + + tlsSocket.on('secure', common.mustCall(() => { + console.log('hello'); + tlsSocket.end(); + server.close(); + })); +})).listen(0, () => { + const opts = { + servername: 'test.test', + port: server.address().port, + rejectUnauthorized: false, + requestOCSP: true + }; + + const client = tls.connect(opts, function() { + client.end(); + }); +}); From caa7ba1e5d73f92126e8e064af34692fb9eb0478 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 9 Jan 2017 11:45:05 -0500 Subject: [PATCH 2/7] fix --- test/parallel/test-tls-starttls-server.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-tls-starttls-server.js b/test/parallel/test-tls-starttls-server.js index 5093ed843598a4..6944a9bfb784b0 100644 --- a/test/parallel/test-tls-starttls-server.js +++ b/test/parallel/test-tls-starttls-server.js @@ -25,14 +25,13 @@ const server = net.createServer(common.mustCall((s) => { }), SNICallback: common.mustCall((hostname, callback) => { - assert.equal(hostname, 'test.test'); + assert.deepEqual(hostname, 'test.test'); callback(null, null); }) }); tlsSocket.on('secure', common.mustCall(() => { - console.log('hello'); tlsSocket.end(); server.close(); })); From 3ef170fff2c801da82c2d774df56cf5f1d1ab049 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 9 Jan 2017 20:58:04 -0500 Subject: [PATCH 3/7] fix --- test/parallel/test-tls-starttls-server.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-tls-starttls-server.js b/test/parallel/test-tls-starttls-server.js index 6944a9bfb784b0..eb3c261c33e9c1 100644 --- a/test/parallel/test-tls-starttls-server.js +++ b/test/parallel/test-tls-starttls-server.js @@ -6,10 +6,13 @@ if (!common.hasCrypto) { return; } +// Test asynchronous SNI+OCSP on TLSSocket created on with `server` set to +// `net.Server` instead of `tls.Server` + const assert = require('assert'); +const fs = require('fs'); const net = require('net'); const tls = require('tls'); -const fs = require('fs'); const key = fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'); const cert = fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'); @@ -43,7 +46,7 @@ const server = net.createServer(common.mustCall((s) => { requestOCSP: true }; - const client = tls.connect(opts, function() { - client.end(); + tls.connect(opts, function() { + this.end(); }); }); From 7473f5532e4637fdd49e5ca51116cbbd8ae7e577 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 9 Jan 2017 20:59:00 -0500 Subject: [PATCH 4/7] fix --- test/parallel/test-tls-starttls-server.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-tls-starttls-server.js b/test/parallel/test-tls-starttls-server.js index eb3c261c33e9c1..6f832586904a8c 100644 --- a/test/parallel/test-tls-starttls-server.js +++ b/test/parallel/test-tls-starttls-server.js @@ -1,4 +1,8 @@ 'use strict'; + +// Test asynchronous SNI+OCSP on TLSSocket created with `server` set to +// `net.Server` instead of `tls.Server` + const common = require('../common'); if (!common.hasCrypto) { @@ -6,9 +10,6 @@ if (!common.hasCrypto) { return; } -// Test asynchronous SNI+OCSP on TLSSocket created on with `server` set to -// `net.Server` instead of `tls.Server` - const assert = require('assert'); const fs = require('fs'); const net = require('net'); From 584b455f8e5f7fbc9c3ff34cad5265cc333134ad Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 10 Jan 2017 08:26:17 -0500 Subject: [PATCH 5/7] fix --- test/parallel/test-tls-starttls-server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-tls-starttls-server.js b/test/parallel/test-tls-starttls-server.js index 6f832586904a8c..ca6a00b25ddc03 100644 --- a/test/parallel/test-tls-starttls-server.js +++ b/test/parallel/test-tls-starttls-server.js @@ -29,7 +29,7 @@ const server = net.createServer(common.mustCall((s) => { }), SNICallback: common.mustCall((hostname, callback) => { - assert.deepEqual(hostname, 'test.test'); + assert.strictEqual(hostname, 'test.test'); callback(null, null); }) From 20b187b1b658bce2a710c55cb636f9dd1c07108a Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 3 Feb 2017 12:21:43 -0500 Subject: [PATCH 6/7] fix --- lib/_tls_wrap.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 66e373e55a6392..b0015b09af931d 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -115,7 +115,8 @@ function requestOCSP(self, hello, ctx, cb) { if (!ctx) ctx = self.server._sharedCreds; - // Running on non-TLS server + // TLS socket is using a `net.Server` instead of a tls.TLSServer. + // Some TLS properties will not be present. if (!ctx) return cb(null); From 36fd2ce745646942cf7d3133acbb5097d7f14d6f Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 3 Feb 2017 13:00:34 -0500 Subject: [PATCH 7/7] fix --- lib/_tls_wrap.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index b0015b09af931d..3d34192a649905 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -116,7 +116,7 @@ function requestOCSP(self, hello, ctx, cb) { ctx = self.server._sharedCreds; // TLS socket is using a `net.Server` instead of a tls.TLSServer. - // Some TLS properties will not be present. + // Some TLS properties like `server._sharedCreds` will not be present if (!ctx) return cb(null);