This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new b3ff835 Improve the SSLValve so it is able to handle the ssl_client_escaped_cert header from Nginx b3ff835 is described below commit b3ff8357a9559e781ae6f9bcd61738cd7ccae2bf Author: Florent Guillaume <fguilla...@nuxeo.com> AuthorDate: Fri Mar 5 16:49:13 2021 +0100 Improve the SSLValve so it is able to handle the ssl_client_escaped_cert header from Nginx --- java/org/apache/catalina/valves/SSLValve.java | 20 +++++++++++++++++++- test/org/apache/catalina/valves/TestSSLValve.java | 23 +++++++++++++++++++++++ webapps/docs/changelog.xml | 5 +++++ webapps/docs/config/valve.xml | 8 ++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/java/org/apache/catalina/valves/SSLValve.java b/java/org/apache/catalina/valves/SSLValve.java index e2ea6ae..05b60f7 100644 --- a/java/org/apache/catalina/valves/SSLValve.java +++ b/java/org/apache/catalina/valves/SSLValve.java @@ -30,6 +30,7 @@ import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.buf.UDecoder; /** * When using mod_proxy_http, the client SSL information is not included in the @@ -65,6 +66,7 @@ public class SSLValve extends ValveBase { private static final Log log = LogFactory.getLog(SSLValve.class); private String sslClientCertHeader = "ssl_client_cert"; + private String sslClientEscapedCertHeader = "ssl_client_escaped_cert"; private String sslCipherHeader = "ssl_cipher"; private String sslSessionIdHeader = "ssl_session_id"; private String sslCipherUserKeySizeHeader = "ssl_cipher_usekeysize"; @@ -83,6 +85,14 @@ public class SSLValve extends ValveBase { this.sslClientCertHeader = sslClientCertHeader; } + public String getSslClientEscapedCertHeader() { + return sslClientEscapedCertHeader; + } + + public void setSslClientEscapedCertHeader(String sslClientEscapedCertHeader) { + this.sslClientEscapedCertHeader = sslClientEscapedCertHeader; + } + public String getSslCipherHeader() { return sslCipherHeader; } @@ -128,6 +138,8 @@ public class SSLValve extends ValveBase { * processing below: * - mod_header converts the '\n' into ' ' * - nginx converts the '\n' into multiple ' ' + * - nginx ssl_client_escaped_cert uses "uri component" escaping, + * keeping only ALPHA, DIGIT, "-", ".", "_", "~" * * The code assumes that the trimmed header value starts with * '-----BEGIN CERTIFICATE-----' and ends with @@ -137,7 +149,13 @@ public class SSLValve extends ValveBase { * separate lines, the CertificateFactory is tolerant of any * additional whitespace. */ - String headerValue = mygetHeader(request, sslClientCertHeader); + String headerValue; + String headerEscapedValue = mygetHeader(request, sslClientEscapedCertHeader); + if (headerEscapedValue != null) { + headerValue = UDecoder.URLDecode(headerEscapedValue, null); + } else { + headerValue = mygetHeader(request, sslClientCertHeader); + } if (headerValue != null) { headerValue = headerValue.trim(); if (headerValue.length() > 27) { diff --git a/test/org/apache/catalina/valves/TestSSLValve.java b/test/org/apache/catalina/valves/TestSSLValve.java index 7e0b073..3d8df7b 100644 --- a/test/org/apache/catalina/valves/TestSSLValve.java +++ b/test/org/apache/catalina/valves/TestSSLValve.java @@ -29,6 +29,7 @@ import org.apache.catalina.Valve; import org.apache.catalina.connector.Connector; import org.apache.catalina.connector.Request; import org.apache.tomcat.unittest.TesterLogValidationFilter; +import org.apache.tomcat.util.buf.UEncoder; import org.easymock.EasyMock; public class TestSSLValve { @@ -184,6 +185,17 @@ public class TestSSLValve { @Test + public void testSslClientCertHeaderEscaped() throws Exception { + String cert = certificateEscaped(); + mockRequest.setHeader(valve.getSslClientEscapedCertHeader(), cert); + + valve.invoke(mockRequest, null); + + assertCertificateParsed(); + } + + + @Test public void testSslClientCertNull() throws Exception { TesterLogValidationFilter f = TesterLogValidationFilter.add(null, "", null, "org.apache.catalina.valves.SSLValve"); @@ -318,6 +330,17 @@ public class TestSSLValve { } + private static String certificateEscaped() throws Exception { + String cert = certificateSingleLine(CERTIFICATE_LINES, "\n"); + String escaped = new UEncoder(UEncoder.SafeCharsSet.DEFAULT).encodeURL(cert, 0, cert.length()).toString(); + Assert.assertTrue(escaped, escaped.contains("%0a")); // newline is escaped + Assert.assertTrue(escaped, escaped.contains("%20")); // space is escaped + Assert.assertTrue(escaped, escaped.contains("%2b")); // + is escaped + Assert.assertTrue(escaped, escaped.contains("%2f")); // / is escaped + return escaped; + } + + private void assertCertificateParsed() throws Exception { TesterLogValidationFilter f = TesterLogValidationFilter.add(null, "", null, "org.apache.catalina.valves.SSLValve"); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 528665e..d127949 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -179,6 +179,11 @@ resulting in one of the deployments failing and errors being reported. (markt) </fix> + <fix> + Improve the <code>SSLValve</code> so it is able to handle escaped + client certificate headers from Nginx. Based on a patch by Florent + Guillaume. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml index a3246e2..2969255 100644 --- a/webapps/docs/config/valve.xml +++ b/webapps/docs/config/valve.xml @@ -1218,6 +1218,14 @@ used.</p> </attribute> + <attribute name="sslClientEscapedCertHeader" required="false"> + <p>Allows setting a custom name for the ssl_client_escaped_cert header. + If not specified, the default of <code>ssl_client_escaped_cert</code> is + used.</p> + <p>This header is useful for Nginx proxying, and takes precedence over + the ssl_client_cert header.</p> + </attribute> + <attribute name="sslCipherHeader" required="false"> <p>Allows setting a custom name for the ssl_cipher header. If not specified, the default of <code>ssl_cipher</code> is --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org