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

Reply via email to