You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2019/10/06 20:07:56 UTC
[jmeter] branch master updated: JmeterKeyStore: Improve readability
and add spock test (#505)
This is an automated email from the ASF dual-hosted git repository.
pmouawad pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jmeter.git
The following commit(s) were added to refs/heads/master by this push:
new d1a5856 JmeterKeyStore: Improve readability and add spock test (#505)
d1a5856 is described below
commit d1a58566466c70a1855318b4bf4c76f4e05a04dd
Author: Graham Russell <gr...@ham1.co.uk>
AuthorDate: Sun Oct 6 21:07:52 2019 +0100
JmeterKeyStore: Improve readability and add spock test (#505)
* Formatting and comments
* Use Objects.requireNonNull instead of custom method.
* Used System.arraycopy instead of for loop,
* Added test for JmeterKeyStore and fixed JavaDocs
* Removed misleading comment and improved readability.
* Used Arrays.copyOf and updated JavaDoc
* Updated getCertificateChain to match behaviour of documentation.
* Fix import order
* Reverted change to getCertificateChain and added clarifying comment
* Clarified description of start and end index.
---
.../config/KeystoreConfigResources.properties | 6 +-
.../jmeter/util/keystore/JmeterKeyStore.java | 200 +++++++++------------
.../jmeter/util/keystore/JmeterKeyStoreSpec.groovy | 41 +++++
3 files changed, 128 insertions(+), 119 deletions(-)
diff --git a/src/components/src/main/resources/org/apache/jmeter/config/KeystoreConfigResources.properties b/src/components/src/main/resources/org/apache/jmeter/config/KeystoreConfigResources.properties
index ae4e196..62db180 100644
--- a/src/components/src/main/resources/org/apache/jmeter/config/KeystoreConfigResources.properties
+++ b/src/components/src/main/resources/org/apache/jmeter/config/KeystoreConfigResources.properties
@@ -19,9 +19,9 @@ aliases.displayName=Aliases selection configuration
# fields
preload.displayName=Preload
preload.shortDescription=Preload Keystore before test. Setting is to true is usually the best option.
-startIndex.displayName=Alias Start index (0-based)
+startIndex.displayName=Alias Start index (0 based)
startIndex.shortDescription=First index of Alias in Keystore
-endIndex.displayName=Alias End index (0-based)
-endIndex.shortDescription=Last index of Alias in Keystore. When using Variable name ensure it is large enough so that all keys are loaded at startup.
+endIndex.displayName=Alias End index (0 based)
+endIndex.shortDescription=Last index of Alias in Keystore. When using Variable name ensure it is large enough so that all keys are loaded at startup. A startIndex of 0 and endIndex of -1 returns the first alias only.
clientCertAliasVarName.displayName=Variable name holding certificate alias
clientCertAliasVarName.shortDescription=Variable name that will contain the alias to use for Cert authentication. Var content can come from CSV Data Set.
diff --git a/src/core/src/main/java/org/apache/jmeter/util/keystore/JmeterKeyStore.java b/src/core/src/main/java/org/apache/jmeter/util/keystore/JmeterKeyStore.java
index 3c40117..0cec9b4 100644
--- a/src/core/src/main/java/org/apache/jmeter/util/keystore/JmeterKeyStore.java
+++ b/src/core/src/main/java/org/apache/jmeter/util/keystore/JmeterKeyStore.java
@@ -30,10 +30,12 @@ import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import org.apache.commons.lang3.StringUtils;
import org.apache.jmeter.threads.JMeterContextService;
@@ -42,7 +44,6 @@ import org.slf4j.LoggerFactory;
/**
* Use this Keystore for JMeter specific KeyStores.
- *
*/
public final class JmeterKeyStore {
@@ -61,27 +62,25 @@ public final class JmeterKeyStore {
/** name of the default alias */
private String clientCertAliasVarName;
- private String[] names = new String[0]; // default empty array to prevent NPEs
+ private String[] names = new String[0];
private Map<String, PrivateKey> privateKeyByAlias = new HashMap<>();
private Map<String, X509Certificate[]> certsByAlias = new HashMap<>();
private int lastAliasIndex;
/**
- * @param type
- * type of the {@link KeyStore}
- * @param startIndex which keys should be considered starting from <code>0</code>
- * @param endIndex which keys should be considered up to <code>count - 1</code>
+ * @param type type of the {@link KeyStore}
+ * @param startIndex which keys should be considered, starting from <code>0</code>
+ * @param endIndex which keys should be considered, up to <code>count - 1</code>
* @param clientCertAliasVarName name for the default key, if empty use the first key available
- * @throws KeyStoreException
- * when the type of the keystore is not supported
- * @throws IllegalArgumentException
- * when <code>startIndex</code> < 0, <code>endIndex</code>
- * < 0 or <code>endIndex</code> < </code>startIndex</code>
+ * @throws KeyStoreException when the type of the keystore is not supported
+ * @throws IllegalArgumentException when <code>startIndex</code> < 0, <code>endIndex</code>
+ * < -1 or <code>endIndex</code> < </code>startIndex</code>
*/
private JmeterKeyStore(String type, int startIndex, int endIndex, String clientCertAliasVarName) throws KeyStoreException {
if (startIndex < 0 || (endIndex != -1 && endIndex < startIndex)) {
- throw new IllegalArgumentException("Invalid index(es). Start="+startIndex+", end="+endIndex);
+ throw new IllegalArgumentException(
+ "Invalid index(es). Start=" + startIndex + ", end=" + endIndex);
}
this.store = KeyStore.getInstance(type);
this.startIndex = startIndex;
@@ -92,24 +91,17 @@ public final class JmeterKeyStore {
/**
* Process the input stream and try to read the keys from the store
*
- * @param is
- * {@link InputStream} from which the store should be loaded
- * @param pword
- * the password used to check the integrity of the store
- * @throws IOException
- * if there is a problem decoding or reading the store. A bad
- * password might be the cause for this, or an empty store
- * @throws CertificateException
- * if any of the certificated in the store can not be loaded
- * @throws NoSuchAlgorithmException
- * if the algorithm to check the integrity of the store can not
- * be found
- * @throws KeyStoreException
- * if the store has not been initialized (should not happen
- * here)
- * @throws UnrecoverableKeyException
- * if the key can not be recovered from the store (should not
- * happen here, either)
+ * @param is {@link InputStream} from which the store should be loaded
+ * @param pword the password used to check the integrity of the store
+ * @throws IOException if there is a problem decoding or reading the store. A bad
+ * password might be the cause for this, or an empty store
+ * @throws CertificateException if any of the certificated in the store can not be loaded
+ * @throws NoSuchAlgorithmException if the algorithm to check the integrity of the store can not
+ * be found
+ * @throws KeyStoreException if the store has not been initialized (should not happen
+ * here)
+ * @throws UnrecoverableKeyException if the key can not be recovered from the store (should not
+ * happen here, either)
*/
public void load(InputStream is, String pword)
throws NoSuchAlgorithmException, CertificateException, IOException, KeyStoreException, UnrecoverableKeyException {
@@ -120,34 +112,35 @@ public final class JmeterKeyStore {
this.privateKeyByAlias = new HashMap<>();
this.certsByAlias = new HashMap<>();
- PrivateKey privateKey = null;
- int index = 0;
- Enumeration<String> aliases = store.aliases();
- while (aliases.hasMoreElements()) {
- String alias = aliases.nextElement();
- if (store.isKeyEntry(alias)) {
- if (isIndexInConfiguredRange(index)) {
- privateKey = validateNotNull(
- (PrivateKey) store.getKey(alias, pw),
- "No key found for alias: " + alias);
- Certificate[] chain = validateNotNull(
- store.getCertificateChain(alias),
- "No certificate chain found for alias" + alias);
- aliasesList.add(alias);
- privateKeyByAlias.put(alias, privateKey);
- certsByAlias.put(alias, toX509Certificates(chain));
- }
- index++;
- }
- }
+ PrivateKey privateKey = null;
+ int index = 0;
+ Enumeration<String> aliases = store.aliases();
+ while (aliases.hasMoreElements()) {
+ String alias = aliases.nextElement();
+ if (!store.isKeyEntry(alias)) {
+ continue;
+ }
+ if (isIndexInConfiguredRange(index)) {
+ privateKey = Objects.requireNonNull(
+ (PrivateKey) store.getKey(alias, pw),
+ "No key found for alias: " + alias);
+ Certificate[] chain = Objects.requireNonNull(
+ store.getCertificateChain(alias),
+ "No certificate chain found for alias" + alias);
+ aliasesList.add(alias);
+ privateKeyByAlias.put(alias, privateKey);
+ certsByAlias.put(alias, toX509Certificates(chain));
+ }
+ index++;
+ }
- if (is != null) { // only check for keys, if we were given a file as inputstream
- validateNotNull(privateKey, "No key(s) found");
+ if (is != null) { // only check for keys, if we were given a file as inputstream
+ Objects.requireNonNull(privateKey, "No key(s) found");
if (endIndex != -1 && index <= endIndex - startIndex && log.isWarnEnabled()) {
log.warn("Did not find as much aliases as configured in indexes Start={}, end={}, found={}", startIndex,
endIndex, certsByAlias.size());
}
- }
+ }
/*
* Note: if is == null and no pkcs11 store is configured, the arrays will be empty
@@ -155,17 +148,10 @@ public final class JmeterKeyStore {
this.names = aliasesList.toArray(new String[aliasesList.size()]);
}
- private <T> T validateNotNull(T object, String message) throws IOException {
- if (null == object) {
- throw new IOException(message);
- }
- return object;
- }
-
private X509Certificate[] toX509Certificates(Certificate[] chain) {
X509Certificate[] x509certs = new X509Certificate[chain.length];
for (int i = 0; i < x509certs.length; i++) {
- x509certs[i] = (X509Certificate) chain[i];
+ x509certs[i] = (X509Certificate) chain[i];
}
return x509certs;
}
@@ -184,37 +170,34 @@ public final class JmeterKeyStore {
/**
* Get the ordered certificate chain for a specific alias.
*
- * @param alias
- * the alias for which the certificate chain should be given
- * @return the certificate chain for the alias
- * @throws IllegalArgumentException
- * if no chain could be found for the alias
+ * @param alias the alias for which the certificate chain should be given
+ * @return the certificate chain for the alias or null if not found
+ * @see javax.net.ssl.X509KeyManager#getCertificateChain(String)
*/
public X509Certificate[] getCertificateChain(String alias) {
X509Certificate[] result = this.certsByAlias.get(alias);
- if(result != null) {
+ if (result != null) {
return result;
}
- // API expects null not empty array, see http://docs.oracle.com/javase/7/docs/api/javax/net/ssl/X509KeyManager.html
- throw new IllegalArgumentException("No certificate found for alias:'"+alias+"'");
+ // API expects null not empty array.
+ // See http://docs.oracle.com/javase/7/docs/api/javax/net/ssl/X509KeyManager.html
+ // However, throwing here to provide a better error message to the user
+ throw new IllegalArgumentException("No certificate found for alias:'" + alias + "'");
}
/**
* Get the next or only alias.
*
* @return the next or only alias.
- * @throws IllegalArgumentException
- * if {@link JmeterKeyStore#clientCertAliasVarName
- * clientCertAliasVarName} is not empty and no key for this
- * alias could be found
+ * @throws IllegalArgumentException if {@link #clientCertAliasVarName}
+ * is not empty and no key for this alias could be found
*/
public String getAlias() {
- if(!StringUtils.isEmpty(clientCertAliasVarName)) {
- // We return even if result is null
+ if (StringUtils.isNotEmpty(clientCertAliasVarName)) {
String aliasName = JMeterContextService.getContext().getVariables().get(clientCertAliasVarName);
- if(StringUtils.isEmpty(aliasName)) {
+ if (StringUtils.isEmpty(aliasName)) {
log.error("No var called '{}' found", clientCertAliasVarName);
- throw new IllegalArgumentException("No var called '"+clientCertAliasVarName+"' found");
+ throw new IllegalArgumentException("No var called '" + clientCertAliasVarName + "' found");
}
return aliasName;
}
@@ -243,51 +226,41 @@ public final class JmeterKeyStore {
/**
* Return the private Key for a specific alias
*
- * @param alias
- * the name of the alias for the private key
+ * @param alias the name of the alias for the private key
* @return the private key for the given <code>alias</code>
- * @throws IllegalArgumentException
- * when no private key could be found
+ * @throws IllegalArgumentException when no private key could be found
*/
public PrivateKey getPrivateKey(String alias) {
PrivateKey pk = this.privateKeyByAlias.get(alias);
- if(pk != null) {
+ if (pk != null) {
return pk;
}
- throw new IllegalArgumentException("No PrivateKey found for alias:'"+alias+"'");
+ throw new IllegalArgumentException("No PrivateKey found for alias:'" + alias + "'");
}
/**
* Create a keystore which returns a range of aliases (if available)
*
- * @param type
- * store type (e.g. JKS)
- * @param startIndex
- * first index (from 0)
- * @param endIndex
- * last index (to count -1)
- * @param clientCertAliasVarName
- * name of the default key to, if empty the first key will be
- * used as default key
+ * @param type store type (e.g. JKS)
+ * @param startIndex first index (from 0)
+ * @param endIndex last index (to count-1)
+ * @param clientCertAliasVarName name of the default key to, if empty the first key will be
+ * used as default key
* @return the keystore
- * @throws KeyStoreException
- * when the type of the store is not supported
- * @throws IllegalArgumentException
- * when <code>startIndex</code> < 0, <code>endIndex</code>
- * < 0, or <code>endIndex</code> < <code>startIndex</code>
+ * @throws KeyStoreException when the type of the store is not supported
+ * @throws IllegalArgumentException when <code>startIndex</code> < 0, <code>endIndex</code>
+ * < -1, or <code>endIndex</code> < <code>startIndex</code>
*/
- public static JmeterKeyStore getInstance(String type, int startIndex, int endIndex, String clientCertAliasVarName) throws KeyStoreException {
+ public static JmeterKeyStore getInstance(String type, int startIndex, int endIndex, String clientCertAliasVarName) throws KeyStoreException {
return new JmeterKeyStore(type, startIndex, endIndex, clientCertAliasVarName);
}
/**
* Create a keystore which returns the first alias only.
*
- * @param type
- * of the store e.g. JKS
+ * @param type of the store e.g. JKS
* @return the keystore
- * @throws KeyStoreException
- * when the type of the store is not supported
+ * @throws KeyStoreException when the type of the store is not supported
*/
public static JmeterKeyStore getInstance(String type) throws KeyStoreException {
return getInstance(type, 0, -1, DEFAULT_ALIAS_VAR_NAME);
@@ -295,10 +268,11 @@ public final class JmeterKeyStore {
/**
* Gets current index and increment by rolling if index is equal to length
+ *
* @param length Number of keys to roll
*/
private int getIndexAndIncrement(int length) {
- synchronized(this) {
+ synchronized (this) {
int result = lastAliasIndex++;
if (lastAliasIndex >= length) {
lastAliasIndex = 0;
@@ -312,22 +286,16 @@ public final class JmeterKeyStore {
* TODO Currently, keyType and issuers are both ignored.
*
* @param keyType the key algorithm type name (RSA, DSA, etc.)
- * @param issuers the CA certificates we are narrowing our selection on.
- *
- * @return the array of aliases; may be empty
+ * @param issuers the CA certificates we are narrowing our selection on.
+ * @return the array of aliases; null if none.
+ * @see javax.net.ssl.X509KeyManager#getClientAliases
*/
public String[] getClientAliases(String keyType, Principal[] issuers) {
- int count = getAliasCount();
- String[] aliases = new String[count];
- for(int i = 0; i < aliases.length; i++) {
- aliases[i] = this.names[i];
- }
- if(aliases.length>0) {
- return aliases;
- } else {
- // API expects null not empty array, see http://docs.oracle.com/javase/7/docs/api/javax/net/ssl/X509KeyManager.html
- return null; // NOSONAR
+ int count = this.names.length;
+ if (count == 0) {
+ return null;
}
+ return Arrays.copyOf(this.names, count);
}
}
diff --git a/src/core/src/test/groovy/org/apache/jmeter/util/keystore/JmeterKeyStoreSpec.groovy b/src/core/src/test/groovy/org/apache/jmeter/util/keystore/JmeterKeyStoreSpec.groovy
new file mode 100644
index 0000000..c655ed9
--- /dev/null
+++ b/src/core/src/test/groovy/org/apache/jmeter/util/keystore/JmeterKeyStoreSpec.groovy
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.jmeter.util.keystore
+
+import java.security.KeyStore
+
+import spock.lang.Specification
+import spock.lang.Unroll
+
+@Unroll
+class JmeterKeyStoreSpec extends Specification {
+
+ def "IllegalArgumentException expected when (#startIndex, #endIndex) startIndex < 0, endIndex < -1, or endIndex < startIndex"() {
+ when:
+ JmeterKeyStore.getInstance(KeyStore.defaultType, startIndex, endIndex, "defaultName")
+ then:
+ thrown IllegalArgumentException
+ where:
+ startIndex | endIndex
+ -1 | 0
+ 0 | -2 // -1 indicates to return the first alias only
+ 1 | 0
+ }
+
+}