On 12/08/2014 11:56, Konstantin Kolinko wrote: > 2014-08-12 14:41 GMT+04:00 <ma...@apache.org>: >> Author: markt >> Date: Tue Aug 12 10:41:49 2014 >> New Revision: 1617445 >> >> URL: http://svn.apache.org/r1617445 >> Log: >> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56825 >> Enable pre-emptive authentication to work with the SSL authenticator. Based >> on a patch by jlmonteiro. >> >> Modified: >> >> tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java >> tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java >> tomcat/trunk/test/org/apache/tomcat/util/net/TestClientCert.java >> tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java >> tomcat/trunk/webapps/docs/changelog.xml >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java?rev=1617445&r1=1617444&r2=1617445&view=diff >> ============================================================================== >> --- >> tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java >> (original) >> +++ >> tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java >> Tue Aug 12 10:41:49 2014 >> @@ -41,6 +41,7 @@ import org.apache.catalina.connector.Req >> import org.apache.catalina.connector.Response; >> import org.apache.catalina.util.SessionIdGenerator; >> import org.apache.catalina.valves.ValveBase; >> +import org.apache.coyote.ActionCode; >> import org.apache.juli.logging.Log; >> import org.apache.juli.logging.LogFactory; >> import org.apache.tomcat.util.descriptor.web.LoginConfig; >> @@ -566,8 +567,7 @@ public abstract class AuthenticatorBase >> } >> >> if (!authRequired && context.getPreemptiveAuthentication()) { >> - X509Certificate[] certs = (X509Certificate[]) >> request.getAttribute( >> - Globals.CERTIFICATES_ATTR); >> + X509Certificate[] certs = getRequestCertificates(request); >> authRequired = certs != null && certs.length > 0; >> } >> >> @@ -619,6 +619,35 @@ public abstract class AuthenticatorBase >> >> // ------------------------------------------------------ Protected >> Methods >> >> + /** >> + * Look for the X509 certificate chain in the Request under the key >> + * <code>javax.servlet.request.X509Certificate</code>. If not found, >> trigger >> + * extracting the certificate chain from the Coyote request. >> + * >> + * @param request Request to be processed >> + * >> + * @return The X509 certificate chain if found, >> <code>null</code> >> + * otherwise. >> + */ >> + protected X509Certificate[] getRequestCertificates(final Request >> request) >> + throws IllegalStateException { >> + >> + X509Certificate certs[] = >> + (X509Certificate[]) >> request.getAttribute(Globals.CERTIFICATES_ATTR); >> + >> + if ((certs == null) || (certs.length < 1)) { >> + try { >> + request.getCoyoteRequest().action >> (ActionCode.REQ_SSL_CERTIFICATE, null); > > This ActionCode causes re-regotiation of HTTPS.
Ah. I didn't spot that. I need to look more closely at exactly what is going on where. > It is understandable in case if webapp expects a certificate (in > SSLAuthenticator), but in AuthenticatorBase this is generic code. Indeed. > Will it (erroneously) trigger re-authentication for webapps that do > not need it, e.g. protected by BASIC authentication? Let me work through some examples. This might need changing / removing. > I also wonder whether there is some check if renegotiation has already > been attempted once on this connection. I'm not sure. Logically, there must be something stopping re-negotiation every time the certs are requested for an authenticated resource but I need look at the code in a bit more depth. The other thing to keep in mind is that setting clientAuth="want" is probably the better way to handle this use case. Thanks for the review. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
