[patch] nodejs-11.2.0-openssl-compat.patch

nodejs-11.2.0-openssl-compat.patch (text/plain), 11.31 KB, created by Guillaume Ceccarelli on 2018-11-21 18:47:39 UTC

diff --git a/doc/api/tls.md b/doc/api/tls.md
index 392052986e..b0b8180351 100644
--- a/doc/api/tls.md
+++ b/doc/api/tls.md
@@ -1043,6 +1043,10 @@ changes:
     pr-url: https://github.com/nodejs/node/pull/4099
     description: The `ca` option can now be a single string containing multiple
                  CA certificates.
+  - version: XXX
+    pr-url: XXX
+    description: The `min_version` and `max_version` can be used to restrict
+                 the allowed TLS protocol versions.
 -->
 
 * `options` {Object}
@@ -1103,6 +1107,16 @@ changes:
     passphrase: <string>]}`. The object form can only occur in an array.
     `object.passphrase` is optional. Encrypted keys will be decrypted with
     `object.passphrase` if provided, or `options.passphrase` if it is not.
+  * `max_version`: Maximum TLS version to allow. One of `'TLSv1.3'`, `TLSv1.2'`,
+    `'TLSv1.1'`, or `'TLSv1'`. Optional, defaults to `'TLSv1.2'`. Note that
+    TLS1.3 is not currently supported, do not attempt to allow it. If
+    `secureProtocol` is used to select a specific protocol version,
+    `max_version` will be ignored.
+  * `min_version`: Minimum TLS version to allow. One of `'TLSv1.3'`, `TLSv1.2'`,
+    `'TLSv1.1'`, or `'TLSv1'`. Optional, defaults to `'TLSv1'`. Note that
+    TLS1.3 is not currently supported, do not attempt to allow it. If
+    `secureProtocol` is used to select a specific protocol version,
+    `min_version` will be ignored.
   * `passphrase` {string} Shared passphrase used for a single private key and/or
     a PFX.
   * `pfx` {string|string[]|Buffer|Buffer[]|Object[]} PFX or PKCS12 encoded
diff --git a/lib/_tls_common.js b/lib/_tls_common.js
index 843c582b1f..1d5117169c 100644
--- a/lib/_tls_common.js
+++ b/lib/_tls_common.js
@@ -28,16 +28,18 @@ const {
   ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED,
   ERR_INVALID_ARG_TYPE
 } = require('internal/errors').codes;
-
 const { SSL_OP_CIPHER_SERVER_PREFERENCE } = internalBinding('constants').crypto;
 
 // Lazily loaded from internal/crypto/util.
 let toBuf = null;
 
+
 const { SecureContext: NativeSecureContext } = internalBinding('crypto');
-function SecureContext(secureProtocol, secureOptions, context) {
+function SecureContext(secureProtocol, secureOptions, context, min_version,
+                       max_version) {
   if (!(this instanceof SecureContext)) {
-    return new SecureContext(secureProtocol, secureOptions, context);
+    return new SecureContext(secureProtocol, secureOptions, context,
+                             min_version, max_version);
   }
 
   if (context) {
@@ -46,9 +48,9 @@ function SecureContext(secureProtocol, secureOptions, context) {
     this.context = new NativeSecureContext();
 
     if (secureProtocol) {
-      this.context.init(secureProtocol);
+      this.context.init(min_version, max_version, secureProtocol);
     } else {
-      this.context.init();
+      this.context.init(min_version, max_version);
     }
   }
 
@@ -75,7 +77,9 @@ exports.createSecureContext = function createSecureContext(options, context) {
   if (options.honorCipherOrder)
     secureOptions |= SSL_OP_CIPHER_SERVER_PREFERENCE;
 
-  const c = new SecureContext(options.secureProtocol, secureOptions, context);
+  const c = new SecureContext(options.secureProtocol, secureOptions, context,
+                              options.min_version || tls.DEFAULT_MIN_VERSION,
+                              options.max_version || tls.DEFAULT_MAX_VERSION);
   var i;
   var val;
 
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 36440ad4ea..b7f2a49062 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -911,6 +911,16 @@ Server.prototype.setSecureContext = function(options) {
   else
     this.ca = undefined;
 
+  if (options.min_version)
+    this.min_version = options.min_version;
+  else
+    this.min_version = undefined;
+
+  if (options.max_version)
+    this.max_version = options.max_version;
+  else
+    this.max_version = undefined;
+
   if (options.secureProtocol)
     this.secureProtocol = options.secureProtocol;
   else
@@ -967,6 +977,8 @@ Server.prototype.setSecureContext = function(options) {
     ciphers: this.ciphers,
     ecdhCurve: this.ecdhCurve,
     dhparam: this.dhparam,
+    min_version: this.min_version,
+    max_version: this.max_version,
     secureProtocol: this.secureProtocol,
     secureOptions: this.secureOptions,
     honorCipherOrder: this.honorCipherOrder,
@@ -1019,6 +1031,8 @@ Server.prototype.setOptions = function(options) {
   if (options.clientCertEngine)
     this.clientCertEngine = options.clientCertEngine;
   if (options.ca) this.ca = options.ca;
+  if (options.min_version) this.min_version = options.min_version;
+  if (options.max_version) this.max_version = options.max_version;
   if (options.secureProtocol) this.secureProtocol = options.secureProtocol;
   if (options.crl) this.crl = options.crl;
   if (options.ciphers) this.ciphers = options.ciphers;
diff --git a/lib/https.js b/lib/https.js
index 12db2f452c..11a0fae9c8 100644
--- a/lib/https.js
+++ b/lib/https.js
@@ -186,6 +186,14 @@ Agent.prototype.getName = function getName(options) {
   if (options.servername && options.servername !== options.host)
     name += options.servername;
 
+  name += ':';
+  if (options.min_version)
+    name += options.min_version;
+
+  name += ':';
+  if (options.max_version)
+    name += options.max_version;
+
   name += ':';
   if (options.secureProtocol)
     name += options.secureProtocol;
diff --git a/lib/tls.js b/lib/tls.js
index d6b86a4103..0ff8cdb8a2 100644
--- a/lib/tls.js
+++ b/lib/tls.js
@@ -53,6 +53,13 @@ exports.DEFAULT_CIPHERS =
 
 exports.DEFAULT_ECDH_CURVE = 'auto';
 
+// Disable TLS1.3 by default. The only reason for enabling it for now is to work
+// on fixing cipher suite incompatibilities with TLS1.2 that prevent node from
+// working with TLS1.3 in OpenSSL 1.1.1.
+exports.DEFAULT_MAX_VERSION = 'TLSv1.2';
+
+exports.DEFAULT_MIN_VERSION = 'TLSv1';
+
 exports.getCiphers = internalUtil.cachedResult(
   () => internalUtil.filterDuplicateStrings(binding.getSSLCiphers(), true)
 );
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index eab8aae180..98b6be2565 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -391,6 +391,24 @@ void SecureContext::New(const FunctionCallbackInfo<Value>& args) {
 }
 
 
+int string_to_tls_protocol(const char* version_str) {
+  int version;
+
+  if (strcmp(version_str, "TLSv1.3") == 0) {
+    version = TLS1_3_VERSION;
+  } else if (strcmp(version_str, "TLSv1.2") == 0) {
+    version = TLS1_2_VERSION;
+  } else if (strcmp(version_str, "TLSv1.1") == 0) {
+    version = TLS1_1_VERSION;
+  } else if (strcmp(version_str, "TLSv1") == 0) {
+    version = TLS1_VERSION;
+  } else {
+    version = 0;
+  }
+  return version;
+}
+
+
 void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
   SecureContext* sc;
   ASSIGN_OR_RETURN_UNWRAP(&sc, args.Holder());
@@ -398,10 +416,21 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
 
   int min_version = 0;
   int max_version = 0;
+
+  if (args[0]->IsString()) {
+    const node::Utf8Value min(env->isolate(), args[0]);
+    min_version = string_to_tls_protocol(*min);
+  }
+
+  if (args[1]->IsString()) {
+    const node::Utf8Value max(env->isolate(), args[1]);
+    max_version = string_to_tls_protocol(*max);
+  }
+
   const SSL_METHOD* method = TLS_method();
 
-  if (args.Length() == 1 && args[0]->IsString()) {
-    const node::Utf8Value sslmethod(env->isolate(), args[0]);
+  if (args.Length() == 3 && args[2]->IsString()) {
+    const node::Utf8Value sslmethod(env->isolate(), args[2]);
 
     // Note that SSLv2 and SSLv3 are disallowed but SSLv23_method and friends
     // are still accepted.  They are OpenSSL's way of saying that all known
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 52d4e73813..d452a8d673 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -222,7 +222,10 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) {
     }
   }
 
-  if (where & SSL_CB_HANDSHAKE_DONE) {
+  // SSL_CB_HANDSHAKE_START and SSL_CB_HANDSHAKE_DONE are called
+  // sending HelloRequest in OpenSSL-1.1.1.
+  // We need to check whether this is in a renegotiation state or not.
+  if (where & SSL_CB_HANDSHAKE_DONE && !SSL_renegotiate_pending(ssl)) {
     Local<Value> callback;
 
     c->established_ = true;
diff --git a/test/parallel/test-https-agent-getname.js b/test/parallel/test-https-agent-getname.js
index c29e09731d..d27763c215 100644
--- a/test/parallel/test-https-agent-getname.js
+++ b/test/parallel/test-https-agent-getname.js
@@ -12,7 +12,7 @@ const agent = new https.Agent();
 // empty options
 assert.strictEqual(
   agent.getName({}),
-  'localhost:::::::::::::::::'
+  'localhost:::::::::::::::::::'
 );
 
 // pass all options arguments
@@ -39,5 +39,5 @@ const options = {
 assert.strictEqual(
   agent.getName(options),
   '0.0.0.0:443:192.168.1.1:ca:cert::ciphers:key:pfx:false:localhost:' +
-    'secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext'
+    '::secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext'
 );
diff --git a/test/parallel/test-tls-handshake-error.js b/test/parallel/test-tls-handshake-error.js
index 2aef1adcc3..13f366f41f 100644
--- a/test/parallel/test-tls-handshake-error.js
+++ b/test/parallel/test-tls-handshake-error.js
@@ -16,12 +16,12 @@ const server = tls.createServer({
   rejectUnauthorized: true
 }, function(c) {
 }).listen(0, common.mustCall(function() {
-  assert.throws(() => {
-    tls.connect({
-      port: this.address().port,
-      ciphers: 'RC4'
-    }, common.mustNotCall());
-  }, /no cipher match/i);
-
-  server.close();
+  const client = tls.connect({
+    port: this.address().port,
+    ciphers: 'RC4'
+  }, common.mustNotCall());
+  client.on('error', common.mustCall((err) => {
+    assert(/No ciphers/.test(err.message));
+    server.close();
+  }));
 }));
diff --git a/test/parallel/test-tls-set-ciphers-error.js b/test/parallel/test-tls-set-ciphers-error.js
deleted file mode 100644
index 5ef08dda04..0000000000
--- a/test/parallel/test-tls-set-ciphers-error.js
+++ /dev/null
@@ -1,22 +0,0 @@
-'use strict';
-const common = require('../common');
-
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-
-const assert = require('assert');
-const tls = require('tls');
-const fixtures = require('../common/fixtures');
-
-{
-  const options = {
-    key: fixtures.readKey('agent2-key.pem'),
-    cert: fixtures.readKey('agent2-cert.pem'),
-    ciphers: 'aes256-sha'
-  };
-  assert.throws(() => tls.createServer(options, common.mustNotCall()),
-                /no cipher match/i);
-  options.ciphers = 'FOOBARBAZ';
-  assert.throws(() => tls.createServer(options, common.mustNotCall()),
-                /no cipher match/i);
-}
diff --git a/test/sequential/test-tls-connect.js b/test/sequential/test-tls-connect.js
index 291747aea7..680ab7bc86 100644
--- a/test/sequential/test-tls-connect.js
+++ b/test/sequential/test-tls-connect.js
@@ -50,12 +50,13 @@ const tls = require('tls');
   const cert = fixtures.readSync('test_cert.pem');
   const key = fixtures.readSync('test_key.pem');
 
-  assert.throws(() => {
-    tls.connect({
-      cert: cert,
-      key: key,
-      port: common.PORT,
-      ciphers: 'rick-128-roll'
-    }, common.mustNotCall());
-  }, /no cipher match/i);
+  const client = tls.connect({
+    cert: cert,
+    key: key,
+    port: common.PORT,
+    ciphers: 'rick-128-roll'
+  }, common.mustNotCall());
+  client.on('error', common.mustCall((err) => {
+    assert.strictEqual(err.code, 'ECONNREFUSED');
+  }));
 }