Keystore password not reset on reload #5442
Comments
|
@FSchumacher (migrated from Bugzilla): retry-asking-for-password.diffdiff --git a/src/core/src/main/java/org/apache/jmeter/util/SSLManager.java b/src/core/src/main/java/org/apache/jmeter/util/SSLManager.java
index 067ef5c3b1..6b491eb595 100644
--- a/src/core/src/main/java/org/apache/jmeter/util/SSLManager.java
+++ b/src/core/src/main/java/org/apache/jmeter/util/SSLManager.java
@@ -17,18 +17,24 @@
package org.apache.jmeter.util;
-import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
+import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
import java.security.Provider;
import java.security.Security;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
import java.util.Arrays;
import java.util.Locale;
+import javax.swing.JLabel;
import javax.swing.JOptionPane;
+import javax.swing.JPanel;
import javax.swing.JPasswordField;
import org.apache.commons.lang3.Validate;
@@ -37,6 +43,8 @@ import org.apache.jmeter.util.keystore.JmeterKeyStore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import net.miginfocom.swing.MigLayout;
+
/**
* The SSLManager handles the KeyStore information for JMeter. Basically, it
* handles all the logic for loading and initializing all the JSSE parameters
@@ -75,7 +83,7 @@ public abstract class SSLManager {
private volatile boolean truststoreLoaded=false;
/** Have the password available */
- protected String defaultpw = System.getProperty(KEY_STORE_PASSWORD);
+ protected volatile String defaultpw = System.getProperty(KEY_STORE_PASSWORD);
private int keystoreAliasStartIndex;
@@ -130,20 +138,16 @@ public abstract class SSLManager {
// The string 'NONE' is used for the keystore location when using PKCS11
// https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html#JSSE
if ("NONE".equalsIgnoreCase(fileName)) {
- this.keyStore.load(null, Validate.notNull(getPassword(), "Password should not be null"));
+ retryLoadKeys(null, false);
log.info("Total of {} aliases loaded OK from PKCS11", keyStore.getAliasCount());
} else {
File initStore = new File(fileName);
if (fileName.length() > 0 && initStore.exists()) {
- try (InputStream fis = new FileInputStream(initStore);
- InputStream fileInputStream = new BufferedInputStream(fis)) {
- this.keyStore.load(fileInputStream, getPassword());
- if (log.isInfoEnabled()) {
- log.info(
- "Total of {} aliases loaded OK from keystore",
- keyStore.getAliasCount());
- }
- }
+ retryLoadKeys(initStore, true);
+ if (log.isInfoEnabled()) {
+ log.info("Total of {} aliases loaded OK from keystore {}",
+ keyStore.getAliasCount(), fileName);
+ }
} else {
log.warn("Keystore file not found, loading empty keystore");
this.defaultpw = ""; // Ensure not null
@@ -162,6 +166,29 @@ public abstract class SSLManager {
return this.keyStore;
}
+ private void retryLoadKeys(File initStore, boolean allowEmptyPassword) throws NoSuchAlgorithmException, CertificateException, IOException, KeyStoreException,
+ UnrecoverableKeyException {
+ for (int i=0; i<3; i++) {
+ String password = getPassword();
+ if (!allowEmptyPassword) {
+ Validate.notNull(password, "Password for keystore must not be null");
+ }
+ try {
+ if (initStore == null) {
+ this.keyStore.load(null, password);
+ } else {
+ try (InputStream fis = new FileInputStream(initStore)) {
+ this.keyStore.load(fis, password);
+ }
+ }
+ return;
+ } catch (IOException e) {
+ log.debug("Could not load keystore. Wrong password for keystore?", e);
+ }
+ this.defaultpw = null;
+ }
+ }
+
/*
* The password can be defined as a property; this dialogue is provided to allow it
* to be entered at run-time.
@@ -171,26 +198,26 @@ public abstract class SSLManager {
if (null == password) {
final GuiPackage guiInstance = GuiPackage.getInstance();
if (guiInstance != null) {
- synchronized (this) { // TODO is sync really needed?
- JPasswordField pwf = new JPasswordField(64);
- pwf.setEchoChar('*');
- int choice = JOptionPane.showConfirmDialog(
- guiInstance.getMainFrame(),
- pwf,
- JMeterUtils.getResString("ssl_pass_prompt"),
- JOptionPane.OK_CANCEL_OPTION,
- JOptionPane.PLAIN_MESSAGE);
- if (choice == JOptionPane.OK_OPTION) {
- char[] pwchars = pwf.getPassword();
- this.defaultpw = new String(pwchars);
- Arrays.fill(pwchars, '*');
- }
- System.setProperty(KEY_STORE_PASSWORD, this.defaultpw);
- password = this.defaultpw;
- }
- } else {
- log.warn("No password provided, and no GUI present so cannot prompt");
+ JPanel panel = new JPanel(new MigLayout("fillx, wrap 2", "[][fill, grow]"));
+ JLabel passwordLabel = new JLabel("Password: ");
+ JPasswordField pwf = new JPasswordField(64);
+ pwf.setEchoChar('*');
+ passwordLabel.setLabelFor(pwf);
+ panel.add(passwordLabel);
+ panel.add(pwf);
+ int choice = JOptionPane.showConfirmDialog(guiInstance.getMainFrame(), panel,
+ JMeterUtils.getResString("ssl_pass_prompt"), JOptionPane.OK_CANCEL_OPTION,
+ JOptionPane.PLAIN_MESSAGE);
+ if (choice == JOptionPane.OK_OPTION) {
+ char[] pwchars = pwf.getPassword();
+ this.defaultpw = new String(pwchars);
+ Arrays.fill(pwchars, '*');
+ }
+ System.setProperty(KEY_STORE_PASSWORD, this.defaultpw);
+ password = this.defaultpw;
}
+ } else {
+ log.warn("No password provided, and no GUI present so cannot prompt");
}
return password;
}
@@ -239,9 +266,8 @@ public abstract class SSLManager {
File initStore = new File(fileName);
if (initStore.exists()) {
- try (InputStream fis = new FileInputStream(initStore);
- InputStream fileInputStream = new BufferedInputStream(fis)) {
- this.trustStore.load(fileInputStream, null);
+ try (InputStream fis = new FileInputStream(initStore)) {
+ this.trustStore.load(fis, null);
log.info("Truststore loaded OK from file");
}
} else { |
|
@FSchumacher (migrated from Bugzilla): Would you be able to patch and build a JMeter and report back, if it helps in your case? Otherwise you would have to wait a few days, as we are currently doing a release and we have to prepare the trunk for the next cycle. With this patch, I cleaned up the code a bit, too. I removed an old TODO whether a sync was needed in the password dialog. I decided, it was not needed, as it was called from within a synced region, only. The dialog has gotten a label, as I found it irritating to see a lonely input field with no real explanation (it was given in the title, which I did not read :)) Another enhancement would be to get the focus inside the password dialog. |
|
Michael Osipov (migrated from Bugzilla): |
|
@FSchumacher (migrated from Bugzilla): commit 6701514 .../java/org/apache/jmeter/util/SSLManager.java | 94 ++++++++++++++-------- |
|
Michael Osipov (migrated from Bugzilla): Thank you, looking forward to JMeter 5.4.1. |
Michael Osipov (Bug 64955):
020-12-04 09:21:48,307 INFO o.a.j.u.SSLManager: KeyStore created OK
2020-12-04 09:21:48,345 ERROR o.a.j.u.SSLManager: Problem loading keystore: keystore password was incorrect
java.io.IOException: keystore password was incorrect
at sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2078) ~[?:1.8.0_275]
at java.security.KeyStore.load(KeyStore.java:1445) ~[?:1.8.0_275]
at org.apache.jmeter.util.keystore.JmeterKeyStore.load(JmeterKeyStore.java:108) ~[ApacheJMeter_core.jar:5.3]
at org.apache.jmeter.util.SSLManager.getKeyStor...
Obviously the password cache is not reset when a new keystore is loaded. I am forced to restart JMeter and load the other keystore.
Severity: major
OS: All
The text was updated successfully, but these errors were encountered: