diff --git a/main/keychain/pom.xml b/main/keychain/pom.xml index 1983d5873..5c96a8aad 100644 --- a/main/keychain/pom.xml +++ b/main/keychain/pom.xml @@ -13,6 +13,15 @@ org.apache.commons commons-lang3 + + com.google.code.gson + gson + 2.7 + + + commons-codec + commons-codec + org.bouncycastle bcprov-jdk15on diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java index ab593f674..3f2cf6abc 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java @@ -3,18 +3,20 @@ package org.cryptomator.keychain; import java.util.Optional; import java.util.Set; +import org.cryptomator.jni.JniModule; + import com.google.common.collect.Sets; import dagger.Module; import dagger.Provides; import dagger.multibindings.ElementsIntoSet; -@Module +@Module(includes = {JniModule.class}) public class KeychainModule { @Provides @ElementsIntoSet - Set provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsSystemKeychainAccess winKeychain) { + Set provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsProtectedKeychainAccess winKeychain) { return Sets.newHashSet(macKeychain, winKeychain); } diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java index a80d084ef..5e8b27b6c 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java @@ -1,21 +1,21 @@ package org.cryptomator.keychain; +import java.util.Optional; + import javax.inject.Inject; -import javax.inject.Singleton; import org.apache.commons.lang3.SystemUtils; -import org.cryptomator.jni.JniModule; +import org.cryptomator.jni.MacFunctions; import org.cryptomator.jni.MacKeychainAccess; -@Singleton class MacSystemKeychainAccess implements KeychainAccessStrategy { private final MacKeychainAccess keychain; @Inject - public MacSystemKeychainAccess() { - if (JniModule.macFunctions().isPresent()) { - this.keychain = JniModule.macFunctions().get().getKeychainAccess(); + public MacSystemKeychainAccess(Optional macFunctions) { + if (macFunctions.isPresent()) { + this.keychain = macFunctions.get().keychainAccess(); } else { this.keychain = null; } diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java new file mode 100644 index 000000000..205772d15 --- /dev/null +++ b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java @@ -0,0 +1,188 @@ +package org.cryptomator.keychain; + +import static java.nio.charset.StandardCharsets.UTF_8; + +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.charset.StandardCharsets; +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 org.apache.commons.codec.binary.Base64; +import org.apache.commons.lang3.SystemUtils; +import org.cryptomator.jni.WinDataProtection; +import org.cryptomator.jni.WinFunctions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonDeserializationContext; +import com.google.gson.JsonDeserializer; +import com.google.gson.JsonElement; +import com.google.gson.JsonParseException; +import com.google.gson.JsonPrimitive; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; +import com.google.gson.annotations.SerializedName; +import com.google.gson.reflect.TypeToken; + +class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { + + private static final Logger LOG = LoggerFactory.getLogger(WindowsProtectedKeychainAccess.class); + private static final Gson GSON = new GsonBuilder().setPrettyPrinting() // + .registerTypeHierarchyAdapter(byte[].class, new ByteArrayJsonAdapter()) // + .disableHtmlEscaping().create(); + + private final WinDataProtection dataProtection; + private final Path keychainPath; + private Map keychainEntries; + + @Inject + public WindowsProtectedKeychainAccess(Optional winFunctions) { + if (winFunctions.isPresent()) { + this.dataProtection = winFunctions.get().dataProtection(); + } else { + this.dataProtection = null; + } + final String keychainPathProperty = System.getProperty("cryptomator.keychainPath"); + if (dataProtection != null && keychainPathProperty == null) { + LOG.warn("Windows DataProtection module loaded, but no keychainPath configured."); + } + if (keychainPathProperty != null) { + this.keychainPath = FileSystems.getDefault().getPath(keychainPathProperty); + } else { + this.keychainPath = null; + } + } + + @Override + public void storePassphrase(String key, CharSequence passphrase) { + loadKeychainEntriesIfNeeded(); + ByteBuffer buf = UTF_8.encode(CharBuffer.wrap(passphrase)); + byte[] cleartext = new byte[buf.remaining()]; + buf.get(cleartext); + KeychainEntry entry = new KeychainEntry(); + entry.salt = generateSalt(); + entry.ciphertext = dataProtection.protect(cleartext, entry.salt); + Arrays.fill(buf.array(), (byte) 0x00); + Arrays.fill(cleartext, (byte) 0x00); + keychainEntries.put(key, entry); + saveKeychainEntries(); + } + + @Override + public char[] loadPassphrase(String key) { + loadKeychainEntriesIfNeeded(); + KeychainEntry entry = keychainEntries.get(key); + if (entry == null) { + return null; + } + byte[] cleartext = dataProtection.unprotect(entry.ciphertext, entry.salt); + if (cleartext == null) { + return null; + } + CharBuffer buf = UTF_8.decode(ByteBuffer.wrap(cleartext)); + char[] passphrase = new char[buf.remaining()]; + buf.get(passphrase); + Arrays.fill(cleartext, (byte) 0x00); + Arrays.fill(buf.array(), (char) 0x00); + return passphrase; + } + + @Override + public void deletePassphrase(String key) { + loadKeychainEntriesIfNeeded(); + keychainEntries.remove(key); + saveKeychainEntries(); + } + + @Override + public boolean isSupported() { + return SystemUtils.IS_OS_WINDOWS && dataProtection != null && keychainPath != null; + } + + private byte[] generateSalt() { + byte[] result = new byte[2 * Long.BYTES]; + UUID uuid = UUID.randomUUID(); + ByteBuffer buf = ByteBuffer.wrap(result); + buf.putLong(uuid.getMostSignificantBits()); + buf.putLong(uuid.getLeastSignificantBits()); + return result; + } + + 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); + } + if (keychainEntries == null) { + keychainEntries = new HashMap<>(); + } + } + + private void saveKeychainEntries() { + try (OutputStream out = Files.newOutputStream(keychainPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); // + 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); + } + } + + private static class KeychainEntry { + @SerializedName("ciphertext") + byte[] ciphertext; + @SerializedName("salt") + byte[] salt; + } + + private static class ByteArrayJsonAdapter implements JsonSerializer, JsonDeserializer { + + private static final Base64 BASE64 = new Base64(); + + @Override + public byte[] deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { + return BASE64.decode(json.getAsString().getBytes(StandardCharsets.UTF_8)); + } + + @Override + public JsonElement serialize(byte[] src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive(new String(BASE64.encode(src), StandardCharsets.UTF_8)); + } + + } + +} diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsSystemKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsSystemKeychainAccess.java deleted file mode 100644 index 47d56953c..000000000 --- a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsSystemKeychainAccess.java +++ /dev/null @@ -1,51 +0,0 @@ -package org.cryptomator.keychain; - -import java.io.IOException; -import java.security.GeneralSecurityException; -import java.security.KeyStore; - -import javax.inject.Inject; -import javax.inject.Singleton; - -import org.apache.commons.lang3.SystemUtils; - -@Singleton -class WindowsSystemKeychainAccess implements KeychainAccessStrategy { - - private final KeyStore keyStore; - - @Inject - public WindowsSystemKeychainAccess() { - KeyStore ks; - try { - ks = KeyStore.getInstance("Windows-MY", "SunMSCAPI"); - ks.load(null); - } catch (GeneralSecurityException | IOException e) { - ks = null; - } - this.keyStore = ks; - } - - @Override - public void storePassphrase(String key, CharSequence passphrase) { - // TODO Auto-generated method stub - } - - @Override - public char[] loadPassphrase(String key) { - // TODO Auto-generated method stub - return null; - } - - @Override - public void deletePassphrase(String key) { - // TODO Auto-generated method stub - - } - - @Override - public boolean isSupported() { - return SystemUtils.IS_OS_WINDOWS && keyStore != null; - } - -} diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java b/main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java index c15c8c1f1..2d8fe4e9e 100644 --- a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java +++ b/main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java @@ -9,7 +9,7 @@ public class KeychainModuleTest { @Test public void testGetKeychain() { - Optional keychainAccess = DaggerKeychainComponent.builder().keychainModule(new KeychainTestModule()).build().keychainAccess(); + Optional keychainAccess = DaggerTestKeychainComponent.builder().jniModule(new TestJniModule()).keychainModule(new TestKeychainModule()).build().keychainAccess(); Assert.assertTrue(keychainAccess.isPresent()); Assert.assertTrue(keychainAccess.get() instanceof MapKeychainAccess); } diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/MapKeychainAccess.java b/main/keychain/src/test/java/org/cryptomator/keychain/MapKeychainAccess.java index 081a0b0e6..8b3bb3e07 100644 --- a/main/keychain/src/test/java/org/cryptomator/keychain/MapKeychainAccess.java +++ b/main/keychain/src/test/java/org/cryptomator/keychain/MapKeychainAccess.java @@ -23,8 +23,7 @@ class MapKeychainAccess implements KeychainAccessStrategy { @Override public void deletePassphrase(String key) { - // TODO Auto-generated method stub - + map.remove(key); } @Override diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/TestJniModule.java b/main/keychain/src/test/java/org/cryptomator/keychain/TestJniModule.java new file mode 100644 index 000000000..79d770b57 --- /dev/null +++ b/main/keychain/src/test/java/org/cryptomator/keychain/TestJniModule.java @@ -0,0 +1,23 @@ +package org.cryptomator.keychain; + +import java.util.Optional; + +import org.cryptomator.jni.JniModule; +import org.cryptomator.jni.MacFunctions; +import org.cryptomator.jni.WinFunctions; + +import dagger.Lazy; + +public class TestJniModule extends JniModule { + + @Override + public Optional winFunctions(Lazy winFunction) { + return Optional.empty(); + } + + @Override + public Optional macFunctions(Lazy winFunction) { + return Optional.empty(); + } + +} diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainComponent.java b/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java similarity index 86% rename from main/keychain/src/test/java/org/cryptomator/keychain/KeychainComponent.java rename to main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java index 44301ed7f..30e42ee52 100644 --- a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainComponent.java +++ b/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java @@ -8,7 +8,7 @@ import dagger.Component; @Singleton @Component(modules = KeychainModule.class) -interface KeychainComponent { +interface TestKeychainComponent { Optional keychainAccess(); diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainTestModule.java b/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java similarity index 62% rename from main/keychain/src/test/java/org/cryptomator/keychain/KeychainTestModule.java rename to main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java index e2636981b..4bb403906 100644 --- a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainTestModule.java +++ b/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java @@ -4,10 +4,10 @@ import java.util.Set; import com.google.common.collect.Sets; -public class KeychainTestModule extends KeychainModule { +public class TestKeychainModule extends KeychainModule { @Override - Set provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsSystemKeychainAccess winKeychain) { + Set provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsProtectedKeychainAccess winKeychain) { return Sets.newHashSet(new MapKeychainAccess()); } diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/WindowsProtectedKeychainAccessTest.java b/main/keychain/src/test/java/org/cryptomator/keychain/WindowsProtectedKeychainAccessTest.java new file mode 100644 index 000000000..9b3fd94e8 --- /dev/null +++ b/main/keychain/src/test/java/org/cryptomator/keychain/WindowsProtectedKeychainAccessTest.java @@ -0,0 +1,60 @@ +package org.cryptomator.keychain; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; + +import org.cryptomator.jni.WinDataProtection; +import org.cryptomator.jni.WinFunctions; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.Mockito; +import org.mockito.stubbing.Answer; + +public class WindowsProtectedKeychainAccessTest { + + @Rule + public final ExpectedException thrown = ExpectedException.none(); + + private Path tmpFile; + private WindowsProtectedKeychainAccess keychain; + + @Before + public void setup() throws IOException, ReflectiveOperationException { + tmpFile = Files.createTempFile("unit-tests", ".tmp"); + System.setProperty("cryptomator.keychainPath", tmpFile.toAbsolutePath().normalize().toString()); + WinFunctions winFunctions = Mockito.mock(WinFunctions.class); + WinDataProtection winDataProtection = Mockito.mock(WinDataProtection.class); + Mockito.when(winFunctions.dataProtection()).thenReturn(winDataProtection); + Answer answerReturningFirstArg = invocation -> invocation.getArgumentAt(0, byte[].class).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)); + } + + @After + public void teardown() throws IOException { + Files.deleteIfExists(tmpFile); + } + + @Test + public void testStoreAndLoad() { + String storedPw1 = "topSecret"; + String storedPw2 = "bottomSecret"; + keychain.storePassphrase("myPassword", storedPw1); + keychain.storePassphrase("myOtherPassword", storedPw2); + String loadedPw1 = new String(keychain.loadPassphrase("myPassword")); + String loadedPw2 = new String(keychain.loadPassphrase("myOtherPassword")); + Assert.assertEquals(storedPw1, loadedPw1); + Assert.assertEquals(storedPw2, loadedPw2); + keychain.deletePassphrase("myPassword"); + Assert.assertNull(keychain.loadPassphrase("myPassword")); + Assert.assertNull(keychain.loadPassphrase("nonExistingPassword")); + } + +} diff --git a/main/keychain/src/test/resources/log4j2.xml b/main/keychain/src/test/resources/log4j2.xml new file mode 100644 index 000000000..39c2f8545 --- /dev/null +++ b/main/keychain/src/test/resources/log4j2.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/main/pom.xml b/main/pom.xml index 1a412a88a..4716aed5b 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -41,7 +41,7 @@ 3.1 2.4.4 1.10.19 - 2.4 + 2.6.1 diff --git a/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java b/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java index 3de672b47..d4f5f7b38 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java @@ -10,7 +10,6 @@ package org.cryptomator.ui; import static java.util.stream.Collectors.toList; -import java.util.Optional; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -24,7 +23,6 @@ import org.cryptomator.frontend.FrontendId; import org.cryptomator.frontend.webdav.WebDavModule; import org.cryptomator.frontend.webdav.WebDavServer; import org.cryptomator.jni.JniModule; -import org.cryptomator.jni.MacFunctions; import org.cryptomator.keychain.KeychainModule; import org.cryptomator.ui.model.Vault; import org.cryptomator.ui.model.VaultObjectMapperProvider; @@ -43,7 +41,7 @@ import javafx.application.Application; import javafx.beans.Observable; import javafx.stage.Stage; -@Module(includes = {CryptoEngineModule.class, CommonsModule.class, WebDavModule.class, KeychainModule.class}) +@Module(includes = {CryptoEngineModule.class, CommonsModule.class, WebDavModule.class, KeychainModule.class, JniModule.class}) class CryptomatorModule { private static final Logger LOG = LoggerFactory.getLogger(CryptomatorModule.class); @@ -111,12 +109,6 @@ class CryptomatorModule { return closer.closeLater(webDavServer, WebDavServer::stop).get().orElseThrow(IllegalStateException::new); } - @Provides - @Singleton - Optional provideMacFunctions() { - return JniModule.macFunctions(); - } - private void setValidFrontendIds(WebDavServer webDavServer, Vaults vaults) { webDavServer.setValidFrontendIds(vaults.stream() // .map(Vault::getId).map(FrontendId::from).collect(toList()));