Skip to content

Commit

Permalink
Secretstore windows fixes.
Browse files Browse the repository at this point in the history
* Can't create file with POSIX flags, since not all Windows are POSIX
* Can't explicitly lock the underlying file on Windows, since the
keystore already implicity (implementation detail) locks and does not
allow any interactions if locked.  This is very similar to the *nix
behavior, except on *nix you can read while locked and need to manually
manage the lock.

Fixes elastic#8938
  • Loading branch information
jakelandis committed Jan 15, 2018
1 parent 5d52329 commit 01ea0d4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public final class JavaKeyStore implements SecretStore {
private Lock writeLock;
//package private for testing
static String filePermissions = "rw-rw----";
private static final boolean IS_WINDOWS = System.getProperty("os.name").startsWith("Windows");

/**
* {@inheritDoc}
Expand All @@ -72,7 +73,8 @@ public JavaKeyStore create(SecureConfig config) {
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
Files.createFile(keyStorePath, PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString(keyStorePermissions)));
//can not set posix attributes on create here since not all Windows are posix, *nix will get the umask default and posix permissions will be set below
Files.createFile(keyStorePath);
try {
keyStore = KeyStore.Builder.newInstance(KEYSTORE_TYPE, null, protectionParameter).getKeyStore();
SecretKeyFactory factory = SecretKeyFactory.getInstance("PBE");
Expand Down Expand Up @@ -370,11 +372,14 @@ public byte[] retrieveSecret(SecretIdentifier identifier) {
* Saves the keystore with some extra meta data if needed. Note - need two output streams here to allow checking the with the append flag, and the other without an append.
*/
private void saveKeyStore() throws IOException, CertificateException, NoSuchAlgorithmException, KeyStoreException {
FileLock fileLock;
try (final FileOutputStream appendOs = new FileOutputStream(keyStorePath.toFile(), true)) {
fileLock = appendOs.getChannel().tryLock();
if (fileLock == null) {
throw new IllegalStateException("Can not save Logstash keystore. Some other process has a lock on the file: " + keyStorePath.toAbsolutePath());
FileLock fileLock = null;
// The keystore.store method on Windows checks for the file lock and does not allow _any_ interaction with the keystore if it is locked.
if(!IS_WINDOWS){
fileLock = appendOs.getChannel().tryLock();
if (fileLock == null) {
throw new IllegalStateException("Can not save Logstash keystore. Some other process has locked on the file: " + keyStorePath.toAbsolutePath());
}
}
try (final OutputStream os = Files.newOutputStream(keyStorePath, StandardOpenOption.WRITE)) {
keyStore.store(os, keyStorePass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ public void testExternalUpdateRead() throws IOException {
*/
@Test
public void testFileLock() throws Exception {
boolean isWindows = System.getProperty("os.name").startsWith("Windows");
Path magicFile = folder.newFolder().toPath().resolve(EXTERNAL_TEST_FILE_LOCK);

String java = System.getProperty("java.home") + File.separator + "bin" + File.separator + "java";
Expand All @@ -491,17 +492,19 @@ public void testFileLock() throws Exception {
try {
keyStore.persistSecret(new SecretIdentifier("foo"), "bar".getBytes(StandardCharsets.UTF_8));
} catch (SecretStoreException.PersistException e) {
assertThat(e.getCause()).isInstanceOf(IllegalStateException.class);
assertThat(e.getCause().getMessage()).contains("has a lock on the file");
assertThat(e.getCause().getMessage()).contains("locked");
passed = true;
}
break;
}
assertThat(passed).isTrue();

//can still read
byte[] marker = keyStore.retrieveSecret(LOGSTASH_MARKER);
assertThat(new String(marker, StandardCharsets.UTF_8)).isEqualTo(LOGSTASH_MARKER.getKey());
// The keystore.store method on Windows checks for the file lock and does not allow _any_ interaction with the keystore if it is locked.
if (!isWindows) {
//can still read
byte[] marker = keyStore.retrieveSecret(LOGSTASH_MARKER);
assertThat(new String(marker, StandardCharsets.UTF_8)).isEqualTo(LOGSTASH_MARKER.getKey());
}

//block until other JVM finishes
future.get();
Expand Down

0 comments on commit 01ea0d4

Please sign in to comment.