-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
tls: support changing credentials dynamically #23644
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -833,22 +833,11 @@ function Server(options, listener) { | |
| // Handle option defaults: | ||
| this.setOptions(options); | ||
|
|
||
| this._sharedCreds = tls.createSecureContext({ | ||
| pfx: this.pfx, | ||
| key: this.key, | ||
| passphrase: this.passphrase, | ||
| cert: this.cert, | ||
| clientCertEngine: this.clientCertEngine, | ||
| ca: this.ca, | ||
| ciphers: this.ciphers, | ||
| ecdhCurve: this.ecdhCurve, | ||
| dhparam: this.dhparam, | ||
| secureProtocol: this.secureProtocol, | ||
| secureOptions: this.secureOptions, | ||
| honorCipherOrder: this.honorCipherOrder, | ||
| crl: this.crl, | ||
| sessionIdContext: this.sessionIdContext | ||
| }); | ||
| // setSecureContext() overlaps with setOptions() quite a bit. setOptions() | ||
| // is an undocumented API that was probably never intended to be exposed | ||
| // publicly. Unfortunately, it would be a breaking change to just remove it, | ||
| // and there is at least one test that depends on it. | ||
| this.setSecureContext(options); | ||
|
|
||
| this[kHandshakeTimeout] = options.handshakeTimeout || (120 * 1000); | ||
| this[kSNICallback] = options.SNICallback; | ||
|
|
@@ -863,14 +852,6 @@ function Server(options, listener) { | |
| 'options.SNICallback', 'function', options.SNICallback); | ||
| } | ||
|
|
||
| if (this.sessionTimeout) { | ||
| this._sharedCreds.context.setSessionTimeout(this.sessionTimeout); | ||
| } | ||
|
|
||
| if (this.ticketKeys) { | ||
| this._sharedCreds.context.setTicketKeys(this.ticketKeys); | ||
| } | ||
|
|
||
| // constructor call | ||
| net.Server.call(this, tlsConnectionListener); | ||
|
|
||
|
|
@@ -886,6 +867,115 @@ exports.createServer = function createServer(options, listener) { | |
| }; | ||
|
|
||
|
|
||
| Server.prototype.setSecureContext = function(options) { | ||
| if (options === null || typeof options !== 'object') | ||
| throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); | ||
|
|
||
| if (options.pfx) | ||
| this.pfx = options.pfx; | ||
| else | ||
| this.pfx = undefined; | ||
|
|
||
| if (options.key) | ||
| this.key = options.key; | ||
| else | ||
| this.key = undefined; | ||
|
|
||
| if (options.passphrase) | ||
| this.passphrase = options.passphrase; | ||
| else | ||
| this.passphrase = undefined; | ||
|
|
||
| if (options.cert) | ||
| this.cert = options.cert; | ||
| else | ||
| this.cert = undefined; | ||
|
|
||
| if (options.clientCertEngine) | ||
| this.clientCertEngine = options.clientCertEngine; | ||
| else | ||
| this.clientCertEngine = undefined; | ||
|
|
||
| if (options.ca) | ||
| this.ca = options.ca; | ||
| else | ||
| this.ca = undefined; | ||
|
|
||
| if (options.secureProtocol) | ||
| this.secureProtocol = options.secureProtocol; | ||
| else | ||
| this.secureProtocol = undefined; | ||
|
|
||
| if (options.crl) | ||
| this.crl = options.crl; | ||
| else | ||
| this.crl = undefined; | ||
|
|
||
| if (options.ciphers) | ||
| this.ciphers = options.ciphers; | ||
| else | ||
| this.ciphers = undefined; | ||
|
|
||
| if (options.ecdhCurve !== undefined) | ||
| this.ecdhCurve = options.ecdhCurve; | ||
| else | ||
| this.ecdhCurve = undefined; | ||
|
|
||
| if (options.dhparam) | ||
| this.dhparam = options.dhparam; | ||
| else | ||
| this.dhparam = undefined; | ||
|
|
||
| if (options.honorCipherOrder !== undefined) | ||
| this.honorCipherOrder = !!options.honorCipherOrder; | ||
| else | ||
| this.honorCipherOrder = true; | ||
|
|
||
| const secureOptions = options.secureOptions || 0; | ||
|
|
||
| if (secureOptions) | ||
| this.secureOptions = secureOptions; | ||
| else | ||
| this.secureOptions = undefined; | ||
|
|
||
| if (options.sessionIdContext) { | ||
| this.sessionIdContext = options.sessionIdContext; | ||
| } else { | ||
| this.sessionIdContext = crypto.createHash('sha1') | ||
| .update(process.argv.join(' ')) | ||
| .digest('hex') | ||
| .slice(0, 32); | ||
| } | ||
|
||
|
|
||
| this._sharedCreds = tls.createSecureContext({ | ||
| pfx: this.pfx, | ||
| key: this.key, | ||
| passphrase: this.passphrase, | ||
| cert: this.cert, | ||
| clientCertEngine: this.clientCertEngine, | ||
| ca: this.ca, | ||
| ciphers: this.ciphers, | ||
| ecdhCurve: this.ecdhCurve, | ||
| dhparam: this.dhparam, | ||
| secureProtocol: this.secureProtocol, | ||
| secureOptions: this.secureOptions, | ||
| honorCipherOrder: this.honorCipherOrder, | ||
| crl: this.crl, | ||
| sessionIdContext: this.sessionIdContext | ||
| }); | ||
|
|
||
| if (this.sessionTimeout) | ||
| this._sharedCreds.context.setSessionTimeout(this.sessionTimeout); | ||
|
|
||
| if (options.ticketKeys) { | ||
| this.ticketKeys = options.ticketKeys; | ||
| this.setTicketKeys(this.ticketKeys); | ||
| } else { | ||
| this.setTicketKeys(this.getTicketKeys()); | ||
|
||
| } | ||
| }; | ||
|
|
||
|
|
||
| Server.prototype._getServerData = function() { | ||
| return { | ||
| ticketKeys: this.getTicketKeys().toString('hex') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| 'use strict'; | ||
| const common = require('../common'); | ||
|
|
||
| if (!common.hasCrypto) | ||
| common.skip('missing crypto'); | ||
|
|
||
| const assert = require('assert'); | ||
| const https = require('https'); | ||
| const fixtures = require('../common/fixtures'); | ||
| const credentialOptions = [ | ||
| { | ||
| key: fixtures.readKey('agent1-key.pem'), | ||
| cert: fixtures.readKey('agent1-cert.pem'), | ||
| ca: fixtures.readKey('ca1-cert.pem') | ||
| }, | ||
| { | ||
| key: fixtures.readKey('agent2-key.pem'), | ||
| cert: fixtures.readKey('agent2-cert.pem'), | ||
| ca: fixtures.readKey('ca2-cert.pem') | ||
| } | ||
| ]; | ||
| let requestsCount = 0; | ||
| let firstResponse; | ||
|
|
||
| const server = https.createServer(credentialOptions[0], (req, res) => { | ||
| requestsCount++; | ||
|
|
||
| if (requestsCount === 1) { | ||
| firstResponse = res; | ||
| firstResponse.write('multi-'); | ||
| return; | ||
| } else if (requestsCount === 3) { | ||
| firstResponse.write('success-'); | ||
| } | ||
|
|
||
| res.end('success'); | ||
| }); | ||
|
|
||
| server.listen(0, common.mustCall(async () => { | ||
| const { port } = server.address(); | ||
| const firstRequest = makeRequest(port); | ||
|
|
||
| assert.strictEqual(await makeRequest(port), 'success'); | ||
|
|
||
| server.setSecureContext(credentialOptions[1]); | ||
| firstResponse.write('request-'); | ||
| await assert.rejects(async () => { | ||
| await makeRequest(port); | ||
| }, /^Error: self signed certificate$/); | ||
|
|
||
| server.setSecureContext(credentialOptions[0]); | ||
| assert.strictEqual(await makeRequest(port), 'success'); | ||
|
|
||
| server.setSecureContext(credentialOptions[1]); | ||
| firstResponse.end('fun!'); | ||
| await assert.rejects(async () => { | ||
| await makeRequest(port); | ||
| }, /^Error: self signed certificate$/); | ||
|
|
||
| assert.strictEqual(await firstRequest, 'multi-request-success-fun!'); | ||
| server.close(); | ||
| })); | ||
|
|
||
| function makeRequest(port) { | ||
| return new Promise((resolve, reject) => { | ||
| const options = { | ||
| rejectUnauthorized: true, | ||
| ca: credentialOptions[0].ca, | ||
| servername: 'agent1' | ||
| }; | ||
|
|
||
| https.get(`https://localhost:${port}`, options, (res) => { | ||
| let response = ''; | ||
|
|
||
| res.setEncoding('utf8'); | ||
|
|
||
| res.on('data', (chunk) => { | ||
| response += chunk; | ||
| }); | ||
|
|
||
| res.on('end', common.mustCall(() => { | ||
| resolve(response); | ||
| })); | ||
| }).on('error', (err) => { | ||
| reject(err); | ||
| }); | ||
| }); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could get a bit messy with so many options, but the following pattern may save a bit of boilerplate code in this method...
Not sure it's actually that much better tho ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasnell I like that pattern, because I think it would be helpful for code coverage purposes. But I'm a little worried that it's just slightly different enough from the existing options handling (
setOptions()) to cause problems. For example, right now falsy values get filtered out in most cases, but that would no longer be the case. The current structure also allows us to make the validation stricter, because right now it seems rather loose.I already plan to follow this up with a deprecation PR for
setOptions(). I'd like to also revisit the argument validation here, but that will be semver-major.