Skip to content

Commit

Permalink
tls: add PSK support
Browse files Browse the repository at this point in the history 
Add the `pskCallback` client/server option, which resolves an identity
or identity hint to a pre-shared key.

Add the `pskIdentityHint` server option to set the identity hint for the
ServerKeyExchange message.

Co-authored-by: Chris Osborn <chris.osborn@sitelier.com>
Co-authored-by: stephank <gh@stephank.nl>
Co-authored-by: Taylor Zane Glaeser <tzglaeser@gmail.com>

PR-URL: #23188
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
@lundibundi @targos
lundibundi authored and targos committed Jan 14, 2020
1 parent 1f0a1d5 commit bfccc00
Show file tree
Hide file tree
Showing 12 changed files with 646 additions and 9 deletions.
5 changes: 5 additions & 0 deletions doc/api/errors.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,11 @@ vector for denial-of-service attacks.
An attempt was made to issue Server Name Indication from a TLS server-side
socket, which is only valid from a client.

<a id="ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED"></a>
### ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED

Failed to set PSK identity hint. Hint may be too long.

<a id="ERR_TRACE_EVENTS_CATEGORY_REQUIRED"></a>
### `ERR_TRACE_EVENTS_CATEGORY_REQUIRED`

Expand Down
80 changes: 79 additions & 1 deletion doc/api/tls.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,40 @@ SNI (Server Name Indication) are TLS handshake extensions:
* SNI: Allows the use of one TLS server for multiple hostnames with different
SSL certificates.

### Pre-shared keys

<!-- type=misc -->

TLS-PSK support is available as an alternative to normal certificate-based
authentication. It uses a pre-shared key instead of certificates to
authenticate a TLS connection, providing mutual authentication.
TLS-PSK and public key infrastructure are not mutually exclusive. Clients and
servers can accommodate both, choosing either of them during the normal cipher
negotiation step.

TLS-PSK is only a good choice where means exist to securely share a
key with every connecting machine, so it does not replace PKI
(Public Key Infrastructure) for the majority of TLS uses.
The TLS-PSK implementation in OpenSSL has seen many security flaws in
recent years, mostly because it is used only by a minority of applications.
Please consider all alternative solutions before switching to PSK ciphers.
Upon generating PSK it is of critical importance to use sufficient entropy as
discussed in [RFC 4086][]. Deriving a shared secret from a password or other
low-entropy sources is not secure.

PSK ciphers are disabled by default, and using TLS-PSK thus requires explicitly
specifying a cipher suite with the `ciphers` option. The list of available
ciphers can be retrieved via `openssl ciphers -v 'PSK'`. All TLS 1.3
ciphers are eligible for PSK but currently only those that use SHA256 digest are
supported they can be retrieved via `openssl ciphers -v -s -tls1_3 -psk`.

According to the [RFC 4279][], PSK identities up to 128 bytes in length and
PSKs up to 64 bytes in length must be supported. As of OpenSSL 1.1.0
maximum identity size is 128 bytes, and maximum PSK length is 256 bytes.

The current implementation doesn't support asynchronous PSK callbacks due to the
limitations of the underlying OpenSSL API.

### Client-initiated renegotiation attack mitigation

<!-- type=misc -->
Expand Down Expand Up @@ -1207,6 +1241,9 @@ being issued by trusted CA (`options.ca`).
<!-- YAML
added: v0.11.3
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23188
description: The `pskCallback` option is now supported.
- version: v12.9.0
pr-url: https://github.com/nodejs/node/pull/27836
description: Support the `allowHalfOpen` option.
Expand Down Expand Up @@ -1258,6 +1295,23 @@ changes:
verified against the list of supplied CAs. An `'error'` event is emitted if
verification fails; `err.code` contains the OpenSSL error code. **Default:**
`true`.
* `pskCallback` {Function}
* hint: {string} optional message sent from the server to help client
decide which identity to use during negotiation.
Always `null` if TLS 1.3 is used.
* Returns: {Object} in the form
`{ psk: <Buffer|TypedArray|DataView>, identity: <string> }`
or `null` to stop the negotiation process. `psk` must be
compatible with the selected cipher's digest.
`identity` must use UTF-8 encoding.
When negotiating TLS-PSK (pre-shared keys), this function is called
with optional identity `hint` provided by the server or `null`
in case of TLS 1.3 where `hint` was removed.
It will be necessary to provide a custom `tls.checkServerIdentity()`
for the connection as the default one will try to check hostname/IP
of the server against the certificate but that's not applicable for PSK
because there won't be a certificate present.
More information can be found in the [RFC 4279][].
* `ALPNProtocols`: {string[]|Buffer[]|TypedArray[]|DataView[]|Buffer|
TypedArray|DataView}
An array of strings, `Buffer`s or `TypedArray`s or `DataView`s, or a
Expand Down Expand Up @@ -1593,8 +1647,30 @@ changes:
provided the default callback with high-level API will be used (see below).
* `ticketKeys`: {Buffer} 48-bytes of cryptographically strong pseudo-random
data. See [Session Resumption][] for more information.
* `pskCallback` {Function}
* socket: {tls.TLSSocket} the server [`tls.TLSSocket`][] instance for
this connection.
* identity: {string} identity parameter sent from the client.
* Returns: {Buffer|TypedArray|DataView} pre-shared key that must either be
a buffer or `null` to stop the negotiation process. Returned PSK must be
compatible with the selected cipher's digest.
When negotiating TLS-PSK (pre-shared keys), this function is called
with the identity provided by the client.
If the return value is `null` the negotiation process will stop and an
"unknown_psk_identity" alert message will be sent to the other party.
If the server wishes to hide the fact that the PSK identity was not known,
the callback must provide some random data as `psk` to make the connection
fail with "decrypt_error" before negotiation is finished.
PSK ciphers are disabled by default, and using TLS-PSK thus
requires explicitly specifying a cipher suite with the `ciphers` option.
More information can be found in the [RFC 4279][].
* `pskIdentityHint` {string} optional hint to send to a client to help
with selecting the identity during TLS-PSK negotiation. Will be ignored
in TLS 1.3. Upon failing to set pskIdentityHint `'tlsClientError'` will be
emitted with `'ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED'` code.
* ...: Any [`tls.createSecureContext()`][] option can be provided. For
servers, the identity options (`pfx` or `key`/`cert`) are usually required.
servers, the identity options (`pfx`, `key`/`cert` or `pskCallback`)
are usually required.
* ...: Any [`net.createServer()`][] option can be provided.
* `secureConnectionListener` {Function}
* Returns: {tls.Server}
Expand Down Expand Up @@ -1870,3 +1946,5 @@ where `secureSocket` has the same API as `pair.cleartext`.
[cipher list format]: https://www.openssl.org/docs/man1.1.1/man1/ciphers.html#CIPHER-LIST-FORMAT
[modifying the default cipher suite]: #tls_modifying_the_default_tls_cipher_suite
[specific attacks affecting larger AES key sizes]: https://www.schneier.com/blog/archives/2009/07/another_new_aes.html
[RFC 4279]: https://tools.ietf.org/html/rfc4279
[RFC 4086]: https://tools.ietf.org/html/rfc4086
117 changes: 114 additions & 3 deletions lib/_tls_wrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ const { TCP, constants: TCPConstants } = internalBinding('tcp_wrap');
const tls_wrap = internalBinding('tls_wrap');
const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap');
const { owner_symbol } = require('internal/async_hooks').symbols;
const { isArrayBufferView } = require('internal/util/types');
const { SecureContext: NativeSecureContext } = internalBinding('crypto');
const { connResetException, codes } = require('internal/errors');
const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_CALLBACK,
ERR_MULTIPLE_CALLBACK,
ERR_SOCKET_CLOSED,
Expand All @@ -58,8 +60,9 @@ const {
ERR_TLS_SESSION_ATTACK,
ERR_TLS_SNI_FROM_SERVER
} = codes;
const { onpskexchange: kOnPskExchange } = internalBinding('symbols');
const { getOptionValue } = require('internal/options');
const { validateString } = require('internal/validators');
const { validateString, validateBuffer } = require('internal/validators');
const traceTls = getOptionValue('--trace-tls');
const tlsKeylog = getOptionValue('--tls-keylog');
const { appendFile } = require('fs');
Expand All @@ -70,6 +73,8 @@ const kHandshakeTimeout = Symbol('handshake-timeout');
const kRes = Symbol('res');
const kSNICallback = Symbol('snicallback');
const kEnableTrace = Symbol('enableTrace');
const kPskCallback = Symbol('pskcallback');
const kPskIdentityHint = Symbol('pskidentityhint');

const noop = () => {};

Expand Down Expand Up @@ -289,6 +294,67 @@ function onnewsession(sessionId, session) {
done();
}

function onPskServerCallback(identity, maxPskLen) {
const owner = this[owner_symbol];
const ret = owner[kPskCallback](owner, identity);
if (ret == null)
return undefined;

let psk;
if (isArrayBufferView(ret)) {
psk = ret;
} else {
if (typeof ret !== 'object') {
throw new ERR_INVALID_ARG_TYPE(
'ret',
['Object', 'Buffer', 'TypedArray', 'DataView'],
ret
);
}
psk = ret.psk;
validateBuffer(psk, 'psk');
}

if (psk.length > maxPskLen) {
throw new ERR_INVALID_ARG_VALUE(
'psk',
psk,
`Pre-shared key exceeds ${maxPskLen} bytes`
);
}

return psk;
}

function onPskClientCallback(hint, maxPskLen, maxIdentityLen) {
const owner = this[owner_symbol];
const ret = owner[kPskCallback](hint);
if (ret == null)
return undefined;

if (typeof ret !== 'object')
throw new ERR_INVALID_ARG_TYPE('ret', 'Object', ret);

validateBuffer(ret.psk, 'psk');
if (ret.psk.length > maxPskLen) {
throw new ERR_INVALID_ARG_VALUE(
'psk',
ret.psk,
`Pre-shared key exceeds ${maxPskLen} bytes`
);
}

validateString(ret.identity, 'identity');
if (Buffer.byteLength(ret.identity) > maxIdentityLen) {
throw new ERR_INVALID_ARG_VALUE(
'identity',
ret.identity,
`PSK identity exceeds ${maxIdentityLen} bytes`
);
}

return { psk: ret.psk, identity: ret.identity };
}

function onkeylogclient(line) {
debug('client onkeylog');
Expand Down Expand Up @@ -687,6 +753,32 @@ TLSSocket.prototype._init = function(socket, wrap) {
ssl.setALPNProtocols(ssl._secureContext.alpnBuffer);
}

if (options.pskCallback && ssl.enablePskCallback) {
if (typeof options.pskCallback !== 'function') {
throw new ERR_INVALID_ARG_TYPE('pskCallback',
'function',
options.pskCallback);
}

ssl[kOnPskExchange] = options.isServer ?
onPskServerCallback : onPskClientCallback;

this[kPskCallback] = options.pskCallback;
ssl.enablePskCallback();

if (options.pskIdentityHint) {
if (typeof options.pskIdentityHint !== 'string') {
throw new ERR_INVALID_ARG_TYPE(
'options.pskIdentityHint',
'string',
options.pskIdentityHint
);
}
ssl.setPskIdentityHint(options.pskIdentityHint);
}
}


if (options.handshakeTimeout > 0)
this.setTimeout(options.handshakeTimeout, this._handleTimeout);

Expand Down Expand Up @@ -898,7 +990,7 @@ function makeSocketMethodProxy(name) {
TLSSocket.prototype[method] = makeSocketMethodProxy(method);
});

// TODO: support anonymous (nocert) and PSK
// TODO: support anonymous (nocert)


function onServerSocketSecure() {
Expand Down Expand Up @@ -954,6 +1046,8 @@ function tlsConnectionListener(rawSocket) {
SNICallback: this[kSNICallback] || SNICallback,
enableTrace: this[kEnableTrace],
pauseOnConnect: this.pauseOnConnect,
pskCallback: this[kPskCallback],
pskIdentityHint: this[kPskIdentityHint],
});

socket.on('secure', onServerSocketSecure);
Expand Down Expand Up @@ -1058,6 +1152,8 @@ function Server(options, listener) {

this[kHandshakeTimeout] = options.handshakeTimeout || (120 * 1000);
this[kSNICallback] = options.SNICallback;
this[kPskCallback] = options.pskCallback;
this[kPskIdentityHint] = options.pskIdentityHint;

if (typeof this[kHandshakeTimeout] !== 'number') {
throw new ERR_INVALID_ARG_TYPE(
Expand All @@ -1069,6 +1165,18 @@ function Server(options, listener) {
'options.SNICallback', 'function', options.SNICallback);
}

if (this[kPskCallback] && typeof this[kPskCallback] !== 'function') {
throw new ERR_INVALID_ARG_TYPE(
'options.pskCallback', 'function', options.pskCallback);
}
if (this[kPskIdentityHint] && typeof this[kPskIdentityHint] !== 'string') {
throw new ERR_INVALID_ARG_TYPE(
'options.pskIdentityHint',
'string',
options.pskIdentityHint
);
}

// constructor call
net.Server.call(this, options, tlsConnectionListener);

Expand Down Expand Up @@ -1265,6 +1373,8 @@ Server.prototype.setOptions = deprecate(function(options) {
.digest('hex')
.slice(0, 32);
}
if (options.pskCallback) this[kPskCallback] = options.pskCallback;
if (options.pskIdentityHint) this[kPskIdentityHint] = options.pskIdentityHint;
}, 'Server.prototype.setOptions() is deprecated', 'DEP0122');

// SNI Contexts High-Level API
Expand Down Expand Up @@ -1440,7 +1550,8 @@ exports.connect = function connect(...args) {
session: options.session,
ALPNProtocols: options.ALPNProtocols,
requestOCSP: options.requestOCSP,
enableTrace: options.enableTrace
enableTrace: options.enableTrace,
pskCallback: options.pskCallback,
});

tlssock[kConnectOptions] = options;
Expand Down
13 changes: 8 additions & 5 deletions src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,12 @@ constexpr size_t kFsStatsBufferLength =

// Symbols are per-isolate primitives but Environment proxies them
// for the sake of convenience.
#define PER_ISOLATE_SYMBOL_PROPERTIES(V) \
V(handle_onclose_symbol, "handle_onclose") \
V(no_message_symbol, "no_message_symbol") \
V(oninit_symbol, "oninit") \
V(owner_symbol, "owner") \
#define PER_ISOLATE_SYMBOL_PROPERTIES(V) \
V(handle_onclose_symbol, "handle_onclose") \
V(no_message_symbol, "no_message_symbol") \
V(oninit_symbol, "oninit") \
V(owner_symbol, "owner") \
V(onpskexchange_symbol, "onpskexchange") \

// Strings are per-isolate primitives but Environment proxies them
// for the sake of convenience. Strings should be ASCII-only.
Expand Down Expand Up @@ -254,6 +255,7 @@ constexpr size_t kFsStatsBufferLength =
V(host_string, "host") \
V(hostmaster_string, "hostmaster") \
V(http_1_1_string, "http/1.1") \
V(identity_string, "identity") \
V(ignore_string, "ignore") \
V(import_string, "import") \
V(infoaccess_string, "infoAccess") \
Expand Down Expand Up @@ -325,6 +327,7 @@ constexpr size_t kFsStatsBufferLength =
V(priority_string, "priority") \
V(process_string, "process") \
V(promise_string, "promise") \
V(psk_string, "psk") \
V(pubkey_string, "pubkey") \
V(query_string, "query") \
V(raw_string, "raw") \
Expand Down
10 changes: 10 additions & 0 deletions src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2620,6 +2620,16 @@ void SSLWrap<Base>::VerifyError(const FunctionCallbackInfo<Value>& args) {
if (X509* peer_cert = SSL_get_peer_certificate(w->ssl_.get())) {
X509_free(peer_cert);
x509_verify_error = SSL_get_verify_result(w->ssl_.get());
} else {
const SSL_CIPHER* curr_cipher = SSL_get_current_cipher(w->ssl_.get());
const SSL_SESSION* sess = SSL_get_session(w->ssl_.get());
// Allow no-cert for PSK authentication in TLS1.2 and lower.
// In TLS1.3 check that session was reused because TLS1.3 PSK
// looks like session resumption. Is there a better way?
if (SSL_CIPHER_get_auth_nid(curr_cipher) == NID_auth_psk ||
(SSL_SESSION_get_protocol_version(sess) == TLS1_3_VERSION &&
SSL_session_reused(w->ssl_.get())))
return args.GetReturnValue().SetNull();
}

if (x509_verify_error == X509_V_OK)
Expand Down
2 changes: 2 additions & 0 deletions src/node_errors.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ void PrintErrorString(const char* format, ...);
V(ERR_STRING_TOO_LONG, Error) \
V(ERR_TLS_INVALID_PROTOCOL_METHOD, TypeError) \
V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, TypeError) \
V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, Error) \

#define V(code, type) \
inline v8::Local<v8::Value> code(v8::Isolate* isolate, \
Expand Down Expand Up @@ -101,6 +102,7 @@ void PrintErrorString(const char* format, ...);
"Script execution was interrupted by `SIGINT`") \
V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, \
"Cannot serialize externalized SharedArrayBuffer") \
V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, "Failed to set PSK identity hint") \

#define V(code, message) \
inline v8::Local<v8::Value> code(v8::Isolate* isolate) { \
Expand Down
Loading

0 comments on commit bfccc00

Please sign in to comment.