Skip to content

Commit

Permalink
lib,src: remove openssl feature conditionals
Browse files Browse the repository at this point in the history
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: #21094
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
bnoordhuis authored and targos committed Jun 13, 2018
1 parent 17954c2 commit a657984
Show file tree
Hide file tree
Showing 13 changed files with 13 additions and 99 deletions.
15 changes: 4 additions & 11 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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
Expand Down
28 changes: 7 additions & 21 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2383,30 +2383,16 @@ static Local<Object> 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<Boolean> tls_alpn = True(env->isolate());
#ifdef HAVE_OPENSSL
Local<Boolean> have_openssl = True(env->isolate());
#else
Local<Boolean> tls_alpn = False(env->isolate());
Local<Boolean> 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<Boolean> tls_sni = True(env->isolate());
#else
Local<Boolean> 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<Boolean> tls_ocsp = True(env->isolate());
#else
Local<Boolean> 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);
}
Expand Down
27 changes: 1 addition & 26 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,24 +133,17 @@ template int SSLWrap<TLSWrap>::NewSessionCallback(SSL* s,
template void SSLWrap<TLSWrap>::OnClientHello(
void* arg,
const ClientHelloParser::ClientHello& hello);

#ifdef NODE__HAVE_TLSEXT_STATUS_CB
template int SSLWrap<TLSWrap>::TLSExtStatusCallback(SSL* s, void* arg);
#endif

template void SSLWrap<TLSWrap>::DestroySSL();
template int SSLWrap<TLSWrap>::SSLCertCallback(SSL* s, void* arg);
template void SSLWrap<TLSWrap>::WaitForCertCb(CertCb cb, void* arg);

#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
template int SSLWrap<TLSWrap>::SelectALPNCallback(
SSL* s,
const unsigned char** out,
unsigned char* outlen,
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) {
Expand Down Expand Up @@ -1387,11 +1380,9 @@ void SSLWrap<Base>::AddMethods(Environment* env, Local<FunctionTemplate> t) {

template <class Base>
void SSLWrap<Base>::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
}


Expand Down Expand Up @@ -2019,7 +2010,6 @@ void SSLWrap<Base>::NewSessionDone(const FunctionCallbackInfo<Value>& args) {

template <class Base>
void SSLWrap<Base>::SetOCSPResponse(const FunctionCallbackInfo<Value>& args) {
#ifdef NODE__HAVE_TLSEXT_STATUS_CB
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
Environment* env = w->env();
Expand All @@ -2030,18 +2020,15 @@ void SSLWrap<Base>::SetOCSPResponse(const FunctionCallbackInfo<Value>& args) {
THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "OCSP response");

w->ocsp_response_.Reset(args.GetIsolate(), args[0].As<Object>());
#endif // NODE__HAVE_TLSEXT_STATUS_CB
}


template <class Base>
void SSLWrap<Base>::RequestOCSP(const FunctionCallbackInfo<Value>& 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
}


Expand Down Expand Up @@ -2226,7 +2213,6 @@ void SSLWrap<Base>::GetProtocol(const FunctionCallbackInfo<Value>& args) {
}


#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
template <class Base>
int SSLWrap<Base>::SelectALPNCallback(SSL* s,
const unsigned char** out,
Expand Down Expand Up @@ -2256,13 +2242,11 @@ int SSLWrap<Base>::SelectALPNCallback(SSL* s,
return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK
: SSL_TLSEXT_ERR_NOACK;
}
#endif // TLSEXT_TYPE_application_layer_protocol_negotiation


template <class Base>
void SSLWrap<Base>::GetALPNNegotiatedProto(
const FunctionCallbackInfo<Value>& args) {
#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());

Expand All @@ -2276,13 +2260,11 @@ void SSLWrap<Base>::GetALPNNegotiatedProto(

args.GetReturnValue().Set(
OneByteString(args.GetIsolate(), alpn_proto, alpn_proto_len));
#endif // TLSEXT_TYPE_application_layer_protocol_negotiation
}


template <class Base>
void SSLWrap<Base>::SetALPNProtocols(const FunctionCallbackInfo<Value>& args) {
#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
Environment* env = w->env();
Expand All @@ -2306,11 +2288,9 @@ void SSLWrap<Base>::SetALPNProtocols(const FunctionCallbackInfo<Value>& args) {
SelectALPNCallback,
nullptr);
}
#endif // TLSEXT_TYPE_application_layer_protocol_negotiation
}


#ifdef NODE__HAVE_TLSEXT_STATUS_CB
template <class Base>
int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
Base* w = static_cast<Base*>(SSL_get_app_data(s));
Expand Down Expand Up @@ -2354,7 +2334,6 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
return SSL_TLSEXT_ERR_OK;
}
}
#endif // NODE__HAVE_TLSEXT_STATUS_CB


template <class Base>
Expand Down Expand Up @@ -2396,11 +2375,7 @@ int SSLWrap<Base>::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();

Expand Down
9 changes: 0 additions & 9 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@
#include <openssl/rand.h>
#include <openssl/pkcs12.h>

#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 {

Expand Down Expand Up @@ -331,13 +327,8 @@ class SSLWrap {

ClientHelloParser hello_parser_;

#ifdef NODE__HAVE_TLSEXT_STATUS_CB
Persistent<v8::Object> ocsp_response_;
#endif // NODE__HAVE_TLSEXT_STATUS_CB

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
Persistent<v8::Value> sni_context_;
#endif

friend class SecureContext;
};
Expand Down
8 changes: 0 additions & 8 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);

Expand Down Expand Up @@ -779,7 +777,6 @@ void TLSWrap::OnClientHelloParseEnd(void* arg) {
}


#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
void TLSWrap::GetServername(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand Down Expand Up @@ -811,10 +808,8 @@ void TLSWrap::SetServername(const FunctionCallbackInfo<Value>& args) {

CHECK_NOT_NULL(wrap->ssl_);

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
node::Utf8Value servername(env->isolate(), args[0].As<String>());
SSL_set_tlsext_host_name(wrap->ssl_.get(), *servername);
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
}


Expand Down Expand Up @@ -853,7 +848,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<Value>& info) {
Expand Down Expand Up @@ -904,10 +898,8 @@ void TLSWrap::Initialize(Local<Object> target,
StreamBase::AddMethods<TLSWrap>(env, t, StreamBase::kFlagHasWritev);
SSLWrap<TLSWrap>::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());

Expand Down
3 changes: 0 additions & 3 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,9 @@ class TLSWrap : public AsyncWrap,
static void EnableCertCb(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void DestroySSL(const v8::FunctionCallbackInfo<v8::Value>& args);

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
static void GetServername(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetServername(const v8::FunctionCallbackInfo<v8::Value>& args);
static int SelectSNIContextCallback(SSL* s, int* ad, void* arg);
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB

crypto::SecureContext* sc_;
BIO* enc_in_;
Expand Down
5 changes: 0 additions & 5 deletions test/parallel/test-tls-alpn-server-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-tls-empty-sni-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-tls-ocsp-callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.');

Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-tls-sni-option.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-tls-sni-server-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit a657984

Please sign in to comment.