diff --git a/logstash-core/src/main/java/org/logstash/secret/store/backend/JavaKeyStore.java b/logstash-core/src/main/java/org/logstash/secret/store/backend/JavaKeyStore.java index b0424ff589f..1aabae79a62 100644 --- a/logstash-core/src/main/java/org/logstash/secret/store/backend/JavaKeyStore.java +++ b/logstash-core/src/main/java/org/logstash/secret/store/backend/JavaKeyStore.java @@ -27,8 +27,7 @@ import java.security.cert.CertificateException; import java.util.*; import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.concurrent.locks.ReentrantLock; import static org.logstash.secret.store.SecretStoreFactory.LOGSTASH_MARKER; @@ -46,9 +45,8 @@ public final class JavaKeyStore implements SecretStore { private char[] keyStorePass; private Path keyStorePath; private ProtectionParameter protectionParameter; - private Lock readLock; + private Lock lock; private boolean useDefaultPass = false; - private Lock writeLock; //package private for testing static String filePermissions = "rw-r--r--"; private static final boolean IS_WINDOWS = System.getProperty("os.name").startsWith("Windows"); @@ -69,7 +67,7 @@ public JavaKeyStore create(SecureConfig config) { } try { init(config); - writeLock.lock(); + lock.lock(); LOGGER.debug("Creating new keystore at {}.", keyStorePath.toAbsolutePath()); String keyStorePermissions = filePermissions; //create the keystore on disk with a default entry to identify this as a logstash keystore @@ -100,7 +98,7 @@ public JavaKeyStore create(SecureConfig config) { } catch (Exception e) { //should never happen throw new SecretStoreException.UnknownException("Error while trying to create the Logstash keystore. ", e); } finally { - releaseLock(writeLock); + releaseLock(lock); config.clearValues(); } } @@ -109,7 +107,7 @@ public JavaKeyStore create(SecureConfig config) { public void delete(SecureConfig config) { try { initLocks(); - writeLock.lock(); + lock.lock(); if (exists(config)) { Files.delete(Paths.get(new String(config.getPlainText(PATH_KEY)))); } @@ -118,7 +116,7 @@ public void delete(SecureConfig config) { } catch (Exception e) { //should never happen throw new SecretStoreException.UnknownException("Error while trying to delete the Logstash keystore", e); } finally { - releaseLock(writeLock); + releaseLock(lock); config.clearValues(); } } @@ -214,16 +212,14 @@ private void init(SecureConfig config) throws IOException, KeyStoreException { } private void initLocks(){ - ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); - readLock = readWriteLock.readLock(); - writeLock = readWriteLock.writeLock(); + lock = new ReentrantLock(); } @Override public Collection list() { Set identifiers = new HashSet<>(); try { - readLock.lock(); + lock.lock(); loadKeyStore(); Enumeration aliases = keyStore.aliases(); while (aliases.hasMoreElements()) { @@ -233,7 +229,7 @@ public Collection list() { } catch (Exception e) { throw new SecretStoreException.ListException(e); } finally { - releaseLock(readLock); + releaseLock(lock); } return identifiers; } @@ -255,7 +251,7 @@ public JavaKeyStore load(SecureConfig config) { } try { init(config); - readLock.lock(); + lock.lock(); try (final InputStream is = Files.newInputStream(keyStorePath)) { try { keyStore.load(is, this.keyStorePass); @@ -282,7 +278,7 @@ public JavaKeyStore load(SecureConfig config) { } catch (Exception e) { //should never happen throw new SecretStoreException.UnknownException("Error while trying to load the Logstash keystore", e); } finally { - releaseLock(readLock); + releaseLock(lock); config.clearValues(); } } @@ -299,7 +295,7 @@ private void loadKeyStore() throws CertificateException, NoSuchAlgorithmExceptio @Override public void persistSecret(SecretIdentifier identifier, byte[] secret) { try { - writeLock.lock(); + lock.lock(); loadKeyStore(); SecretKeyFactory factory = SecretKeyFactory.getInstance("PBE"); //PBEKey requires an ascii password, so base64 encode it @@ -317,14 +313,14 @@ public void persistSecret(SecretIdentifier identifier, byte[] secret) { } catch (Exception e) { throw new SecretStoreException.PersistException(identifier, e); } finally { - releaseLock(writeLock); + releaseLock(lock); } } @Override public void purgeSecret(SecretIdentifier identifier) { try { - writeLock.lock(); + lock.lock(); loadKeyStore(); keyStore.deleteEntry(identifier.toExternalForm()); saveKeyStore(); @@ -332,7 +328,7 @@ public void purgeSecret(SecretIdentifier identifier) { } catch (Exception e) { throw new SecretStoreException.PurgeException(identifier, e); } finally { - releaseLock(writeLock); + releaseLock(lock); } } @@ -346,7 +342,7 @@ private void releaseLock(Lock lock) { public byte[] retrieveSecret(SecretIdentifier identifier) { if (identifier != null && identifier.getKey() != null && !identifier.getKey().isEmpty()) { try { - readLock.lock(); + lock.lock(); loadKeyStore(); SecretKeyFactory factory = SecretKeyFactory.getInstance("PBE"); KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry) keyStore.getEntry(identifier.toExternalForm(), protectionParameter); @@ -365,7 +361,7 @@ public byte[] retrieveSecret(SecretIdentifier identifier) { } catch (Exception e) { throw new SecretStoreException.RetrievalException(identifier, e); } finally { - releaseLock(readLock); + releaseLock(lock); } } return null; diff --git a/logstash-core/src/test/java/org/logstash/secret/store/backend/JavaKeyStoreTest.java b/logstash-core/src/test/java/org/logstash/secret/store/backend/JavaKeyStoreTest.java index ed2013cd962..6d09df42b53 100644 --- a/logstash-core/src/test/java/org/logstash/secret/store/backend/JavaKeyStoreTest.java +++ b/logstash-core/src/test/java/org/logstash/secret/store/backend/JavaKeyStoreTest.java @@ -26,15 +26,18 @@ import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermission; import java.util.*; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.IntStream; import static java.nio.file.attribute.PosixFilePermission.*; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Fail.fail; -import static org.hamcrest.CoreMatchers.isA; import static org.logstash.secret.store.SecretStoreFactory.LOGSTASH_MARKER; /** @@ -669,4 +672,31 @@ public void wrongPassword() throws Exception { withDefinedPassConfig.add(SecretStoreFactory.KEYSTORE_ACCESS_KEY, "wrongpassword".toCharArray()); new JavaKeyStore().load(withDefinedPassConfig); } + + @Test(timeout = 40_000) + public void concurrentReadTest() throws Exception { + + final int KEYSTORE_COUNT = 250; + + final ExecutorService executorService = Executors.newFixedThreadPool(KEYSTORE_COUNT); + String password = "pAssW3rd!"; + keyStore.persistSecret(new SecretIdentifier("password"), password.getBytes(StandardCharsets.UTF_8)); + try{ + Callable reader = () -> keyStore.retrieveSecret(new SecretIdentifier("password")); + + List> futures = new ArrayList<>(); + for (int i = 0; i < KEYSTORE_COUNT; i++) { + futures.add(executorService.submit(reader)); + } + + for (Future future : futures) { + byte[] result = future.get(); + assertThat(result).isNotNull(); + assertThat(new String(result, StandardCharsets.UTF_8)).isEqualTo(password); + } + } finally { + executorService.shutdownNow(); + executorService.awaitTermination(Long.MAX_VALUE, TimeUnit.MILLISECONDS); + } + } } \ No newline at end of file