Bug 48545 - truststorePass used in JSSESocketFactory should be optional (nillable)
truststorePass used in JSSESocketFactory should be optional (nillable)
Status: RESOLVED FIXED
Product: Tomcat 6
Classification: Unclassified
Component: Catalina
6.0.20
PC Linux
: P2 normal (vote)
: default
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2010-01-14 07:58 UTC by smmwpf54
Modified: 2010-11-25 11:03 UTC (History)
1 user (show)



Attachments
Patched JSSESocketFactory.java based on Tomcat 6.0.20 (2.24 KB, patch)
2010-01-14 07:58 UTC, smmwpf54
Details | Diff
Patch that provides better backwards compatibility (4.25 KB, text/plain)
2010-08-05 12:49 UTC, Mark Thomas
Details
2010-11-08_tc6_bug48545.patch - Updated version of the patch (5.18 KB, patch)
2010-11-08 03:25 UTC, Konstantin Kolinko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description smmwpf54 2010-01-14 07:58:48 UTC
Created attachment 24845 [details]
Patched JSSESocketFactory.java based on Tomcat 6.0.20

For the moment, a user must set the "truststorePass" in the SSL connector, even if this is not required by the JSEE API (Keystore.load() with null password is possible for truststores) and is also unwanted in a production environment with "real" truststores, because this may give someone the possibility to manipulate a productive trustore file or give more information than needed.
If the "truststorePass" is not set in the connector element, the current implementation will use the "keystorePass" as the value for "truststorePass" (strange wrong behaviour) and this will lead to an exception.
Proposal: do not set the "truststorePass" if omitted, leave it with null and the SSL connector still works.
This should also not affect old tomcat configurations, where the truststore password equals to the keystore password.
See my attached JSSESocketFactory patch (based on 6.0.20)
Comment 1 Mark Thomas 2010-02-15 17:17:34 UTC
Thanks for the patch. It has been applied to 7.0.x and proposed for 6.0.x

For future reference, please provide patches in diff -u format as they are easier to work with. Also, patches should update documentation where appropriate and should delete code rather than comment it out.
Comment 2 Mark Thomas 2010-08-05 12:49:56 UTC
Created attachment 25848 [details]
Patch that provides better backwards compatibility
Comment 3 Konstantin Kolinko 2010-11-08 03:25:58 UTC
Created attachment 26268 [details]
2010-11-08_tc6_bug48545.patch - Updated version of the patch

I'm attaching a slightly improved version of Mark's patch. The changes are:
- Do not retry with null password if the password was already null, or if there was a FileNotFoundException.
- Log the "jsse.keystore_load_failed" message in getKeystore() and getTrustStore(), when getStore() does not log it anymore.
Comment 4 Mark Thomas 2010-11-25 11:03:09 UTC
The updated patch has been applied to 6.0.x and will be included in 6.0.30 onwards.