Closed asfimport closed 6 years ago
asfimport
commented
8 years ago Rainer Jung (migrated from Bugzilla):
Both parts could then be controlled via static flags instead of instance flags. One flag would be in HTTPHCAbstractImpl and controls reset of the SSLContext, one flag would be in HTTPHC4Impl and HTTPHC3Impl and controls the HTTPClient resets.
Correction: static -> static ThreadLocal
asfimport
commented
8 years ago Rainer Jung (migrated from Bugzilla): The patch fixes reuse/reset between test iterations (on the same thread) of SSLContext, HTTPClient and HTTP connections for HC4 (and likely HC3).
For experiments the reuse of the three objects can be controlled using three different system properties
"true" means "reuse" in all three cases, "false" means "reset/close" (do not reuse).
Resetting the HTTPClient results in also resetting HTTP Connections, but not vice versa.
It is not yet clear to me if we need to support both cases, resetting connection plus HTTPClient and also resetting Connections but reusing HTTPClient, or if one of the twi would suffice.
Feedback welcome.
Created attachment SSLContext-HTTPClient-Connection-reuse.patch: Fixing SSLContext/HTTPClient/Connection reuse
asfimport
commented
8 years ago Sebb (migrated from Bugzilla): (In reply to Rainer Jung from comment 2)
For experiments the reuse of the three objects can be controlled using three different system properties
They seem to be JMeter properties, which is better.
asfimport
commented
8 years ago Rainer Jung (migrated from Bugzilla): Yes, sorry, JMeter properties.
I think we don't need/want all three of them (individual control over connection reuse, HTTPClient reuse and SSLContext reuse). After some experimenting IMHO the useful cases are:
The default should be full reuse like it is today.
The question is though, whether the name "https.use.cached.ssl.context" for the property is good. Once could use a uniform switch to determine reuse in the HTTP case as well, something like reset.http.objects.before.iteration. Default would be "false" (no reset = reuse) and to start each iteration with fresh objects you would switch to "true".
The old flag could be deprecated. If a user would use both, the old and the new, we could warn during startup and the new flag would win.
asfimport
commented
8 years ago Sebb (migrated from Bugzilla): (In reply to Rainer Jung from comment 4)
The question is though, whether the name "https.use.cached.ssl.context" for the property is good.
Seems OK to me; the only problem is it does not fully describe what is re-used. But that could be fixed by better docn in the jmeter.properties file.
Once could use a uniform switch to determine reuse in the HTTP case as well, something like reset.http.objects.before.iteration.
Why do we need the reuse flag for http?
asfimport
commented
8 years ago Rainer Jung (migrated from Bugzilla): As far as I understand the situation reusing (the default) means e.g. connections are kept open and are reused via HTTP Keep-Alive during the next iteration. I do see this behavior using https, I expect it to happen for http too. That's of course efficient but might not be what you want. A new iteration typically is a new user who would start with new connections.
Of course modelling the connection behavior in an exact way is beyond what we support, but the decision to start a new iteration with new connections could be interesting for some (approximate modeling). It is somehow analogous to modeling the handshake rate by resetting the SSLContext, although a new connection is of cours less disruptive than a new ssl handshake. All in all not a critical feature.
In addition I think the optional behavior "reset instead of reuse" for http and https is a good safety net against problems that might creep in due to reuse of objects that might have more context than we expect (wrong basic auth, wrong cookie, wrong client cert, wrong SNI etc.). Of course most of these things do work now, but reusing the connections and the objects from the HC libs as well as the SSLContext might introduce future bugs of the sort "next iteration uses context data from the previous one" which could then be worked around quickly by disabling the reuse. In general since the risks are quite vague so I'd stay on the performance side here for the default value (reuse). But probably based on my incomplete understanding of the lifecycles of HC objects, I'd feel better if we had such a "reset between iterations" switch.
asfimport
commented
8 years ago Sebb (migrated from Bugzilla): (In reply to Rainer Jung from comment 6)
As far as I understand the situation reusing (the default) means e.g. connections are kept open and are reused via HTTP Keep-Alive during the next iteration. I do see this behavior using https, I expect it to happen for http too. That's of course efficient but might not be what you want. A new iteration typically is a new user who would start with new connections.
Of course modelling the connection behavior in an exact way is beyond what we support, but the decision to start a new iteration with new connections could be interesting for some (approximate modeling). It is somehow analogous to modeling the handshake rate by resetting the SSLContext, although a new connection is of cours less disruptive than a new ssl handshake. All in all not a critical feature.
Surelt that's already catered for by the "Use Keepalive" checkbox? If not checked, JMeter sends Connection: close.
In addition I think the optional behavior "reset instead of reuse" for http and https is a good safety net against problems that might creep in due to reuse of objects that might have more context than we expect (wrong basic auth, wrong cookie, wrong client cert, wrong SNI etc.). Of course most of these things do work now, but reusing the connections and the objects from the HC libs as well as the SSLContext might introduce future bugs of the sort "next iteration uses context data from the previous one" which could then be worked around quickly by disabling the reuse. In general since the risks are quite vague so I'd stay on the performance side here for the default value (reuse). But probably based on my incomplete understanding of the lifecycles of HC objects, I'd feel better if we had such a "reset between iterations" switch.
I think we already do.
asfimport
commented
8 years ago Rainer Jung (migrated from Bugzilla): (In reply to Sebb from comment 7)
(In reply to Rainer Jung from comment 6) > As far as I understand the situation reusing (the default) means e.g. > connections are kept open and are reused via HTTP Keep-Alive during the next > iteration. I do see this behavior using https, I expect it to happen for > http too. That's of course efficient but might not be what you want. A new > iteration typically is a new user who would start with new connections. > > Of course modelling the connection behavior in an exact way is beyond what > we support, but the decision to start a new iteration with new connections > could be interesting for some (approximate modeling). It is somehow > analogous to modeling the handshake rate by resetting the SSLContext, > although a new connection is of cours less disruptive than a new ssl > handshake. All in all not a critical feature.
Surelt that's already catered for by the "Use Keepalive" checkbox? If not checked, JMeter sends Connection: close.
I didn't mean to completely disable Keep-Alive but to not alow Keep-Alive from one itertaion to the next. The "Use Keepalive" checkbox would disable Keep-Alive completely.
> In addition I think the optional behavior "reset instead of reuse" for http > and https is a good safety net against problems that might creep in due to > reuse of objects that might have more context than we expect (wrong basic > auth, wrong cookie, wrong client cert, wrong SNI etc.). Of course most of > these things do work now, but reusing the connections and the objects from > the HC libs as well as the SSLContext might introduce future bugs of the > sort "next iteration uses context data from the previous one" which could > then be worked around quickly by disabling the reuse. In general since the > risks are quite vague so I'd stay on the performance side here for the > default value (reuse). But probably based on my incomplete understanding of > the lifecycles of HC objects, I'd feel better if we had such a "reset > between iterations" switch.
I think we already do.
I don't understand you. The current impl of reset (https.use.cached.ssl.context) is a NOP in the http case.
asfimport
commented
8 years ago @pmouawad (migrated from Bugzilla): (In reply to Rainer Jung from comment 4)
Yes, sorry, JMeter properties.
I think we don't need/want all three of them (individual control over connection reuse, HTTPClient reuse and SSLContext reuse). After some experimenting IMHO the useful cases are:
- full reuse of SSLContext, HTTPClient and existing connections
- no reuse of SSLContext, HTTPClient and existing connections
Looks ok to me.
The default should be full reuse like it is today.
The question is though, whether the name "https.use.cached.ssl.context" for the property is good. Once could use a uniform switch to determine reuse in the HTTP case as well, something like reset.http.objects.before.iteration. Default would be "false" (no reset = reuse) and to start each iteration with fresh objects you would switch to "true".
The old flag could be deprecated. If a user would use both, the old and the new, we could warn during startup and the new flag would win.
Thanks for fix
asfimport
commented
8 years ago Sebb (migrated from Bugzilla): (In reply to Rainer Jung from comment 8)
(In reply to Sebb from comment 7) > (In reply to Rainer Jung from comment 6) > > As far as I understand the situation reusing (the default) means e.g. > > connections are kept open and are reused via HTTP Keep-Alive during the next > > iteration. I do see this behavior using https, I expect it to happen for > > http too. That's of course efficient but might not be what you want. A new > > iteration typically is a new user who would start with new connections. > > > > Of course modelling the connection behavior in an exact way is beyond what > > we support, but the decision to start a new iteration with new connections > > could be interesting for some (approximate modeling). It is somehow > > analogous to modeling the handshake rate by resetting the SSLContext, > > although a new connection is of cours less disruptive than a new ssl > > handshake. All in all not a critical feature. > > Surelt that's already catered for by the "Use Keepalive" checkbox? > If not checked, JMeter sends Connection: close.
I didn't mean to completely disable Keep-Alive but to not alow Keep-Alive from one itertaion to the next.
What do you mean by iteration? If you mean the start of the next loop, one just needs to clear Use KeepAlive on the last sampler.
The "Use Keepalive" checkbox would disable Keep-Alive completely.
Use Keepalive only applies to a single sample. If deselected, it means the connection will be reset after it completes.
> > In addition I think the optional behavior "reset instead of reuse" for http > > and https is a good safety net against problems that might creep in due to > > reuse of objects that might have more context than we expect (wrong basic > > auth, wrong cookie, wrong client cert, wrong SNI etc.). Of course most of > > these things do work now, but reusing the connections and the objects from > > the HC libs as well as the SSLContext might introduce future bugs of the > > sort "next iteration uses context data from the previous one" which could > > then be worked around quickly by disabling the reuse. In general since the > > risks are quite vague so I'd stay on the performance side here for the > > default value (reuse). But probably based on my incomplete understanding of > > the lifecycles of HC objects, I'd feel better if we had such a "reset > > between iterations" switch. > > I think we already do.
I don't understand you. The current impl of reset (https.use.cached.ssl.context) is a NOP in the http case.
I meant that Use KeepAlive acts as the reset if deselected.
So I think we already have the means to do a reset each iteration.
asfimport
commented
8 years ago Rainer Jung (migrated from Bugzilla): (In reply to Sebb from comment 10)
(In reply to Rainer Jung from comment 8) > (In reply to Sebb from comment 7) > > (In reply to Rainer Jung from comment 6) > > > As far as I understand the situation reusing (the default) means e.g. > > > connections are kept open and are reused via HTTP Keep-Alive during the next > > > iteration. I do see this behavior using https, I expect it to happen for > > > http too. That's of course efficient but might not be what you want. A new > > > iteration typically is a new user who would start with new connections. > > > > > > Of course modelling the connection behavior in an exact way is beyond what > > > we support, but the decision to start a new iteration with new connections > > > could be interesting for some (approximate modeling). It is somehow > > > analogous to modeling the handshake rate by resetting the SSLContext, > > > although a new connection is of cours less disruptive than a new ssl > > > handshake. All in all not a critical feature. > > > > Surelt that's already catered for by the "Use Keepalive" checkbox? > > If not checked, JMeter sends Connection: close. > > I didn't mean to completely disable Keep-Alive but to not alow Keep-Alive > from one itertaion to the next.
What do you mean by iteration?
One loop for the same thread.
If you mean the start of the next loop, one just needs to clear Use KeepAlive on the last sampler.
Depending on the logic controllers in the plan, there is no easy way to decide upon the last sampler, more precisely the last execution of the last sampler in one loop. It could change during plan execution. Most often it will be a defined logout sample or similar, but not necessarily. You e.g. might want to loop the last sampler until a condition becomes true.
> The "Use Keepalive" checkbox would disable > Keep-Alive completely.
Use Keepalive only applies to a single sample. If deselected, it means the connection will be reset after it completes.
OK, you were talking about a single HTTP sampler, I was talking about the HTTP sampler default config element. See above for problem of defining the "last sampler".
> > > In addition I think the optional behavior "reset instead of reuse" for http > > > and https is a good safety net against problems that might creep in due to > > > reuse of objects that might have more context than we expect (wrong basic > > > auth, wrong cookie, wrong client cert, wrong SNI etc.). Of course most of > > > these things do work now, but reusing the connections and the objects from > > > the HC libs as well as the SSLContext might introduce future bugs of the > > > sort "next iteration uses context data from the previous one" which could > > > then be worked around quickly by disabling the reuse. In general since the > > > risks are quite vague so I'd stay on the performance side here for the > > > default value (reuse). But probably based on my incomplete understanding of > > > the lifecycles of HC objects, I'd feel better if we had such a "reset > > > between iterations" switch. > > > > I think we already do. > > I don't understand you. The current impl of reset > (https.use.cached.ssl.context) is a NOP in the http case.
I meant that Use KeepAlive acts as the reset if deselected.
So I think we already have the means to do a reset each iteration.
As I wrote, connection keep alive is only one of the arguments. General object reuse between loop iterations (HTTPClient instance) is what makes me feel more uncomfortable. If someone with enough HC knowledge than me comments strongly that such a reset will most likely not be necessary, I'm OK to forget about that reset feature for http.
asfimport
commented
8 years ago @pmouawad (migrated from Bugzilla): Hi, Can the patch be commited ? Thanks
asfimport
commented
8 years ago @pmouawad (migrated from Bugzilla): Hello,
I suggest we delay this bugfix to 3.1 unless there is a volunteer to test it and confirm it is OK (including Parallel Download)
Regards Philippe
asfimport
commented
8 years ago bamboocha324 (migrated from Bugzilla): Hi,
well in jmeter 2.13 i had the following (so far not clear to me) issue with https where the https WAS mutual ssl. I set up the ssl debug on and watched what happend with my http samplers.
I had a java key store whith 2 client certs. I had a key store configuration element which starts at 0 and ends at 1 (2 elements).
Now i had one thread group and 2 http samplers both pointing to the same https server. One sampler was targeting https://server/uri and the second ../uri2.
Now i had the following problem. The first http sampler picked the first ssl cert from my keystore [0] made a mutual ssl connection and execute .../uri. -great.
The second http sampler DOES NOT REUSE the connection and DOES NOT executed the ../uri2 BUT the second http sampler made a NEW connection (ssl) and picked a NEW client cert from the keystore [1]
It was STILL the SAME Thread just the next http sampler. It was a mess because the user with the client cert on [1] was not allowed to execute the ../uri2.
Then...i changed the parameter "https.use.cached.ssl.context" and put value "true". I have no idea why...but it WORKED?!?. Means the first http sampler did mutual ssl and execute ../uri The second http sampler did NOT the handshake again but execute the ../uri2 as the same user as the first sampler. perfect. But why?
asfimport
commented
6 years ago @pmouawad (migrated from Bugzilla): Still present in 4.0
asfimport
commented
6 years ago @pmouawad (migrated from Bugzilla): Following dev mailing list discussion "https://github.com/apache/jmeter/issues/3759" , this is a patch implementing what I suggested.
Created attachment BUG_58807.patch: Amended Patch
asfimport
commented
6 years ago @pmouawad (migrated from Bugzilla): Created attachment BUG_58807.jmx: Test plan with different cases
asfimport
commented
6 years ago @pmouawad (migrated from Bugzilla): NodeJS project that can be run :
node server.js
Created attachment client-certificate.zip: NodeJS Project for client certificate authentication
asfimport
commented
6 years ago @pmouawad (migrated from Bugzilla): Author: pmouawad Date: Sun Feb 25 21:19:42 2018 New Revision: 1825328
URL: http://svn.apache.org/viewvc?rev=1825328&view=rev Log: https://github.com/apache/jmeter/issues/3759 - https.use.cached.ssl.context=false is broken Based partly on Rainer Jung analysis and patch. https://github.com/apache/jmeter/issues/3759
Modified: jmeter/trunk/bin/jmeter.properties jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHCAbstractImpl.java jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java jmeter/trunk/xdocs/changes.xml jmeter/trunk/xdocs/usermanual/properties_reference.xml
asfimport
commented
6 years ago @pmouawad (migrated from Bugzilla): Author: pmouawad Date: Tue Aug 28 21:12:36 2018 New Revision: 1839506
URL: http://svn.apache.org/viewvc?rev=1839506&view=rev Log: https://github.com/apache/jmeter/issues/3759 - Reset SSL State on Thread Group iteration only (was https.use.cached.ssl.context=false is broken) Document https://github.com/apache/jmeter/issues/3759
Modified: jmeter/trunk/xdocs/changes.xml jmeter/trunk/xdocs/usermanual/component_reference.xml
pmouawad@apache.org
11:13 PM (0 minutes ago)
to commits Author: pmouawad Date: Tue Aug 28 21:13:48 2018 New Revision: 1839507
URL: http://svn.apache.org/viewvc?rev=1839507&view=rev Log: https://github.com/apache/jmeter/issues/3759 - Reset SSL State on Thread Group iteration only (was https.use.cached.ssl.context=false is broken) Document https://github.com/apache/jmeter/issues/3759
Modified: jmeter/trunk/xdocs/usermanual/properties_reference.xml
asfimport
commented
5 years ago Vasili U (migrated from Bugzilla): Hello all,
After migrating to Jmeter 5.1.1, I have the issue when I'm trying to send several requests with different client ssl certificates inside one tread group and one iteration. In Jmeter 4.0 I could set https.use.cached.ssl.context=false, but it's deprecated since Jmeter 5.0. I tried to close connection for the first request, but it didn't help me. Could you please help me how can I do it in Jmeter 5.0 ?
Thanks, Vasili
Rainer Jung (Bug 58807): All of the following assumes you have set https.use.cached.ssl.context=false and are doing HTTPS tests using HC3 or HC4. The property is supposed the close/reset SSLContext, HTTPS connections and similar per thread at the beginning of each test iteration.
Interface TestIterationListener has a method testIterationStart() that was used in r1172147 to implement this (feature request BZ 51380). Originally the implemention of the method in HTTPSamplerBase called sslMgr.resetContext() and then notifySSLContextWasReset() which in turn immediately closed all thread local HC3 and HC4 connections using closeThreadLocalConnections() in the respective implementations.
Unfortunately there was a negative side effect, namely closing of http connections as well (BZ 55023). Since all the info needed to decide which protocol was used etc. would only be available when a sampler actually executes a request, the implementation was switched to delay the close/reset of the objects in the samplers until the next sample of the respective sampler executes. The TestIterationListener would only set a flag "resetSSLContext" in the sampler and the sampler would reset its flag once it has closed/reset the objects during the next execution. The flag is per sampler instance.
Now here's the problem: multiple samplers can and often will share the HTTP connections and SSLContext etc. So in a test plan with e.g. two HC4 samplers s1 and s2 both requesting some URI from the same HTTPS host, after the iteration starts s1 would close/reset, then create new objects and run its sample, next s2 would close/reset again, again create new objects and run its sample.
More or less this means that https.use.cached.ssl.context=false also disables reusing SSLContext, connection Keep-Alive etc. between samples in the same iteration.
As far as I can see, resetting the SSLContext via JsseSSLManager should be done as one single call at the start of an iteration. It is only needed
The other part of the close/reset, namely cleaning up the httpClient and its connections etc. is mor etricky, because there is a whole map of HTTPClient instances. Currently each sampler cares about "his" HTTPClient instance, even if that one is shared with other samplers. I think it would be better to add a static method to HTTPHC4Impl (and HC3) which would do the reset/close for all HTTPClient instances in the (thread-local) Map HTTPCLIENTS_CACHE_PER_THREAD_AND_HTTPCLIENTKEY.
Both parts could then be controlled via static flags instead of instance flags. One flag would be in HTTPHCAbstractImpl and controls reset of the SSLContext, one flag would be in HTTPHC4Impl and HTTPHC3Impl and controls the HTTPClient resets.
I'll try that and see whether it makes sense. If it works, one should also be able to support config for SSLContext reset independent of HTTPClient and Connection reset (this feature then also in the HTTP case, not only for HTTPS).
Severity: major OS: All
Duplicates: