I looked in a bit more detail,
static is needed
I think AtomicInteger won't do it because we need to increment and %
aliasCount in the same time, don't know if we can do that  so maybe just a
custom lock for this static field would do it.

On Sun, Oct 23, 2011 at 12:49 PM, sebb <seb...@gmail.com> wrote:

> On 23 October 2011 11:38, Philippe Mouawad <philippe.moua...@gmail.com>
> wrote:
> > Hello,
> > Regarding this last commit,  I think there is an issue in last_user
> either
> > being static or being synchronized on this.
> >
> > In my opinion, as it seems JsseSSLManager is a singleton, last_user
> should
> > be instance variable.
> > If it's not the case, then synchronized block in getNextIndex should be
> on
> > JsseSSLManager.class or field should use AtomicInteger .
>
> Good catch indeed!
>
> I originally was going to use AtomicInteger, but would still have had
> to protect the wrap-around code.
>
> I'll fix it shortly.
>
> > Regards
> > Philippe
> >
> > On Sun, Oct 23, 2011 at 3:50 AM, <s...@apache.org> wrote:
> >
> >> Author: sebb
> >> Date: Sun Oct 23 01:50:55 2011
> >> New Revision: 1187840
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1187840&view=rev
> >> Log:
> >> Bug 52033 - Allowing multiple certificates (JKS)
> >>
> >> Modified:
> >>    jakarta/jmeter/trunk/bin/jmeter.properties
> >>
>  jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/JsseSSLManager.java
> >>    jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/SSLManager.java
> >>
> >>
>  
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/DefaultKeyStore.java
> >>
> >>
>  
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/JmeterKeyStore.java
> >>    jakarta/jmeter/trunk/xdocs/changes.xml
> >>
> >> Modified: jakarta/jmeter/trunk/bin/jmeter.properties
> >> URL:
> >>
> http://svn.apache.org/viewvc/jakarta/jmeter/trunk/bin/jmeter.properties?rev=1187840&r1=1187839&r2=1187840&view=diff
> >>
> >>
> ==============================================================================
> >> --- jakarta/jmeter/trunk/bin/jmeter.properties (original)
> >> +++ jakarta/jmeter/trunk/bin/jmeter.properties Sun Oct 23 01:50:55 2011
> >> @@ -78,6 +78,11 @@ xml.parser=org.apache.xerces.parsers.SAX
> >>  # set the value to 'false' to reset the SSL context each iteration
> >>  #https.use.cached.ssl.context=true
> >>
> >> +# Start and end index to be used with keystores with many entries
> >> +# The default is to use entry 0, i.e. the first
> >> +#https.keyStoreStartIndex=0
> >> +#https.keyStoreEndIndex=0
> >> +
> >>
> >>
>  #---------------------------------------------------------------------------
> >>  # Look and Feel configuration
> >>
> >>
>  #---------------------------------------------------------------------------
> >>
> >> Modified:
> >> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/JsseSSLManager.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/JsseSSLManager.java?rev=1187840&r1=1187839&r2=1187840&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/JsseSSLManager.java
> >> (original)
> >> +++
> >> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/JsseSSLManager.java
> Sun
> >> Oct 23 01:50:55 2011
> >> @@ -70,6 +70,9 @@ public class JsseSSLManager extends SSLM
> >>
> >>     private static final int cps;
> >>
> >> +    //@GuardedBy("this")
> >> +    private static int  last_user;
> >> +
> >>     static {
> >>         log.info("Using default SSL protocol: "+DEFAULT_SSL_PROTOCOL);
> >>         log.info("SSL session context: "+(SHARED_SESSION_CONTEXT ?
> >> "shared" : "per-thread"));
> >> @@ -314,8 +317,12 @@ public class JsseSSLManager extends SSLM
> >>          */
> >>         public String[] getClientAliases(String keyType, Principal[]
> >> issuers) {
> >>             log.debug("WrappedX509Manager: getClientAliases: ");
> >> -            log.debug(this.store.getAlias());
> >> -            return new String[] { this.store.getAlias() };
> >> +            int count = this.store.getAliasCount();
> >> +            String[] aliases = new String[count];
> >> +            for(int i = 0; i < aliases.length; i++) {
> >> +                aliases[i] = this.store.getAlias(i);
> >> +            }
> >> +             return aliases;
> >>         }
> >>
> >>         /**
> >> @@ -343,7 +350,7 @@ public class JsseSSLManager extends SSLM
> >>          */
> >>         public X509Certificate[] getCertificateChain(String alias) {
> >>             log.debug("WrappedX509Manager: getCertificateChain(" + alias
> +
> >> ")");
> >> -            return this.store.getCertificateChain();
> >> +            return this.store.getCertificateChain(alias);
> >>         }
> >>
> >>         /**
> >> @@ -354,8 +361,9 @@ public class JsseSSLManager extends SSLM
> >>          * @return The PrivateKey value
> >>          */
> >>         public PrivateKey getPrivateKey(String alias) {
> >> -            log.debug("WrappedX509Manager: getPrivateKey: " +
> >> this.store.getPrivateKey());
> >> -            return this.store.getPrivateKey();
> >> +            PrivateKey privateKey = this.store.getPrivateKey(alias);
> >> +            log.debug("WrappedX509Manager: getPrivateKey: " +
> privateKey);
> >> +            return privateKey;
> >>         }
> >>
> >>         /**
> >> @@ -372,14 +380,28 @@ public class JsseSSLManager extends SSLM
> >>          * @see javax.net.ssl.X509KeyManager#chooseClientAlias(String[],
> >> Principal[], Socket)
> >>          */
> >>         public String chooseClientAlias(String[] keyType, Principal[]
> >> issuers, Socket socket) {
> >> -            String alias = this.store.getAlias();
> >> -            log.debug("ClientAlias: " + alias);
> >> +            log.debug("keyType: " + keyType[0]);
> >> +            int aliasCount = this.store.getAliasCount();
> >> +            String alias =
> this.store.getAlias(getNextIndex(aliasCount));
> >>             if (alias == null || alias.length() == 0) {
> >>                 log.debug("ClientAlias not found.");
> >>             }
> >>             return alias;
> >>         }
> >>
> >> +        private int getNextIndex(int aliasCount) {
> >> +            if (aliasCount == 1) {
> >> +                return 0;
> >> +            }
> >> +            synchronized(this) {
> >> +                last_user ++;
> >> +                if (last_user >= aliasCount) {
> >> +                    last_user = 0;
> >> +                }
> >> +                return last_user;
> >> +            }
> >> +        }
> >> +
> >>         /**
> >>          * Choose the server alias for the SSLServerSockets. This are
> not
> >> used
> >>          * in JMeter.
> >>
> >> Modified:
> >> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/SSLManager.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/SSLManager.java?rev=1187840&r1=1187839&r2=1187840&view=diff
> >>
> >>
> ==============================================================================
> >> --- jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/SSLManager.java
> >> (original)
> >> +++ jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/SSLManager.java
> >> Sun Oct 23 01:50:55 2011
> >> @@ -64,7 +64,7 @@ public abstract class SSLManager {
> >>     private static final Provider sslProvider = null;
> >>
> >>     /** Cache the KeyStore instance */
> >> -    private JmeterKeyStore keyStore;
> >> +    private volatile JmeterKeyStore keyStore;
> >>
> >>     /** Cache the TrustStore instance - null if no truststore name was
> >> provided */
> >>     private KeyStore trustStore = null;
> >> @@ -126,7 +126,9 @@ public abstract class SSLManager {
> >>                 if (initStore.exists()) {
> >>                     fileInputStream = new FileInputStream(initStore);
> >>                     this.keyStore.load(fileInputStream, getPassword());
> >> -                    log.info("Keystore loaded OK from file, found
> alias:
> >> "+keyStore.getAlias());
> >> +                    if (log.isInfoEnabled()) {
> >> +                        log.info("Total of " +
> keyStore.getAliasCount() +
> >> " aliases loaded OK from keystore");
> >> +                    }
> >>                 } else {
> >>                     log.warn("Keystore file not found, loading empty
> >> keystore");
> >>                     this.defaultpw = ""; // Ensure not null
> >>
> >> Modified:
> >>
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/DefaultKeyStore.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/DefaultKeyStore.java?rev=1187840&r1=1187839&r2=1187840&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/DefaultKeyStore.java
> >> (original)
> >> +++
> >>
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/DefaultKeyStore.java
> >> Sun Oct 23 01:50:55 2011
> >> @@ -23,21 +23,35 @@ import java.security.KeyStore;
> >>  import java.security.PrivateKey;
> >>  import java.security.cert.Certificate;
> >>  import java.security.cert.X509Certificate;
> >> +import java.util.ArrayList;
> >>  import java.util.Enumeration;
> >>
> >> +import org.apache.jmeter.util.JMeterUtils;
> >> +
> >>  /**
> >>  * Use this Keystore to wrap the normal KeyStore implementation.
> >>  *
> >>  */
> >>  public class DefaultKeyStore extends JmeterKeyStore {
> >> -    private X509Certificate[] certChain;
> >> +    private X509Certificate[][] certChains;
> >>
> >> -    private PrivateKey key;
> >> +    private PrivateKey[] keys;
> >>
> >> -    private String alias;
> >> +    private String[] names;
> >>
> >>     private final KeyStore store;
> >>
> >> +    private static final String KEY_STORE_START_INDEX =
> >> "https.keyStoreStartIndex"; // $NON-NLS-1$
> >> +    private static final String KEY_STORE_END_INDEX   =
> >> "https.keyStoreEndIndex"; // $NON-NLS-1$
> >> +
> >> +    private static final int startIndex;
> >> +    private static final int endIndex;
> >> +
> >> +    static {
> >> +        startIndex = JMeterUtils.getPropDefault(KEY_STORE_START_INDEX,
> 0);
> >> +        endIndex = JMeterUtils.getPropDefault(KEY_STORE_END_INDEX, 0);
> >> +    }
> >> +
> >>     public DefaultKeyStore(String type) throws Exception {
> >>         this.store = KeyStore.getInstance(type);
> >>     }
> >> @@ -46,54 +60,96 @@ public class DefaultKeyStore extends Jme
> >>     @Override
> >>     public void load(InputStream is, String pword) throws Exception {
> >>         store.load(is, pword.toCharArray());
> >> -        PrivateKey _key = null;
> >> -        X509Certificate[] _certChain = null;
> >>
> >> -        if (null != is){ // No point checking an empty keystore
> >> +        ArrayList<String> v_names = new ArrayList<String>();
> >> +        ArrayList<PrivateKey> v_keys = new ArrayList<PrivateKey>();
> >> +        ArrayList<X509Certificate[]> v_certChains = new
> >> ArrayList<X509Certificate[]>();
> >>
> >> +        if (null != is){ // No point checking an empty keystore
> >> +            PrivateKey _key = null;
> >> +            int index = 0;
> >>             Enumeration<String> aliases = store.aliases();
> >>             while (aliases.hasMoreElements()) {
> >> -                this.alias = aliases.nextElement();
> >> +                String alias = aliases.nextElement();
> >>                 if (store.isKeyEntry(alias)) {
> >> -                    _key = (PrivateKey) store.getKey(alias,
> >> pword.toCharArray());
> >> -                    Certificate[] chain =
> >> store.getCertificateChain(alias);
> >> -                    _certChain = new X509Certificate[chain.length];
> >> -
> >> -                    for (int i = 0; i < chain.length; i++) {
> >> -                        _certChain[i] = (X509Certificate) chain[i];
> >> +                    if ((index >= startIndex && index <= endIndex)) {
> >> +                        _key = (PrivateKey) store.getKey(alias,
> >> pword.toCharArray());
> >> +                        if (null == _key) {
> >> +                            throw new Exception("No key found for
> alias: "
> >> + alias); // Should not happen
> >> +                        }
> >> +                        Certificate[] chain =
> >> store.getCertificateChain(alias);
> >> +                        if (null == chain) {
> >> +                            throw new Exception("No certificate chain
> >> found for alias: " + alias);
> >> +                        }
> >> +                        v_names.add(alias);
> >> +                        v_keys.add(_key);
> >> +                        v_certChains.add((X509Certificate[]) chain);
> >>                     }
> >> -
> >> -                    break;
> >>                 }
> >> +                index++;
> >>             }
> >>
> >>             if (null == _key) {
> >> -                throw new Exception("No key found");
> >> -            }
> >> -            if (null == _certChain) {
> >> -                throw new Exception("No certificate chain found");
> >> +                throw new Exception("No key(s) found");
> >>             }
> >>         }
> >>
> >> -        this.key = _key;
> >> -        this.certChain = _certChain;
> >> +        /*
> >> +         * Note: if is == null, the arrays will be empty
> >> +         */
> >> +        int v_size = v_names.size();
> >> +
> >> +        this.names = new String[v_size];
> >> +        this.names = v_names.toArray(names);
> >> +
> >> +        this.keys = new PrivateKey[v_size];
> >> +        this.keys = v_keys.toArray(keys);
> >> +
> >> +        this.certChains = new X509Certificate[v_size][];
> >> +        this.certChains = v_certChains.toArray(certChains);
> >>     }
> >>
> >> -    /** {@inheritDoc} */
> >>     @Override
> >> -    public final X509Certificate[] getCertificateChain() {
> >> -        return this.certChain;
> >> +    public final X509Certificate[] getCertificateChain(String alias) {
> >> +        int entry = findAlias(alias);
> >> +        if (entry >=0) {
> >> +            return this.certChains[entry];
> >> +        }
> >> +        return null;
> >>     }
> >>
> >> -    /** {@inheritDoc} */
> >>     @Override
> >> -    public final PrivateKey getPrivateKey() {
> >> -        return this.key;
> >> +    public final PrivateKey getPrivateKey(String alias) {
> >> +        int entry = findAlias(alias);
> >> +        if (entry >=0) {
> >> +            return this.keys[entry];
> >> +        }
> >> +        return null;
> >>     }
> >>
> >> -    /** {@inheritDoc} */
> >>     @Override
> >> -    public final String getAlias() {
> >> -        return this.alias;
> >> +    public final String getAlias(int index) {
> >> +        int length = this.names.length;
> >> +        if (length == 0 && index == 0) { // i.e. is == null
> >> +            return null;
> >> +        }
> >> +        if (index >= length || index < 0) {
> >> +            throw new ArrayIndexOutOfBoundsException(index);
> >> +        }
> >> +        return this.names[index];
> >> +    }
> >> +
> >> +    @Override
> >> +    public int getAliasCount() {
> >> +        return this.names.length;
> >> +    }
> >> +
> >> +    private int findAlias(String alias) {
> >> +        for(int i = 0; i < names.length; i++) {
> >> +            if (alias.equals(names[i])){
> >> +                return i;
> >> +            }
> >> +        }
> >> +        return -1;
> >>     }
> >>  }
> >>
> >> Modified:
> >>
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/JmeterKeyStore.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/JmeterKeyStore.java?rev=1187840&r1=1187839&r2=1187840&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/JmeterKeyStore.java
> >> (original)
> >> +++
> >>
> jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/JmeterKeyStore.java
> >> Sun Oct 23 01:50:55 2011
> >> @@ -34,16 +34,18 @@ public abstract class JmeterKeyStore {
> >>     public abstract void load(InputStream is, String password) throws
> >> Exception;
> >>
> >>     /**
> >> -     * Get the ordered certificate chain.
> >> +     * Get the ordered certificate chain for a specific alias.
> >>      */
> >> -    public abstract X509Certificate[] getCertificateChain();
> >> +    public abstract X509Certificate[] getCertificateChain(String
> alias);
> >>
> >> -    public abstract String getAlias();
> >> +    public abstract int getAliasCount();
> >> +
> >> +    public abstract String getAlias(int index);
> >>
> >>     /**
> >> -     * Return the private Key
> >> +     * Return the private Key for a specific alias
> >>      */
> >> -    public abstract PrivateKey getPrivateKey();
> >> +    public abstract PrivateKey getPrivateKey(String alias);
> >>
> >>     public static final JmeterKeyStore getInstance(String type) throws
> >> Exception {
> >>         // JAVA 1.4 now handles all keystore types, so just use default
> >>
> >> Modified: jakarta/jmeter/trunk/xdocs/changes.xml
> >> URL:
> >>
> http://svn.apache.org/viewvc/jakarta/jmeter/trunk/xdocs/changes.xml?rev=1187840&r1=1187839&r2=1187840&view=diff
> >>
> >>
> ==============================================================================
> >> --- jakarta/jmeter/trunk/xdocs/changes.xml (original)
> >> +++ jakarta/jmeter/trunk/xdocs/changes.xml Sun Oct 23 01:50:55 2011
> >> @@ -137,6 +137,7 @@ Mirror server now uses default port 8081
> >>  <h3>HTTP Samplers</h3>
> >>  <ul>
> >>  <li>Bug 51981 - Better support for file: protocol in HTTP sampler</li>
> >> +<li>Bug 52033 - Allowing multiple certificates (JKS)</li>
> >>  </ul>
> >>
> >>  <h3>Other samplers</h3>
> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: notifications-unsubscr...@jakarta.apache.org
> >> For additional commands, e-mail: notifications-h...@jakarta.apache.org
> >>
> >>
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
> For additional commands, e-mail: dev-h...@jakarta.apache.org
>
>


-- 
Cordialement.
Philippe Mouawad.

Reply via email to