cache keystore password in memory cf. #68 #69

todo: sanitize config loading behavior

7 files changed, 148 insertions, 39 deletions

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<String> 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);
diff --git a/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages.properties b/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages.properties
index e6af48fa..060023e0 100644
--- a/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages.properties
+++ b/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages.properties
@@ -149,6 +149,7 @@ error.KeyStore=Error loading they keystore. Wrong password?
 error.KeyStoreAliasExist=Key alias %s not found in keystore
 error.KeyStoreAliasNoKey=Alias %s is not a key
 error.KeyStoreFileNotExist=Keystore file %s does not exist\!
+error.KeyStoreStorePass=Keystore password invalid
 error.KeyStoreKeyPass=Key password invalid
 error.LocalBKU=Please check if a local CCE (citizen card environment) is running\n\nYou need a CCE to access your citizen card. Further information under www.buergerkarte.at
 error.MayNotBeAPDF=This may not be a PDF file
diff --git a/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages_de.properties b/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages_de.properties
index f216e15c..15b7f337 100644
--- a/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages_de.properties
+++ b/pdf-over-gui/src/main/resources/at/asit/pdfover/gui/messages_de.properties
@@ -139,6 +139,7 @@ error.KeyStore=Fehler beim Laden des KeyStores. Falsches Passwort?
 error.KeyStoreAliasExist=Key-Alias %s nicht im Keystore gefunden
 error.KeyStoreAliasNoKey=Alias %s ist kein Schlüssel
 error.KeyStoreFileNotExist=Keystore-Datei %s existiert nicht\!
+error.KeyStoreStorePass=Keystore-Passwort nicht akzeptiert
 error.KeyStoreKeyPass=Schlüssel-Passwort nicht akzeptiert
 error.LocalBKU=Bitte prüfen sie, ob Ihre lokale BKU (Bürgerkartenumgebung) läuft\n\nSie benötigen eine BKU, um auf Ihre Bürgerkarte zuzugreifen. Weitere Informationen unter www.buergerkarte.at
 error.MayNotBeAPDF=Dies ist möglicherweise keine PDF-Datei