mirrors/cryptomator
1
0
Fork 0
mirror of https://github.com/cryptomator/cryptomator.git synced 2026-10-22 14:22:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Restore moved keychain entries from retained copies when switching backends fails

Browse Source
This commit is contained in:
Tobias Hagemann
2026-10-20 18:26:34 +02:00
parent 154e290755
commit 67c92e43e8
3 changed files with 95 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/org/cryptomator/common/keychain/KeychainManager.java
+52 -32
View File
@@ -162,46 +162,66 @@ public class KeychainManager implements KeychainAccessProvider {
if (oldProvider instanceof KeychainManager || newProvider instanceof KeychainManager) {
throw new IllegalArgumentException("KeychainManger must not be the source or target of migration");
}
var migrated = new LinkedHashMap<String, String>();
var migrated = new LinkedHashMap<String, Passphrase>();
try {
for (var entry : idsAndNames.entrySet()) {
if (move(oldProvider, newProvider, entry.getKey(), entry.getValue())) {
migrated.put(entry.getKey(), entry.getValue());
var passphrase = move(oldProvider, newProvider, entry.getKey(), entry.getValue());
if (passphrase != null) {
migrated.put(entry.getKey(), passphrase);
}
}
} catch (KeychainAccessException e) {
for (var entry : migrated.entrySet()) {
try {
move(newProvider, oldProvider, entry.getKey(), entry.getValue());
} catch (KeychainAccessException rollbackException) {
e.addSuppressed(rollbackException);
}
moveBack(newProvider, oldProvider, entry.getKey(), idsAndNames.get(entry.getKey()), entry.getValue(), e);
}
throw e;
} finally {
migrated.values().forEach(Passphrase::destroy);
}
}
// returns the moved passphrase, which the caller must destroy, or null if the source has none
private static Passphrase move(KeychainAccessProvider source, KeychainAccessProvider target, String key, String displayName) throws KeychainAccessException {
var loaded = source.loadPassphrase(key);
if (loaded == null) {
return null;
}
var passphrase = new Passphrase(loaded);
try {
source.deletePassphrase(key); // delete before storing: both providers may address the same keychain item (e.g., Touch ID and regular keychain on macOS), so deleting afterwards would remove the moved entry
storeOrRestore(source, target, key, displayName, passphrase);
} catch (KeychainAccessException | RuntimeException e) {
passphrase.destroy();
throw e;
}
return passphrase;
}
// restores from the retained copy, since the new provider may no longer be readable after the failure
private static void moveBack(KeychainAccessProvider newProvider, KeychainAccessProvider oldProvider, String key, String displayName, Passphrase passphrase, KeychainAccessException failure) {
try {
newProvider.deletePassphrase(key);
} catch (KeychainAccessException deleteException) {
failure.addSuppressed(deleteException);
}
try {
storeOrRestore(newProvider, oldProvider, key, displayName, passphrase);
} catch (KeychainAccessException restoreException) {
failure.addSuppressed(restoreException);
}
}
// if the target refuses the passphrase, it goes back into the source, so the entry isn't lost from both
private static void storeOrRestore(KeychainAccessProvider source, KeychainAccessProvider target, String key, String displayName, Passphrase passphrase) throws KeychainAccessException {
try {
target.storePassphrase(key, displayName, passphrase);
} catch (KeychainAccessException e) {
try {
source.storePassphrase(key, displayName, passphrase);
} catch (KeychainAccessException restoreException) {
e.addSuppressed(restoreException);
}
throw e;
}
}
private static boolean move(KeychainAccessProvider source, KeychainAccessProvider target, String key, String displayName) throws KeychainAccessException {
var passphrase = source.loadPassphrase(key);
if (passphrase == null) {
return false;
}
var wrapper = new Passphrase(passphrase);
try {
source.deletePassphrase(key); // delete before storing: both providers may address the same keychain item (e.g., Touch ID and regular keychain on macOS), so deleting afterwards would remove the moved entry
try {
target.storePassphrase(key, displayName, wrapper);
} catch (KeychainAccessException e) {
try {
source.storePassphrase(key, displayName, wrapper);
} catch (KeychainAccessException restoreException) {
e.addSuppressed(restoreException);
}
throw e;
}
} finally {
wrapper.destroy();
}
return true;
}
}
src/test/java/org/cryptomator/common/keychain/KeychainManagerTest.java
+42 -1
View File
@@ -112,7 +112,7 @@ class KeychainManagerTest {
}
@Test
public void testMigrateRollsBackRemainingEntriesIfOneRollbackFails() throws KeychainAccessException {
public void testMigrateRollsBackRemainingEntriesAndKeepsUnrestorableEntryIfOneRollbackFails() throws KeychainAccessException {
var refuseStores = new AtomicBoolean(false);
var oldProvider = new MapKeychainAccess() {
@Override
@@ -145,10 +145,51 @@ class KeychainManagerTest {
Assertions.assertEquals("store failed", e.getMessage());
Assertions.assertEquals(1, e.getSuppressed().length);
Assertions.assertArrayEquals("asd".toCharArray(), newProvider.loadPassphrase("first"));
Assertions.assertArrayEquals("qwe".toCharArray(), oldProvider.loadPassphrase("second"));
Assertions.assertNull(newProvider.loadPassphrase("second"));
}
@Test
public void testMigrateRestoresEntriesIfNewProviderBecomesUnavailable() throws KeychainAccessException {
var oldProvider = new MapKeychainAccess();
var newProvider = new MapKeychainAccess() {
private boolean unavailable;
@Override
public void storePassphrase(String key, String displayName, CharSequence passphrase) throws KeychainAccessException {
if (key.equals("second")) {
unavailable = true;
throw new KeychainAccessException("store failed");
}
super.storePassphrase(key, displayName, passphrase);
}
@Override
public char[] loadPassphrase(String key) {
return unavailable ? null : super.loadPassphrase(key);
}
@Override
public void deletePassphrase(String key) throws KeychainAccessException {
if (unavailable) {
throw new KeychainAccessException("delete failed");
}
super.deletePassphrase(key);
}
};
oldProvider.storePassphrase("first", "First", "asd");
oldProvider.storePassphrase("second", "Second", "qwe");
var idsAndNames = new LinkedHashMap<String, String>();
idsAndNames.put("first", "First");
idsAndNames.put("second", "Second");
Assertions.assertThrows(KeychainAccessException.class, () -> KeychainManager.migrate(oldProvider, newProvider, idsAndNames));
Assertions.assertArrayEquals("asd".toCharArray(), oldProvider.loadPassphrase("first"));
Assertions.assertArrayEquals("qwe".toCharArray(), oldProvider.loadPassphrase("second"));
}
}
}
src/test/java/org/cryptomator/common/keychain/MapKeychainAccess.java
+1 -1
View File
@@ -38,7 +38,7 @@ class MapKeychainAccess implements KeychainAccessProvider {
}
@Override
public void deletePassphrase(String key) {
public void deletePassphrase(String key) throws KeychainAccessException {
map.remove(key);
}