When using Tomcat SSL coyote connector, the request does not by default contain the certificate chain under the key javax.servlet.request.X509Certificate The following coyote action must be invoked in order to extract the certificate chain and enrich the request under the right key. This makes it impossible to use the SSLAuthenticator with preemptive mode for example. Provided a test to reproduce and the fix within the patch file. I tried to follow Tomcat guidelines and coding rules. If not lemme know so that I can resubmit a new patch. Not tested under Tomcat 6 and 8 but, the AuthenticatorBase does not change a lot over versions so I guess the bug existed before Tomcat 7 and still exists in Tomcat 8.
Created attachment 31885 [details] Patch with the test to reproduce and a fix
Generally we apply patches to the latest release and then back-port. Therefore patches should be against 8.0.x. The current patch changes import order and white space which a) makes the patch harder to review and b) those changes would need to be reverted before the patch was committed.
Thanks for reviewing. I'll checkout the 8.0.x trunk and submit a new patch. I'll take care as well of the formatting (including imports).
Sorry, I wanted to get the next 8.0.x release out so I have taken you patch and applied to to 8.0.x myself. In addition to the changes I mentioned previously, I also did the unit tests slightly differently to reduce the number of changes required. This has been fixed in 8.0.x for 8.0.11 onwards and in 7.0.x for 7.0.56 onwards.
Thank you very much Mark. I'll check your commit so that I can avoid such mistakes next time.
REOPENing for Tomcat 7. As discussed in "Re r1617445" on dev, the change trigger re-authentication for webapps that do not need it. The fix in Tomcat 8 is r1617461 but it has not been backported to Tomcat 7 yet.
Re-authentication patch back-ported from truk for 7.0.56 onwards.
Re-reviewing the changes in Tomcat 7 (revisions r1617447 r1620827 and r1622328 ) I have a question. There exists ActionCode.REQ_SSL_ATTRIBUTE. The method org.apache.catalina.connector.Request.getAttribute() does "if (isSSLAttribute(name)) coyoteRequest.action(ActionCode.REQ_SSL_ATTRIBUTE, ...)" This action populates the "javax.servlet.request.X509Certificate" attribute (aka Globals.CERTIFICATES_ATTR). I mean that it is effectively equivalent to the new API of using ActionCode.REQ_SSL_CERTIFICATE with parameter Boolean.FALSE. > When using Tomcat SSL coyote connector, the request does not by default contain > the certificate chain under the key javax.servlet.request.X509Certificate > > The following coyote action must be invoked in order to extract the certificate > chain and enrich the request under the right key. Is the above really true? Why was the old code not working properly? Was all this fix really needed? Was the new API really needed? I did the following at tc7.0.x\trunk: I reverted to the state before those fixes and updated the tests to their current versions: svn up -r 1617446 cd test/org/apache/tomcat/util/net svn up TestClientCert.java svn up TesterSupport.java Then I run test.entry=org.apache.tomcat.util.net.TestClientCert test with BIO, NIO, APR (java.7.home=JDK 7u67). Results are: 1) With APR the tests were skipped, "SKIPPED: SSL renegotiation has to be supported for this test" 2) With BIO and NIO the tests passed. So it looks like there was no issue.
Hi, (In reply to Konstantin Kolinko from comment #8) > Re-reviewing the changes in Tomcat 7 (revisions r1617447 r1620827 and > r1622328 ) I have a question. > > There exists ActionCode.REQ_SSL_ATTRIBUTE. > > The method org.apache.catalina.connector.Request.getAttribute() does > > "if (isSSLAttribute(name)) > coyoteRequest.action(ActionCode.REQ_SSL_ATTRIBUTE, ...)" > > This action populates the "javax.servlet.request.X509Certificate" attribute > (aka Globals.CERTIFICATES_ATTR). Right the getAttribute invokes ActionCode.REQ_SSL_ATTRIBUTE, but the main difference between REQ_SSL_ATTRIBUTE and REQ_SSL_CERTIFICATE is the following invocation: sslO = sslSupport.getPeerCertificateChain(<force>); REQ_SSL_ATTRIBUTE --> force is false REQ_SSL_CERTIFICATE --> force is true REQ_SSL_ATTRIBUTE --> the certificate entry is never populated cause the certificate chain is never extracted (in the use case above) > > I mean that it is effectively equivalent to the new API of using > ActionCode.REQ_SSL_CERTIFICATE with parameter Boolean.FALSE. > > > When using Tomcat SSL coyote connector, the request does not by default contain > > the certificate chain under the key javax.servlet.request.X509Certificate > > > > The following coyote action must be invoked in order to extract the certificate > > chain and enrich the request under the right key. > > Is the above really true? Why was the old code not working properly? Was all > this fix really needed? Was the new API really needed? I created the test to reproduce before proposing a fix. So if now it does not fail anymore, there must be something else. Did the following with this revision $ svn info Path: . Working Copy Root Path: /Users/jlmonteiro/devs/asf/tomcat/tc7.0.x/trunk URL: http://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk Repository Root: http://svn.apache.org/repos/asf Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68 Revision: 1616257 Node Kind: directory Schedule: normal Last Changed Author: markt Last Changed Rev: 1615951 Last Changed Date: 2014-08-05 17:50:13 +0200 (Mar, 05 aoû 2014) Kept the test case portion of my patch and it actually still fails. So either my test is wrong which is definitely possible, or I missed something. What do you think?
I've dug a little more into this. The unit test setting clientAuth="want" is hiding the fact that this still isn't quite right. Some new API is going to be necessary. That said, I'm not sure that the current API is exactly right either.
Fixed in 8.0.x for 8.0.13 onwards and in 7.0.x for 7.0.56 onwards. In the end, no new API was required as I was able to get the info I needed vai an existing API.