Skip to content

Commit 598b6e4

Browse files
mkristianmkristian
committed
clean up synchronizing Store
just keep all synchronization inside the Store class and keep the synchronize "small". removed GLOBAL Store lock. part of fixing #14 Sponsored by Lookout Inc.
1 parent 2a9ad81 commit 598b6e4

File tree

4 files changed

+52
-52
lines changed

4 files changed

+52
-52
lines changed

‎src/main/java/org/jruby/ext/openssl/x509store/Lookup.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
***** END LICENSE BLOCK *****/
2828
package org.jruby.ext.openssl.x509store;
2929

30-
import static org.jruby.ext.openssl.x509store.X509Utils.CRYPTO_LOCK_X509_STORE;
3130
import static org.jruby.ext.openssl.x509store.X509Utils.X509_CERT_DIR;
3231
import static org.jruby.ext.openssl.x509store.X509Utils.X509_FILETYPE_ASN1;
3332
import static org.jruby.ext.openssl.x509store.X509Utils.X509_FILETYPE_DEFAULT;
@@ -595,11 +594,9 @@ else if ( type == X509_LU_CRL ) {
595594
}
596595
}
597596
X509Object tmp = null;
598-
synchronized ( CRYPTO_LOCK_X509_STORE ) {
599-
for ( X509Object obj : lookup.store.getObjects() ) {
600-
if ( obj.type() == type && obj.isName(name) ) {
601-
tmp = obj; break;
602-
}
597+
for ( X509Object obj : lookup.store.getObjects() ) {
598+
if ( obj.type() == type && obj.isName(name) ) {
599+
tmp = obj; break;
603600
}
604601
}
605602
if ( tmp != null ) {

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
***** END LICENSE BLOCK *****/
2828
package org.jruby.ext.openssl.x509store;
2929

30-
import static org.jruby.ext.openssl.x509store.X509Utils.CRYPTO_LOCK_X509_STORE;
3130
import static org.jruby.ext.openssl.x509store.X509Utils.X509_FILETYPE_DEFAULT;
3231
import static org.jruby.ext.openssl.x509store.X509Utils.X509_FILETYPE_PEM;
3332
import static org.jruby.ext.openssl.x509store.X509Utils.X509_R_CERT_ALREADY_IN_HASH_TABLE;
@@ -148,11 +147,15 @@ public Store() {
148147
}
149148

150149
public List<X509Object> getObjects() {
151-
return objects;
150+
synchronized(objects) {
151+
return new ArrayList<X509Object>(objects);
152+
}
152153
}
153154

154155
public List<Lookup> getCertificateMethods() {
155-
return certificateMethods;
156+
synchronized(certificateMethods) {
157+
return new ArrayList<Lookup>(certificateMethods);
158+
}
156159
}
157160

158161
public VerifyParameter getVerifyParameter() {
@@ -185,7 +188,7 @@ public void setVerifyCallbackFunction(VerifyCallbackFunction func) {
185188
* c: X509_STORE_free
186189
*/
187190
public void free() throws Exception {
188-
synchronized(CRYPTO_LOCK_X509_STORE) {
191+
synchronized(certificateMethods) {
189192
for (Lookup lu : certificateMethods) {
190193
lu.shutdown();
191194
lu.free();
@@ -200,15 +203,19 @@ public void free() throws Exception {
200203
* c: X509_set_ex_data
201204
*/
202205
public int setExtraData(int idx, Object data) {
203-
extraData.set(idx,data);
204-
return 1;
206+
synchronized(extraData) {
207+
extraData.set(idx,data);
208+
return 1;
209+
}
205210
}
206211

207212
/**
208213
* c: X509_get_ex_data
209214
*/
210215
public Object getExtraData(int idx) {
211-
return extraData.get(idx);
216+
synchronized(extraData) {
217+
return extraData.get(idx);
218+
}
212219
}
213220

214221
/**
@@ -251,7 +258,7 @@ public int setParam(VerifyParameter pm) {
251258
* c: X509_STORE_add_lookup
252259
*/
253260
public Lookup addLookup(Ruby runtime, final LookupMethod method) throws Exception {
254-
synchronized(CRYPTO_LOCK_X509_STORE) {
261+
synchronized(certificateMethods) {
255262
for ( Lookup lookup : certificateMethods ) {
256263
if ( lookup.equals(method) ) return lookup;
257264
}
@@ -272,7 +279,7 @@ public int addCertificate(final X509Certificate cert) {
272279
certObj.x509 = StoreContext.ensureAux(cert);
273280

274281
int ret = 1;
275-
synchronized (CRYPTO_LOCK_X509_STORE) {
282+
synchronized (objects) {
276283
if ( X509Object.retrieveMatch(objects,certObj) != null ) {
277284
X509Error.addError(X509_R_CERT_ALREADY_IN_HASH_TABLE);
278285
ret = 0;
@@ -293,7 +300,7 @@ public int addCRL(final java.security.cert.CRL crl) {
293300
final CRL crlObj = new CRL(); crlObj.crl = crl;
294301

295302
int ret = 1;
296-
synchronized (CRYPTO_LOCK_X509_STORE) {
303+
synchronized (objects) {
297304
if ( X509Object.retrieveMatch(objects,crlObj) != null ) {
298305
X509Error.addError(X509_R_CERT_ALREADY_IN_HASH_TABLE);
299306
ret = 0;
@@ -373,7 +380,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) {
373380

374381
@Override
375382
public X509Certificate[] getAcceptedIssuers() {
376-
synchronized(CRYPTO_LOCK_X509_STORE) {
383+
synchronized(objects) {
377384
ArrayList<X509Certificate> issuers = new ArrayList<X509Certificate>(objects.size());
378385
for ( X509Object object : objects ) {
379386
if ( object instanceof Certificate ) {

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

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -150,24 +150,23 @@ else if ( ok != X509Utils.X509_LU_FAIL ) {
150150
return 1;
151151
}
152152

153-
synchronized(X509Utils.CRYPTO_LOCK_X509_STORE) {
154-
int idx = X509Object.indexBySubject(store.getObjects(), X509Utils.X509_LU_X509, xn);
155-
if ( idx == -1 ) return 0;
156-
157-
/* Look through all matching certificates for a suitable issuer */
158-
for ( int i = idx; i < store.getObjects().size(); i++ ) {
159-
final X509Object pobj = store.getObjects().get(i);
160-
if ( pobj.type() != X509Utils.X509_LU_X509 ) {
161-
return 0;
162-
}
163-
final X509AuxCertificate x509 = ((Certificate) pobj).x509;
164-
if ( ! xn.equalTo( x509.getSubjectX500Principal() ) ) {
165-
return 0;
166-
}
167-
if ( checkIssued.call(this, x, x509) != 0 ) {
168-
issuers[0] = x509;
169-
return 1;
170-
}
153+
List<X509Object> objects = store.getObjects();
154+
int idx = X509Object.indexBySubject(objects, X509Utils.X509_LU_X509, xn);
155+
if ( idx == -1 ) return 0;
156+
157+
/* Look through all matching certificates for a suitable issuer */
158+
for ( int i = idx; i < objects.size(); i++ ) {
159+
final X509Object pobj = objects.get(i);
160+
if ( pobj.type() != X509Utils.X509_LU_X509 ) {
161+
return 0;
162+
}
163+
final X509AuxCertificate x509 = ((Certificate) pobj).x509;
164+
if ( ! xn.equalTo( x509.getSubjectX500Principal() ) ) {
165+
return 0;
166+
}
167+
if ( checkIssued.call(this, x, x509) != 0 ) {
168+
issuers[0] = x509;
169+
return 1;
171170
}
172171
}
173172
return 0;
@@ -607,21 +606,20 @@ public int setDefault(String name) {
607606
public int getBySubject(int type,Name name,X509Object[] ret) throws Exception {
608607
Store c = store;
609608

610-
synchronized(X509Utils.CRYPTO_LOCK_X509_STORE) {
611-
X509Object tmp = X509Object.retrieveBySubject(c.getObjects(),type,name);
612-
if ( tmp == null ) {
613-
for(int i=currentMethod; i<c.getCertificateMethods().size(); i++) {
614-
Lookup lu = c.getCertificateMethods().get(i);
615-
X509Object[] stmp = new X509Object[1];
616-
int j = lu.bySubject(type,name,stmp);
617-
if ( j < 0 ) {
618-
currentMethod = i;
619-
return j;
620-
}
621-
else if( j > 0 ) {
622-
tmp = stmp[0];
623-
break;
624-
}
609+
X509Object tmp = X509Object.retrieveBySubject(c.getObjects(),type,name);
610+
if ( tmp == null ) {
611+
List<Lookup> certificateMethods = c.getCertificateMethods();
612+
for(int i=currentMethod; i<certificateMethods.size(); i++) {
613+
Lookup lu = certificateMethods.get(i);
614+
X509Object[] stmp = new X509Object[1];
615+
int j = lu.bySubject(type,name,stmp);
616+
if ( j < 0 ) {
617+
currentMethod = i;
618+
return j;
619+
}
620+
else if( j > 0 ) {
621+
tmp = stmp[0];
622+
break;
625623
}
626624
}
627625
currentMethod = 0;

‎src/main/java/org/jruby/ext/openssl/x509store/X509Utils.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,6 @@ else if ( keyUsage != null && ! keyUsage[5] ) { // KU_KEY_CERT_SIGN
292292
public static final String X509_CERT_DIR_EVP = "SSL_CERT_DIR";
293293
public static final String X509_CERT_FILE_EVP = "SSL_CERT_FILE";
294294

295-
public static final Object CRYPTO_LOCK_X509_STORE = new Object();
296-
297295
public static final int X509_LU_RETRY=-1;
298296
public static final int X509_LU_FAIL=0;
299297
public static final int X509_LU_X509=1;

0 commit comments

Comments
 (0)