Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace GSON by Jackson #23

Merged
merged 3 commits into from
Sep 17, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
<!-- runtime dependencies -->
<api.version>1.2.0</api.version>
<slf4j.version>1.7.36</slf4j.version>
<gson.version>2.9.0</gson.version>
<jackson.version>2.15.2</jackson.version>

<!-- test dependencies -->
<junit.jupiter.version>5.8.2</junit.jupiter.version>
Expand Down Expand Up @@ -69,11 +69,16 @@
<artifactId>slf4j-api</artifactId>
<version>${slf4j.version}</version>
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>${gson.version}</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>${jackson.version}</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
<version>${jackson.version}</version>
</dependency>

<!-- JUnit / Mockito / Hamcrest -->
<dependency>
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
package org.cryptomator.windows.keychain;

import com.google.gson.annotations.SerializedName;

class KeychainEntry {
import com.fasterxml.jackson.annotation.JsonProperty;

@SerializedName("ciphertext")
byte[] ciphertext;

@SerializedName("salt")
byte[] salt;
record KeychainEntry(@JsonProperty("ciphertext") byte[] ciphertext, @JsonProperty("salt") byte[] salt) {
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Path> keychainPaths;
private final WinDataProtection dataProtection;
Expand Down Expand Up @@ -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();
}

Expand All @@ -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;
}
Expand Down Expand Up @@ -184,15 +177,18 @@ private void loadKeychainEntriesIfNeeded() throws KeychainAccessException {
//visible for testing
Optional<Map<String, KeychainEntry>> loadKeychainEntries(Path keychainPath) throws KeychainAccessException {
LOG.debug("Attempting to load keychain from {}", keychainPath);
Type type = new TypeToken<Map<String, KeychainEntry>>() {
}.getType();
TypeReference<Map<String, KeychainEntry>> 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.");
}
infeo marked this conversation as resolved.
Show resolved Hide resolved
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);
infeo marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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());
}
}

}
Original file line number Diff line number Diff line change
@@ -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=="
}
}