tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1797694 - in /tomcat/trunk: java/org/apache/catalina/valves/SSLValve.java test/org/apache/catalina/valves/TestSSLValve.java webapps/docs/changelog.xml
Date Mon, 05 Jun 2017 20:10:43 GMT
Author: markt
Date: Mon Jun  5 20:10:43 2017
New Revision: 1797694

URL: http://svn.apache.org/viewvc?rev=1797694&view=rev
Log:
Improve the <code>SSLValve</code> so it is able to handle client certificate headers
from Nginx. Based on a patch by Lucas Ventura Carro.

Added:
    tomcat/trunk/test/org/apache/catalina/valves/TestSSLValve.java   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/valves/SSLValve.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/valves/SSLValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/SSLValve.java?rev=1797694&r1=1797693&r2=1797694&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/SSLValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/SSLValve.java Mon Jun  5 20:10:43 2017
@@ -119,54 +119,66 @@ public class SSLValve extends ValveBase
         }
         return strcert0;
     }
-    @Override
-    public void invoke(Request request, Response response)
-        throws IOException, ServletException {
 
-        /* mod_header converts the '\n' into ' ' so we have to rebuild the client certificate
*/
-        String strcert0 = mygetHeader(request, sslClientCertHeader);
-        if (strcert0 != null && strcert0.length()>28) {
-            String strcert1 = strcert0.replace(' ', '\n');
-            String strcert2 = strcert1.substring(28, strcert1.length()-26);
-            String strcert3 = "-----BEGIN CERTIFICATE-----\n";
-            String strcert4 = strcert3.concat(strcert2);
-            String strcerts = strcert4.concat("\n-----END CERTIFICATE-----\n");
-            // ByteArrayInputStream bais = new ByteArrayInputStream(strcerts.getBytes("UTF-8"));
-            ByteArrayInputStream bais = new ByteArrayInputStream(
-                    strcerts.getBytes(StandardCharsets.ISO_8859_1));
-            X509Certificate jsseCerts[] = null;
-            String providerName = (String) request.getConnector().getProperty(
-                    "clientCertProvider");
-            try {
-                CertificateFactory cf;
-                if (providerName == null) {
-                    cf = CertificateFactory.getInstance("X.509");
-                } else {
-                    cf = CertificateFactory.getInstance("X.509", providerName);
+
+    @Override
+    public void invoke(Request request, Response response) throws IOException, ServletException
{
+        /*
+         * Known behaviours of reverse proxies that are handled by the
+         * processing below:
+         * - mod_header converts the '\n' into ' '
+         * - nginx converts the '\n' into multiple ' '
+         *
+         * The code assumes that the trimmed header value starts with
+         * '-----BEGIN CERTIFICATE-----' and ends with
+         * '-----END CERTIFICATE-----'.
+         *
+         * Note: As long as the BEGIN marker and the rest of the content are on
+         *       separate lines, the CertificateFactory is tolerant of any
+         *       additional whitespace.
+         */
+        String headerValue = mygetHeader(request, sslClientCertHeader);
+        if (headerValue != null) {
+            headerValue = headerValue.trim();
+            if (headerValue.length() > 27) {
+                String body = headerValue.substring(27);
+                String header = "-----BEGIN CERTIFICATE-----\n";
+                String strcerts = header.concat(body);
+                ByteArrayInputStream bais = new ByteArrayInputStream(
+                        strcerts.getBytes(StandardCharsets.ISO_8859_1));
+                X509Certificate jsseCerts[] = null;
+                String providerName = (String) request.getConnector().getProperty(
+                        "clientCertProvider");
+                try {
+                    CertificateFactory cf;
+                    if (providerName == null) {
+                        cf = CertificateFactory.getInstance("X.509");
+                    } else {
+                        cf = CertificateFactory.getInstance("X.509", providerName);
+                    }
+                    X509Certificate cert = (X509Certificate) cf.generateCertificate(bais);
+                    jsseCerts = new X509Certificate[1];
+                    jsseCerts[0] = cert;
+                } catch (java.security.cert.CertificateException e) {
+                    log.warn(sm.getString("sslValve.certError", strcerts), e);
+                } catch (NoSuchProviderException e) {
+                    log.error(sm.getString(
+                            "sslValve.invalidProvider", providerName), e);
                 }
-                X509Certificate cert = (X509Certificate) cf.generateCertificate(bais);
-                jsseCerts = new X509Certificate[1];
-                jsseCerts[0] = cert;
-            } catch (java.security.cert.CertificateException e) {
-                log.warn(sm.getString("sslValve.certError", strcerts), e);
-            } catch (NoSuchProviderException e) {
-                log.error(sm.getString(
-                        "sslValve.invalidProvider", providerName), e);
+                request.setAttribute(Globals.CERTIFICATES_ATTR, jsseCerts);
             }
-            request.setAttribute(Globals.CERTIFICATES_ATTR, jsseCerts);
         }
-        strcert0 = mygetHeader(request, sslCipherHeader);
-        if (strcert0 != null) {
-            request.setAttribute(Globals.CIPHER_SUITE_ATTR, strcert0);
+        headerValue = mygetHeader(request, sslCipherHeader);
+        if (headerValue != null) {
+            request.setAttribute(Globals.CIPHER_SUITE_ATTR, headerValue);
         }
-        strcert0 = mygetHeader(request, sslSessionIdHeader);
-        if (strcert0 != null) {
-            request.setAttribute(Globals.SSL_SESSION_ID_ATTR, strcert0);
+        headerValue = mygetHeader(request, sslSessionIdHeader);
+        if (headerValue != null) {
+            request.setAttribute(Globals.SSL_SESSION_ID_ATTR, headerValue);
         }
-        strcert0 = mygetHeader(request, sslCipherUserKeySizeHeader);
-        if (strcert0 != null) {
-            request.setAttribute(Globals.KEY_SIZE_ATTR,
-                    Integer.valueOf(strcert0));
+        headerValue = mygetHeader(request, sslCipherUserKeySizeHeader);
+        if (headerValue != null) {
+            request.setAttribute(Globals.KEY_SIZE_ATTR, Integer.valueOf(headerValue));
         }
         getNext().invoke(request, response);
     }

Added: tomcat/trunk/test/org/apache/catalina/valves/TestSSLValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/valves/TestSSLValve.java?rev=1797694&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/valves/TestSSLValve.java (added)
+++ tomcat/trunk/test/org/apache/catalina/valves/TestSSLValve.java Mon Jun  5 20:10:43 2017
@@ -0,0 +1,333 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.valves;
+
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.logging.Level;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.catalina.Globals;
+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.easymock.EasyMock;
+
+public class TestSSLValve {
+
+    public static class MockRequest extends Request {
+
+        public MockRequest() {
+            super(EasyMock.createMock(Connector.class));
+            setCoyoteRequest(new org.apache.coyote.Request());
+        }
+
+        @Override
+        public void setAttribute(String name, Object value) {
+            getCoyoteRequest().getAttributes().put(name, value);
+        }
+
+        @Override
+        public Object getAttribute(String name) {
+            return getCoyoteRequest().getAttribute(name);
+        }
+
+        public void setHeader(String header, String value) {
+            getCoyoteRequest().getMimeHeaders().setValue(header).setString(value);
+        }
+
+        public void addHeader(String header, String value) {
+            getCoyoteRequest().getMimeHeaders().addValue(header).setString(value);
+        }
+    }
+
+    private static final String[] CERTIFICATE_LINES = new String[] { "-----BEGIN CERTIFICATE-----",
+            "MIIFXTCCA0WgAwIBAgIJANFf3YTJgYifMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV",
+            "BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX",
+            "aWRnaXRzIFB0eSBMdGQwHhcNMTcwNTI2MjEzNjM3WhcNMTgwNTI2MjEzNjM3WjBF",
+            "MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50",
+            "ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIIC",
+            "CgKCAgEA2ykNBanZz4cVITNpZcWNVmErUzqgSNrK361mj9vEdB1UkHatwal9jVrR",
+            "QvFgfiZ8Gl+/85t0ebJhJ+rIr1ww6JE7v2s2MThENj95K5EwZOmgvw+CBlBYsFIz",
+            "8BtjlVYy+v7RaGPXfjrFkexQP9UIaiIIog2ClDZirRvb+QxS930/YW5Qo+X6EX6W",
+            "/m/HvlorD25U4ni2FQ0y+EMO2e1jD88cAAMoP5f+Mf6NBK8I6yUeaSuMq7WqtHGV",
+            "e4F1WOg5z9J5c/M69rB0iQr5NUQwZ1mPYf5Kr0P6+TLh8DJphbVvmHJyT3bgofeV",
+            "JYl/kdjiXS5P/jwY9tfmhu04tsyzopWRUFCcj5zCiqZYaMn0wtDn08KaAh9oOlg8",
+            "Z6mJ9i5EybkLm63W7z7LxuM+qnYzq4wKkKdx8hbpASwPqzJkJeXFL/LzhKdZuHiR",
+            "clgPVYnm98URwhObh073dKguG/gkhcnpXcVBBVdVTJZYGBvTpQh0afXd9bcBwOzY",
+            "t4MDpGiQB2fLzBOEZhQ37kUcWPmZw5bNPxhx4yE96Md0rx/Gu4ipAHuqLemb1SL5",
+            "uWNesVmgY3OXaIamQIm9BCwkf8mMvoYdAT+lukTUZLtJ6s2w+Oxnl10tmb+6sTXy",
+            "UB3WcBTp/o3YjAyJPnM1Wq6nVNQ4W2+NbV5purGAP09sumxeJj8CAwEAAaNQME4w",
+            "HQYDVR0OBBYEFCGOYMvymUG2ZZT+lK4LvwEvx731MB8GA1UdIwQYMBaAFCGOYMvy",
+            "mUG2ZZT+lK4LvwEvx731MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggIB",
+            "AG6m4nDYCompUtRVude1qulwwAaYEMHyIIsfymI8uAE7d2o4bGjVpAUOdH/VWSOp",
+            "Rzx0oK6K9cHyiBlKHw5zSZdqcRi++tDX3P9Iy5tXO//zkhMEnSpk6RF2+9JXtyhx",
+            "Gma4yAET1yES+ybiFT21uZrGCC9r69rWG8JRZshc4RVWGwZsd0zrATVqfY0mZurm",
+            "xLgU4UOvkTczjlrLiklwwU68M1DLcILJ5FZGTWeTJ/q1wpIn9isK2siAW/VOcbuG",
+            "xdbGladnIFv+iQfuZG0yjcuMsBFsQiXi6ONM8GM+dr+61V63/1s73jYcOToEsTMM",
+            "3bHeVffoSkhZvOGTRCI6QhK9wqnIKhAYqu+NbV4OphfE3gOaK+T1cASXUtSQPXoa",
+            "sEoIVmbQsWRBhWvYShVqvINsH/hAT3Cf/+SslprtQUqiyt2ljdgrRFZdoyB3S7ky",
+            "KWoZRvHRj2cKU65LVYwx6U1A8SGmViz4aHMSai0wwKzOVv9MGHeRaVhlMmMsbdfu",
+            "wKoKJv0xYoVwEh1rB8TH8PjbL+6eFLeZXYVZnH71d5JHCghZ8W0a11ZuYkscSQWk",
+            "yoTBqEpJloWksrypqp3iL4PAL5+KkB2zp66+MVAg8LcEDFJggBBJCtv4SCWV7ZOB",
+            "WLu8gep+XCwSn0Wb6D3eFs4DoIiMvQ6g2rS/pk7o5eWj", "-----END CERTIFICATE-----" };
+
+    private SSLValve valve = new SSLValve();
+
+    private MockRequest mockRequest = new MockRequest();
+    private Valve mockNext = EasyMock.createMock(Valve.class);
+
+
+    @Before
+    public void setUp() throws Exception {
+        valve.setNext(mockNext);
+        mockNext.invoke(mockRequest, null);
+        EasyMock.replay(mockNext);
+    }
+
+
+    @Test
+    public void testSslHeader() {
+        final String headerName = "myheader";
+        final String headerValue = "BASE64_HEADER_VALUE";
+        mockRequest.setHeader(headerName, headerValue);
+
+        Assert.assertEquals(headerValue, valve.mygetHeader(mockRequest, headerName));
+    }
+
+
+    @Test
+    public void testSslHeaderNull() {
+        final String headerName = "myheader";
+        mockRequest.setHeader(headerName, null);
+
+        Assert.assertNull(valve.mygetHeader(mockRequest, headerName));
+    }
+
+
+    @Test
+    public void testSslHeaderNullModHeader() {
+        final String headerName = "myheader";
+        final String nullModHeaderValue = "(null)";
+        mockRequest.setHeader(headerName, nullModHeaderValue);
+
+        Assert.assertNull(valve.mygetHeader(mockRequest, nullModHeaderValue));
+    }
+
+
+    @Test
+    public void testSslHeaderNullName() throws Exception {
+        Assert.assertNull(valve.mygetHeader(mockRequest, null));
+    }
+
+
+    @Test
+    public void testSslHeaderMultiples() throws Exception {
+        final String headerName = "myheader";
+        final String headerValue = "BASE64_HEADER_VALUE";
+        mockRequest.addHeader(headerName, headerValue);
+        mockRequest.addHeader(headerName, "anyway won't be found");
+
+        Assert.assertEquals(headerValue, valve.mygetHeader(mockRequest, headerName));
+    }
+
+
+    @Test
+    public void testSslClientCertHeaderSingleSpace() throws Exception {
+        String singleSpaced = certificateSingleLine(" ");
+        mockRequest.setHeader(valve.getSslClientCertHeader(), singleSpaced);
+
+        valve.invoke(mockRequest, null);
+
+        assertCertificateParsed();
+    }
+
+
+    @Test
+    public void testSslClientCertHeaderMultiSpace() throws Exception {
+        String singleSpaced = certificateSingleLine("    ");
+        mockRequest.setHeader(valve.getSslClientCertHeader(), singleSpaced);
+
+        valve.invoke(mockRequest, null);
+
+        assertCertificateParsed();
+    }
+
+
+    @Test
+    public void testSslClientCertHeaderTab() throws Exception {
+        String singleSpaced = certificateSingleLine("\t");
+        mockRequest.setHeader(valve.getSslClientCertHeader(), singleSpaced);
+
+        valve.invoke(mockRequest, null);
+
+        assertCertificateParsed();
+    }
+
+
+    @Test
+    public void testSslClientCertNull() throws Exception {
+        TesterLogValidationFilter f = TesterLogValidationFilter.add(null, "", null,
+                "org.apache.catalina.valves.SSLValve");
+
+        valve.invoke(mockRequest, null);
+
+        EasyMock.verify(mockNext);
+        Assert.assertNull(mockRequest.getAttribute(Globals.CERTIFICATES_ATTR));
+        Assert.assertEquals(0, f.getMessageCount());
+    }
+
+
+    @Test
+    public void testSslClientCertShorter() throws Exception {
+        mockRequest.setHeader(valve.getSslClientCertHeader(), "shorter than hell");
+
+        TesterLogValidationFilter f = TesterLogValidationFilter.add(null, "", null,
+                "org.apache.catalina.valves.SSLValve");
+
+        valve.invoke(mockRequest, null);
+
+        EasyMock.verify(mockNext);
+        Assert.assertNull(mockRequest.getAttribute(Globals.CERTIFICATES_ATTR));
+        Assert.assertEquals(0, f.getMessageCount());
+    }
+
+
+    @Test
+    public void testSslClientCertIgnoredBegin() throws Exception {
+        String[] linesBegin = Arrays.copyOf(CERTIFICATE_LINES, CERTIFICATE_LINES.length);
+        linesBegin[0] = "3fisjcme3kdsakasdfsadkafsd3";
+        String begin = certificateSingleLine(linesBegin, " ");
+        mockRequest.setHeader(valve.getSslClientCertHeader(), begin);
+
+        valve.invoke(mockRequest, null);
+
+        assertCertificateParsed();
+    }
+
+
+    @Test
+    public void testSslClientCertBadFormat() throws Exception {
+        String[] linesDeleted = Arrays.copyOf(CERTIFICATE_LINES, CERTIFICATE_LINES.length
/ 2);
+        String deleted = certificateSingleLine(linesDeleted, " ");
+        mockRequest.setHeader(valve.getSslClientCertHeader(), deleted);
+
+        TesterLogValidationFilter f = TesterLogValidationFilter.add(Level.WARNING, null,
+                "java.security.cert.CertificateException", "org.apache.catalina.valves.SSLValve");
+
+        valve.invoke(mockRequest, null);
+
+        EasyMock.verify(mockNext);
+        Assert.assertNull(mockRequest.getAttribute(Globals.CERTIFICATES_ATTR));
+        Assert.assertEquals(1, f.getMessageCount());
+    }
+
+
+    @Test
+    public void testClientCertProviderNotFound() throws Exception {
+        EasyMock.expect(mockRequest.getConnector().getProperty("clientCertProvider")).andStubReturn("wontBeFound");
+        EasyMock.replay(mockRequest.getConnector());
+        mockRequest.setHeader(valve.getSslClientCertHeader(), certificateSingleLine(" "));
+
+        TesterLogValidationFilter f = TesterLogValidationFilter.add(Level.SEVERE, null,
+                "java.security.NoSuchProviderException", "org.apache.catalina.valves.SSLValve");
+
+        valve.invoke(mockRequest, null);
+
+        Assert.assertNull(mockRequest.getAttribute(Globals.CERTIFICATES_ATTR));
+        Assert.assertEquals(1, f.getMessageCount());
+    }
+
+
+    @Test
+    public void testSslCipherHeaderPresent() throws Exception {
+        String cipher = "ciphered-with";
+        mockRequest.setHeader(valve.getSslCipherHeader(), cipher);
+
+        valve.invoke(mockRequest, null);
+
+        Assert.assertEquals(cipher, mockRequest.getAttribute(Globals.CIPHER_SUITE_ATTR));
+    }
+
+
+    @Test
+    public void testSslSessionIdHeaderPresent() throws Exception {
+        String session = "ssl-session";
+        mockRequest.setHeader(valve.getSslSessionIdHeader(), session);
+
+        valve.invoke(mockRequest, null);
+
+        Assert.assertEquals(session, mockRequest.getAttribute(Globals.SSL_SESSION_ID_ATTR));
+    }
+
+
+    @Test
+    public void testSslCipherUserKeySizeHeaderPresent() throws Exception {
+        Integer keySize = Integer.valueOf(452);
+        mockRequest.setHeader(valve.getSslCipherUserKeySizeHeader(), String.valueOf(keySize));
+
+        valve.invoke(mockRequest, null);
+
+        Assert.assertEquals(keySize, mockRequest.getAttribute(Globals.KEY_SIZE_ATTR));
+    }
+
+
+    @Test(expected = NumberFormatException.class)
+    public void testSslCipherUserKeySizeHeaderBadFormat() throws Exception {
+        mockRequest.setHeader(valve.getSslCipherUserKeySizeHeader(), "not-an-integer");
+
+        try {
+            valve.invoke(mockRequest, null);
+        } catch (NumberFormatException e) {
+            Assert.assertNull(mockRequest.getAttribute(Globals.KEY_SIZE_ATTR));
+            throw e;
+        }
+    }
+
+
+    private static String certificateSingleLine(String[] lines, String separator) {
+        StringBuilder singleSpaced = new StringBuilder();
+        for (String current : lines) {
+            singleSpaced.append(current).append(separator);
+        }
+        singleSpaced.deleteCharAt(singleSpaced.length() - 1);
+        return singleSpaced.toString();
+    }
+
+
+    private static String certificateSingleLine(String separator) {
+        return certificateSingleLine(CERTIFICATE_LINES, separator);
+    }
+
+
+    private void assertCertificateParsed() throws Exception {
+        TesterLogValidationFilter f = TesterLogValidationFilter.add(null, "", null,
+                "org.apache.catalina.valves.SSLValve");
+
+        EasyMock.verify(mockNext);
+
+        X509Certificate[] certificates = (X509Certificate[]) mockRequest.getAttribute(Globals.CERTIFICATES_ATTR);
+        Assert.assertNotNull(certificates);
+        Assert.assertEquals(1, certificates.length);
+        Assert.assertNotNull(certificates[0]);
+        Assert.assertEquals(0, f.getMessageCount());
+    }
+}
\ No newline at end of file

Propchange: tomcat/trunk/test/org/apache/catalina/valves/TestSSLValve.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1797694&r1=1797693&r2=1797694&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Jun  5 20:10:43 2017
@@ -76,6 +76,11 @@
         required for the correct detection of JSP modifications when the JSP is
         packaged in a WAR file. (markt)
       </fix>
+      <fix>
+        Improve the <code>SSLValve</code> so it is able to handle client
+        certificate headers from Nginx. Based on a patch by Lucas Ventura Carro.
+        (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


Mime
View raw message