Skip to content

Commit

Permalink
tls: use SSL_set_cert_cb for async SNI/OCSP
Browse files Browse the repository at this point in the history 
Do not enable ClientHello parser for async SNI/OCSP. Use new
OpenSSL-1.0.2's API `SSL_set_cert_cb` to pause the handshake process and
load the cert/OCSP response asynchronously. Hopefuly this will make
whole async SNI/OCSP process much faster and will eventually let us
remove the ClientHello parser itself (which is currently used only for
async session, see nodejs#1462 for the discussion of removing it).

NOTE: Ported our code to `SSL_CTX_add1_chain_cert` to use
`SSL_CTX_get0_chain_certs` in `CertCbDone`. Test provided for this
feature.

Fix: nodejs#1423
PR-URL: nodejs#1464
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
  • Loading branch information
@indutny @Fishrock123
indutny authored and Fishrock123 committed May 19, 2015
1 parent c5d7c7e commit 592230a
Show file tree
Hide file tree
Showing 23 changed files with 438 additions and 95 deletions.
49 changes: 23 additions & 26 deletions lib/_tls_wrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,29 +141,23 @@ function onclienthello(hello) {
if (err)
return self.destroy(err);

// Servername came from SSL session
// NOTE: TLS Session ticket doesn't include servername information
//
// Another note, From RFC3546:
//
// If, on the other hand, the older
// session is resumed, then the server MUST ignore extensions appearing
// in the client hello, and send a server hello containing no
// extensions; in this case the extension functionality negotiated
// during the original session initiation is applied to the resumed
// session.
//
// Therefore we should account session loading when dealing with servername
var servername = session && session.servername || hello.servername;
loadSNI(self, servername, function(err, ctx) {
self._handle.endParser();
});
}


function oncertcb(info) {
var self = this;
var servername = info.servername;

loadSNI(self, servername, function(err, ctx) {
if (err)
return self.destroy(err);
requestOCSP(self, info, ctx, function(err) {
if (err)
return self.destroy(err);
requestOCSP(self, hello, ctx, function(err) {
if (err)
return self.destroy(err);

self._handle.endParser();
});
self._handle.certCbDone();
});
});
}
Expand Down Expand Up @@ -333,15 +327,18 @@ TLSSocket.prototype._init = function(socket, wrap) {
ssl.onhandshakestart = onhandshakestart.bind(this);
ssl.onhandshakedone = onhandshakedone.bind(this);
ssl.onclienthello = onclienthello.bind(this);
ssl.oncertcb = oncertcb.bind(this);
ssl.onnewsession = onnewsession.bind(this);
ssl.lastHandshakeTime = 0;
ssl.handshakes = 0;

if (this.server &&
(listenerCount(this.server, 'resumeSession') > 0 ||
listenerCount(this.server, 'newSession') > 0 ||
listenerCount(this.server, 'OCSPRequest') > 0)) {
ssl.enableSessionCallbacks();
if (this.server) {
if (listenerCount(this.server, 'resumeSession') > 0 ||
listenerCount(this.server, 'newSession') > 0) {
ssl.enableSessionCallbacks();
}
if (listenerCount(this.server, 'OCSPRequest') > 0)
ssl.enableCertCb();
}
} else {
ssl.onhandshakestart = function() {};
Expand Down Expand Up @@ -382,7 +379,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
options.server._contexts.length)) {
assert(typeof options.SNICallback === 'function');
this._SNICallback = options.SNICallback;
ssl.enableHelloParser();
ssl.enableCertCb();
}

if (process.features.tls_npn && options.NPNProtocols)
Expand Down
1 change: 1 addition & 0 deletions src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ namespace node {
V(bytes_parsed_string, "bytesParsed") \
V(callback_string, "callback") \
V(change_string, "change") \
V(oncertcb_string, "oncertcb") \
V(onclose_string, "_onclose") \
V(code_string, "code") \
V(compare_string, "compare") \
Expand Down
132 changes: 129 additions & 3 deletions src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ template int SSLWrap<TLSWrap>::SelectNextProtoCallback(
#endif
template int SSLWrap<TLSWrap>::TLSExtStatusCallback(SSL* s, void* arg);
template void SSLWrap<TLSWrap>::DestroySSL();
template int SSLWrap<TLSWrap>::SSLCertCallback(SSL* s, void* arg);
template void SSLWrap<TLSWrap>::WaitForCertCb(CertCb cb, void* arg);


static void crypto_threadid_cb(CRYPTO_THREADID* tid) {
Expand Down Expand Up @@ -511,7 +513,8 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
}

while ((ca = PEM_read_bio_X509(in, nullptr, CryptoPemCallback, nullptr))) {
r = SSL_CTX_add_extra_chain_cert(ctx, ca);
// NOTE: Increments reference count on `ca`
r = SSL_CTX_add1_chain_cert(ctx, ca);

if (!r) {
X509_free(ca);
Expand Down Expand Up @@ -987,6 +990,7 @@ void SSLWrap<Base>::AddMethods(Environment* env, Handle<FunctionTemplate> t) {
env->SetProtoMethod(t, "verifyError", VerifyError);
env->SetProtoMethod(t, "getCurrentCipher", GetCurrentCipher);
env->SetProtoMethod(t, "endParser", EndParser);
env->SetProtoMethod(t, "certCbDone", CertCbDone);
env->SetProtoMethod(t, "renegotiate", Renegotiate);
env->SetProtoMethod(t, "shutdownSSL", Shutdown);
env->SetProtoMethod(t, "getTLSTicket", GetTLSTicket);
Expand Down Expand Up @@ -1869,6 +1873,122 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
#endif // NODE__HAVE_TLSEXT_STATUS_CB


template <class Base>
void SSLWrap<Base>::WaitForCertCb(CertCb cb, void* arg) {
cert_cb_ = cb;
cert_cb_arg_ = arg;
}


template <class Base>
int SSLWrap<Base>::SSLCertCallback(SSL* s, void* arg) {
Base* w = static_cast<Base*>(SSL_get_app_data(s));

if (!w->is_server())
return 1;

if (!w->is_waiting_cert_cb())
return 1;

if (w->cert_cb_running_)
return -1;

Environment* env = w->env();
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
w->cert_cb_running_ = true;

Local<Object> info = Object::New(env->isolate());

SSL_SESSION* sess = SSL_get_session(s);
if (sess != nullptr) {
if (sess->tlsext_hostname == nullptr) {
info->Set(env->servername_string(), String::Empty(env->isolate()));
} else {
Local<String> servername = OneByteString(env->isolate(),
sess->tlsext_hostname,
strlen(sess->tlsext_hostname));
info->Set(env->servername_string(), servername);
}
info->Set(env->tls_ticket_string(),
Boolean::New(env->isolate(), sess->tlsext_ticklen != 0));
}
bool ocsp = s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp;
info->Set(env->ocsp_request_string(), Boolean::New(env->isolate(), ocsp));

Local<Value> argv[] = { info };
w->MakeCallback(env->oncertcb_string(), ARRAY_SIZE(argv), argv);

if (!w->cert_cb_running_)
return 1;

// Performing async action, wait...
return -1;
}


template <class Base>
void SSLWrap<Base>::CertCbDone(const FunctionCallbackInfo<Value>& args) {
Base* w = Unwrap<Base>(args.Holder());
Environment* env = w->env();

CHECK(w->is_waiting_cert_cb() && w->cert_cb_running_);

Local<Object> object = w->object();
Local<Value> ctx = object->Get(env->sni_context_string());
Local<FunctionTemplate> cons = env->secure_context_constructor_template();

// Not an object, probably undefined or null
if (!ctx->IsObject())
goto fire_cb;

if (cons->HasInstance(ctx)) {
SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
w->sni_context_.Reset();
w->sni_context_.Reset(env->isolate(), ctx);

int rv;

// NOTE: reference count is not increased by this API methods
X509* x509 = SSL_CTX_get0_certificate(sc->ctx_);
EVP_PKEY* pkey = SSL_CTX_get0_privatekey(sc->ctx_);
STACK_OF(X509)* chain;

rv = SSL_CTX_get0_chain_certs(sc->ctx_, &chain);
if (rv)
rv = SSL_use_certificate(w->ssl_, x509);
if (rv)
rv = SSL_use_PrivateKey(w->ssl_, pkey);
if (rv && chain != nullptr)
rv = SSL_set1_chain(w->ssl_, chain);
if (!rv) {
unsigned long err = ERR_get_error();
if (!err)
return env->ThrowError("CertCbDone");
return ThrowCryptoError(env, err);
}
} else {
// Failure: incorrect SNI context object
Local<Value> err = Exception::TypeError(env->sni_context_err_string());
w->MakeCallback(env->onerror_string(), 1, &err);
return;
}

fire_cb:
CertCb cb;
void* arg;

cb = w->cert_cb_;
arg = w->cert_cb_arg_;

w->cert_cb_running_ = false;
w->cert_cb_ = nullptr;
w->cert_cb_arg_ = nullptr;

cb(arg);
}


template <class Base>
void SSLWrap<Base>::SSLGetter(Local<String> property,
const PropertyCallbackInfo<Value>& info) {
Expand Down Expand Up @@ -1975,6 +2095,10 @@ int Connection::HandleSSLError(const char* func,
DEBUG_PRINT("[%p] SSL: %s want read\n", ssl_, func);
return 0;

} else if (err == SSL_ERROR_WANT_X509_LOOKUP) {
DEBUG_PRINT("[%p] SSL: %s want x509 lookup\n", ssl_, func);
return 0;

} else if (err == SSL_ERROR_ZERO_RETURN) {
HandleScope scope(ssl_env()->isolate());

Expand Down Expand Up @@ -2140,7 +2264,7 @@ int Connection::SelectSNIContextCallback_(SSL *s, int *ad, void* arg) {

// Call the SNI callback and use its return value as context
if (!conn->sniObject_.IsEmpty()) {
conn->sniContext_.Reset();
conn->sni_context_.Reset();

Local<Value> arg = PersistentToLocal(env->isolate(), conn->servername_);
Local<Value> ret = conn->MakeCallback(env->onselect_string(), 1, &arg);
Expand All @@ -2149,7 +2273,7 @@ int Connection::SelectSNIContextCallback_(SSL *s, int *ad, void* arg) {
Local<FunctionTemplate> secure_context_constructor_template =
env->secure_context_constructor_template();
if (secure_context_constructor_template->HasInstance(ret)) {
conn->sniContext_.Reset(env->isolate(), ret);
conn->sni_context_.Reset(env->isolate(), ret);
SecureContext* sc = Unwrap<SecureContext>(ret.As<Object>());
InitNPN(sc);
SSL_set_SSL_CTX(s, sc->ctx_);
Expand Down Expand Up @@ -2188,6 +2312,8 @@ void Connection::New(const FunctionCallbackInfo<Value>& args) {

InitNPN(sc);

SSL_set_cert_cb(conn->ssl_, SSLWrap<Connection>::SSLCertCallback, conn);

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
if (is_server) {
SSL_CTX_set_tlsext_servername_callback(sc->ctx_, SelectSNIContextCallback_);
Expand Down
26 changes: 23 additions & 3 deletions src/node_crypto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ class SSLWrap {
kind_(kind),
next_sess_(nullptr),
session_callbacks_(false),
new_session_wait_(false) {
new_session_wait_(false),
cert_cb_(nullptr),
cert_cb_arg_(nullptr),
cert_cb_running_(false) {
ssl_ = SSL_new(sc->ctx_);
env_->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize);
CHECK_NE(ssl_, nullptr);
Expand All @@ -160,6 +163,9 @@ class SSLWrap {
npn_protos_.Reset();
selected_npn_proto_.Reset();
#endif
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
sni_context_.Reset();
#endif
#ifdef NODE__HAVE_TLSEXT_STATUS_CB
ocsp_response_.Reset();
#endif // NODE__HAVE_TLSEXT_STATUS_CB
Expand All @@ -170,8 +176,11 @@ class SSLWrap {
inline bool is_server() const { return kind_ == kServer; }
inline bool is_client() const { return kind_ == kClient; }
inline bool is_waiting_new_session() const { return new_session_wait_; }
inline bool is_waiting_cert_cb() const { return cert_cb_ != nullptr; }

protected:
typedef void (*CertCb)(void* arg);

// Size allocated by OpenSSL: one for SSL structure, one for SSL3_STATE and
// some for buffers.
// NOTE: Actually it is much more than this
Expand Down Expand Up @@ -199,6 +208,7 @@ class SSLWrap {
static void VerifyError(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetCurrentCipher(const v8::FunctionCallbackInfo<v8::Value>& args);
static void EndParser(const v8::FunctionCallbackInfo<v8::Value>& args);
static void CertCbDone(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Renegotiate(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Shutdown(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetTLSTicket(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down Expand Up @@ -227,10 +237,12 @@ class SSLWrap {
void* arg);
#endif // OPENSSL_NPN_NEGOTIATED
static int TLSExtStatusCallback(SSL* s, void* arg);
static int SSLCertCallback(SSL* s, void* arg);
static void SSLGetter(v8::Local<v8::String> property,
const v8::PropertyCallbackInfo<v8::Value>& info);

void DestroySSL();
void WaitForCertCb(CertCb cb, void* arg);

inline Environment* ssl_env() const {
return env_;
Expand All @@ -242,6 +254,12 @@ class SSLWrap {
SSL* ssl_;
bool session_callbacks_;
bool new_session_wait_;

// SSL_set_cert_cb
CertCb cert_cb_;
void* cert_cb_arg_;
bool cert_cb_running_;

ClientHelloParser hello_parser_;

#ifdef NODE__HAVE_TLSEXT_STATUS_CB
Expand All @@ -253,6 +271,10 @@ class SSLWrap {
v8::Persistent<v8::Value> selected_npn_proto_;
#endif // OPENSSL_NPN_NEGOTIATED

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

friend class SecureContext;
};

Expand All @@ -264,7 +286,6 @@ class Connection : public SSLWrap<Connection>, public AsyncWrap {
~Connection() override {
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
sniObject_.Reset();
sniContext_.Reset();
servername_.Reset();
#endif
}
Expand All @@ -279,7 +300,6 @@ class Connection : public SSLWrap<Connection>, public AsyncWrap {

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
v8::Persistent<v8::Object> sniObject_;
v8::Persistent<v8::Value> sniContext_;
v8::Persistent<v8::String> servername_;
#endif

Expand Down
Loading

0 comments on commit 592230a

Please sign in to comment.