From b5a3841f05ee843f8e0ae484bb914b54ae1a0a75 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 1 Jun 2018 14:57:51 -0700 Subject: [PATCH 1/2] process,tls: stop relying on process.features The `process.features` property is mutable by userland such that `delete process.features` will cause SNI and ALPN support for fail. Add the necessary properties to `process.config('binding')` so that we are not relying on a user-modifiable object. Also clean up the `GetFeatures` funciton just a bit. --- lib/_tls_wrap.js | 12 ++++++++---- lib/https.js | 3 ++- src/node.cc | 40 +++++++++++++++++++++------------------- src/node_config.cc | 12 ++++++++++++ 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 77b37c54f0aee0..291fb980d7513e 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -38,6 +38,10 @@ const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); const { SecureContext: NativeSecureContext } = process.binding('crypto'); +const { + tlsALPN, + tlsSNI +} = process.binding('config'); const { ERR_INVALID_ARG_TYPE, ERR_MULTIPLE_CALLBACK, @@ -512,7 +516,7 @@ TLSSocket.prototype._init = function(socket, wrap) { // If custom SNICallback was given, or if // there're SNI contexts to perform match against - // set `.onsniselect` callback. - if (process.features.tls_sni && + if (tlsSNI && options.isServer && options.SNICallback && (options.SNICallback !== SNICallback || @@ -522,7 +526,7 @@ TLSSocket.prototype._init = function(socket, wrap) { ssl.enableCertCb(); } - if (process.features.tls_alpn && options.ALPNProtocols) { + if (tlsALPN && options.ALPNProtocols) { // keep reference in secureContext not to be GC-ed ssl._secureContext.alpnBuffer = options.ALPNProtocols; ssl.setALPNProtocols(ssl._secureContext.alpnBuffer); @@ -620,11 +624,11 @@ TLSSocket.prototype._releaseControl = function() { }; TLSSocket.prototype._finishInit = function() { - if (process.features.tls_alpn) { + if (tlsALPN) { this.alpnProtocol = this._handle.getALPNNegotiatedProtocol(); } - if (process.features.tls_sni) { + if (tlsSNI) { this.servername = this._handle.getServername(); } diff --git a/lib/https.js b/lib/https.js index 169c61900cff1c..8fffe94fcdd083 100644 --- a/lib/https.js +++ b/lib/https.js @@ -31,6 +31,7 @@ const { Server: HttpServer, _connectionListener } = require('_http_server'); +const { tlsALPN } = process.binding('config'); const { ClientRequest } = require('_http_client'); const { inherits } = util; const debug = util.debuglog('https'); @@ -48,7 +49,7 @@ function Server(opts, requestListener) { } opts = util._extend({}, opts); - if (process.features.tls_alpn && !opts.ALPNProtocols) { + if (tlsALPN && !opts.ALPNProtocols) { // http/1.0 is not defined as Protocol IDs in IANA // http://www.iana.org/assignments/tls-extensiontype-values // /tls-extensiontype-values.xhtml#alpn-protocol-ids diff --git a/src/node.cc b/src/node.cc index bf3aae2d35f773..19c4b65e20dedb 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2456,44 +2456,46 @@ static void GetParentProcessId(Local property, static Local GetFeatures(Environment* env) { - EscapableHandleScope scope(env->isolate()); + Isolate* isolate = env->isolate(); + EscapableHandleScope scope(isolate); + Local trueValue = True(isolate); + Local falseValue = False(isolate); - Local obj = Object::New(env->isolate()); + Local obj = Object::New(isolate); #if defined(DEBUG) && DEBUG - Local debug = True(env->isolate()); + Local debug = trueValue; #else - Local debug = False(env->isolate()); + Local debug = falseValue; #endif // defined(DEBUG) && DEBUG + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "debug"), debug); - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "debug"), debug); - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "uv"), True(env->isolate())); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "uv"), trueValue); // TODO(bnoordhuis) ping libuv - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "ipv6"), True(env->isolate())); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "ipv6"), trueValue); #ifdef TLSEXT_TYPE_application_layer_protocol_negotiation - Local tls_alpn = True(env->isolate()); + Local tls_alpn = trueValue; #else - Local tls_alpn = False(env->isolate()); + Local tls_alpn = falseValue; #endif - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_alpn"), tls_alpn); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "tls_alpn"), tls_alpn); #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB - Local tls_sni = True(env->isolate()); + Local tls_sni = trueValue; #else - Local tls_sni = False(env->isolate()); + Local tls_sni = falseValue; #endif - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_sni"), tls_sni); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "tls_sni"), tls_sni); #if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) - Local tls_ocsp = True(env->isolate()); + Local tls_ocsp = trueValue; #else - Local tls_ocsp = False(env->isolate()); + Local tls_ocsp = falseValue; #endif // !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_ocsp"), tls_ocsp); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "tls_ocsp"), tls_ocsp); - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls"), - Boolean::New(env->isolate(), - get_builtin_module("crypto") != nullptr)); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "tls"), + Boolean::New(isolate, get_builtin_module("crypto") != nullptr)); return scope.Escape(obj); } diff --git a/src/node_config.cc b/src/node_config.cc index 603d55491a259b..082f4d1c301efa 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -4,6 +4,10 @@ #include "util-inl.h" #include "node_debug_options.h" +#if HAVE_OPENSSL +#include "node_crypto.h" +#endif + namespace node { using v8::Boolean; @@ -48,6 +52,14 @@ static void Initialize(Local target, READONLY_BOOLEAN_PROPERTY("fipsForced"); #endif +#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation + READONLY_BOOLEAN_PROPERTY("tlsALPN"); +#endif + +#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB + READONLY_BOOLEAN_PROPERTY("tlsSNI"); +#endif + #ifdef NODE_HAVE_I18N_SUPPORT READONLY_BOOLEAN_PROPERTY("hasIntl"); From f91ce4ca4138d6217d8a2dab12b2983b24a6f36e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 1 Jun 2018 15:11:58 -0700 Subject: [PATCH 2/2] [Squash] nit --- src/node.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/node.cc b/src/node.cc index 19c4b65e20dedb..c4a05a6a54c31f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2458,39 +2458,39 @@ static void GetParentProcessId(Local property, static Local GetFeatures(Environment* env) { Isolate* isolate = env->isolate(); EscapableHandleScope scope(isolate); - Local trueValue = True(isolate); - Local falseValue = False(isolate); + Local true_value = True(isolate); + Local false_value = False(isolate); Local obj = Object::New(isolate); #if defined(DEBUG) && DEBUG - Local debug = trueValue; + Local debug = true_value; #else - Local debug = falseValue; + Local debug = false_value; #endif // defined(DEBUG) && DEBUG obj->Set(FIXED_ONE_BYTE_STRING(isolate, "debug"), debug); - obj->Set(FIXED_ONE_BYTE_STRING(isolate, "uv"), trueValue); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "uv"), true_value); // TODO(bnoordhuis) ping libuv - obj->Set(FIXED_ONE_BYTE_STRING(isolate, "ipv6"), trueValue); + obj->Set(FIXED_ONE_BYTE_STRING(isolate, "ipv6"), true_value); #ifdef TLSEXT_TYPE_application_layer_protocol_negotiation - Local tls_alpn = trueValue; + Local tls_alpn = true_value; #else - Local tls_alpn = falseValue; + Local tls_alpn = false_value; #endif obj->Set(FIXED_ONE_BYTE_STRING(isolate, "tls_alpn"), tls_alpn); #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB - Local tls_sni = trueValue; + Local tls_sni = true_value; #else - Local tls_sni = falseValue; + Local tls_sni = false_value; #endif obj->Set(FIXED_ONE_BYTE_STRING(isolate, "tls_sni"), tls_sni); #if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) - Local tls_ocsp = trueValue; + Local tls_ocsp = true_value; #else - Local tls_ocsp = falseValue; + Local tls_ocsp = false_value; #endif // !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) obj->Set(FIXED_ONE_BYTE_STRING(isolate, "tls_ocsp"), tls_ocsp);