Skip to content

Commit 68530c8

Browse files
rlm2002rlm2002
authored
Merge pull request #165 from cconlon/wksSynchronization
Remove synchronization on some WolfSSLKeyStore (WKS) methods
2 parents d7339f0 + 589cf30 commit 68530c8

File tree

2 files changed

+176
-10
lines changed

2 files changed

+176
-10
lines changed

‎src/main/java/com/wolfssl/provider/jce/WolfSSLKeyStore.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ private static byte[] decryptKey(byte[] encKey, byte[] kek, byte[] iv)
621621
* @throws UnrecoverableKeyException if the key cannot be recovered
622622
*/
623623
@Override
624-
public synchronized Key engineGetKey(String alias, char[] password)
624+
public Key engineGetKey(String alias, char[] password)
625625
throws NoSuchAlgorithmException, UnrecoverableKeyException {
626626

627627
int algoId = 0;
@@ -721,7 +721,7 @@ else if (entry instanceof WKSSecretKey) {
721721
* does not exist or does not contain a certificate chain.
722722
*/
723723
@Override
724-
public synchronized Certificate[] engineGetCertificateChain(String alias) {
724+
public Certificate[] engineGetCertificateChain(String alias) {
725725

726726
Object entry = null;
727727

@@ -752,7 +752,7 @@ public synchronized Certificate[] engineGetCertificateChain(String alias) {
752752
* does not match any entries.
753753
*/
754754
@Override
755-
public synchronized Certificate engineGetCertificate(String alias) {
755+
public Certificate engineGetCertificate(String alias) {
756756

757757
Object entry = null;
758758

@@ -783,7 +783,7 @@ else if (entry instanceof WKSPrivateKey) {
783783
* alias does not exist.
784784
*/
785785
@Override
786-
public synchronized Date engineGetCreationDate(String alias) {
786+
public Date engineGetCreationDate(String alias) {
787787

788788
Object entry = null;
789789

@@ -1146,7 +1146,7 @@ public synchronized void engineDeleteEntry(String alias)
11461146
* @return enumeration of all aliases
11471147
*/
11481148
@Override
1149-
public synchronized Enumeration<String> engineAliases() {
1149+
public Enumeration<String> engineAliases() {
11501150

11511151
log("returning all alias names in KeyStore");
11521152

@@ -1161,7 +1161,7 @@ public synchronized Enumeration<String> engineAliases() {
11611161
* @return true if alias is in KeyStore, otherwise false
11621162
*/
11631163
@Override
1164-
public synchronized boolean engineContainsAlias(String alias) {
1164+
public boolean engineContainsAlias(String alias) {
11651165

11661166
log("checking if KeyStore contains alias: " + alias);
11671167

@@ -1174,7 +1174,7 @@ public synchronized boolean engineContainsAlias(String alias) {
11741174
* @return number of entries
11751175
*/
11761176
@Override
1177-
public synchronized int engineSize() {
1177+
public int engineSize() {
11781178

11791179
log("returning size of KeyStore: " + entries.size());
11801180

@@ -1193,7 +1193,7 @@ public synchronized int engineSize() {
11931193
* private key entry or alias does not exist
11941194
*/
11951195
@Override
1196-
public synchronized boolean engineIsKeyEntry(String alias) {
1196+
public boolean engineIsKeyEntry(String alias) {
11971197

11981198
Object entry;
11991199
boolean isKey = false;
@@ -1224,7 +1224,7 @@ public synchronized boolean engineIsKeyEntry(String alias) {
12241224
* certificate entry or alias does not exist
12251225
*/
12261226
@Override
1227-
public synchronized boolean engineIsCertificateEntry(String alias) {
1227+
public boolean engineIsCertificateEntry(String alias) {
12281228

12291229
Object entry = null;
12301230
boolean isCert = false;
@@ -1259,7 +1259,7 @@ public synchronized boolean engineIsCertificateEntry(String alias) {
12591259
* certificate, or null if no entry is found.
12601260
*/
12611261
@Override
1262-
public synchronized String engineGetCertificateAlias(Certificate cert) {
1262+
public String engineGetCertificateAlias(Certificate cert) {
12631263

12641264
Certificate tmp = null;
12651265

‎src/test/java/com/wolfssl/provider/jce/test/WolfSSLKeyStoreTest.java‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2208,5 +2208,171 @@ public void testMixedIterationCountsInSameKeyStore()
22082208
}
22092209
}
22102210
}
2211+
2212+
/**
2213+
* Test concurrent access to engineGetCertificateAlias() while other
2214+
* threads are modifying the same KeyStore instance via setKeyEntry()
2215+
* and setCertificateEntry().
2216+
*/
2217+
@Test
2218+
public void testGetCertificateAliasConcurrent()
2219+
throws KeyStoreException, IOException, FileNotFoundException,
2220+
NoSuchProviderException, NoSuchAlgorithmException,
2221+
CertificateException, InvalidKeySpecException,
2222+
UnrecoverableKeyException, InterruptedException {
2223+
2224+
int numThreads = 10;
2225+
int iterationsPerThread = 100;
2226+
final KeyStore store = KeyStore.getInstance(storeType,
2227+
storeProvider);
2228+
final LinkedBlockingQueue<Integer> results =
2229+
new LinkedBlockingQueue<>();
2230+
final CountDownLatch startLatch = new CountDownLatch(1);
2231+
final CountDownLatch doneLatch = new CountDownLatch(numThreads);
2232+
2233+
/* Initialize KeyStore with some initial entries */
2234+
store.load(null, null);
2235+
store.setKeyEntry("initialKey", serverKeyRsa,
2236+
storePass.toCharArray(), rsaServerChain);
2237+
store.setCertificateEntry("initialCert", serverCertRsa);
2238+
2239+
/* Create thread pool */
2240+
ExecutorService executor = Executors.newFixedThreadPool(numThreads);
2241+
2242+
/* Start reader threads that call getCertificateAlias() */
2243+
for (int i = 0; i < numThreads / 2; i++) {
2244+
final int threadId = i;
2245+
executor.submit(new Runnable() {
2246+
@Override
2247+
public void run() {
2248+
try {
2249+
/* Wait for all threads to be ready */
2250+
startLatch.await();
2251+
2252+
for (int j = 0; j < iterationsPerThread; j++) {
2253+
/* Look up alias for existing certificate */
2254+
String alias = store.getCertificateAlias(
2255+
serverCertRsa);
2256+
if (alias == null) {
2257+
/* Certificate should exist, either as
2258+
* initialCert or in a key entry chain */
2259+
results.add(1);
2260+
return;
2261+
}
2262+
2263+
/* Verify alias is valid */
2264+
if (!alias.equals("initialCert") &&
2265+
!alias.startsWith("writerKey")) {
2266+
results.add(1);
2267+
return;
2268+
}
2269+
2270+
/* Look up alias for certificate that might
2271+
* be added/removed by writers */
2272+
alias = store.getCertificateAlias(
2273+
clientCertRsa);
2274+
/* Result can be null or valid alias, both OK */
2275+
if (alias != null &&
2276+
!alias.startsWith("writerCert")) {
2277+
results.add(1);
2278+
return;
2279+
}
2280+
}
2281+
2282+
/* Success */
2283+
results.add(0);
2284+
2285+
} catch (Exception e) {
2286+
e.printStackTrace();
2287+
results.add(1);
2288+
2289+
} finally {
2290+
doneLatch.countDown();
2291+
}
2292+
}
2293+
});
2294+
}
2295+
2296+
/* Start writer threads that modify KeyStore */
2297+
for (int i = numThreads / 2; i < numThreads; i++) {
2298+
final int threadId = i;
2299+
executor.submit(new Runnable() {
2300+
@Override
2301+
public void run() {
2302+
try {
2303+
/* Wait for all threads to be ready */
2304+
startLatch.await();
2305+
2306+
for (int j = 0; j < iterationsPerThread; j++) {
2307+
String keyAlias = "writerKey" + threadId +
2308+
"_" + j;
2309+
String certAlias = "writerCert" + threadId +
2310+
"_" + j;
2311+
2312+
/* Add key entry */
2313+
store.setKeyEntry(keyAlias, serverKeyRsa,
2314+
storePass.toCharArray(), rsaServerChain);
2315+
2316+
/* Add certificate entry */
2317+
store.setCertificateEntry(certAlias,
2318+
clientCertRsa);
2319+
2320+
/* Delete some entries periodically */
2321+
if (j % 10 == 0 && j > 0) {
2322+
String oldKeyAlias = "writerKey" + threadId +
2323+
"_" + (j - 10);
2324+
String oldCertAlias = "writerCert" +
2325+
threadId + "_" + (j - 10);
2326+
try {
2327+
store.deleteEntry(oldKeyAlias);
2328+
store.deleteEntry(oldCertAlias);
2329+
} catch (KeyStoreException e) {
2330+
/* Entry might not exist, ignore */
2331+
}
2332+
}
2333+
}
2334+
2335+
/* Success */
2336+
results.add(0);
2337+
2338+
} catch (Exception e) {
2339+
e.printStackTrace();
2340+
results.add(1);
2341+
2342+
} finally {
2343+
doneLatch.countDown();
2344+
}
2345+
}
2346+
});
2347+
}
2348+
2349+
/* Start all threads simultaneously */
2350+
startLatch.countDown();
2351+
2352+
/* Wait for all threads to complete */
2353+
doneLatch.await();
2354+
2355+
/* Shutdown executor */
2356+
executor.shutdown();
2357+
2358+
/* Check results - all threads should have succeeded */
2359+
assertEquals("Expected " + numThreads + " results",
2360+
numThreads, results.size());
2361+
2362+
Iterator<Integer> listIterator = results.iterator();
2363+
while (listIterator.hasNext()) {
2364+
Integer cur = listIterator.next();
2365+
if (cur != 0) {
2366+
fail("Threading error in concurrent " +
2367+
"getCertificateAlias test");
2368+
}
2369+
}
2370+
2371+
/* Verify KeyStore is still in valid state */
2372+
assertNotNull(store.getCertificate("initialCert"));
2373+
Certificate[] chain = store.getCertificateChain("initialKey");
2374+
assertNotNull(chain);
2375+
assertTrue(chain.length > 0);
2376+
}
22112377
}
22122378

0 commit comments

Comments
 (0)