Bug 61901 - Support for https.cipherSuites property
Summary: Support for https.cipherSuites property
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: HTTP (show other bugs)
Version: 3.1
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2017-12-13 20:11 UTC by Jeremy Arnold
Modified: 2018-02-13 13:28 UTC (History)
2 users (show)



Attachments
Patch to support the https.cipherSuites system property (5.70 KB, patch)
2017-12-13 20:11 UTC, Jeremy Arnold
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Arnold 2017-12-13 20:11:43 UTC
Created attachment 35609 [details]
Patch to support the https.cipherSuites system property

I ran an HTTPS-based JMeter test (using JMeter 3.1) with several hundred users against one of my servers.  I then switched to a different server to run a similar test, but the results were terrible. The issue turned out to be that the new server supported stronger encryption cipher suites, and the stronger encryption was driving up the CPU on the JMeter client so that it could no longer support as many users.  In my particular case, it wasn't important to test with the stronger encryption -- it was more important to be able to scale up the large number of users.

Both HttpsURLConnection and Apache HttpClient support the https.cipherSuites system property to allow configuration of what cipher suites are supported by the client.  But JMeter uses a custom SSLSocketFactory that doesn't use this property.

JMeter should support the https.cipherSuites system property so that users can set this value in system.properties when needed.

I've written a patch (generated by Eclipse) against the current trunk code to add this support.  This fix addressed the issue for my test.
Comment 1 Philippe Mouawad 2017-12-13 20:25:45 UTC
Hello Jeremy,
thanks for your patch.
You mention System property in your comment but in your code you use a JMeter property set through:

-Jhttps.cipherSuites=

Maybe it would be better to make JMeter also accept definition through System property no ?

Regarding the performance issue , what exactly was the cause ? the negotiation of cipher  ?
Thanks
Comment 2 Jeremy Arnold 2017-12-13 20:40:11 UTC
I apologize for the confusion between System property vs JMeter property.  I intended for this to be a System property (set in system.properties) to match the behavior of HttpsURLConnection / HttpClient.  But when I wrote the patch I was following the example of https.socket.protocols, and thus used JMeterUtils.getPropDefault.

Since JMeterUtils.loadJMeterProperties initializes its local properties using "Properties p = new Properties(System.getProperties())", I believe the code does work with System properties in addition to JMeter properties.  (In my testing, I set the value in the system.properties file.)

I'd be happy to update the patch to use System.getProperty instead so this value can only be set as a System property and not a JMeter property.  Please let me know if that's your preference.


I haven't gotten completely to the bottom of the performance issue.  In my test the JMeter system CPU was maxed out, primarily performing SSL handshakes.  I wouldn't expect quite as big of a difference in CPU utilization between the two cipher suites that were being used.  My suspicion was that one of them was being hardware accelerated and the other not, but I have not yet confirmed that.
Comment 3 Philippe Mouawad 2017-12-13 20:50:16 UTC
(In reply to Jeremy Arnold from comment #2)
> I apologize for the confusion between System property vs JMeter property.  I
> intended for this to be a System property (set in system.properties) to
> match the behavior of HttpsURLConnection / HttpClient.  But when I wrote the
> patch I was following the example of https.socket.protocols, and thus used
> JMeterUtils.getPropDefault.

Ok
> 
> Since JMeterUtils.loadJMeterProperties initializes its local properties
> using "Properties p = new Properties(System.getProperties())", I believe the
> code does work with System properties in addition to JMeter properties.  (In
> my testing, I set the value in the system.properties file.)

You are right.

> 
> I'd be happy to update the patch to use System.getProperty instead so this
> value can only be set as a System property and not a JMeter property. 
> Please let me know if that's your preference.

It looks fine.
For your information if you submit future patches, documentation is in xdocs, docs html are generated from there.
> 
> 
> I haven't gotten completely to the bottom of the performance issue.  In my
> test the JMeter system CPU was maxed out, primarily performing SSL
> handshakes.  I wouldn't expect quite as big of a difference in CPU
> utilization between the two cipher suites that were being used.  My
> suspicion was that one of them was being hardware accelerated and the other
> not, but I have not yet confirmed that.


Thanks
Comment 4 Jeremy Arnold 2017-12-19 18:58:17 UTC
Changing status back to NEW as I believe I've addressed all of the questions.  If any additional info is needed, please let me know.
Comment 5 Philippe Mouawad 2017-12-20 17:51:16 UTC
Author: pmouawad
Date: Wed Dec 20 17:50:52 2017
New Revision: 1818839

URL: http://svn.apache.org/viewvc?rev=1818839&view=rev
Log:
Bug 61901 - Support for https.cipherSuites property
Contributed by  Jeremy Arnold
Bugzilla Id: 61901

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/util/HttpSSLProtocolSocketFactory.java
    jmeter/trunk/xdocs/changes.xml
    jmeter/trunk/xdocs/usermanual/properties_reference.xml
Comment 6 bamboocha324 2018-02-13 13:28:37 UTC
Hi,

i'm also using strong https connection during my jmeter tests since jmeter 3.0.

What i've done and is perfectly working for me is i've set this:

JMETER_OPTS="-Dhttps.cipherSuites=TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"

to my jmeter.sh file.

Doing so my jmeter connect (via ssl) using this cipher string with my server.
I also set in the jmeter.properties the tls version to 1.2 (only) cuz this cipher is only in tlsv1.2

Best Regards.