tls: drop NPN (next protocol negotiation) support

NPN has been superseded by ALPN.  Chrome and Firefox removed support for
NPN in 2016 and 2017 respectively to no ill effect.

Fixes: #14602
PR-URL: #19403
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Author: bnoordhuis

deps/openssl/openssl.gypi

@@ -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',

doc/api/crypto.md

@@ -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>

doc/api/tls.md

@@ -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 -->
@@ -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.
@@ -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.
@@ -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
@@ -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.
@@ -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
@@ -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.
@@ -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)`,
@@ -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.

lib/_tls_wrap.js

@@ -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);
 
@@ -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;
@@ -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;
@@ -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();
   }
@@ -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
   });
@@ -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) {
@@ -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
   });

lib/https.js

@@ -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

node.gyp

@@ -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.

src/env.h

@@ -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")                                        \
 

src/node.cc

@@ -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

src/node_constants.cc

@@ -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);