From ffa68589c668f4bfff32828d6aa582bf184934d2 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 14 Sep 2023 16:31:09 +0200 Subject: [PATCH 1/3] first impl with explicit byte array conversion --- pom.xml | 17 ++++--- src/main/java/module-info.java | 5 +- .../keychain/ByteArrayJsonAdapter.java | 46 ++++++++++++------- .../windows/keychain/KeychainEntry.java | 18 +++++--- .../WindowsProtectedKeychainAccess.java | 40 ++++++++-------- 5 files changed, 73 insertions(+), 53 deletions(-) diff --git a/pom.xml b/pom.xml index 30a7fd3..b53a1aa 100644 --- a/pom.xml +++ b/pom.xml @@ -39,7 +39,7 @@ 1.2.0 1.7.36 - 2.9.0 + 2.15.2 5.8.2 @@ -69,11 +69,16 @@ slf4j-api ${slf4j.version} - - com.google.code.gson - gson - ${gson.version} - + + com.fasterxml.jackson.core + jackson-databind + ${jackson.version} + + + com.fasterxml.jackson.core + jackson-annotations + ${jackson.version} + diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 1e6e53a..97af1f7 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -10,9 +10,10 @@ module org.cryptomator.integrations.win { requires org.cryptomator.integrations.api; requires org.slf4j; - requires com.google.gson; + requires com.fasterxml.jackson.annotation; + requires com.fasterxml.jackson.databind; - opens org.cryptomator.windows.keychain to com.google.gson; + opens org.cryptomator.windows.keychain to com.fasterxml.jackson.databind; provides AutoStartProvider with WindowsAutoStart; provides KeychainAccessProvider with WindowsProtectedKeychainAccess; diff --git a/src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java b/src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java index 0761e36..6444373 100644 --- a/src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java +++ b/src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java @@ -1,26 +1,40 @@ package org.cryptomator.windows.keychain; -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 java.lang.reflect.Type; +import com.fasterxml.jackson.core.JacksonException; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.deser.std.StdDeserializer; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; + +import java.io.IOException; import java.util.Base64; -class ByteArrayJsonAdapter implements JsonSerializer, JsonDeserializer { +class ByteArrayJsonAdapter { + static class Serializer extends StdSerializer { + + public Serializer() { + super((Class) null); + } - @Override - public byte[] deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { - return Base64.getDecoder().decode(json.getAsString()); + @Override + public void serialize(byte[] value, JsonGenerator gen, SerializerProvider provider) throws IOException { + gen.writeString(Base64.getEncoder().encodeToString(value)); + } } - @Override - public JsonElement serialize(byte[] src, Type typeOfSrc, JsonSerializationContext context) { - return new JsonPrimitive(Base64.getEncoder().encodeToString(src)); + static class Deserializer extends StdDeserializer { + + public Deserializer() { + super(byte[].class); + } + + @Override + public byte[] deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JacksonException { + String base64 = p.getValueAsString(); + return Base64.getDecoder().decode(base64); + } } } diff --git a/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java b/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java index 7a22db5..0e00f49 100644 --- a/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java +++ b/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java @@ -1,12 +1,16 @@ package org.cryptomator.windows.keychain; -import com.google.gson.annotations.SerializedName; -class KeychainEntry { +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; - @SerializedName("ciphertext") - byte[] ciphertext; - - @SerializedName("salt") - byte[] salt; +record KeychainEntry(@JsonProperty("ciphertext") // + @JsonSerialize(using = ByteArrayJsonAdapter.Serializer.class) // + @JsonDeserialize(using = ByteArrayJsonAdapter.Deserializer.class) // + byte[] ciphertext, + @JsonProperty("salt") // + @JsonSerialize(using = ByteArrayJsonAdapter.Serializer.class) // + @JsonDeserialize(using = ByteArrayJsonAdapter.Deserializer.class) // + byte[] salt) { } diff --git a/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java b/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java index 9e3e591..333f2cb 100644 --- a/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java +++ b/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java @@ -1,9 +1,8 @@ package org.cryptomator.windows.keychain; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; -import com.google.gson.JsonParseException; -import com.google.gson.reflect.TypeToken; +import com.fasterxml.jackson.core.JacksonException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; import org.cryptomator.integrations.common.OperatingSystem; import org.cryptomator.integrations.common.Priority; import org.cryptomator.integrations.keychain.KeychainAccessException; @@ -19,7 +18,6 @@ import java.io.OutputStreamWriter; import java.io.Reader; import java.io.Writer; -import java.lang.reflect.Type; import java.nio.ByteBuffer; import java.nio.CharBuffer; import java.nio.file.Files; @@ -49,11 +47,7 @@ public class WindowsProtectedKeychainAccess implements KeychainAccessProvider { private static final Logger LOG = LoggerFactory.getLogger(WindowsProtectedKeychainAccess.class); private static final Path USER_HOME_REL = Path.of("~"); private static final Path USER_HOME = Path.of(System.getProperty("user.home")); - private static final Gson GSON = new GsonBuilder() // - .setPrettyPrinting() // - .registerTypeHierarchyAdapter(byte[].class, new ByteArrayJsonAdapter()) // - .disableHtmlEscaping() // - .create(); + private static final ObjectMapper JSON_MAPPER = new ObjectMapper(); private final List keychainPaths; private final WinDataProtection dataProtection; @@ -103,12 +97,11 @@ public void storePassphrase(String key, String displayName, CharSequence passphr 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); + var salt = generateSalt(); + var ciphertext = dataProtection.protect(cleartext, salt); Arrays.fill(buf.array(), (byte) 0x00); Arrays.fill(cleartext, (byte) 0x00); - keychainEntries.put(key, entry); + keychainEntries.put(key, new KeychainEntry(ciphertext, salt)); saveKeychainEntries(); } @@ -119,7 +112,7 @@ public char[] loadPassphrase(String key) throws KeychainAccessException { if (entry == null) { return null; } - byte[] cleartext = dataProtection.unprotect(entry.ciphertext, entry.salt); + byte[] cleartext = dataProtection.unprotect(entry.ciphertext(), entry.salt()); if (cleartext == null) { return null; } @@ -184,15 +177,18 @@ private void loadKeychainEntriesIfNeeded() throws KeychainAccessException { //visible for testing Optional> loadKeychainEntries(Path keychainPath) throws KeychainAccessException { LOG.debug("Attempting to load keychain from {}", keychainPath); - Type type = new TypeToken>() { - }.getType(); + TypeReference> type = new TypeReference<>() { + }; try (InputStream in = Files.newInputStream(keychainPath, StandardOpenOption.READ); // Reader reader = new InputStreamReader(in, UTF_8)) { - return Optional.ofNullable(GSON.fromJson(reader, type)); - } catch (NoSuchFileException | JsonParseException e) { + return Optional.ofNullable(JSON_MAPPER.readValue(reader, type)); + } catch (NoSuchFileException | JacksonException e) { + if (e instanceof JacksonException) { + LOG.warn("Unable to parse keychain file, overwriting existing one."); + } return Optional.empty(); - } catch (IOException e) { - throw new KeychainAccessException("Could not read keychain from path " + keychainPath, e); + } catch (IOException ioe) { + throw new KeychainAccessException("Could not read keychain from path " + keychainPath, ioe); } } @@ -206,7 +202,7 @@ private void saveKeychainEntries() throws KeychainAccessException { private void saveKeychainEntries(Path keychainPath) throws KeychainAccessException { try (OutputStream out = Files.newOutputStream(keychainPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); // Writer writer = new OutputStreamWriter(out, UTF_8)) { - GSON.toJson(keychainEntries, writer); + JSON_MAPPER.writeValue(writer, keychainEntries); } catch (IOException e) { throw new KeychainAccessException("Could not read keychain from path " + keychainPath, e); } From b4827ff2d7d0eed0d40f95d0f564060de92a2f77 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 14 Sep 2023 16:59:03 +0200 Subject: [PATCH 2/3] rely on default jackson mapper settings --- .../keychain/ByteArrayJsonAdapter.java | 40 ------------------- .../windows/keychain/KeychainEntry.java | 11 +---- ...rotectedKeychainAccessIntegrationTest.java | 10 +++++ .../windows/keychain/keychain.v1.2.2.json | 14 +++++++ 4 files changed, 25 insertions(+), 50 deletions(-) delete mode 100644 src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java create mode 100644 src/test/resources/org/cryptomator/windows/keychain/keychain.v1.2.2.json diff --git a/src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java b/src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java deleted file mode 100644 index 6444373..0000000 --- a/src/main/java/org/cryptomator/windows/keychain/ByteArrayJsonAdapter.java +++ /dev/null @@ -1,40 +0,0 @@ -package org.cryptomator.windows.keychain; - -import com.fasterxml.jackson.core.JacksonException; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.deser.std.StdDeserializer; -import com.fasterxml.jackson.databind.ser.std.StdSerializer; - -import java.io.IOException; -import java.util.Base64; - -class ByteArrayJsonAdapter { - static class Serializer extends StdSerializer { - - public Serializer() { - super((Class) null); - } - - @Override - public void serialize(byte[] value, JsonGenerator gen, SerializerProvider provider) throws IOException { - gen.writeString(Base64.getEncoder().encodeToString(value)); - } - } - - static class Deserializer extends StdDeserializer { - - public Deserializer() { - super(byte[].class); - } - - @Override - public byte[] deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JacksonException { - String base64 = p.getValueAsString(); - return Base64.getDecoder().decode(base64); - } - } - -} diff --git a/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java b/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java index 0e00f49..fd295f1 100644 --- a/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java +++ b/src/main/java/org/cryptomator/windows/keychain/KeychainEntry.java @@ -2,15 +2,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; -record KeychainEntry(@JsonProperty("ciphertext") // - @JsonSerialize(using = ByteArrayJsonAdapter.Serializer.class) // - @JsonDeserialize(using = ByteArrayJsonAdapter.Deserializer.class) // - byte[] ciphertext, - @JsonProperty("salt") // - @JsonSerialize(using = ByteArrayJsonAdapter.Serializer.class) // - @JsonDeserialize(using = ByteArrayJsonAdapter.Deserializer.class) // - byte[] salt) { +record KeychainEntry(@JsonProperty("ciphertext") byte[] ciphertext, @JsonProperty("salt") byte[] salt) { } diff --git a/src/test/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccessIntegrationTest.java b/src/test/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccessIntegrationTest.java index 69d1eb9..c948440 100644 --- a/src/test/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccessIntegrationTest.java +++ b/src/test/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccessIntegrationTest.java @@ -11,6 +11,7 @@ import org.junit.jupiter.api.io.TempDir; import java.io.IOException; +import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; @@ -58,6 +59,15 @@ public void testEmptyFileReturnsEmpty() throws KeychainAccessException, IOExcept Assertions.assertTrue(result.isEmpty()); } + + @Test + public void testLegacyKeychainFiles() throws URISyntaxException, KeychainAccessException { + var keychainPath = Path.of(this.getClass().getResource("keychain.v1.2.2.json").toURI()); + var result = keychainAccess.loadKeychainEntries(keychainPath); + + Assertions.assertTrue(result.isPresent()); + Assertions.assertEquals(3, result.get().size()); + } } } diff --git a/src/test/resources/org/cryptomator/windows/keychain/keychain.v1.2.2.json b/src/test/resources/org/cryptomator/windows/keychain/keychain.v1.2.2.json new file mode 100644 index 0000000..4f12b0a --- /dev/null +++ b/src/test/resources/org/cryptomator/windows/keychain/keychain.v1.2.2.json @@ -0,0 +1,14 @@ +{ + "cryptomator-device-p12": { + "ciphertext": "AQAAANCMnd8BFdERjHoAwE/Cl+sBAAAAMKzd7acJg0ScvBUOCjUb3wAAAAACAAAAAAAQZgAAAAEAACAAAABS6cSKuWqEOp+1HHcsSF0bSzQRRbotDIZPQIuMvbSdowAAAAAOgAAAAAIAACAAAABatZ2hp39vFTysmXRSO1rkk9vLx2S6rnRq6RIGkXlCnzAAAAAgTeOMtOKhnMzIjSwJEFE22UaB3sPGH6UxQ+EWMuZ9WPGHmOp8/WzTzFLCwbiIh9ZAAAAAf7DQSu40eCLUDKQzwFNJrBJsEWX2pK2GS2wOYFTlcMhSij7WkSHh9nAeZtRf14PyBpuLxCiBIayLJMpeK4HIXg==", + "salt": "Rwzfb0IHSqOXJw5MGRmCTw==" + }, + "-JgknuFBfItX": { + "ciphertext": "AQAAANCMnd8BFdERjHoAwE/Cl+sBAAAAMKzd7acJg0ScvBUOCjUb3wAAAAACAAAAAAAQZgAAAAEAACAAAABnYHm42qCU6kq50NV16IqJbrwrRwagpiYgopjK8xpyVAAAAAAOgAAAAAIAACAAAAC6XB/vmPRR7tK5sTOyY6DFgTXv4/ptTzGsKwJEsn1I8xAAAADUlPA6Qet/WSCbdK4WShMsQAAAAOVsW/9E/YNwHLl/qyzffkp2YR7UKeTcM3EkLoyI9Q2RJjmdidJc4wAet9zlp4qUPST+ukTxXAvMW/+RNxEAeZM=", + "salt": "CqEUWMVSQtqsDNhKeNCZHg==" + }, + "vkEAWUv_NmbN": { + "ciphertext": "AQAAANCMnd8BFdERjHoAwE/Cl+sBAAAAcLFOdGbRSEiT1nQ2ILvP/gAAAAACAAAAAAAQZgAAAAEAACAAAABYjK3wVUE/is3SHpdzLRaijVbLw6UYMSBizyiDDWNrqAAAAAAOgAAAAAIAACAAAAALkAoBV3/RwpSXwkTyDgOKiR7+wMnXq099WW4tt873NRAAAACH2Ki+R4fSi+569Y0RpoS0QAAAAEXbbfM0mUuIgajCfO7yfH1ysWdRUWfZYnfqDjeR5Up/FKqLbPkgZr+0fgb/sZEw+HyQ781A7+fXHFqB3hQoDV0=", + "salt": "evllr3H5SHyJN/1SciqlVQ==" + } +} \ No newline at end of file From 0872aec65fff6b2764ba8454f0a1157ee2e69ad3 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 15 Sep 2023 11:29:55 +0200 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Sebastian Stenzel --- .../keychain/WindowsProtectedKeychainAccess.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java b/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java index 333f2cb..abb71f1 100644 --- a/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java +++ b/src/main/java/org/cryptomator/windows/keychain/WindowsProtectedKeychainAccess.java @@ -182,13 +182,13 @@ Optional> loadKeychainEntries(Path keychainPath) thro try (InputStream in = Files.newInputStream(keychainPath, StandardOpenOption.READ); // Reader reader = new InputStreamReader(in, UTF_8)) { return Optional.ofNullable(JSON_MAPPER.readValue(reader, type)); - } catch (NoSuchFileException | JacksonException e) { - if (e instanceof JacksonException) { - LOG.warn("Unable to parse keychain file, overwriting existing one."); - } + } catch (NoSuchFileException e) { + return Optional.empty(); + } catch (JacksonException je) { + LOG.warn("Unable to parse keychain file, overwriting existing one."); return Optional.empty(); - } catch (IOException ioe) { - throw new KeychainAccessException("Could not read keychain from path " + keychainPath, ioe); + } catch (IOException e) { + throw new KeychainAccessException("Could not read keychain from path " + keychainPath, e); } }