Skip to content
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

tls: remove NPN (next protocol negotation) support #19403

Merged
merged 2 commits into from
Mar 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions deps/openssl/openssl.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,9 @@
# the real driver but that poses a security liability when an attacker
# is able to create a malicious DLL in one of the default search paths.
'OPENSSL_NO_HW',

# Disable NPN (Next Protocol Negotiation), superseded by ALPN.
'OPENSSL_NO_NEXTPROTONEG',
],
'openssl_default_defines_win': [
'MK1MF_BUILD',
Expand Down
4 changes: 0 additions & 4 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -2412,10 +2412,6 @@ the `crypto`, `tls`, and `https` modules and are generally specific to OpenSSL.
<td><code>DH_NOT_SUITABLE_GENERATOR</code></td>
<td></td>
</tr>
<tr>
<td><code>NPN_ENABLED</code></td>
<td></td>
</tr>
<tr>
<td><code>ALPN_ENABLED</code></td>
<td></td>
Expand Down
8 changes: 8 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,14 @@ initialization vectors. It is recommended to derive a key using
[`crypto.createDecipheriv()`][] to obtain the [`Cipher`][] and [`Decipher`][]
objects respectively.

<a id="DEP0107"></a>
### DEP0107: tls.convertNPNProtocols()

Type: Runtime

This was an undocumented helper function not intended for use outside Node.js
core and obsoleted by the removal of NPN (Next Protocol Negotiation) support.

[`--pending-deprecation`]: cli.html#cli_pending_deprecation
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
Expand Down
52 changes: 12 additions & 40 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,22 +104,17 @@ not required and a default ECDHE curve will be used. The `ecdhCurve` property
can be used when creating a TLS Server to specify the list of names of supported
curves to use, see [`tls.createServer()`] for more info.

### ALPN, NPN, and SNI
### ALPN and SNI

<!-- type=misc -->

ALPN (Application-Layer Protocol Negotiation Extension), NPN (Next
Protocol Negotiation) and, SNI (Server Name Indication) are TLS
handshake extensions:
ALPN (Application-Layer Protocol Negotiation Extension) and
SNI (Server Name Indication) are TLS handshake extensions:

* ALPN/NPN - Allows the use of one TLS server for multiple protocols (HTTP,
SPDY, HTTP/2)
* ALPN - Allows the use of one TLS server for multiple protocols (HTTP, HTTP/2)
* SNI - Allows the use of one TLS server for multiple hostnames with different
SSL certificates.

Use of ALPN is recommended over NPN. The NPN extension has never been
formally defined or documented and generally not recommended for use.

### Client-initiated renegotiation attack mitigation

<!-- type=misc -->
Expand Down Expand Up @@ -332,12 +327,9 @@ server. If `tlsSocket.authorized` is `false`, then `socket.authorizationError`
is set to describe how authorization failed. Note that depending on the settings
of the TLS server, unauthorized connections may still be accepted.

The `tlsSocket.npnProtocol` and `tlsSocket.alpnProtocol` properties are strings
that contain the selected NPN and ALPN protocols, respectively. When both NPN
and ALPN extensions are received, ALPN takes precedence over NPN and the next
protocol is selected by ALPN.

When ALPN has no selected protocol, `tlsSocket.alpnProtocol` returns `false`.
The `tlsSocket.alpnProtocol` property is a string that contains the selected
ALPN protocol. When ALPN has no selected protocol, `tlsSocket.alpnProtocol`
equals `false`.

The `tlsSocket.servername` property is a string containing the server name
requested via SNI.
Expand Down Expand Up @@ -468,7 +460,6 @@ changes:
(`isServer` is true) may optionally set `requestCert` to true to request a
client certificate.
* `rejectUnauthorized`: Optional, see [`tls.createServer()`][]
* `NPNProtocols`: Optional, see [`tls.createServer()`][]
* `ALPNProtocols`: Optional, see [`tls.createServer()`][]
* `SNICallback`: Optional, see [`tls.createServer()`][]
* `session` {Buffer} An optional `Buffer` instance containing a TLS session.
Expand Down Expand Up @@ -509,9 +500,9 @@ regardless of whether or not the server's certificate has been authorized. It
is the client's responsibility to check the `tlsSocket.authorized` property to
determine if the server certificate was signed by one of the specified CAs. If
`tlsSocket.authorized === false`, then the error can be found by examining the
`tlsSocket.authorizationError` property. If either ALPN or NPN was used,
the `tlsSocket.alpnProtocol` or `tlsSocket.npnProtocol` properties can be
checked to determine the negotiated protocol.
`tlsSocket.authorizationError` property. If ALPN was used, the
`tlsSocket.alpnProtocol` property can be checked to determine the negotiated
protocol.

### tlsSocket.address()
<!-- YAML
Expand Down Expand Up @@ -841,8 +832,7 @@ changes:
description: The `lookup` option is supported now.
- version: v8.0.0
pr-url: https://github.com/nodejs/node/pull/11984
description: The `ALPNProtocols` and `NPNProtocols` options can
be `Uint8Array`s now.
description: The `ALPNProtocols` option can be a `Uint8Array` now.
- version: v5.3.0, v4.7.0
pr-url: https://github.com/nodejs/node/pull/4246
description: The `secureContext` option is supported now.
Expand All @@ -869,12 +859,6 @@ changes:
verified against the list of supplied CAs. An `'error'` event is emitted if
verification fails; `err.code` contains the OpenSSL error code. Defaults to
`true`.
* `NPNProtocols` {string[]|Buffer[]|Uint8Array[]|Buffer|Uint8Array}
An array of strings, `Buffer`s or `Uint8Array`s, or a single `Buffer` or
`Uint8Array` containing supported NPN protocols. `Buffer`s should have the
format `[len][name][len][name]...` e.g. `0x05hello0x05world`, where the
first byte is the length of the next protocol name. Passing an array is
usually much simpler, e.g. `['hello', 'world']`.
* `ALPNProtocols`: {string[]|Buffer[]|Uint8Array[]|Buffer|Uint8Array}
An array of strings, `Buffer`s or `Uint8Array`s, or a single `Buffer` or
`Uint8Array` containing the supported ALPN protocols. `Buffer`s should have
Expand Down Expand Up @@ -1116,8 +1100,7 @@ changes:
description: The `options` parameter can now include `clientCertEngine`.
- version: v8.0.0
pr-url: https://github.com/nodejs/node/pull/11984
description: The `ALPNProtocols` and `NPNProtocols` options can
be `Uint8Array`s now.
description: The `ALPNProtocols` option can be a `Uint8Array` now.
- version: v5.0.0
pr-url: https://github.com/nodejs/node/pull/2564
description: ALPN options are supported now.
Expand All @@ -1136,23 +1119,13 @@ changes:
* `rejectUnauthorized` {boolean} If not `false` the server will reject any
connection which is not authorized with the list of supplied CAs. This
option only has an effect if `requestCert` is `true`. Defaults to `true`.
* `NPNProtocols` {string[]|Buffer[]|Uint8Array[]|Buffer|Uint8Array}
An array of strings, `Buffer`s or `Uint8Array`s, or a single `Buffer` or
`Uint8Array` containing supported NPN protocols. `Buffer`s should have the
format `[len][name][len][name]...` e.g. `0x05hello0x05world`, where the
first byte is the length of the next protocol name. Passing an array is
usually much simpler, e.g. `['hello', 'world']`.
(Protocols should be ordered by their priority.)
* `ALPNProtocols`: {string[]|Buffer[]|Uint8Array[]|Buffer|Uint8Array}
An array of strings, `Buffer`s or `Uint8Array`s, or a single `Buffer` or
`Uint8Array` containing the supported ALPN protocols. `Buffer`s should have
the format `[len][name][len][name]...` e.g. `0x05hello0x05world`, where the
first byte is the length of the next protocol name. Passing an array is
usually much simpler, e.g. `['hello', 'world']`.
(Protocols should be ordered by their priority.)
When the server receives both NPN and ALPN extensions from the client,
ALPN takes precedence over NPN and the server does not send an NPN
extension to the client.
* `SNICallback(servername, cb)` {Function} A function that will be called if
the client supports SNI TLS extension. Two arguments will be passed when
called: `servername` and `cb`. `SNICallback` should invoke `cb(null, ctx)`,
Expand Down Expand Up @@ -1333,7 +1306,6 @@ changes:
* `server` {net.Server} An optional [`net.Server`][] instance
* `requestCert`: Optional, see [`tls.createServer()`][]
* `rejectUnauthorized`: Optional, see [`tls.createServer()`][]
* `NPNProtocols`: Optional, see [`tls.createServer()`][]
* `ALPNProtocols`: Optional, see [`tls.createServer()`][]
* `SNICallback`: Optional, see [`tls.createServer()`][]
* `session` {Buffer} An optional `Buffer` instance containing a TLS session.
Expand Down
13 changes: 0 additions & 13 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,6 @@ function initRead(tls, wrapped) {
function TLSSocket(socket, opts) {
const tlsOptions = Object.assign({}, opts);

if (tlsOptions.NPNProtocols)
tls.convertNPNProtocols(tlsOptions.NPNProtocols, tlsOptions);
if (tlsOptions.ALPNProtocols)
tls.convertALPNProtocols(tlsOptions.ALPNProtocols, tlsOptions);

Expand All @@ -306,7 +304,6 @@ function TLSSocket(socket, opts) {
this._controlReleased = false;
this._SNICallback = null;
this.servername = null;
this.npnProtocol = null;
this.alpnProtocol = null;
this.authorized = false;
this.authorizationError = null;
Expand Down Expand Up @@ -529,9 +526,6 @@ TLSSocket.prototype._init = function(socket, wrap) {
ssl.enableCertCb();
}

if (process.features.tls_npn && options.NPNProtocols)
ssl.setNPNProtocols(options.NPNProtocols);

if (process.features.tls_alpn && options.ALPNProtocols) {
// keep reference in secureContext not to be GC-ed
ssl._secureContext.alpnBuffer = options.ALPNProtocols;
Expand Down Expand Up @@ -630,10 +624,6 @@ TLSSocket.prototype._releaseControl = function() {
};

TLSSocket.prototype._finishInit = function() {
if (process.features.tls_npn) {
this.npnProtocol = this._handle.getNegotiatedProtocol();
}

if (process.features.tls_alpn) {
this.alpnProtocol = this._handle.getALPNNegotiatedProtocol();
}
Expand Down Expand Up @@ -790,7 +780,6 @@ function tlsConnectionListener(rawSocket) {
requestCert: this.requestCert,
rejectUnauthorized: this.rejectUnauthorized,
handshakeTimeout: this[kHandshakeTimeout],
NPNProtocols: this.NPNProtocols,
ALPNProtocols: this.ALPNProtocols,
SNICallback: this[kSNICallback] || SNICallback
});
Expand Down Expand Up @@ -982,7 +971,6 @@ Server.prototype.setOptions = function(options) {
else
this.honorCipherOrder = true;
if (secureOptions) this.secureOptions = secureOptions;
if (options.NPNProtocols) tls.convertNPNProtocols(options.NPNProtocols, this);
if (options.ALPNProtocols)
tls.convertALPNProtocols(options.ALPNProtocols, this);
if (options.sessionIdContext) {
Expand Down Expand Up @@ -1149,7 +1137,6 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
requestCert: true,
rejectUnauthorized: options.rejectUnauthorized !== false,
session: options.session,
NPNProtocols: options.NPNProtocols,
ALPNProtocols: options.ALPNProtocols,
requestOCSP: options.requestOCSP
});
Expand Down
4 changes: 0 additions & 4 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ function Server(opts, requestListener) {
}
opts = util._extend({}, opts);

if (process.features.tls_npn && !opts.NPNProtocols) {
opts.NPNProtocols = ['http/1.1', 'http/1.0'];
}

if (process.features.tls_alpn && !opts.ALPNProtocols) {
// http/1.0 is not defined as Protocol IDs in IANA
// http://www.iana.org/assignments/tls-extensiontype-values
Expand Down
4 changes: 2 additions & 2 deletions lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ function convertProtocols(protocols) {
return buff;
}

exports.convertNPNProtocols = function(protocols, out) {
exports.convertNPNProtocols = internalUtil.deprecate(function(protocols, out) {
// If protocols is Array - translate it into buffer
if (Array.isArray(protocols)) {
out.NPNProtocols = convertProtocols(protocols);
} else if (isUint8Array(protocols)) {
// Copy new buffer not to be modified by user.
out.NPNProtocols = Buffer.from(protocols);
}
};
}, 'tls.convertNPNProtocols() is deprecated.', 'DEP0107');

exports.convertALPNProtocols = function(protocols, out) {
// If protocols is Array - translate it into buffer
Expand Down
4 changes: 2 additions & 2 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,8 @@
'mkssldef_flags': [
# Categories to export.
'-CAES,BF,BIO,DES,DH,DSA,EC,ECDH,ECDSA,ENGINE,EVP,HMAC,MD4,MD5,'
'NEXTPROTONEG,PSK,RC2,RC4,RSA,SHA,SHA0,SHA1,SHA256,SHA512,SOCK,'
'STDIO,TLSEXT,FP_API',
'PSK,RC2,RC4,RSA,SHA,SHA0,SHA1,SHA256,SHA512,SOCK,STDIO,TLSEXT,'
'FP_API',
# Defines.
'-DWIN32',
# Symbols to filter from the export list.
Expand Down
2 changes: 0 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ struct PackageConfig {
V(contextify_context_private_symbol, "node:contextify:context") \
V(contextify_global_private_symbol, "node:contextify:global") \
V(decorated_private_symbol, "node:decorated") \
V(npn_buffer_private_symbol, "node:npnBuffer") \
V(selected_npn_buffer_private_symbol, "node:selectedNpnBuffer") \
V(napi_env, "node:napi:env") \
V(napi_wrapper, "node:napi:wrapper") \

Expand Down
7 changes: 0 additions & 7 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2824,13 +2824,6 @@ static Local<Object> GetFeatures(Environment* env) {
// TODO(bnoordhuis) ping libuv
obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "ipv6"), True(env->isolate()));

#ifndef OPENSSL_NO_NEXTPROTONEG
Local<Boolean> tls_npn = True(env->isolate());
#else
Local<Boolean> tls_npn = False(env->isolate());
#endif
obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_npn"), tls_npn);

#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
Local<Boolean> tls_alpn = True(env->isolate());
#else
Expand Down
5 changes: 0 additions & 5 deletions src/node_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -971,11 +971,6 @@ void DefineOpenSSLConstants(Local<Object> target) {
NODE_DEFINE_CONSTANT(target, DH_NOT_SUITABLE_GENERATOR);
#endif

#ifndef OPENSSL_NO_NEXTPROTONEG
#define NPN_ENABLED 1
NODE_DEFINE_CONSTANT(target, NPN_ENABLED);
#endif

#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
#define ALPN_ENABLED 1
NODE_DEFINE_CONSTANT(target, ALPN_ENABLED);
Expand Down
Loading