-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Added TLS-PSK support. #1162
Added TLS-PSK support. #1162
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 |
|---|---|---|
|
|
@@ -36,6 +36,8 @@ if (process.env.NODE_DEBUG && /tls/.test(process.env.NODE_DEBUG)) { | |
| debug = function() { }; | ||
| } | ||
|
|
||
| // ciphers to use if a server or client is configured for TLS-PSK but does not specify a cipher list | ||
| var DefaultPSKCiphers = 'PSK-AES256-CBC-SHA:PSK-3DES-EDE-CBC-SHA:PSK-AES128-CBC-SHA:PSK-RC4-SHA'; | ||
|
||
|
|
||
| var Connection = null; | ||
| try { | ||
|
|
@@ -69,7 +71,7 @@ function convertNPNProtocols(NPNProtocols, out) { | |
| if (Buffer.isBuffer(NPNProtocols)) { | ||
| out.NPNProtocols = NPNProtocols; | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| // Base class of both CleartextStream and EncryptedStream | ||
| function CryptoStream(pair) { | ||
|
|
@@ -478,15 +480,17 @@ EncryptedStream.prototype._pusher = function(pool, offset, length) { | |
| */ | ||
|
|
||
| function SecurePair(credentials, isServer, requestCert, rejectUnauthorized, | ||
| NPNProtocols) { | ||
| NPNProtocols, pskCallback) { | ||
| if (!(this instanceof SecurePair)) { | ||
| return new SecurePair(credentials, | ||
| isServer, | ||
| requestCert, | ||
| rejectUnauthorized, | ||
| NPNProtocols); | ||
| NPNProtocols, | ||
| pskCallback); | ||
| } | ||
|
|
||
|
|
||
| var self = this; | ||
|
|
||
| events.EventEmitter.call(this); | ||
|
|
@@ -523,6 +527,10 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized, | |
| this.npnProtocol = null; | ||
| } | ||
|
|
||
| if (pskCallback) { | ||
| this.ssl.setPskClientCallback(pskCallback); | ||
| } | ||
|
|
||
| /* Acts as a r/w stream to the cleartext side of the stream. */ | ||
| this.cleartext = new CleartextStream(this); | ||
|
|
||
|
|
@@ -541,11 +549,14 @@ util.inherits(SecurePair, events.EventEmitter); | |
| exports.createSecurePair = function(credentials, | ||
| isServer, | ||
| requestCert, | ||
| rejectUnauthorized) { | ||
| rejectUnauthorized, | ||
| pskCallback) { | ||
| var pair = new SecurePair(credentials, | ||
| isServer, | ||
| requestCert, | ||
| rejectUnauthorized); | ||
| rejectUnauthorized, | ||
| null, | ||
| pskCallback); | ||
| return pair; | ||
| }; | ||
|
|
||
|
|
@@ -675,7 +686,7 @@ SecurePair.prototype.error = function() { | |
| } | ||
| }; | ||
|
|
||
| // TODO: support anonymous (nocert) and PSK | ||
| // TODO: support anonymous (nocert) | ||
|
|
||
|
|
||
| // AUTHENTICATION MODES | ||
|
|
@@ -774,11 +785,11 @@ function Server(/* [options], listener */) { | |
| ciphers: self.ciphers, | ||
| secureProtocol: self.secureProtocol, | ||
| secureOptions: self.secureOptions, | ||
| crl: self.crl | ||
| crl: self.crl, | ||
| pskServerCallback: self.pskCallback, | ||
| pskServerHint: self.pskHint | ||
| }); | ||
|
|
||
| sharedCreds.context.setCiphers('RC4-SHA:AES128-SHA:AES256-SHA'); | ||
|
|
||
| // constructor call | ||
| net.Server.call(this, function(socket) { | ||
| var creds = crypto.createCredentials(null, sharedCreds.context); | ||
|
|
@@ -795,25 +806,35 @@ function Server(/* [options], listener */) { | |
| pair.on('secure', function() { | ||
| pair.cleartext.authorized = false; | ||
| pair.cleartext.npnProtocol = pair.npnProtocol; | ||
| if (!self.requestCert) { | ||
|
|
||
| // if using PSK, then handshake completion implies authorization | ||
| if (pair.ssl.pskIdentity) { | ||
| pair.cleartext.pskIdentity = pair.ssl.pskIdentity; | ||
| pair.cleartext.authorized = true; | ||
| cleartext._controlReleased = true; | ||
| self.emit('secureConnection', pair.cleartext, pair.encrypted); | ||
| } else { | ||
| var verifyError = pair.ssl.verifyError(); | ||
| if (verifyError) { | ||
| pair.cleartext.authorizationError = verifyError; | ||
|
|
||
| if (self.rejectUnauthorized) { | ||
| socket.destroy(); | ||
| pair.destroy(); | ||
| } | ||
| else { | ||
| // otherwise, might need to check for cert verification errors | ||
| if (!self.requestCert) { | ||
| cleartext._controlReleased = true; | ||
| self.emit('secureConnection', pair.cleartext, pair.encrypted); | ||
| } else { | ||
| var verifyError = pair.ssl.verifyError(); | ||
| if (verifyError) { | ||
| pair.cleartext.authorizationError = verifyError; | ||
| if (self.rejectUnauthorized) { | ||
| socket.destroy(); | ||
| pair.destroy(); | ||
| } else { | ||
| cleartext._controlReleased = true; | ||
| self.emit('secureConnection', pair.cleartext, pair.encrypted); | ||
| } | ||
| } else { | ||
| pair.cleartext.authorized = true; | ||
| cleartext._controlReleased = true; | ||
| self.emit('secureConnection', pair.cleartext, pair.encrypted); | ||
| } | ||
| } else { | ||
| pair.cleartext.authorized = true; | ||
| cleartext._controlReleased = true; | ||
| self.emit('secureConnection', pair.cleartext, pair.encrypted); | ||
| } | ||
| } | ||
| }); | ||
|
|
@@ -847,6 +868,12 @@ Server.prototype.setOptions = function(options) { | |
| this.rejectUnauthorized = false; | ||
| } | ||
|
|
||
| if (options.pskCallback && !options.ciphers) { | ||
| // set default PSK ciphers if it looks like we're trying to use | ||
| // PSK but no preference has been specified | ||
| options.ciphers = DefaultPSKCiphers; | ||
| } | ||
|
|
||
| if (options.key) this.key = options.key; | ||
| if (options.cert) this.cert = options.cert; | ||
| if (options.ca) this.ca = options.ca; | ||
|
|
@@ -856,6 +883,8 @@ Server.prototype.setOptions = function(options) { | |
| if (options.secureProtocol) this.secureProtocol = options.secureProtocol; | ||
| if (options.secureOptions) this.secureOptions = options.secureOptions; | ||
| if (options.NPNProtocols) convertNPNProtocols(options.NPNProtocols, this); | ||
| if (options.pskCallback) this.pskCallback = options.pskCallback; | ||
| if (options.pskHint) this.pskHint = options.pskHint; | ||
| }; | ||
|
|
||
|
|
||
|
|
@@ -893,31 +922,66 @@ exports.connect = function(port /* host, options, cb */) { | |
| } | ||
| } | ||
|
|
||
| // if a PSK identity/key pair was provided, translate it into the | ||
| // callback expected by SecurePair | ||
| if (options.pskIdentity && options.pskKey) { | ||
| options.pskClientCallback = function(hint) { | ||
| return { | ||
| identity: options.pskIdentity, | ||
| key: options.pskKey | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| if (options.pskClientCallback && !options.ciphers) { | ||
| // set default PSK ciphers if it looks like we're trying to use | ||
| // PSK but no preference has been specified | ||
| options.ciphers = DefaultPSKCiphers; | ||
| } | ||
|
|
||
| var socket = new net.Stream(); | ||
|
|
||
| var sslcontext = crypto.createCredentials(options); | ||
| //sslcontext.context.setCiphers('RC4-SHA:AES128-SHA:AES256-SHA'); | ||
|
|
||
| convertNPNProtocols(options.NPNProtocols, this); | ||
| var pair = new SecurePair(sslcontext, false, true, false, | ||
| this.NPNProtocols); | ||
| this.NPNProtocols, options.pskClientCallback); | ||
|
|
||
| var cleartext = pipe(pair, socket); | ||
|
|
||
| // This handler runs if the socket closes before the TLS handshake completes, | ||
| // either because the remote host aborted a normal TLS handshake for some reason | ||
| // or, in the case of TLS-PSK, if the identity and/or secret is invalid. | ||
| function onSocketClose() { | ||
| cleartext.emit('error', new Error('remote host closed the connection')); | ||
| } | ||
| socket.on('close', onSocketClose); | ||
|
||
|
|
||
| socket.connect(port, host); | ||
|
|
||
| pair.on('secure', function() { | ||
| var verifyError = pair.ssl.verifyError(); | ||
|
|
||
| // the handshake is complete, so remove the close listener; | ||
| // a close hereafter is just a close, not an error | ||
| socket.removeListener('close', onSocketClose); | ||
| cleartext.npnProtocol = pair.npnProtocol; | ||
|
|
||
| if (verifyError) { | ||
| cleartext.authorized = false; | ||
| cleartext.authorizationError = verifyError; | ||
| } else { | ||
| if (pair.ssl.pskIdentity) { | ||
| // if there is a PSK identity, then we're authorized, since PSK inherently | ||
| // provides mutual authentication | ||
| cleartext.pskIdentity = pair.ssl.pskIdentity; | ||
| cleartext.authorized = true; | ||
| } else { | ||
| // otherwise check the ssl verifyError and make it | ||
| // available for the client code to inspect | ||
| var verifyError = pair.ssl.verifyError(); | ||
| if (verifyError) { | ||
| cleartext.authorized = false; | ||
| cleartext.authorizationError = verifyError; | ||
| } else { | ||
| cleartext.authorized = true; | ||
| } | ||
| } | ||
|
|
||
| // call the callback, indicating that the handshake is complete | ||
| if (cb) cb(); | ||
| }); | ||
|
|
||
|
|
||
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.
Speaking purely as a consumer of this API, not having a unified behavoir between PSK and certificates for the error handling of a failed to authenticate connection is kinda a bummer.
Is there a better way to unify them so the same code that checked authorized === false would be re-used?
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.
I thought about this a lot before doing it this way, and I'm still not sure of the best strategy. The problem with a unified behavior is that the existing interface makes perfect sense for certs and very little sense for PSK. As I understand it, the callback indicates that the handshake is complete and a secure (but not necessarily authenticated) channel exists. In TLS-PSK the handshake doesn't complete and there is no encrypted channel unless the keys match, so the callback should be called only on success. Obviously we can call the callback unconditionally if that's the desired behavior, but it won't imply that a channel exists.
Perhaps what would be best is a callback that is called unconditionally but receives an error as its first argument, consistent with most other Node interfaces, but I am quite open to better ideas on this point.
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.
Does anyone else care to weigh in on this? I think it is the main issue with this pull request, and I'd like to clear it up in the hopes of getting this change moving along soon.
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.
Started a thread on nodejsdev specifically about this api. Lets try to get a consensus there and then get this pull request landed.