Don't load root certs for each SSL context

nodejs · Apr 1, 2011 · 5c35dff · 5c35dff
1 parent ad861e1
commit 5c35dff
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 27 deletions.
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -131,8 +131,7 @@ Handle<Value> SecureContext::Init(const Arguments& args) {
   SSL_CTX_set_session_cache_mode(sc->ctx_, SSL_SESS_CACHE_SERVER);
   // SSL_CTX_set_session_cache_mode(sc->ctx_,SSL_SESS_CACHE_OFF);
 
-  sc->ca_store_ = X509_STORE_new();
-  SSL_CTX_set_cert_store(sc->ctx_, sc->ca_store_);
+  sc->ca_store_ = NULL;
   return True();
 }
 
@@ -311,6 +310,7 @@ Handle<Value> SecureContext::SetCert(const Arguments& args) {
 
 
 Handle<Value> SecureContext::AddCACert(const Arguments& args) {
+  bool newCAStore = false;
   HandleScope scope;
 
   SecureContext *sc = ObjectWrap::Unwrap<SecureContext>(args.Holder());
@@ -319,6 +319,11 @@ Handle<Value> SecureContext::AddCACert(const Arguments& args) {
     return ThrowException(Exception::TypeError(String::New("Bad parameter")));
   }
 
+  if (!sc->ca_store_) {
+    sc->ca_store_ = X509_STORE_new();
+    newCAStore = true;
+  }
+
   X509* x509 = LoadX509(args[0]);
   if (!x509) return False();
 
@@ -327,6 +332,10 @@ Handle<Value> SecureContext::AddCACert(const Arguments& args) {
 
   X509_free(x509);
 
+  if (newCAStore) {
+    SSL_CTX_set_cert_store(sc->ctx_, sc->ca_store_);
+  }
+
   return True();
 }
 
@@ -362,33 +371,42 @@ Handle<Value> SecureContext::AddCRL(const Arguments& args) {
 }
 
 
+
 Handle<Value> SecureContext::AddRootCerts(const Arguments& args) {
   HandleScope scope;
 
   SecureContext *sc = ObjectWrap::Unwrap<SecureContext>(args.Holder());
 
-  for (int i = 0; root_certs[i]; i++) {
-    // TODO: reuse bp ?
-    BIO *bp = BIO_new(BIO_s_mem());
+  assert(sc->ca_store_ == NULL);
 
-    if (!BIO_write(bp, root_certs[i], strlen(root_certs[i]))) {
-      BIO_free(bp);
-      return False();
-    }
+  if (!root_cert_store) {
+    root_cert_store = X509_STORE_new();
 
-    X509 *x509 = PEM_read_bio_X509(bp, NULL, NULL, NULL);
+    for (int i = 0; root_certs[i]; i++) {
+      BIO *bp = BIO_new(BIO_s_mem());
 
-    if (x509 == NULL) {
-      BIO_free(bp);
-      return False();
-    }
+      if (!BIO_write(bp, root_certs[i], strlen(root_certs[i]))) {
+        BIO_free(bp);
+        return False();
+      }
 
-    X509_STORE_add_cert(sc->ca_store_, x509);
+      X509 *x509 = PEM_read_bio_X509(bp, NULL, NULL, NULL);
 
-    BIO_free(bp);
-    X509_free(x509);
+      if (x509 == NULL) {
+        BIO_free(bp);
+        return False();
+      }
+
+      X509_STORE_add_cert(root_cert_store, x509);
+
+      BIO_free(bp);
+      X509_free(x509);
+    }
   }
 
+  sc->ca_store_ = root_cert_store;
+  SSL_CTX_set_cert_store(sc->ctx_, sc->ca_store_);
+
   return True();
 }
 
@@ -411,19 +429,12 @@ Handle<Value> SecureContext::SetCiphers(const Arguments& args) {
 
 Handle<Value> SecureContext::Close(const Arguments& args) {
   HandleScope scope;
-
   SecureContext *sc = ObjectWrap::Unwrap<SecureContext>(args.Holder());
-
-  if (sc->ctx_ != NULL) {
-    SSL_CTX_free(sc->ctx_);
-    sc->ctx_ = NULL;
-    sc->ca_store_ = NULL;
-    return True();
-  }
-
+  sc->FreeCTXMem();
   return False();
 }
 
+
 #ifdef SSL_PRINT_DEBUG
 # define DEBUG_PRINT(...) fprintf (stderr, __VA_ARGS__)
 #else

diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -39,11 +39,14 @@
 namespace node {
 namespace crypto {
 
+static X509_STORE* root_cert_store;
+
 class SecureContext : ObjectWrap {
  public:
   static void Initialize(v8::Handle<v8::Object> target);
 
   SSL_CTX *ctx_;
+  // TODO: ca_store_ should probably be removed, it's not used anywhere.
   X509_STORE *ca_store_;
 
  protected:
@@ -62,8 +65,15 @@ class SecureContext : ObjectWrap {
     ca_store_ = NULL;
   }
 
-  ~SecureContext() {
+  void FreeCTXMem() {
     if (ctx_) {
+      if (ctx_->cert_store == root_cert_store) {
+        // SSL_CTX_free() will attempt to free the cert_store as well.
+        // Since we want our root_cert_store to stay around forever
+        // we just clear the field. Hopefully OpenSSL will not modify this
+        // struct in future versions.
+        ctx_->cert_store = NULL;
+      }
       SSL_CTX_free(ctx_);
       ctx_ = NULL;
       ca_store_ = NULL;
@@ -72,6 +82,10 @@ class SecureContext : ObjectWrap {
     }
   }
 
+  ~SecureContext() {
+    FreeCTXMem();
+  }
+
  private:
 };