You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/08/12 12:41:50 UTC
svn commit: r1617445 - in /tomcat/trunk:
java/org/apache/catalina/authenticator/ test/org/apache/tomcat/util/net/
webapps/docs/
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);
+ certs = (X509Certificate[]) request.getAttribute(Globals.CERTIFICATES_ATTR);
+ } catch (IllegalStateException ise) {
+ // Request body was too large for save buffer
+ // Return null which will trigger an auth failure
+ }
+ }
+
+ return certs;
+ }
+
/**
* Associate the specified single sign on identifier with the
Modified: tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java?rev=1617445&r1=1617444&r2=1617445&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java (original)
+++ tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java Tue Aug 12 10:41:49 2014
@@ -23,9 +23,7 @@ import java.security.cert.X509Certificat
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.apache.catalina.Globals;
import org.apache.catalina.connector.Request;
-import org.apache.coyote.ActionCode;
/**
* An <b>Authenticator</b> and <b>Valve</b> implementation of authentication
@@ -97,28 +95,15 @@ public class SSLAuthenticator extends Au
containerLog.debug(" Looking up certificates");
}
- X509Certificate certs[] = (X509Certificate[])
- request.getAttribute(Globals.CERTIFICATES_ATTR);
- if ((certs == null) || (certs.length < 1)) {
- try {
- request.getCoyoteRequest().action
- (ActionCode.REQ_SSL_CERTIFICATE, null);
- } catch (IllegalStateException ise) {
- // Request body was too large for save buffer
- response.sendError(HttpServletResponse.SC_UNAUTHORIZED,
- sm.getString("authenticator.certificates"));
- return false;
- }
- certs = (X509Certificate[])
- request.getAttribute(Globals.CERTIFICATES_ATTR);
- }
+ X509Certificate certs[] = getRequestCertificates(request);
+
if ((certs == null) || (certs.length < 1)) {
if (containerLog.isDebugEnabled()) {
containerLog.debug(" No certificates included with this request");
}
response.sendError(HttpServletResponse.SC_UNAUTHORIZED,
- sm.getString("authenticator.certificates"));
- return (false);
+ sm.getString("authenticator.certificates"));
+ return false;
}
// Authenticate the specified certificate chain
Modified: tomcat/trunk/test/org/apache/tomcat/util/net/TestClientCert.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TestClientCert.java?rev=1617445&r1=1617444&r2=1617445&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/net/TestClientCert.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/net/TestClientCert.java Tue Aug 12 10:41:49 2014
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertEqu
import org.junit.Assume;
import org.junit.Test;
+import org.apache.catalina.Context;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.util.buf.ByteChunk;
@@ -35,18 +36,37 @@ import org.apache.tomcat.util.buf.ByteCh
public class TestClientCert extends TomcatBaseTest {
@Test
- public void testClientCertGet() throws Exception {
+ public void testClientCertGetWithoutPreemptive() throws Exception {
+ doTestClientCertGet(false);
+ }
+
+ @Test
+ public void testClientCertGetWithPreemptive() throws Exception {
+ doTestClientCertGet(true);
+ }
+
+ public void doTestClientCertGet(boolean preemtive) throws Exception {
Assume.assumeTrue("SSL renegotiation has to be supported for this test",
TesterSupport.isRenegotiationSupported(getTomcatInstance()));
+ if (preemtive) {
+ // Only one context deployed
+ Context c = (Context) getTomcatInstance().getHost().findChildren()[0];
+ c.setPreemptiveAuthentication(true);
+ }
+
// Unprotected resource
ByteChunk res =
getUrl("https://localhost:" + getPort() + "/unprotected");
- assertEquals("OK", res.toString());
+ if (preemtive) {
+ assertEquals("OK-" + TesterSupport.ROLE, res.toString());
+ } else {
+ assertEquals("OK", res.toString());
+ }
// Protected resource
res = getUrl("https://localhost:" + getPort() + "/protected");
- assertEquals("OK", res.toString());
+ assertEquals("OK-" + TesterSupport.ROLE, res.toString());
}
@Test
Modified: tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java?rev=1617445&r1=1617444&r2=1617445&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java Tue Aug 12 10:41:49 2014
@@ -50,6 +50,8 @@ import org.apache.tomcat.util.descriptor
public final class TesterSupport {
+ public static final String ROLE = "testrole";
+
public static void initSsl(Tomcat tomcat) {
initSsl(tomcat, "localhost.jks", null, null);
}
@@ -161,14 +163,14 @@ public final class TesterSupport {
SecurityCollection collection = new SecurityCollection();
collection.addPattern("/protected");
SecurityConstraint sc = new SecurityConstraint();
- sc.addAuthRole("testrole");
+ sc.addAuthRole(ROLE);
sc.addCollection(collection);
ctx.addConstraint(sc);
// Configure the Realm
TesterMapRealm realm = new TesterMapRealm();
realm.addUser("CN=user1, C=US", "not used");
- realm.addUserRole("CN=user1, C=US", "testrole");
+ realm.addUserRole("CN=user1, C=US", ROLE);
ctx.setRealm(realm);
// Configure the authenticator
@@ -189,6 +191,9 @@ public final class TesterSupport {
throws ServletException, IOException {
resp.setContentType("text/plain");
resp.getWriter().print("OK");
+ if (req.isUserInRole(ROLE)) {
+ resp.getWriter().print("-" + ROLE);
+ }
}
@Override
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1617445&r1=1617444&r2=1617445&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Aug 12 10:41:49 2014
@@ -128,6 +128,10 @@
Add simple caching for calls to <code>StandardRoot.getResources()</code>
in the new (for 8.0.x) resources implementation. (markt)
</add>
+ <fix>
+ <bug>56825</bug>: Enable pre-emptive authentication to work with the
+ SSL authenticator. Based on a patch by jlmonteiro. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1617445 - in /tomcat/trunk: java/org/apache/catalina/authenticator/
test/org/apache/tomcat/util/net/ webapps/docs/
Posted by Mark Thomas <ma...@apache.org>.
On 12/08/2014 12:15, Mark Thomas wrote:
> 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.
Should be fixed now.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1617445 - in /tomcat/trunk: java/org/apache/catalina/authenticator/
test/org/apache/tomcat/util/net/ webapps/docs/
Posted by Mark Thomas <ma...@apache.org>.
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1617445 - in /tomcat/trunk: java/org/apache/catalina/authenticator/
test/org/apache/tomcat/util/net/ webapps/docs/
Posted by Konstantin Kolinko <kn...@gmail.com>.
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.
It is understandable in case if webapp expects a certificate (in
SSLAuthenticator), but in AuthenticatorBase this is generic code.
Will it (erroneously) trigger re-authentication for webapps that do
not need it, e.g. protected by BASIC authentication?
I also wonder whether there is some check if renegotiation has already
been attempted once on this connection.
> + certs = (X509Certificate[]) request.getAttribute(Globals.CERTIFICATES_ATTR);
> + } catch (IllegalStateException ise) {
> + // Request body was too large for save buffer
> + // Return null which will trigger an auth failure
Regarding the comment. This code may return a zero-length array as
well. (It is processed in the same way as null by the callers, so it
makes no difference).
> + }
> + }
> +
> + return certs;
> + }
> +
> (...)
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org