From 53b0d5cb9f83f9ef47495f036ea028a1def88f24 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 21 Feb 2019 13:38:30 +0100 Subject: [PATCH] Environment now being injected into WindowsProtectedKeychainAccess --- main/keychain/pom.xml | 5 + .../WindowsProtectedKeychainAccess.java | 115 +++++++++--------- .../WindowsProtectedKeychainAccessTest.java | 24 ++-- 3 files changed, 76 insertions(+), 68 deletions(-) diff --git a/main/keychain/pom.xml b/main/keychain/pom.xml index 8ae41d506..29471ed1d 100644 --- a/main/keychain/pom.xml +++ b/main/keychain/pom.xml @@ -10,6 +10,11 @@ System Keychain Access + + org.cryptomator + commons + + org.apache.commons commons-lang3 diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java index 57d9cdfa7..ce8c5744d 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java @@ -5,30 +5,6 @@ *******************************************************************************/ package org.cryptomator.keychain; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.Reader; -import java.io.UncheckedIOException; -import java.io.Writer; -import java.lang.reflect.Type; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.NoSuchFileException; -import java.nio.file.Path; -import java.nio.file.StandardOpenOption; -import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; -import java.util.UUID; - -import javax.inject.Inject; - import com.google.common.io.BaseEncoding; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -42,11 +18,36 @@ import com.google.gson.JsonSerializer; import com.google.gson.annotations.SerializedName; import com.google.gson.reflect.TypeToken; import org.apache.commons.lang3.SystemUtils; +import org.cryptomator.common.Environment; import org.cryptomator.jni.WinDataProtection; import org.cryptomator.jni.WinFunctions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.inject.Inject; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Reader; +import java.io.UncheckedIOException; +import java.io.Writer; +import java.lang.reflect.Type; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; + import static java.nio.charset.StandardCharsets.UTF_8; class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { @@ -57,24 +58,13 @@ class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { .disableHtmlEscaping().create(); private final Optional winFunctions; - private final Path keychainPath; + private final List keychainPaths; private Map keychainEntries; @Inject - public WindowsProtectedKeychainAccess(Optional winFunctions) { + public WindowsProtectedKeychainAccess(Optional winFunctions, Environment environment) { this.winFunctions = winFunctions; - String keychainPathProperty = System.getProperty("cryptomator.keychainPath"); - if (keychainPathProperty == null) { - LOG.warn("Windows DataProtection module loaded, but no cryptomator.keychainPath property found."); - } - if (keychainPathProperty != null) { - if (keychainPathProperty.startsWith("~/")) { - keychainPathProperty = SystemUtils.USER_HOME + keychainPathProperty.substring(1); - } - this.keychainPath = FileSystems.getDefault().getPath(keychainPathProperty); - } else { - this.keychainPath = null; - } + this.keychainPaths = environment.getKeychainPath().collect(Collectors.toList()); } private WinDataProtection dataProtection() { @@ -124,7 +114,7 @@ class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { @Override public boolean isSupported() { - return SystemUtils.IS_OS_WINDOWS && winFunctions.isPresent() && keychainPath != null; + return SystemUtils.IS_OS_WINDOWS && winFunctions.isPresent() && !keychainPaths.isEmpty(); } private byte[] generateSalt() { @@ -138,30 +128,44 @@ class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { private void loadKeychainEntriesIfNeeded() { if (keychainEntries == null) { - loadKeychainEntries(); - } - assert keychainEntries != null; - } - - private void loadKeychainEntries() { - Type type = new TypeToken>() { - }.getType(); - try (InputStream in = Files.newInputStream(keychainPath, StandardOpenOption.READ); // - Reader reader = new InputStreamReader(in, UTF_8)) { - keychainEntries = GSON.fromJson(reader, type); - } catch (JsonParseException | NoSuchFileException e) { - LOG.info("Creating new keychain at path {}", keychainPath); - } catch (IOException e) { - throw new UncheckedIOException("Could not read keychain from path " + keychainPath, e); + for (Path keychainPath : keychainPaths) { + Optional> keychain = loadKeychainEntries(keychainPath); + if (keychain.isPresent()) { + keychainEntries = keychain.get(); + break; + } + } } if (keychainEntries == null) { + LOG.info("Unable to load existing keychain file, creating new keychain."); keychainEntries = new HashMap<>(); } } + private Optional> loadKeychainEntries(Path keychainPath) { + LOG.debug("Attempting to load keychain from {}", keychainPath); + Type type = new TypeToken>() { + }.getType(); + try (InputStream in = Files.newInputStream(keychainPath, StandardOpenOption.READ); // + Reader reader = new InputStreamReader(in, UTF_8)) { + return Optional.of(GSON.fromJson(reader, type)); + } catch (NoSuchFileException | JsonParseException e) { + return Optional.empty(); + } catch (IOException e) { + throw new UncheckedIOException("Could not read keychain from path " + keychainPath, e); + } + } + private void saveKeychainEntries() { + if (keychainPaths.isEmpty()) { + throw new IllegalStateException("Can't save keychain if no keychain path is specified."); + } + saveKeychainEntries(keychainPaths.get(0)); + } + + private void saveKeychainEntries(Path keychainPath) { try (OutputStream out = Files.newOutputStream(keychainPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); // - Writer writer = new OutputStreamWriter(out, UTF_8)) { + Writer writer = new OutputStreamWriter(out, UTF_8)) { GSON.toJson(keychainEntries, writer); } catch (IOException e) { throw new UncheckedIOException("Could not read keychain from path " + keychainPath, e); @@ -169,6 +173,7 @@ class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { } private static class KeychainEntry { + @SerializedName("ciphertext") byte[] ciphertext; @SerializedName("salt") diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/WindowsProtectedKeychainAccessTest.java b/main/keychain/src/test/java/org/cryptomator/keychain/WindowsProtectedKeychainAccessTest.java index 88c23ffa1..796634f5e 100644 --- a/main/keychain/src/test/java/org/cryptomator/keychain/WindowsProtectedKeychainAccessTest.java +++ b/main/keychain/src/test/java/org/cryptomator/keychain/WindowsProtectedKeychainAccessTest.java @@ -5,45 +5,42 @@ *******************************************************************************/ package org.cryptomator.keychain; +import org.cryptomator.common.Environment; import org.cryptomator.jni.WinDataProtection; import org.cryptomator.jni.WinFunctions; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mockito; import org.mockito.stubbing.Answer; import java.io.IOException; -import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; +import java.util.stream.Stream; public class WindowsProtectedKeychainAccessTest { - private Path tmpFile; + private Path keychainPath; private WindowsProtectedKeychainAccess keychain; @BeforeEach - public void setup() throws IOException { - tmpFile = Files.createTempFile("unit-tests", ".tmp"); - System.setProperty("cryptomator.keychainPath", tmpFile.toAbsolutePath().normalize().toString()); + public void setup(@TempDir Path tempDir) throws IOException { + keychainPath = tempDir.resolve("keychainfile.tmp"); + Environment env = Mockito.mock(Environment.class); + Mockito.when(env.getKeychainPath()).thenReturn(Stream.of(keychainPath)); WinFunctions winFunctions = Mockito.mock(WinFunctions.class); WinDataProtection winDataProtection = Mockito.mock(WinDataProtection.class); Mockito.when(winFunctions.dataProtection()).thenReturn(winDataProtection); Answer answerReturningFirstArg = invocation -> ((byte[]) invocation.getArgument(0)).clone(); Mockito.when(winDataProtection.protect(Mockito.any(), Mockito.any())).thenAnswer(answerReturningFirstArg); Mockito.when(winDataProtection.unprotect(Mockito.any(), Mockito.any())).thenAnswer(answerReturningFirstArg); - keychain = new WindowsProtectedKeychainAccess(Optional.of(winFunctions)); - } - - @AfterEach - public void teardown() throws IOException { - Files.deleteIfExists(tmpFile); + keychain = new WindowsProtectedKeychainAccess(Optional.of(winFunctions), env); } @Test - public void testStoreAndLoad() { + public void testStoreAndLoad() throws IOException { String storedPw1 = "topSecret"; String storedPw2 = "bottomSecret"; keychain.storePassphrase("myPassword", storedPw1); @@ -54,6 +51,7 @@ public class WindowsProtectedKeychainAccessTest { Assertions.assertEquals(storedPw2, loadedPw2); keychain.deletePassphrase("myPassword"); Assertions.assertNull(keychain.loadPassphrase("myPassword")); + Assertions.assertNotNull(keychain.loadPassphrase("myOtherPassword")); Assertions.assertNull(keychain.loadPassphrase("nonExistingPassword")); }