From b3ae550f4196dfc7b3b45739a04c5b1ae1859623 Mon Sep 17 00:00:00 2001 From: Jakob Heher Date: Fri, 8 Jul 2022 13:42:20 +0200 Subject: cache keystore password in memory cf. #68 #69 todo: sanitize config loading behavior --- .../KeystoreConfigurationComposite.java | 20 ++-- .../pdfover/gui/controls/PasswordInputDialog.java | 2 +- .../asit/pdfover/gui/keystore/KeystoreUtils.java | 28 +++++ .../gui/workflow/config/ConfigurationManager.java | 8 +- .../asit/pdfover/gui/workflow/states/KSState.java | 127 +++++++++++++++++---- 5 files changed, 146 insertions(+), 39 deletions(-) create mode 100644 pdf-over-gui/src/main/java/at/asit/pdfover/gui/keystore/KeystoreUtils.java (limited to 'pdf-over-gui/src/main/java') diff --git a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/KeystoreConfigurationComposite.java b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/KeystoreConfigurationComposite.java index 993e408f..e293958f 100644 --- a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/KeystoreConfigurationComposite.java +++ b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/KeystoreConfigurationComposite.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; +import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; import java.util.Enumeration; import java.util.HashMap; @@ -57,12 +58,12 @@ import at.asit.pdfover.gui.exceptions.KeystoreAliasDoesntExistException; import at.asit.pdfover.gui.exceptions.KeystoreAliasNoKeyException; import at.asit.pdfover.gui.exceptions.KeystoreDoesntExistException; import at.asit.pdfover.gui.exceptions.KeystoreKeyPasswordException; +import at.asit.pdfover.gui.keystore.KeystoreUtils; import at.asit.pdfover.commons.Messages; import at.asit.pdfover.gui.workflow.config.ConfigurationManager; import at.asit.pdfover.gui.workflow.config.ConfigurationDataInMemory.KeyStorePassStorageType; import at.asit.pdfover.gui.workflow.config.ConfigurationDataInMemory; import at.asit.pdfover.gui.workflow.states.State; -import iaik.security.provider.IAIK; /** * @@ -250,6 +251,9 @@ public class KeystoreConfigurationComposite extends ConfigurationCompositeBase { } catch (NullPointerException ex) { log.error("Error loading keystore - NPE?", ex); showErrorDialog(Messages.getString("error.KeyStore")); + } catch (UnrecoverableKeyException ex) { + log.warn("Error loading keystore, invalid password", ex); + showErrorDialog(Messages.getString("error.KeyStoreStorePass")); } } }); @@ -296,13 +300,9 @@ public class KeystoreConfigurationComposite extends ConfigurationCompositeBase { e.open(); } - void loadKeystore() throws KeyStoreException, NoSuchAlgorithmException, CertificateException, IOException { - ConfigurationDataInMemory config = - KeystoreConfigurationComposite.this.configurationContainer; - File f = new File(config.keystoreFile); - this.ks = KeyStore.getInstance(config.keystoreType); - FileInputStream fis = new FileInputStream(f); - this.ks.load(fis, config.keystoreStorePass.toCharArray()); + void loadKeystore() throws KeyStoreException, NoSuchAlgorithmException, CertificateException, IOException, UnrecoverableKeyException { + ConfigurationDataInMemory config = this.configurationContainer; + this.ks = KeystoreUtils.tryLoadKeystore(new File(config.keystoreFile), config.keystoreType, config.keystoreStorePass); this.cmbKeystoreAlias.remove(0, this.cmbKeystoreAlias.getItemCount()-1); Enumeration aliases = this.ks.aliases(); while (aliases.hasMoreElements()) @@ -460,8 +460,8 @@ public class KeystoreConfigurationComposite extends ConfigurationCompositeBase { store.setKeyStoreType(config.keystoreType); store.setKeyStoreAlias(config.keystoreAlias); store.setKeyStorePassStorageType(config.keystorePassStorageType); - store.setKeyStoreStorePass(config.keystoreStorePass); - store.setKeyStoreKeyPass(config.keystoreKeyPass); + store.setKeyStoreStorePassPersistent(config.keystoreStorePass); + store.setKeyStoreKeyPassPersistent(config.keystoreKeyPass); } /* diff --git a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/controls/PasswordInputDialog.java b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/controls/PasswordInputDialog.java index 12b41a3e..3a15c63a 100644 --- a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/controls/PasswordInputDialog.java +++ b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/controls/PasswordInputDialog.java @@ -31,6 +31,6 @@ public class PasswordInputDialog extends InputDialog { */ public PasswordInputDialog(Shell parent, String title, String prompt) { super(parent, title, prompt); - TEXT_FLAGS = SWT.BORDER | SWT.PASSWORD; + PasswordInputDialog.TEXT_FLAGS = SWT.BORDER | SWT.PASSWORD; } } diff --git a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/keystore/KeystoreUtils.java b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/keystore/KeystoreUtils.java new file mode 100644 index 00000000..8b7bb59c --- /dev/null +++ b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/keystore/KeystoreUtils.java @@ -0,0 +1,28 @@ +package at.asit.pdfover.gui.keystore; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.UnrecoverableKeyException; +import java.security.cert.CertificateException; + +public class KeystoreUtils { + public static KeyStore tryLoadKeystore(File location, String storeType, String storePass) throws KeyStoreException, NoSuchAlgorithmException, CertificateException, IOException, UnrecoverableKeyException { + KeyStore ks = KeyStore.getInstance(storeType); + FileInputStream fis = new FileInputStream(location); + try + { + ks.load(fis, storePass.toCharArray()); + } catch (IOException e) { + UnrecoverableKeyException keyCause = (UnrecoverableKeyException)e.getCause(); + if (keyCause != null) + throw keyCause; + else + throw e; + } + return ks; + } +} diff --git a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/config/ConfigurationManager.java b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/config/ConfigurationManager.java index 1cfa72b3..340c125a 100644 --- a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/config/ConfigurationManager.java +++ b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/config/ConfigurationManager.java @@ -230,8 +230,8 @@ public class ConfigurationManager { setKeyStoreFile(diskConfig.getProperty(Constants.CFG_KEYSTORE_FILE)); setKeyStoreType(diskConfig.getProperty(Constants.CFG_KEYSTORE_TYPE)); setKeyStoreAlias(diskConfig.getProperty(Constants.CFG_KEYSTORE_ALIAS)); - setKeyStoreStorePass(diskConfig.getProperty(Constants.CFG_KEYSTORE_STOREPASS)); - setKeyStoreKeyPass(diskConfig.getProperty(Constants.CFG_KEYSTORE_KEYPASS)); + setKeyStoreStorePassPersistent(diskConfig.getProperty(Constants.CFG_KEYSTORE_STOREPASS)); + setKeyStoreKeyPassPersistent(diskConfig.getProperty(Constants.CFG_KEYSTORE_KEYPASS)); String storeTypeOnDisk = diskConfig.getProperty(Constants.CFG_KEYSTORE_PASSSTORETYPE); if (storeTypeOnDisk == null) /* auto-detect based on old config */ { @@ -877,7 +877,7 @@ public class ConfigurationManager { return this.configuration.keystorePassStorageType; } - public void setKeyStoreStorePass(String storePass) { + public void setKeyStoreStorePassPersistent(String storePass) { this.configuration.keystoreStorePass = storePass; } @@ -898,7 +898,7 @@ public class ConfigurationManager { return this.configuration.keystoreStorePass; } - public void setKeyStoreKeyPass(String keyPass) { + public void setKeyStoreKeyPassPersistent(String keyPass) { this.configuration.keystoreKeyPass = keyPass; } diff --git a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/states/KSState.java b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/states/KSState.java index 94f6993c..d8231e99 100644 --- a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/states/KSState.java +++ b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/workflow/states/KSState.java @@ -17,6 +17,10 @@ package at.asit.pdfover.gui.workflow.states; // Imports import java.io.File; +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.UnrecoverableKeyException; import org.eclipse.swt.SWT; import org.slf4j.Logger; @@ -25,12 +29,14 @@ import org.slf4j.LoggerFactory; import at.asit.pdfover.gui.MainWindow.Buttons; import at.asit.pdfover.gui.MainWindowBehavior; import at.asit.pdfover.gui.controls.Dialog.BUTTONS; +import at.asit.pdfover.gui.keystore.KeystoreUtils; import at.asit.pdfover.gui.controls.ErrorDialog; import at.asit.pdfover.gui.controls.PasswordInputDialog; import at.asit.pdfover.commons.Messages; import at.asit.pdfover.gui.workflow.StateMachine; import at.asit.pdfover.gui.workflow.Status; import at.asit.pdfover.gui.workflow.config.ConfigurationManager; +import at.asit.pdfover.gui.workflow.config.ConfigurationDataInMemory.KeyStorePassStorageType; import at.asit.pdfover.signator.SignatureException; import at.asit.pdfover.signator.SigningState; @@ -52,6 +58,16 @@ public class KSState extends State { super(stateMachine); } + private void showError(String messageKey, Object... args) + { + new ErrorDialog(getStateMachine().getMainShell(), String.format(Messages.getString(messageKey), args), BUTTONS.OK).open(); + } + + private boolean askShouldRetry(String messageKey, Object... args) + { + return SWT.RETRY == (new ErrorDialog(getStateMachine().getMainShell(), String.format(Messages.getString(messageKey), args), BUTTONS.RETRY_CANCEL).open()); + } + /* * (non-Javadoc) * @@ -71,37 +87,100 @@ public class KSState extends State { File f = new File(file); if (!f.isFile()) { log.error("Keystore not found"); - ErrorDialog dialog = new ErrorDialog( - getStateMachine().getMainShell(), - String.format(Messages.getString("error.KeyStoreFileNotExist"), f.getName()), - BUTTONS.RETRY_CANCEL); - if (dialog.open() != SWT.RETRY) { - //getStateMachine().exit(); + if (askShouldRetry("error.KeyStoreFileNotExist", f.getName())) + this.run(); + else this.setNextState(new BKUSelectionState(getStateMachine())); - return; - } - this.run(); return; } - String alias = config.getKeyStoreAlias(); + String type = config.getKeyStoreType(); + KeyStore keyStore = null; String storePass = config.getKeyStoreStorePass(); - // TODO trial and error - if (storePass == null) { - PasswordInputDialog pwd = new PasswordInputDialog( - getStateMachine().getMainShell(), - Messages.getString("keystore_config.KeystoreStorePass"), - Messages.getString("keystore.KeystoreStorePassEntry")); - storePass = pwd.open(); + while (keyStore == null) { + if (storePass == null) + { + PasswordInputDialog pwd = new PasswordInputDialog( + getStateMachine().getMainShell(), + Messages.getString("keystore_config.KeystoreStorePass"), + Messages.getString("keystore.KeystoreStorePassEntry")); + storePass = pwd.open(); + if (storePass == null) + { + this.setNextState(new BKUSelectionState(getStateMachine())); + return; + } + } + + try { + keyStore = KeystoreUtils.tryLoadKeystore(f, type, storePass); + } catch (UnrecoverableKeyException e) { + showError("error.KeyStoreStorePass"); + storePass = null; + } catch (Exception e) { + throw new SignatureException("Failed to load keystore", e); + } } + + /* we've successfully unlocked the key store, save the entered password if requested */ + if (config.getKeyStorePassStorageType() == KeyStorePassStorageType.DISK) + { + /* only save to disk if the current keystore file is the one saved to disk */ + /* (might not be true if overridden from CLI) */ + if (file.equals(config.getKeyStoreFilePersistent())) + config.setKeyStoreStorePassPersistent(storePass); + else + config.setKeyStoreStorePassOverlay(storePass); + } + else if (config.getKeyStorePassStorageType() == KeyStorePassStorageType.MEMORY) + config.setKeyStoreStorePassOverlay(storePass); + + /* next, try to load the key from the now-unlocked keystore */ + String alias = config.getKeyStoreAlias(); + Key key = null; String keyPass = config.getKeyStoreKeyPass(); - if (keyPass == null) { - PasswordInputDialog pwd = new PasswordInputDialog( - getStateMachine().getMainShell(), - Messages.getString("keystore_config.KeystoreKeyPass"), - Messages.getString("keystore.KeystoreKeyPassEntry")); - keyPass = pwd.open(); + while (key == null) { + if (keyPass == null) { + PasswordInputDialog pwd = new PasswordInputDialog( + getStateMachine().getMainShell(), + Messages.getString("keystore_config.KeystoreKeyPass"), + Messages.getString("keystore.KeystoreKeyPassEntry")); + keyPass = pwd.open(); + if (keyPass == null) + { + this.setNextState(new BKUSelectionState(getStateMachine())); + return; + } + } + + try { + key = keyStore.getKey(alias, keyPass.toCharArray()); + if (key == null) /* alias does not exist */ + { + if (!askShouldRetry("error.KeyStoreAliasExist", alias)) + { + this.setNextState(new BKUSelectionState(getStateMachine())); + return; + } + continue; + } + } catch (UnrecoverableKeyException e) { + showError("error.KeyStoreKeyPass"); + keyPass = null; + } catch (Exception e) { + throw new SignatureException("Failed to load key from store", e); + } } - String type = config.getKeyStoreType(); + + if (config.getKeyStorePassStorageType() == KeyStorePassStorageType.DISK) + { + if (file.equals(config.getKeyStoreFilePersistent())) + config.setKeyStoreKeyPassPersistent(keyPass); + else + config.setKeyStoreKeyPassOverlay(keyPass); + } + else if (config.getKeyStorePassStorageType() == KeyStorePassStorageType.MEMORY) + config.setKeyStoreKeyPassOverlay(keyPass); + signingState.setKSSigner(file, alias, storePass, keyPass, type); } catch (SignatureException e) { log.error("Error loading keystore", e); -- cgit v1.2.3