Bug 52033

Hello,
Thanks for the patch but can you recreate it by removing all tabs that have been added ?
We generally use 4 spaces indentation (never any tabs).

Thanks

Created attachment 27778 [details]
New patch with spaces.

Sorry.

Thanks for the patch; unfortunately it does not apply cleanly to the code in trunk.

Also, there are several instances of using @SuppressWarnings.
As far as possible we don't allow these.
The instance ArrayList fields in DefaultKeyStore should be local variables.
The mutable static caches in JsseSLManager are not thread-safe.

But we can fix these.

I'm more concerned about the use of the system property names javax.net.ssl.keyStoreStartIndex and javax.net.ssl.keyStoreEndIndex.

Are these standard property names?
I could not find any reference to them.
If these are invented names, then they should not use the javax namespace.
Probably should be JMeter properties.

The proposal needs a bit more consideration.

I was waiting for us to reach this point, this properties is used to load a range of certificates/keys of jks, we use this when using distributed tests, we can help to write a documentation about this and we consider that this feature is very important.
About the names of properties, really dont exists in documentation, we invented that! however we can change it.

We appreciate your patience, we intend to help more with less mistakes in the future, thank you!

OK

In which case the only problem is that a system property in the javax namespace is not appropriate.

I'll see about incorporating the patch in due course.

Sebba, we tested the proposal that appears to be stable. We are very interested in including this feature to prevent us to use an unofficial version of JMeter. There are planning on which version will the use of multiple certificates as proposed? How long could we wait?

Found a few other issues with the patch:
- the behaviour for start = end = 0 was changed
- unnecessary class fields
- unsafe caching of fields

Applied changes to SVN:

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

Please check if the updated code works for you.
Note that I changed the property names, and the meaning of the last index.
If start = end, then that is the index of the single alias to be used.

Also, at present the index range has to be chosen before starting a test.
Is there a use case for allowing the range of aliases to vary between samples?
Or more likely, using a different fixed alias for different samples?
For example, when connecting to different hosts, perhaps these will need different aliases?

Made another change to improve thread-safety in r1187876.

Please try with any build from that fix and report if the issue is solved.

Message from Julian Cesar:
Hello Philippe,

We test and we found a problem.
In line 89 (v_certChains.add((X509Certificate[]) chain)) of DefaultKeyStore we have a class cast problem:

2011/11/01 10:19:10 ERROR - jmeter.util.SSLManager: Problem loading keystore: [Ljava.security.cert.Certificate; cannot be cast to [Ljava.security.cert.X509Certificate;

We are using the revision 1195404.

Maybe we need to iterate one by one of certificates to be cast to x509. In the code originally sent we did this way.

Would it be possible for you to test again with upcoming revision, I just
commited the fix to this:

Date: Tue Nov  1 13:11:56 2011
New Revision: 1195978

URL: http://svn.apache.org/viewvc?rev=1195978&view=rev
Log:
Bug 52033 - Allowing multiple certificates (JKS):
Fixed classcastexception

Modified:
  
jakarta/jmeter/trunk/src/core/org/apache/jmeter/util/keystore/DefaultKeyStore.java

Message from Julian Cesar:
The problem with the class cast is solved, but 2 problems still exists.

1. Always the same certificate is used, if you use the keystore was sent you can see this;
2. Is interesting that the certificates are loaded at startup jmeter or as soon as a key store is selected, because this load takes around 3 seconds, and this can cause noise in the test and also the incomplete certificates loading can cause a malfunction of user requests.

Hello,
You can achieve the first by setting:
- HTTPClient 3.1 or 4 as implementation
- setting https.use.cached.ssl.context=false


For the second , we will make it an option.
Regards
Philippe

if you create a option to load key store on start jmeter would be perfect.
We would like a nightly builds for the final tests, when we have it?

Date: Tue Nov  1 21:51:46 2011
New Revision: 1196306

URL: http://svn.apache.org/viewvc?rev=1196306&view=rev
Log:
Bug 52033 - Allowing multiple certificates (JKS)
Added Keystore Config component to enable configuring:
- Preload
- Start alias index
- End alias index

Added:
   jmeter/trunk/src/components/org/apache/jmeter/config/KeystoreConfig.java   (with props)
   jmeter/trunk/src/components/org/apache/jmeter/config/KeystoreConfigBeanInfo.java   (with props)
   jmeter/trunk/src/components/org/apache/jmeter/config/KeystoreConfigResources.properties   (with props)
   jmeter/trunk/xdocs/images/screenshots/keystore_config.png   (with props)
Modified:
   jmeter/trunk/src/core/org/apache/jmeter/util/SSLManager.java
   jmeter/trunk/src/core/org/apache/jmeter/util/keystore/DefaultKeyStore.java
   jmeter/trunk/src/core/org/apache/jmeter/util/keystore/JmeterKeyStore.java
   jmeter/trunk/xdocs/usermanual/component_reference.xml

Hello Julian,
Preload was implemented through new "KKeystore Config" new config element.
AS soon as Nightly build is available you can test it.
You will need to change your test plan to add "Keystore Config" component.

Regards
Philippe

I tested with 7000 certs and everything is ok!

It would be interesting if not put values ​​for the start and index end on Keystore Configuration, we try to get the https.keyStoreStartIndex and https.keyStoreEndIndex contained in the file jmeter.properties, because if it is distributed test is a necessary thing. This makes sense for you?

Implemented as required.
I will close issue as solved, please reopen if you find any issue and in all cases, comment to say tests are OK on your side.

Thanks for your contribution on this.

Date: Thu Nov  3 16:29:32 2011
New Revision: 1197207

URL: http://svn.apache.org/viewvc?rev=1197207&view=rev
Log:
BUG_52033 :
Default to jmeter.properties if start and end not set
Handle errors in inputed start and endIndex

Modified:
   jmeter/trunk/src/components/org/apache/jmeter/config/KeystoreConfig.java

*** Bug 22510 has been marked as a duplicate of this bug. ***

Created attachment 28381 [details]
Correction

Hi, I made a dumb mistake in this feature! 
We did not have detected this problem until we need to get a specific certificate.
The correction is atached in ticket.

Can you explain the issue ?

Maybe you should create a new Bugzilla for this.

Thank you
Regards
Philippe

Done! The bug 52762 was created.

Thanks

This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/2604