[SWS-997] Thread proliferation and memory leak when using WSS4JSecurityInterceptor Created: 11/Oct/17 Updated: 10/Nov/17
|Project:||Spring Web Services|
|Remaining Estimate:||Not Specified|
|Time Spent:||Not Specified|
|Original Estimate:||Not Specified|
We're using a WSS4JSecurityInterceptor to validate incoming messages and sign outgoing messages:
When using the interceptor this way, a lot of EHCache threads named "wss4j%002etimestamp%002ecache-e%0058ga%0058l%0058%004b%0057g%004ah%0050w==.data" (and similar) start to appear, one for each request that has been made.
I believe this happens because of the way the Wss4jSecurityInterceptor initializes the WSS4J RequestData structure in initializeRequestData and initializeValidationRequestData. By default the RequestData class has caching enabled for detecting various replay attacks. However the default implementation is to just create one cache per RequestData structure using the ReplayCacheFactory.newInstance() method.
Now when you send a request and this is validated, this at some point calls WSS4J's SignatureProcessor class, which in turn calls RequestData.getTimestampReplayCache() which by default creates a new EHCache instance (if EHCache happens to be on the classpath which it is for our project).
I found out about this issue reading through http://apache-xml-project.6118.n7.nabble.com/nonce-cache-thread-proliferation-td42797.html for a similar problem. The recommended solution is to either disable replay caches or to properly initialize them by calling the setXXXReplayCache methods when creating the RequestData structure.
So since the current implementation wouldn't detect replay attacks either (because having one cache per request basically disables replay attack checks, albeit in a very expensive manner), i would suggest changing the implementation of the Wss4jSecurityInterceptor initializeRequestData and initializeValidationRequestData methods and to append these lines before returning the RequestData structure:
|Comment by László Csontos [ 10/Nov/17 ]|
I've also faced the very same issue about a year ago and worked it around this way: https://gist.github.com/laszlocsontos/c0b1beac76e02cbaa0514a19f7db923c
Basically this was a bug in WSS4J (WSS-584) and they fixed it in 2.x by removing automatic creation of replay caches from RequestData: https://fisheye.apache.org/changelog/wss4j?cs=1753264.
What this means for Spring-WS v2.4 is that this is a bug, because it's still using WSS4J 1.6.x. For the upcoming v3 however, this is a loss of functionality, because WSS4J has been recently upgraded to 2.x and replay caches aren't created at all.
I'd gladly contribute a fix for this along with a junit test and would appritiate some guidence from the Spring-WS team how to do that?
The idea is that users of Wss4jSecurityInterceptor could indicate with a flag if they want to use replay caches of not, by default I would leave it on. After that at the relevant places Wss4jSecurityInterceptor would ensure that caches are created only once. See a working PoC above.
Should I submit the fix first to master and then backport it to 2.4.0.RELEASE or the other way around?
This is indeed a major issue and it's not easy to spot what's going on. Nice catch Jan!
|Comment by Greg Turnquist [ 10/Nov/17 ]|
To clarify, Spring WS 2.x will continue supporting both Wss4j 1.6.x and 2.x because we cannot remove 1.6.x support for backwards compatibility reasons. Spring WS 3.x IS the place where we drop support for Wss4j 1.6.x.
Any contribution to fix Wss4jSecurityInterceptor would be highly appreciated. My recommendation is to work against the master branch. Once completed, we can assess if we wish to backport the change or not. Simple stuff is typically quite simple.
Also, this change cannot break existing APIs in either branch. That would demand a 3.1 release of Spring WS and we aren't ready for that yet.