tls: use SSL_set_cert_cb for async SNI/OCSP

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 <[email protected]>

lib/_tls_wrap.js

@@ -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();
     });
   });
 }
@@ -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() {};
@@ -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)

src/env.h

@@ -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")                                                \

src/node_crypto.cc

@@ -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) {
@@ -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);
@@ -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);
@@ -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) {
@@ -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());
 
@@ -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);
@@ -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_);
@@ -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_);

src/node_crypto.h

@@ -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);
@@ -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
@@ -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
@@ -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);
@@ -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_;
@@ -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
@@ -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;
 };
 
@@ -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
   }
@@ -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