From 8cb9728565ae8d6c1c2493e377b8b9f428dc260a Mon Sep 17 00:00:00 2001 From: Ralph Plawetzki Date: Tue, 12 May 2020 07:41:10 +0200 Subject: [PATCH] Implement changePassphrase from the secret-service API (#1191) Fixes #1189 --- .../cryptomator/keychain/KeychainAccess.java | 7 +++++ .../LinuxSecretServiceKeychainAccess.java | 5 ++++ .../LinuxSecretServiceKeychainAccessImpl.java | 16 +++++++++- .../keychain/MacSystemKeychainAccess.java | 5 ++++ .../WindowsProtectedKeychainAccess.java | 5 ++++ .../keychain/MapKeychainAccess.java | 6 ++++ .../ChangePasswordController.java | 30 ++++++++++++------- 7 files changed, 63 insertions(+), 11 deletions(-) diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java index 6c20ba50f..563794dc0 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java @@ -28,4 +28,11 @@ public interface KeychainAccess { */ void deletePassphrase(String key) throws KeychainAccessException; + /** + * Updates a passphrase with a given key. + * + * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}. + * @param passphrase The secret to be updated in this keychain. + */ + void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException; } diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java index 1387bf0eb..7cf0b5e7b 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java @@ -48,4 +48,9 @@ public class LinuxSecretServiceKeychainAccess implements KeychainAccessStrategy public void deletePassphrase(String key) throws KeychainAccessException { delegate.orElseThrow(IllegalStateException::new).deletePassphrase(key); } + + @Override + public void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException { + delegate.orElseThrow(IllegalStateException::new).changePassphrase(key, passphrase); + } } diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccessImpl.java b/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccessImpl.java index 36a139a9f..675992e5c 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccessImpl.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccessImpl.java @@ -9,6 +9,8 @@ import java.util.Map; class LinuxSecretServiceKeychainAccessImpl implements KeychainAccessStrategy { + private final String LABEL_FOR_SECRET_IN_KEYRING = "Cryptomator"; + @Override public boolean isSupported() { try (@SuppressWarnings("unused") SimpleCollection keyring = new SimpleCollection()) { @@ -24,7 +26,7 @@ class LinuxSecretServiceKeychainAccessImpl implements KeychainAccessStrategy { try (SimpleCollection keyring = new SimpleCollection()) { List list = keyring.getItems(createAttributes(key)); if (list == null) { - keyring.createItem("Cryptomator", passphrase, createAttributes(key)); + keyring.createItem(LABEL_FOR_SECRET_IN_KEYRING, passphrase, createAttributes(key)); } } catch (IOException e) { throw new KeychainAccessException(e); @@ -57,6 +59,18 @@ class LinuxSecretServiceKeychainAccessImpl implements KeychainAccessStrategy { } } + @Override + public void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException { + try (SimpleCollection keyring = new SimpleCollection()) { + List list = keyring.getItems(createAttributes(key)); + if (list != null) { + keyring.updateItem(list.get(0), LABEL_FOR_SECRET_IN_KEYRING, passphrase, createAttributes(key)); + } + } catch (IOException e) { + throw new KeychainAccessException(e); + } + } + private Map createAttributes(String key) { Map attributes = new HashMap(); attributes.put("Vault", key); 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 b3b27a4ae..382ec0aff 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java @@ -46,4 +46,9 @@ class MacSystemKeychainAccess implements KeychainAccessStrategy { keychain().deletePassword(key); } + @Override + public void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException { + storePassphrase(key, passphrase); + } + } 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 ce8c5744d..15525aca3 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java @@ -112,6 +112,11 @@ class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { saveKeychainEntries(); } + @Override + public void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException { + storePassphrase(key, passphrase); + } + @Override public boolean isSupported() { return SystemUtils.IS_OS_WINDOWS && winFunctions.isPresent() && !keychainPaths.isEmpty(); 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 2d5f66857..26b301377 100644 --- a/main/keychain/src/test/java/org/cryptomator/keychain/MapKeychainAccess.java +++ b/main/keychain/src/test/java/org/cryptomator/keychain/MapKeychainAccess.java @@ -31,6 +31,12 @@ class MapKeychainAccess implements KeychainAccessStrategy { map.remove(key); } + @Override + public void changePassphrase(String key, CharSequence passphrase) { + map.get(key); + storePassphrase(key, passphrase); + } + @Override public boolean isSupported() { return true; diff --git a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java index ca15a5105..acb2fd12d 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java @@ -2,32 +2,28 @@ package org.cryptomator.ui.changepassword; import javafx.beans.binding.Bindings; import javafx.beans.binding.BooleanBinding; -import javafx.beans.property.IntegerProperty; import javafx.beans.property.ObjectProperty; -import javafx.beans.property.SimpleIntegerProperty; import javafx.fxml.FXML; import javafx.scene.control.Button; import javafx.scene.control.CheckBox; -import javafx.scene.control.Label; -import javafx.scene.layout.HBox; import javafx.stage.Stage; import org.cryptomator.common.vaults.Vault; import org.cryptomator.cryptofs.CryptoFileSystemProvider; import org.cryptomator.cryptolib.api.InvalidPassphraseException; +import org.cryptomator.keychain.KeychainAccess; +import org.cryptomator.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.controls.FontAwesome5IconView; import org.cryptomator.ui.controls.NiceSecurePasswordField; -import org.cryptomator.ui.common.PasswordStrengthUtil; -import org.fxmisc.easybind.EasyBind; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; import java.io.IOException; -import java.util.ResourceBundle; +import java.nio.CharBuffer; +import java.util.Optional; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -40,17 +36,19 @@ public class ChangePasswordController implements FxController { private final Vault vault; private final ObjectProperty newPassword; private final ErrorComponent.Builder errorComponent; + private final Optional keychain; public NiceSecurePasswordField oldPasswordField; public CheckBox finalConfirmationCheckbox; public Button finishButton; @Inject - public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent) { + public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent, Optional keychain) { this.window = window; this.vault = vault; this.newPassword = newPassword; this.errorComponent = errorComponent; + this.keychain = keychain; } @FXML @@ -69,8 +67,9 @@ public class ChangePasswordController implements FxController { public void finish() { try { CryptoFileSystemProvider.changePassphrase(vault.getPath(), MASTERKEY_FILENAME, oldPasswordField.getCharacters(), newPassword.get()); - LOG.info("Successful changed password for {}", vault.getDisplayableName()); + LOG.info("Successfully changed password for {}", vault.getDisplayableName()); window.close(); + updatePasswordInSystemkeychain(); } catch (IOException e) { LOG.error("IO error occured during password change. Unable to perform operation.", e); errorComponent.cause(e).window(window).returnToScene(window.getScene()).build().showErrorScene(); @@ -81,6 +80,17 @@ public class ChangePasswordController implements FxController { } } + private void updatePasswordInSystemkeychain() { + if (keychain.isPresent()) { + try { + keychain.get().changePassphrase(vault.getId(), CharBuffer.wrap(newPassword.get())); + LOG.info("Successfully updated password in system keychain for {}", vault.getDisplayableName()); + } catch (KeychainAccessException e) { + LOG.error("Failed to update password in system keychain.", e); + } + } + } + /* Getter/Setter */ public Vault getVault() {