Bug 48208 - allow to configure a custom client certificate Trust Manager in server.xml per connector attribute "trustManagerClassName"
Summary: allow to configure a custom client certificate Trust Manager in server.xml pe...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Connectors (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-17 00:44 UTC by Ralf Hauser
Modified: 2013-07-11 15:24 UTC (History)
2 users (show)



Attachments
patch_48158_c5_wildCard.txt (3.45 KB, text/plain)
2009-11-17 00:44 UTC, Ralf Hauser
Details
patch for tomcat 6 svn revision 1065625 (2.63 KB, text/plain)
2011-01-31 11:23 UTC, Luciana Moreira
Details
Proposed patch for Tomcat 6 (4.22 KB, patch)
2011-03-06 11:39 UTC, Mark Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Hauser 2009-11-17 00:44:09 UTC
Created attachment 24546 [details]
patch_48158_c5_wildCard.txt

as per bug 48158 comment 8, this is now an RFE on its own:

Luciana has come up with a patch to allow accepting any client certificate on a
per Connector basis.

In server.xml the following attribute should be added in the Connector element:

<Connector ... acceptAllCerts="true"/>

If this argument is present and set to true or yes, then the
AcceptAllTrustManager will be used as Trust Manager.

The "truststoreFile" of
http://tomcat.apache.org/tomcat-5.5-doc/ssl-howto.html#Edit%20the%20Tomcat%20Configuration%20File
can be left empty

originally was attachment (id=24542)
Comment 1 Mark Thomas 2011-01-28 18:39:49 UTC
Accepting all client certificates defeats the purpose of CLIENT_CERT.
Comment 2 Ralf Hauser 2011-01-29 00:34:24 UTC
Mark, I have to disagree
in the context of CLIENT_CERT, the user still is forced to use a client certificate. If your policy is of a complexity that the standard container-provided security features do not match, the application can then make sure it only provides services to the client-certs provided that conform with its requirements
Comment 3 Mark Thomas 2011-01-29 07:19:18 UTC
Then we disagree.

Regardless of the complexity of the rules you may wish to apply, for there to be any security at all the client certificates have to be issued by a trusted certificate authority. The AcceptAllTrustManager is sufficiently insecure and its use sufficiently dangerous that I do not believe it should be part of the standard Tomcat distribution.

There should be sufficient scope within the current configuration options to install a custom trust manager although I haven't investigated this. If that process is excessively painful then I think an acceptable approach would be to add support for a trustManagerClassName attribute that would override the call to TrustManagerFactory.getTrustManagers() in a similar way to the above patch.
Comment 4 Luciana Moreira 2011-01-31 11:20:59 UTC
Hello Mark.
I do understand your point. Our application has a particular way of enforcing client authentication and authorization through an application level verification of the client certificates.

Opening the possibility for every application to accept any certificates may not be the best approach and lead to security problems to other applications that not necessarily will have the same policy as we do. Nevertheless, I did like your idea of allowing to specify the trustmanager's class.

I have implemented this idea, but I haven't had the time to test this code. But I believe it should work (famous last words). In any case it gives a good idea of how to structure the code in a way to allow proprietary TrustManagers.

I have taken the liberty to re-open the bug for your evaluation. I believe this can be an interesting addition to tomcat.
Comment 5 Luciana Moreira 2011-01-31 11:23:10 UTC
Created attachment 26581 [details]
patch for tomcat 6 svn revision 1065625

This patch allows to specify the TrustManager class name.
Comment 6 Christopher Schultz 2011-01-31 15:42:47 UTC
If you don't need authentication, why bother configuring it?

If you just want ANY client certificate to be available to your webapp, why not configure <Connector clientAuth="want" />?

Enabling CLIENT_CERT *authentication* with no form of validation of the certificate is entirely without merit.

If you just want the client certificate, then you want clientAuth="want" with no authentication configured.

Like Mark, I will -1 any attempt to add this feature unless you can give us a reasonable use case that is not possible to configure in Tomcat.
Comment 7 Ralf Hauser 2011-01-31 19:27:22 UTC
Hi Chris, we do need authentication and "want" is not good enough since the user can opt out not to use client cert auth altogether AFAICR.

To avoid misunderstandings, I have updated the Summary.

Luciana's patch attachment (id=26581) for tomcat 6 svn revision 1065625 exactly tries to implement what Mark suggested as "acceptable approach" in comment 3
Comment 8 Ralf Hauser 2011-02-03 05:24:46 UTC
Just another application scenario the mandates a custom TrustManager as suggested by Mark:

There is a restriction that depending on the time-of-the-day, a different set of certificates is trusted.

Our original solution approach was then to accept all certificates. Once the certificate is provided and the full SSL handshake is finished, we can choose the set of acceptable trusted issuing certificates depending on time-of-day or other context paramters. Only then we do the verification and possibly abort the session.

As I mentioned before, wedo understand your reservations on incorporating an AcceptAllTrustManager patch, however, we would hope that a more generic solution as the  trustManagerClassName suggested by Mark Thomas would be a fair compromise.
Then, we could move the logic from the application into our custom trust manager we define in “trustManagerClassName”.
Comment 9 Mark Thomas 2011-03-06 04:40:27 UTC
This has been fixed in 7.0.x and will be included in 7.0.11 onwards.

The patch has not yet been back-ported to 6.0.x
Comment 10 Mark Thomas 2011-03-06 11:39:29 UTC
Created attachment 26732 [details]
Proposed patch for Tomcat 6

Some slight modifications from the patch provided by Luciana Moreira.
Comment 11 Luciana Moreira 2011-03-07 10:02:21 UTC
I looked and it seems very good from my perspective.
Comment 12 Konstantin Kolinko 2011-03-07 13:55:40 UTC
Regarding implementation in 7.0.x: the tests added in r1078436 in TestCustomSsl do work with BIO connector, but fail with NIO. I have not tested APR.

Tested with JDK 6u23, WinXP, Ant 1.8.2, trunk @1078628
TEST-org.apache.tomcat.util.net.TestCustomSsl.NIO.txt:

Testsuite: org.apache.tomcat.util.net.TestCustomSsl
Tests run: 3, Failures: 1, Errors: 0, Time elapsed: 7,187 sec
------------- Standard Error -----------------
(...)
------------- ---------------- ---------------

Testcase: testCustomSslImplementation took 3,906 sec
Testcase: testCustomTrustManager1 took 1,641 sec
Testcase: testCustomTrustManager2 took 1,64 sec
	FAILED
expected:<200> but was:<-1>
junit.framework.AssertionFailedError: expected:<200> but was:<-1>
	at org.apache.tomcat.util.net.TestCustomSsl.doTestCustomTrustManager(TestCustomSsl.java:133)
	at org.apache.tomcat.util.net.TestCustomSsl.testCustomTrustManager2(TestCustomSsl.java:79)
Comment 13 Konstantin Kolinko 2011-03-07 21:26:55 UTC
(In reply to comment #10)
> Created an attachment (id=26732) [details]
> Proposed patch for Tomcat 6

Regarding this patch for tc6:

When trustManageClassName is specified, there is a lot of work to get and configure a TrustManagerFactory instance (tmf) in JSSESocketFactory#getTrustManagers() and then that getTrustManagers() method just ignores this tmf instance.

The documentation update part of the patch says that "If this attribute is set, the trust store attributes may be ignored." but with the current code they actually cannot be ignored because that (unneeded) tmf.init() call is likely to fail if those values are bogus.
Comment 14 Mark Thomas 2011-03-08 08:41:45 UTC
(In reply to comment #12)
> Regarding implementation in 7.0.x: the tests added in r1078436 in TestCustomSsl
> do work with BIO connector, but fail with NIO. I have not tested APR.

There is a lot of duplication between SSL Support in BIO and NIO. Removing that duplication should fix this and any other (as yet undiscovered issues).

This attribute is not supported for APR.
Comment 15 Mark Thomas 2011-06-14 11:23:33 UTC
This has been fixed in 6.0.x and will be included in 6.0.33 onwards.
Comment 16 Ralf Hauser 2013-07-11 15:24:42 UTC
in tomcat 7, "trustManagerClassName" of http://tomcat.apache.org/tomcat-7.0-doc/config/http.html#SSL%20Support will no longer be dealt with in JSSESocketFactory.java but at least as of version TOMCAT_7_0_28, this is now in AbstractEndpoint.java

http://svn.apache.org/viewvc/tomcat/tc7.0.x/tags/TOMCAT_7_0_28/java/org/apache/tomcat/util/net/AbstractEndpoint.java?view=markup