From a76f0298184115971bac56dd369418786569f76d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 2 Jun 2018 10:52:59 +0200 Subject: [PATCH] lib,src: remove openssl feature conditionals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove compile-time and run-time conditionals for features that OpenSSL 1.0.0 and 1.0.1 didn't support: ALPN, OCSP and/or SNI. They are no longer necessary since our baseline is OpenSSL 1.0.2. PR-URL: https://github.com/nodejs/node/pull/21094 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Tobias Nießen Reviewed-By: Trivikram Kamat Reviewed-By: Colin Ihrig --- lib/_tls_wrap.js | 15 +++------- lib/https.js | 2 +- src/node.cc | 28 +++++-------------- src/node_crypto.cc | 27 +----------------- src/node_crypto.h | 9 ------ src/tls_wrap.cc | 8 ------ src/tls_wrap.h | 3 -- test/parallel/test-tls-alpn-server-client.js | 5 ---- test/parallel/test-tls-empty-sni-context.js | 3 -- test/parallel/test-tls-ocsp-callback.js | 3 -- test/parallel/test-tls-sni-option.js | 3 -- test/parallel/test-tls-sni-server-client.js | 3 -- test/parallel/test-tls-snicallback-error.js | 3 -- ...socket-constructor-alpn-options-parsing.js | 3 -- 14 files changed, 13 insertions(+), 102 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 77b37c54f0aee0..7b16abab87bdc0 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -512,8 +512,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 && - options.isServer && + if (options.isServer && options.SNICallback && (options.SNICallback !== SNICallback || (options.server && options.server._contexts.length))) { @@ -522,7 +521,7 @@ TLSSocket.prototype._init = function(socket, wrap) { ssl.enableCertCb(); } - if (process.features.tls_alpn && options.ALPNProtocols) { + if (options.ALPNProtocols) { // keep reference in secureContext not to be GC-ed ssl._secureContext.alpnBuffer = options.ALPNProtocols; ssl.setALPNProtocols(ssl._secureContext.alpnBuffer); @@ -620,15 +619,9 @@ TLSSocket.prototype._releaseControl = function() { }; TLSSocket.prototype._finishInit = function() { - if (process.features.tls_alpn) { - this.alpnProtocol = this._handle.getALPNNegotiatedProtocol(); - } - - if (process.features.tls_sni) { - this.servername = this._handle.getServername(); - } - debug('secure established'); + this.alpnProtocol = this._handle.getALPNNegotiatedProtocol(); + this.servername = this._handle.getServername(); this._secureEstablished = true; if (this._tlsOptions.handshakeTimeout > 0) this.setTimeout(0, this._handleTimeout); diff --git a/lib/https.js b/lib/https.js index 53a8a2751df9fa..43bd6ee06c775c 100644 --- a/lib/https.js +++ b/lib/https.js @@ -48,7 +48,7 @@ function Server(opts, requestListener) { } opts = util._extend({}, opts); - if (process.features.tls_alpn && !opts.ALPNProtocols) { + if (!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 ff3d149863927e..75dbafa1ab48f6 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2373,30 +2373,16 @@ static Local GetFeatures(Environment* env) { // TODO(bnoordhuis) ping libuv obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "ipv6"), True(env->isolate())); -#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation - Local tls_alpn = True(env->isolate()); +#ifdef HAVE_OPENSSL + Local have_openssl = True(env->isolate()); #else - Local tls_alpn = False(env->isolate()); + Local have_openssl = False(env->isolate()); #endif - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_alpn"), tls_alpn); -#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB - Local tls_sni = True(env->isolate()); -#else - Local tls_sni = False(env->isolate()); -#endif - obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_sni"), tls_sni); - -#if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) - Local tls_ocsp = True(env->isolate()); -#else - Local tls_ocsp = False(env->isolate()); -#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(env->isolate(), "tls"), - Boolean::New(env->isolate(), - get_builtin_module("crypto") != nullptr)); + obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_alpn"), have_openssl); + obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_sni"), have_openssl); + obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_ocsp"), have_openssl); + obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls"), have_openssl); return scope.Escape(obj); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index d5d1b031c5e535..2339dc833590d2 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -133,16 +133,10 @@ template int SSLWrap::NewSessionCallback(SSL* s, template void SSLWrap::OnClientHello( void* arg, const ClientHelloParser::ClientHello& hello); - -#ifdef NODE__HAVE_TLSEXT_STATUS_CB template int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg); -#endif - template void SSLWrap::DestroySSL(); template int SSLWrap::SSLCertCallback(SSL* s, void* arg); template void SSLWrap::WaitForCertCb(CertCb cb, void* arg); - -#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation template int SSLWrap::SelectALPNCallback( SSL* s, const unsigned char** out, @@ -150,7 +144,6 @@ template int SSLWrap::SelectALPNCallback( const unsigned char* in, unsigned int inlen, void* arg); -#endif // TLSEXT_TYPE_application_layer_protocol_negotiation static int PasswordCallback(char* buf, int size, int rwflag, void* u) { @@ -1387,11 +1380,9 @@ void SSLWrap::AddMethods(Environment* env, Local t) { template void SSLWrap::ConfigureSecureContext(SecureContext* sc) { -#ifdef NODE__HAVE_TLSEXT_STATUS_CB // OCSP stapling SSL_CTX_set_tlsext_status_cb(sc->ctx_.get(), TLSExtStatusCallback); SSL_CTX_set_tlsext_status_arg(sc->ctx_.get(), nullptr); -#endif // NODE__HAVE_TLSEXT_STATUS_CB } @@ -2019,7 +2010,6 @@ void SSLWrap::NewSessionDone(const FunctionCallbackInfo& args) { template void SSLWrap::SetOCSPResponse(const FunctionCallbackInfo& args) { -#ifdef NODE__HAVE_TLSEXT_STATUS_CB Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); Environment* env = w->env(); @@ -2030,18 +2020,15 @@ void SSLWrap::SetOCSPResponse(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "OCSP response"); w->ocsp_response_.Reset(args.GetIsolate(), args[0].As()); -#endif // NODE__HAVE_TLSEXT_STATUS_CB } template void SSLWrap::RequestOCSP(const FunctionCallbackInfo& args) { -#ifdef NODE__HAVE_TLSEXT_STATUS_CB Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); SSL_set_tlsext_status_type(w->ssl_.get(), TLSEXT_STATUSTYPE_ocsp); -#endif // NODE__HAVE_TLSEXT_STATUS_CB } @@ -2226,7 +2213,6 @@ void SSLWrap::GetProtocol(const FunctionCallbackInfo& args) { } -#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation template int SSLWrap::SelectALPNCallback(SSL* s, const unsigned char** out, @@ -2256,13 +2242,11 @@ int SSLWrap::SelectALPNCallback(SSL* s, return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK : SSL_TLSEXT_ERR_NOACK; } -#endif // TLSEXT_TYPE_application_layer_protocol_negotiation template void SSLWrap::GetALPNNegotiatedProto( const FunctionCallbackInfo& args) { -#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); @@ -2276,13 +2260,11 @@ void SSLWrap::GetALPNNegotiatedProto( args.GetReturnValue().Set( OneByteString(args.GetIsolate(), alpn_proto, alpn_proto_len)); -#endif // TLSEXT_TYPE_application_layer_protocol_negotiation } template void SSLWrap::SetALPNProtocols(const FunctionCallbackInfo& args) { -#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); Environment* env = w->env(); @@ -2306,11 +2288,9 @@ void SSLWrap::SetALPNProtocols(const FunctionCallbackInfo& args) { SelectALPNCallback, nullptr); } -#endif // TLSEXT_TYPE_application_layer_protocol_negotiation } -#ifdef NODE__HAVE_TLSEXT_STATUS_CB template int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { Base* w = static_cast(SSL_get_app_data(s)); @@ -2354,7 +2334,6 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { return SSL_TLSEXT_ERR_OK; } } -#endif // NODE__HAVE_TLSEXT_STATUS_CB template @@ -2396,11 +2375,7 @@ int SSLWrap::SSLCertCallback(SSL* s, void* arg) { info->Set(context, env->servername_string(), str).FromJust(); } - bool ocsp = false; -#ifdef NODE__HAVE_TLSEXT_STATUS_CB - ocsp = SSL_get_tlsext_status_type(s) == TLSEXT_STATUSTYPE_ocsp; -#endif - + const bool ocsp = (SSL_get_tlsext_status_type(s) == TLSEXT_STATUSTYPE_ocsp); info->Set(context, env->ocsp_request_string(), Boolean::New(env->isolate(), ocsp)).FromJust(); diff --git a/src/node_crypto.h b/src/node_crypto.h index 8d40f85099daca..4587a96e725f86 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -53,10 +53,6 @@ #include #include -#if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) -# define NODE__HAVE_TLSEXT_STATUS_CB -#endif // !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) - namespace node { namespace crypto { @@ -331,13 +327,8 @@ class SSLWrap { ClientHelloParser hello_parser_; -#ifdef NODE__HAVE_TLSEXT_STATUS_CB Persistent ocsp_response_; -#endif // NODE__HAVE_TLSEXT_STATUS_CB - -#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB Persistent sni_context_; -#endif friend class SecureContext; }; diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index e74ee02aaa92d9..65615e3a11e968 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -131,12 +131,10 @@ void TLSWrap::InitSSL() { SSL_set_app_data(ssl_.get(), this); SSL_set_info_callback(ssl_.get(), SSLInfoCallback); -#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB if (is_server()) { SSL_CTX_set_tlsext_servername_callback(sc_->ctx_.get(), SelectSNIContextCallback); } -#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB ConfigureSecureContext(sc_); @@ -777,7 +775,6 @@ void TLSWrap::OnClientHelloParseEnd(void* arg) { } -#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB void TLSWrap::GetServername(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -809,10 +806,8 @@ void TLSWrap::SetServername(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(wrap->ssl_); -#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB node::Utf8Value servername(env->isolate(), args[0].As()); SSL_set_tlsext_host_name(wrap->ssl_.get(), *servername); -#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB } @@ -851,7 +846,6 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) { p->SetSNIContext(sc); return SSL_TLSEXT_ERR_OK; } -#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB void TLSWrap::GetWriteQueueSize(const FunctionCallbackInfo& info) { @@ -902,10 +896,8 @@ void TLSWrap::Initialize(Local target, StreamBase::AddMethods(env, t, StreamBase::kFlagHasWritev); SSLWrap::AddMethods(env, t); -#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB env->SetProtoMethod(t, "getServername", GetServername); env->SetProtoMethod(t, "setServername", SetServername); -#endif // SSL_CRT_SET_TLSEXT_SERVERNAME_CB env->set_tls_wrap_constructor_function(t->GetFunction()); diff --git a/src/tls_wrap.h b/src/tls_wrap.h index 95e0c09cd8f971..1603d8919a472c 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -138,12 +138,9 @@ class TLSWrap : public AsyncWrap, static void EnableCertCb( const v8::FunctionCallbackInfo& args); static void DestroySSL(const v8::FunctionCallbackInfo& args); - -#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB static void GetServername(const v8::FunctionCallbackInfo& args); static void SetServername(const v8::FunctionCallbackInfo& args); static int SelectSNIContextCallback(SSL* s, int* ad, void* arg); -#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB crypto::SecureContext* sc_; BIO* enc_in_; diff --git a/test/parallel/test-tls-alpn-server-client.js b/test/parallel/test-tls-alpn-server-client.js index 8b8ae3e5cf0658..2540831a38f25c 100644 --- a/test/parallel/test-tls-alpn-server-client.js +++ b/test/parallel/test-tls-alpn-server-client.js @@ -4,11 +4,6 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); -if (!process.features.tls_alpn) { - common.skip( - 'Skipping because node compiled without ALPN feature of OpenSSL.'); -} - const assert = require('assert'); const tls = require('tls'); const fixtures = require('../common/fixtures'); diff --git a/test/parallel/test-tls-empty-sni-context.js b/test/parallel/test-tls-empty-sni-context.js index 48f9a52463d3a4..9b963e66294c32 100644 --- a/test/parallel/test-tls-empty-sni-context.js +++ b/test/parallel/test-tls-empty-sni-context.js @@ -4,9 +4,6 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); -if (!process.features.tls_sni) - common.skip('node compiled without OpenSSL or with old OpenSSL version.'); - const assert = require('assert'); const tls = require('tls'); diff --git a/test/parallel/test-tls-ocsp-callback.js b/test/parallel/test-tls-ocsp-callback.js index 9a6df6fb5b2c5b..cf05f6967a967b 100644 --- a/test/parallel/test-tls-ocsp-callback.js +++ b/test/parallel/test-tls-ocsp-callback.js @@ -22,9 +22,6 @@ 'use strict'; const common = require('../common'); -if (!process.features.tls_ocsp) - common.skip('node compiled without OpenSSL or with old OpenSSL version.'); - if (!common.opensslCli) common.skip('node compiled without OpenSSL CLI.'); diff --git a/test/parallel/test-tls-sni-option.js b/test/parallel/test-tls-sni-option.js index b3a5adb47cd6d0..375575c78ab635 100644 --- a/test/parallel/test-tls-sni-option.js +++ b/test/parallel/test-tls-sni-option.js @@ -24,9 +24,6 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); -if (!process.features.tls_sni) - common.skip('node compiled without OpenSSL or with old OpenSSL version.'); - const assert = require('assert'); const tls = require('tls'); const fixtures = require('../common/fixtures'); diff --git a/test/parallel/test-tls-sni-server-client.js b/test/parallel/test-tls-sni-server-client.js index ef1bc09cc0d63f..073e95988a3a08 100644 --- a/test/parallel/test-tls-sni-server-client.js +++ b/test/parallel/test-tls-sni-server-client.js @@ -24,9 +24,6 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); -if (!process.features.tls_sni) - common.skip('node compiled without OpenSSL or with old OpenSSL version.'); - const assert = require('assert'); const tls = require('tls'); const fixtures = require('../common/fixtures'); diff --git a/test/parallel/test-tls-snicallback-error.js b/test/parallel/test-tls-snicallback-error.js index 307a359ebb2ab1..1e1c82225309b4 100644 --- a/test/parallel/test-tls-snicallback-error.js +++ b/test/parallel/test-tls-snicallback-error.js @@ -3,9 +3,6 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); -if (!process.features.tls_sni) - common.skip('compiled without OpenSSL or with old OpenSSL version'); - const assert = require('assert'); const tls = require('tls'); diff --git a/test/parallel/test-tls-socket-constructor-alpn-options-parsing.js b/test/parallel/test-tls-socket-constructor-alpn-options-parsing.js index edbc9f63cf3625..6b0a23f31b6eeb 100644 --- a/test/parallel/test-tls-socket-constructor-alpn-options-parsing.js +++ b/test/parallel/test-tls-socket-constructor-alpn-options-parsing.js @@ -13,9 +13,6 @@ new tls.TLSSocket(null, { ALPNProtocols: ['http/1.1'], }); -if (!process.features.tls_alpn) - common.skip('node compiled without ALPN feature of OpenSSL'); - const assert = require('assert'); const net = require('net'); const fixtures = require('../common/fixtures');