lock the x509store when using the list of certificateMethods

use the already existing lock for the x509store to avoid concurrent
modification of the list of certificateMethods. fixes #14

Sponsored by Lookout Inc.

Author: mkristian

src/main/java/org/jruby/ext/openssl/x509store/Store.java

@@ -115,7 +115,7 @@ public int call(StoreContext context) {
     @Deprecated int cache = 1; // not-used
 
     final List<X509Object> objects;
-    final List<Lookup> certificateMethods;
+    private final List<Lookup> certificateMethods;
 
     public final VerifyParameter verifyParameter;
 
@@ -187,9 +187,11 @@ public void setVerifyCallbackFunction(VerifyCallbackFunction func) {
      * c: X509_STORE_free
      */
     public void free() throws Exception {
-        for (Lookup lu : certificateMethods) {
-            lu.shutdown();
-            lu.free();
+        synchronized(CRYPTO_LOCK_X509_STORE) {
+            for (Lookup lu : certificateMethods) {
+                lu.shutdown();
+                lu.free();
+            }
         }
         if (verifyParameter != null) {
             verifyParameter.free();
@@ -251,13 +253,15 @@ public int setParam(VerifyParameter pm) {
      * c: X509_STORE_add_lookup
      */
     public Lookup addLookup(Ruby runtime, final LookupMethod method) throws Exception {
-        for ( Lookup lookup : certificateMethods ) {
-            if ( lookup.equals(method) ) return lookup;
+        synchronized(CRYPTO_LOCK_X509_STORE) {
+            for ( Lookup lookup : certificateMethods ) {
+                if ( lookup.equals(method) ) return lookup;
+            }
+            Lookup lookup = new Lookup(runtime, method);
+            lookup.store = this;
+            certificateMethods.add(lookup);
+            return lookup;
         }
-        Lookup lookup = new Lookup(runtime, method);
-        lookup.store = this;
-        certificateMethods.add(lookup);
-        return lookup;
     }
 
     /**

src/main/java/org/jruby/ext/openssl/x509store/StoreContext.java

@@ -607,17 +607,19 @@ public int getBySubject(int type,Name name,X509Object[] ret) throws Exception {
 
         X509Object tmp = X509Object.retrieveBySubject(c.objects,type,name);
         if ( tmp == null ) {
-            for(int i=currentMethod; i<c.certificateMethods.size(); i++) {
-                Lookup lu = c.certificateMethods.get(i);
-                X509Object[] stmp = new X509Object[1];
-                int j = lu.bySubject(type,name,stmp);
-                if ( j < 0 ) {
-                    currentMethod = i;
-                    return j;
-                }
-                else if( j > 0 ) {
-                    tmp = stmp[0];
-                    break;
+            synchronized(X509Utils.CRYPTO_LOCK_X509_STORE) {
+                for(int i=currentMethod; i<c.getCertificateMethods().size(); i++) {
+                    Lookup lu = c.getCertificateMethods().get(i);
+                    X509Object[] stmp = new X509Object[1];
+                    int j = lu.bySubject(type,name,stmp);
+                    if ( j < 0 ) {
+                       currentMethod = i;
+                       return j;
+                    }
+                    else if( j > 0 ) {
+                        tmp = stmp[0];
+                        break;
+                    }
                 }
             }
             currentMethod = 0;

src/test/ruby/x509/EntrustnetSecureServerCertificationAuthority.pem

@@ -0,0 +1,28 @@
+-----BEGIN CERTIFICATE-----
+MIIE2DCCBEGgAwIBAgIEN0rSQzANBgkqhkiG9w0BAQUFADCBwzELMAkGA1UEBhMC
+VVMxFDASBgNVBAoTC0VudHJ1c3QubmV0MTswOQYDVQQLEzJ3d3cuZW50cnVzdC5u
+ZXQvQ1BTIGluY29ycC4gYnkgcmVmLiAobGltaXRzIGxpYWIuKTElMCMGA1UECxMc
+KGMpIDE5OTkgRW50cnVzdC5uZXQgTGltaXRlZDE6MDgGA1UEAxMxRW50cnVzdC5u
+ZXQgU2VjdXJlIFNlcnZlciBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTAeFw05OTA1
+MjUxNjA5NDBaFw0xOTA1MjUxNjM5NDBaMIHDMQswCQYDVQQGEwJVUzEUMBIGA1UE
+ChMLRW50cnVzdC5uZXQxOzA5BgNVBAsTMnd3dy5lbnRydXN0Lm5ldC9DUFMgaW5j
+b3JwLiBieSByZWYuIChsaW1pdHMgbGlhYi4pMSUwIwYDVQQLExwoYykgMTk5OSBF
+bnRydXN0Lm5ldCBMaW1pdGVkMTowOAYDVQQDEzFFbnRydXN0Lm5ldCBTZWN1cmUg
+U2VydmVyIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MIGdMA0GCSqGSIb3DQEBAQUA
+A4GLADCBhwKBgQDNKIM0VBuJ8w+vN5Ex/68xYMmo6LIQaO2f55M28Qpku0f1BBc/
+I0dNxScZgSYMVHINiC3ZH5oSn7yzcdOAGT9HZnuMNSjSuQrfJNqc1lB5gXpa0zf3
+wkrYKZImZNHkmGw6AIr1NJtl+O3jEP/9uElY3KDegjlrgbEWGWG5VLbmQwIBA6OC
+AdcwggHTMBEGCWCGSAGG+EIBAQQEAwIABzCCARkGA1UdHwSCARAwggEMMIHeoIHb
+oIHYpIHVMIHSMQswCQYDVQQGEwJVUzEUMBIGA1UEChMLRW50cnVzdC5uZXQxOzA5
+BgNVBAsTMnd3dy5lbnRydXN0Lm5ldC9DUFMgaW5jb3JwLiBieSByZWYuIChsaW1p
+dHMgbGlhYi4pMSUwIwYDVQQLExwoYykgMTk5OSBFbnRydXN0Lm5ldCBMaW1pdGVk
+MTowOAYDVQQDEzFFbnRydXN0Lm5ldCBTZWN1cmUgU2VydmVyIENlcnRpZmljYXRp
+b24gQXV0aG9yaXR5MQ0wCwYDVQQDEwRDUkwxMCmgJ6AlhiNodHRwOi8vd3d3LmVu
+dHJ1c3QubmV0L0NSTC9uZXQxLmNybDArBgNVHRAEJDAigA8xOTk5MDUyNTE2MDk0
+MFqBDzIwMTkwNTI1MTYwOTQwWjALBgNVHQ8EBAMCAQYwHwYDVR0jBBgwFoAU8Bdi
+E1U9s/8KAGv7UISX8+1i0BowHQYDVR0OBBYEFPAXYhNVPbP/CgBr+1CEl/PtYtAa
+MAwGA1UdEwQFMAMBAf8wGQYJKoZIhvZ9B0EABAwwChsEVjQuMAMCBJAwDQYJKoZI
+hvcNAQEFBQADgYEAkNwwAvpkdMKnCqV8IY00F6j7Rw7/JXyNEwr75Ji174z4xRAN
+95K+8cPV1ZVqBLssziY2ZcgxxufuP+NXdYR6Ee9GTxj005i7qIcyunL2POI9n9cd
+2cNgQ4xYDiKWL2KjLB+6rQXvqzJ4h6BUcxm1XAX5Uj5tLUUL9wqT6u0G+bI=
+-----END CERTIFICATE-----

src/test/ruby/x509/test_x509store.rb

@@ -9,6 +9,26 @@ def setup; require 'jopenssl/load' end
     def setup; require 'openssl' end
   end
 
+  def test_add_cert_concurrently
+    pem = File.expand_path('../EntrustnetSecureServerCertificationAuthority.pem', __FILE__)
+    store = OpenSSL::X509::Store.new
+    t = []
+    (0..25).each do |i|
+
+      t << Thread.new do
+        (0..2).each do
+          store.add_file pem
+        end
+      end
+    end
+
+    t.each do |t|
+      t.join
+    end
+    # just ensure there is no concurreny error
+    assert true
+  end
+
   define_method 'test_add_same_cert_twice jruby/jruby-openssl/issues/3' do
     root_key = OpenSSL::PKey::RSA.new 2048 # the CA's public/private key
     root_ca = OpenSSL::X509::Certificate.new
@@ -38,4 +58,4 @@ def setup; require 'openssl' end
     end
   end
 
-end
+end