[tomcat] branch master updated: https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[tomcat] branch master updated: https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight

Mark Thomas-2
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 e9430e1  https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight
e9430e1 is described below

commit e9430e1db97d9ffc31d4d4431af92f2511d1b950
Author: Mark Thomas <[hidden email]>
AuthorDate: Mon Dec 2 14:01:13 2019 +0000

    https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight
   
    Add a new attribute to the standard Authenticator implementations,
    allowCorsPreflight, that allows the Authenticators to be configured to
    allow CORS preflight requests to bypass authentication as required by
    the CORS specification.
---
 .../catalina/authenticator/AuthenticatorBase.java  |  88 ++++++++++
 java/org/apache/catalina/filters/CorsFilter.java   |  34 +---
 java/org/apache/tomcat/util/http/RequestUtil.java  |  43 +++++
 .../TestAuthenticatorBaseCorsPreflight.java        | 177 +++++++++++++++++++++
 .../apache/catalina/filters/TestCorsFilter.java    |  12 +-
 webapps/docs/changelog.xml                         |   8 +
 webapps/docs/config/valve.xml                      |  93 +++++++++--
 7 files changed, 413 insertions(+), 42 deletions(-)

diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index 76e712b..308b019 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -33,6 +33,7 @@ import javax.security.auth.message.config.AuthConfigProvider;
 import javax.security.auth.message.config.RegistrationListener;
 import javax.security.auth.message.config.ServerAuthConfig;
 import javax.security.auth.message.config.ServerAuthContext;
+import javax.servlet.DispatcherType;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.Cookie;
@@ -53,6 +54,7 @@ import org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl;
 import org.apache.catalina.authenticator.jaspic.MessageInfoImpl;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.connector.Response;
+import org.apache.catalina.filters.CorsFilter;
 import org.apache.catalina.filters.RemoteIpFilter;
 import org.apache.catalina.realm.GenericPrincipal;
 import org.apache.catalina.util.SessionIdGeneratorBase;
@@ -63,9 +65,12 @@ import org.apache.coyote.ActionCode;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.ExceptionUtils;
+import org.apache.tomcat.util.descriptor.web.FilterDef;
+import org.apache.tomcat.util.descriptor.web.FilterMap;
 import org.apache.tomcat.util.descriptor.web.LoginConfig;
 import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
 import org.apache.tomcat.util.http.FastHttpDateFormat;
+import org.apache.tomcat.util.http.RequestUtil;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
@@ -237,12 +242,22 @@ public abstract class AuthenticatorBase extends ValveBase
      */
     protected SingleSignOn sso = null;
 
+    private AllowCorsPreflight allowCorsPreflight = AllowCorsPreflight.NEVER;
+
     private volatile String jaspicAppContextID = null;
     private volatile Optional<AuthConfigProvider> jaspicProvider = null;
 
 
     // ------------------------------------------------------------- Properties
 
+    public String getAllowCorsPreflight() {
+        return allowCorsPreflight.name().toLowerCase();
+    }
+
+    public void setAllowCorsPreflight(String allowCorsPreflight) {
+        this.allowCorsPreflight = AllowCorsPreflight.valueOf(allowCorsPreflight.trim().toUpperCase());
+    }
+
     public boolean getAlwaysUseSession() {
         return alwaysUseSession;
     }
@@ -593,6 +608,14 @@ public abstract class AuthenticatorBase extends ValveBase
 
         JaspicState jaspicState = null;
 
+        if ((authRequired || constraints != null) && allowCorsPreflightBypass(request)) {
+            if (log.isDebugEnabled()) {
+                log.debug(" CORS Preflight request bypassing authentication");
+            }
+            getNext().invoke(request, response);
+            return;
+        }
+
         if (authRequired) {
             if (log.isDebugEnabled()) {
                 log.debug(" Calling authenticate()");
@@ -648,6 +671,64 @@ public abstract class AuthenticatorBase extends ValveBase
     }
 
 
+    protected boolean allowCorsPreflightBypass(Request request) {
+        boolean allowBypass = false;
+
+        if (allowCorsPreflight != AllowCorsPreflight.NEVER) {
+            // First check to see if this is a CORS Preflight request
+            // This is a subset of the tests in CorsFilter.checkRequestType
+            if ("OPTIONS".equals(request.getMethod())) {
+                String originHeader = request.getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
+                if (originHeader != null &&
+                        !originHeader.isEmpty() &&
+                        RequestUtil.isValidOrigin(originHeader) &&
+                        !RequestUtil.isSameOrigin(request, originHeader)) {
+                    String accessControlRequestMethodHeader =
+                            request.getHeader(CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_METHOD);
+                    if (accessControlRequestMethodHeader != null &&
+                            !accessControlRequestMethodHeader.isEmpty()) {
+                        // This appears to be a CORS Preflight request
+                        if (allowCorsPreflight == AllowCorsPreflight.ALWAYS) {
+                            allowBypass = true;
+                        } else if (allowCorsPreflight == AllowCorsPreflight.FILTER) {
+                            if (DispatcherType.REQUEST == request.getDispatcherType()) {
+                                // Look at Filter configuration for the Context
+                                // Can't cache this unless we add a listener to
+                                // the Context to clear the cache on reload
+                                for (FilterDef filterDef : request.getContext().findFilterDefs()) {
+                                    if (CorsFilter.class.getName().equals(filterDef.getFilterClass())) {
+                                        for (FilterMap filterMap : context.findFilterMaps()) {
+                                            if (filterMap.getFilterName().equals(filterDef.getFilterName())) {
+                                                if ((filterMap.getDispatcherMapping() & FilterMap.REQUEST) > 0) {
+                                                    for (String urlPattern : filterMap.getURLPatterns()) {
+                                                        if ("/*".equals(urlPattern)) {
+                                                            allowBypass = true;
+                                                            // No need to check other patterns
+                                                            break;
+                                                        }
+                                                    }
+                                                }
+                                                // Found mappings for CORS filter.
+                                                // No need to look further
+                                                break;
+                                            }
+                                        }
+                                        // Found the CORS filter. No need to look further.
+                                        break;
+                                    }
+                                }
+                            }
+                        } else {
+                            // Unexpected enum type
+                        }
+                    }
+                }
+            }
+        }
+        return allowBypass;
+    }
+
+
     @Override
     public boolean authenticate(Request request, HttpServletResponse httpResponse)
             throws IOException {
@@ -1301,4 +1382,11 @@ public abstract class AuthenticatorBase extends ValveBase
         public MessageInfo messageInfo = null;
         public ServerAuthContext serverAuthContext = null;
     }
+
+
+    protected enum AllowCorsPreflight {
+        NEVER,
+        FILTER,
+        ALWAYS
+    }
 }
diff --git a/java/org/apache/catalina/filters/CorsFilter.java b/java/org/apache/catalina/filters/CorsFilter.java
index 4213fb4..294e904 100644
--- a/java/org/apache/catalina/filters/CorsFilter.java
+++ b/java/org/apache/catalina/filters/CorsFilter.java
@@ -19,7 +19,6 @@ package org.apache.catalina.filters;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -590,7 +589,7 @@ public class CorsFilter extends GenericFilter {
         if (originHeader != null) {
             if (originHeader.isEmpty()) {
                 requestType = CORSRequestType.INVALID_CORS;
-            } else if (!isValidOrigin(originHeader)) {
+            } else if (!RequestUtil.isValidOrigin(originHeader)) {
                 requestType = CORSRequestType.INVALID_CORS;
             } else if (RequestUtil.isSameOrigin(request, originHeader)) {
                 return CORSRequestType.NOT_CORS;
@@ -783,34 +782,13 @@ public class CorsFilter extends GenericFilter {
      * @param origin The origin URI
      * @return <code>true</code> if the origin was valid
      * @see <a href="http://tools.ietf.org/html/rfc952">RFC952</a>
+     *
+     * @deprecated This will be removed in Tomcat 10
+     *             Use {@link RequestUtil#isValidOrigin(String)}
      */
+    @Deprecated
     protected static boolean isValidOrigin(String origin) {
-        // Checks for encoded characters. Helps prevent CRLF injection.
-        if (origin.contains("%")) {
-            return false;
-        }
-
-        // "null" is a valid origin
-        if ("null".equals(origin)) {
-            return true;
-        }
-
-        // RFC6454, section 4. "If uri-scheme is file, the implementation MAY
-        // return an implementation-defined value.". No limits are placed on
-        // that value so treat all file URIs as valid origins.
-        if (origin.startsWith("file://")) {
-            return true;
-        }
-
-        URI originURI;
-        try {
-            originURI = new URI(origin);
-        } catch (URISyntaxException e) {
-            return false;
-        }
-        // If scheme for URI is null, return false. Return true otherwise.
-        return originURI.getScheme() != null;
-
+        return RequestUtil.isValidOrigin(origin);
     }
 
 
diff --git a/java/org/apache/tomcat/util/http/RequestUtil.java b/java/org/apache/tomcat/util/http/RequestUtil.java
index cfa9c57..2edd695 100644
--- a/java/org/apache/tomcat/util/http/RequestUtil.java
+++ b/java/org/apache/tomcat/util/http/RequestUtil.java
@@ -16,6 +16,8 @@
  */
 package org.apache.tomcat.util.http;
 
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.Locale;
 
 import javax.servlet.http.HttpServletRequest;
@@ -164,4 +166,45 @@ public class RequestUtil {
         return origin.equals(target.toString());
     }
 
+
+    /**
+     * Checks if a given origin is valid or not. Criteria:
+     * <ul>
+     * <li>If an encoded character is present in origin, it's not valid.</li>
+     * <li>If origin is "null", it's valid.</li>
+     * <li>Origin should be a valid {@link URI}</li>
+     * </ul>
+     *
+     * @param origin The origin URI
+     * @return <code>true</code> if the origin was valid
+     * @see <a href="http://tools.ietf.org/html/rfc952">RFC952</a>
+     */
+    public static boolean isValidOrigin(String origin) {
+        // Checks for encoded characters. Helps prevent CRLF injection.
+        if (origin.contains("%")) {
+            return false;
+        }
+
+        // "null" is a valid origin
+        if ("null".equals(origin)) {
+            return true;
+        }
+
+        // RFC6454, section 4. "If uri-scheme is file, the implementation MAY
+        // return an implementation-defined value.". No limits are placed on
+        // that value so treat all file URIs as valid origins.
+        if (origin.startsWith("file://")) {
+            return true;
+        }
+
+        URI originURI;
+        try {
+            originURI = new URI(origin);
+        } catch (URISyntaxException e) {
+            return false;
+        }
+        // If scheme for URI is null, return false. Return true otherwise.
+        return originURI.getScheme() != null;
+
+    }
 }
diff --git a/test/org/apache/catalina/authenticator/TestAuthenticatorBaseCorsPreflight.java b/test/org/apache/catalina/authenticator/TestAuthenticatorBaseCorsPreflight.java
new file mode 100644
index 0000000..b0a68dd
--- /dev/null
+++ b/test/org/apache/catalina/authenticator/TestAuthenticatorBaseCorsPreflight.java
@@ -0,0 +1,177 @@
+/*
+ * 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.authenticator;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.Realm;
+import org.apache.catalina.authenticator.AuthenticatorBase.AllowCorsPreflight;
+import org.apache.catalina.filters.AddDefaultCharsetFilter;
+import org.apache.catalina.filters.CorsFilter;
+import org.apache.catalina.realm.NullRealm;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.descriptor.web.FilterDef;
+import org.apache.tomcat.util.descriptor.web.FilterMap;
+import org.apache.tomcat.util.descriptor.web.LoginConfig;
+import org.apache.tomcat.util.descriptor.web.SecurityCollection;
+import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
+
+@RunWith(Parameterized.class)
+public class TestAuthenticatorBaseCorsPreflight extends TomcatBaseTest {
+
+    private static final String ALLOWED_ORIGIN = "http://example.com";
+    private static final String EMPTY_ORIGIN = "";
+    private static final String INVALID_ORIGIN = "http://%20";
+    private static final String SAME_ORIGIN = "http://localhost";
+    private static final String ALLOWED_METHOD = "GET";
+    private static final String BLOCKED_METHOD = "POST";
+    private static final String EMPTY_METHOD = "";
+
+    @Parameterized.Parameters(name = "{index}: input[{0}]")
+    public static Collection<Object[]> parameters() {
+        List<Object[]> parameterSets = new ArrayList<>();
+
+        parameterSets.add(new Object[] { AllowCorsPreflight.NEVER,  "/*", "OPTIONS", null,           null,           Boolean.FALSE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", null,           null,           Boolean.FALSE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", ALLOWED_ORIGIN, ALLOWED_METHOD, Boolean.TRUE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", EMPTY_ORIGIN,   ALLOWED_METHOD, Boolean.FALSE});
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", INVALID_ORIGIN, ALLOWED_METHOD, Boolean.FALSE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", SAME_ORIGIN,    ALLOWED_METHOD, Boolean.FALSE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "GET",     ALLOWED_ORIGIN, ALLOWED_METHOD, Boolean.FALSE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", ALLOWED_ORIGIN, BLOCKED_METHOD, Boolean.FALSE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", ALLOWED_ORIGIN, EMPTY_METHOD,   Boolean.FALSE});
+        parameterSets.add(new Object[] { AllowCorsPreflight.ALWAYS, "/*", "OPTIONS", ALLOWED_ORIGIN, null,           Boolean.FALSE});
+        parameterSets.add(new Object[] { AllowCorsPreflight.FILTER, "/*", "OPTIONS", ALLOWED_ORIGIN, ALLOWED_METHOD, Boolean.TRUE });
+        parameterSets.add(new Object[] { AllowCorsPreflight.FILTER, "/x", "OPTIONS", ALLOWED_ORIGIN, ALLOWED_METHOD, Boolean.FALSE });
+
+        return parameterSets;
+    }
+
+    @Parameter(0)
+    public AllowCorsPreflight allowCorsPreflight;
+    @Parameter(1)
+    public String filterMapping;
+    @Parameter(2)
+    public String method;
+    @Parameter(3)
+    public String origin;
+    @Parameter(4)
+    public String accessControl;
+    @Parameter(5)
+    public boolean allow;
+
+
+    @BeforeClass
+    public static void init() {
+        // So the test can set the origin header
+        System.setProperty("sun.net.http.allowRestrictedHeaders", "true");
+    }
+
+
+    @Test
+    public void test() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp");
+        Context ctx = tomcat.addContext("", appDir.getAbsolutePath());
+
+        Tomcat.addServlet(ctx, "default", new DefaultServlet());
+        ctx.addServletMappingDecoded("/", "default");
+
+        LoginConfig loginConfig  = new LoginConfig();
+        loginConfig.setAuthMethod("BASIC");
+        ctx.setLoginConfig(loginConfig);
+
+        BasicAuthenticator basicAuth = new BasicAuthenticator();
+        basicAuth.setAllowCorsPreflight(allowCorsPreflight.toString());
+        ctx.getPipeline().addValve(basicAuth);
+
+        Realm realm = new NullRealm();
+        ctx.setRealm(realm);
+
+        SecurityCollection securityCollection = new SecurityCollection();
+        securityCollection.addPattern("/*");
+        SecurityConstraint constraint = new SecurityConstraint();
+        constraint.setAuthConstraint(true);
+        constraint.addCollection(securityCollection);
+        ctx.addConstraint(constraint);
+
+        // For code coverage
+        FilterDef otherFilter = new FilterDef();
+        otherFilter.setFilterName("other");
+        otherFilter.setFilterClass(AddDefaultCharsetFilter.class.getName());
+        FilterMap otherMap = new FilterMap();
+        otherMap.setFilterName("other");
+        otherMap.addURLPatternDecoded("/other");
+        ctx.addFilterDef(otherFilter);
+        ctx.addFilterMap(otherMap);
+
+        FilterDef corsFilter = new FilterDef();
+        corsFilter.setFilterName("cors");
+        corsFilter.setFilterClass(CorsFilter.class.getName());
+        corsFilter.addInitParameter(CorsFilter.PARAM_CORS_ALLOWED_ORIGINS, ALLOWED_ORIGIN);
+        corsFilter.addInitParameter(CorsFilter.PARAM_CORS_ALLOWED_METHODS, ALLOWED_METHOD);
+        FilterMap corsFilterMap = new FilterMap();
+        corsFilterMap.setFilterName("cors");
+        corsFilterMap.addURLPatternDecoded(filterMapping);
+        ctx.addFilterDef(corsFilter);
+        ctx.addFilterMap(corsFilterMap);
+
+        tomcat.start();
+
+        Map<String,List<String>> reqHead = new HashMap<>();
+        if (origin != null) {
+            List<String> values = new ArrayList<>();
+            if (SAME_ORIGIN.equals(origin)) {
+                values.add(origin + ":" + getPort());
+            } else {
+                values.add(origin);
+            }
+            reqHead.put(CorsFilter.REQUEST_HEADER_ORIGIN, values);
+        }
+        if (accessControl != null) {
+            List<String> values = new ArrayList<>();
+            values.add(accessControl);
+            reqHead.put(CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_METHOD, values);
+        }
+
+        ByteChunk out = new ByteChunk();
+        int rc = methodUrl("http://localhost:" + getPort() + "/target", out, 300000, reqHead, null, method, false);
+
+        if (allow) {
+            Assert.assertEquals(200, rc);
+        } else {
+            Assert.assertEquals(403, rc);
+        }
+    }
+}
diff --git a/test/org/apache/catalina/filters/TestCorsFilter.java b/test/org/apache/catalina/filters/TestCorsFilter.java
index 6fc6dc1..617c5df 100644
--- a/test/org/apache/catalina/filters/TestCorsFilter.java
+++ b/test/org/apache/catalina/filters/TestCorsFilter.java
@@ -32,6 +32,8 @@ import javax.servlet.http.HttpServletResponse;
 import org.junit.Assert;
 import org.junit.Test;
 
+import org.apache.tomcat.util.http.RequestUtil;
+
 public class TestCorsFilter {
     private FilterChain filterChain = new TesterFilterChain();
 
@@ -1425,27 +1427,27 @@ public class TestCorsFilter {
 
     @Test
     public void testValidOrigin() {
-        Assert.assertTrue(CorsFilter.isValidOrigin("http://www.w3.org"));
+        Assert.assertTrue(RequestUtil.isValidOrigin("http://www.w3.org"));
     }
 
     @Test
     public void testInValidOriginCRLF() {
-        Assert.assertFalse(CorsFilter.isValidOrigin("http://www.w3.org\r\n"));
+        Assert.assertFalse(RequestUtil.isValidOrigin("http://www.w3.org\r\n"));
     }
 
     @Test
     public void testInValidOriginEncodedCRLF1() {
-        Assert.assertFalse(CorsFilter.isValidOrigin("http://www.w3.org%0d%0a"));
+        Assert.assertFalse(RequestUtil.isValidOrigin("http://www.w3.org%0d%0a"));
     }
 
     @Test
     public void testInValidOriginEncodedCRLF2() {
-        Assert.assertFalse(CorsFilter.isValidOrigin("http://www.w3.org%0D%0A"));
+        Assert.assertFalse(RequestUtil.isValidOrigin("http://www.w3.org%0D%0A"));
     }
 
     @Test
     public void testInValidOriginEncodedCRLF3() {
-        Assert.assertFalse(CorsFilter
+        Assert.assertFalse(RequestUtil
                 .isValidOrigin("<a href="http://www.w3.org%0%0d%0ad%0%0d%0aa">http://www.w3.org%0%0d%0ad%0%0d%0aa"));
     }
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 359f9a4..c7eb551 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -61,6 +61,14 @@
         changes introduced in 9.0.28. Connections to URLs obtained for JAR
         resources could not be cast to <code>JarURLConnection</code>. (markt)
       </fix>
+      <add>
+        <bug>63937</bug>: Add a new attribute to the standard
+        <code>Authenticator</code> implementations,
+        <code>allowCorsPreflight</code>, that allows the
+        <code>Authenticator</code>s to be configured to allow CORS preflight
+        requests to bypass authentication as required by the CORS specification.
+        (markt)
+      </add>
       <fix>
         <bug>63939</bug>: Correct the same origin check in the CORS filter. An
         origin with an explicit default port is now considered to be the same as
diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml
index 72f0584..ca32e37 100644
--- a/webapps/docs/config/valve.xml
+++ b/webapps/docs/config/valve.xml
@@ -1201,6 +1201,21 @@
 
     <attributes>
 
+      <attribute name="allowCorsPreflight" required="false">
+        <p>Are requests that appear to be CORS preflight requests allowed to
+        bypass the authenticator as required by the CORS specification. The
+        allowed values are <code>never</code>, <code>filter</code> and
+        <code>always</code>. <code>never</code> means that a request will never
+        bypass authentication even if it appears to be a CORS preflight request.
+        <code>filter</code> means that a request will bypass authentication if
+        it appears to be a CORS preflight request and the web application the
+        request maps to has the <a href="filter.html#CORS_Filter">CORS
+        Filter</a> enabled and mapped to <code>/*</code>. <code>always</code>
+        means that all requests that appear to be CORS preflight requests will
+        bypass authentication. If not set, the default value is
+        <code>never</code>.</p>
+      </attribute>
+
       <attribute name="alwaysUseSession" required="false">
         <p>Should a session always be used once a user is authenticated? This
         may offer some performance benefits since the session can then be used
@@ -1344,6 +1359,21 @@
 
     <attributes>
 
+      <attribute name="allowCorsPreflight" required="false">
+        <p>Are requests that appear to be CORS preflight requests allowed to
+        bypass the authenticator as required by the CORS specification. The
+        allowed values are <code>never</code>, <code>filter</code> and
+        <code>always</code>. <code>never</code> means that a request will never
+        bypass authentication even if it appears to be a CORS preflight request.
+        <code>filter</code> means that a request will bypass authentication if
+        it appears to be a CORS preflight request and the web application the
+        request maps to has the <a href="filter.html#CORS_Filter">CORS
+        Filter</a> enabled and mapped to <code>/*</code>. <code>always</code>
+        means that all requests that appear to be CORS preflight requests will
+        bypass authentication. If not set, the default value is
+        <code>never</code>.</p>
+      </attribute>
+
       <attribute name="alwaysUseSession" required="false">
         <p>Should a session always be used once a user is authenticated? This
         may offer some performance benefits since the session can then be used
@@ -1511,6 +1541,21 @@
 
     <attributes>
 
+      <attribute name="allowCorsPreflight" required="false">
+        <p>Are requests that appear to be CORS preflight requests allowed to
+        bypass the authenticator as required by the CORS specification. The
+        allowed values are <code>never</code>, <code>filter</code> and
+        <code>always</code>. <code>never</code> means that a request will never
+        bypass authentication even if it appears to be a CORS preflight request.
+        <code>filter</code> means that a request will bypass authentication if
+        it appears to be a CORS preflight request and the web application the
+        request maps to has the <a href="filter.html#CORS_Filter">CORS
+        Filter</a> enabled and mapped to <code>/*</code>. <code>always</code>
+        means that all requests that appear to be CORS preflight requests will
+        bypass authentication. If not set, the default value is
+        <code>never</code>.</p>
+      </attribute>
+
       <attribute name="changeSessionIdOnAuthentication" required="false">
         <p>Controls if the session ID is changed if a session exists at the
         point where users are authenticated. This is to prevent session fixation
@@ -1637,6 +1682,21 @@
 
     <attributes>
 
+      <attribute name="allowCorsPreflight" required="false">
+        <p>Are requests that appear to be CORS preflight requests allowed to
+        bypass the authenticator as required by the CORS specification. The
+        allowed values are <code>never</code>, <code>filter</code> and
+        <code>always</code>. <code>never</code> means that a request will never
+        bypass authentication even if it appears to be a CORS preflight request.
+        <code>filter</code> means that a request will bypass authentication if
+        it appears to be a CORS preflight request and the web application the
+        request maps to has the <a href="filter.html#CORS_Filter">CORS
+        Filter</a> enabled and mapped to <code>/*</code>. <code>always</code>
+        means that all requests that appear to be CORS preflight requests will
+        bypass authentication. If not set, the default value is
+        <code>never</code>.</p>
+      </attribute>
+
       <attribute name="cache" required="false">
         <p>Should we cache authenticated Principals if the request is part of an
         HTTP session? If not specified, the default value of <code>true</code>
@@ -1737,15 +1797,19 @@
 
     <attributes>
 
-      <attribute name="applyJava8u40Fix" required="false">
-        <p>A fix introduced in Java 8 update 40 (
-        <a href="https://bugs.openjdk.java.net/browse/JDK-8048194">JDK-8048194</a>)
-        onwards broke SPNEGO authentication for IE with Tomcat running on
-        Windows 2008 R2 servers. This option enables a work-around that allows
-        SPNEGO authentication to continue working. The work-around should not
-        impact other configurations so it is enabled by default. If necessary,
-        the workaround can be disabled by setting this attribute to
-        <code>false</code>.</p>
+      <attribute name="allowCorsPreflight" required="false">
+        <p>Are requests that appear to be CORS preflight requests allowed to
+        bypass the authenticator as required by the CORS specification. The
+        allowed values are <code>never</code>, <code>filter</code> and
+        <code>always</code>. <code>never</code> means that a request will never
+        bypass authentication even if it appears to be a CORS preflight request.
+        <code>filter</code> means that a request will bypass authentication if
+        it appears to be a CORS preflight request and the web application the
+        request maps to has the <a href="filter.html#CORS_Filter">CORS
+        Filter</a> enabled and mapped to <code>/*</code>. <code>always</code>
+        means that all requests that appear to be CORS preflight requests will
+        bypass authentication. If not set, the default value is
+        <code>never</code>.</p>
       </attribute>
 
       <attribute name="alwaysUseSession" required="false">
@@ -1760,6 +1824,17 @@
         <code>false</code> will be used.</p>
       </attribute>
 
+      <attribute name="applyJava8u40Fix" required="false">
+        <p>A fix introduced in Java 8 update 40 (
+        <a href="https://bugs.openjdk.java.net/browse/JDK-8048194">JDK-8048194</a>)
+        onwards broke SPNEGO authentication for IE with Tomcat running on
+        Windows 2008 R2 servers. This option enables a work-around that allows
+        SPNEGO authentication to continue working. The work-around should not
+        impact other configurations so it is enabled by default. If necessary,
+        the workaround can be disabled by setting this attribute to
+        <code>false</code>.</p>
+      </attribute>
+
       <attribute name="cache" required="false">
         <p>Should we cache authenticated Principals if the request is part of an
         HTTP session? If not specified, the default value of <code>true</code>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight

Michael Osipov
Am 2019-12-02 um 18:51 schrieb [hidden email]:

> 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 e9430e1  https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight
> e9430e1 is described below
>
> commit e9430e1db97d9ffc31d4d4431af92f2511d1b950
> Author: Mark Thomas <[hidden email]>
> AuthorDate: Mon Dec 2 14:01:13 2019 +0000
>
>      https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight
>      
>      Add a new attribute to the standard Authenticator implementations,
>      allowCorsPreflight, that allows the Authenticators to be configured to
>      allow CORS preflight requests to bypass authentication as required by
>      the CORS specification.
> ---
>   .../catalina/authenticator/AuthenticatorBase.java  |  88 ++++++++++
>   java/org/apache/catalina/filters/CorsFilter.java   |  34 +---
>   java/org/apache/tomcat/util/http/RequestUtil.java  |  43 +++++
>   .../TestAuthenticatorBaseCorsPreflight.java        | 177 +++++++++++++++++++++
>   .../apache/catalina/filters/TestCorsFilter.java    |  12 +-
>   webapps/docs/changelog.xml                         |   8 +
>   webapps/docs/config/valve.xml                      |  93 +++++++++--
>   7 files changed, 413 insertions(+), 42 deletions(-)
>
> diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
> index 76e712b..308b019 100644
> --- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
> +++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
> @@ -33,6 +33,7 @@ import javax.security.auth.message.config.AuthConfigProvider;
>   import javax.security.auth.message.config.RegistrationListener;
>   import javax.security.auth.message.config.ServerAuthConfig;
>   import javax.security.auth.message.config.ServerAuthContext;
> +import javax.servlet.DispatcherType;
>   import javax.servlet.ServletContext;
>   import javax.servlet.ServletException;
>   import javax.servlet.http.Cookie;
> @@ -53,6 +54,7 @@ import org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl;
>   import org.apache.catalina.authenticator.jaspic.MessageInfoImpl;
>   import org.apache.catalina.connector.Request;
>   import org.apache.catalina.connector.Response;
> +import org.apache.catalina.filters.CorsFilter;
>   import org.apache.catalina.filters.RemoteIpFilter;
>   import org.apache.catalina.realm.GenericPrincipal;
>   import org.apache.catalina.util.SessionIdGeneratorBase;
> @@ -63,9 +65,12 @@ import org.apache.coyote.ActionCode;
>   import org.apache.juli.logging.Log;
>   import org.apache.juli.logging.LogFactory;
>   import org.apache.tomcat.util.ExceptionUtils;
> +import org.apache.tomcat.util.descriptor.web.FilterDef;
> +import org.apache.tomcat.util.descriptor.web.FilterMap;
>   import org.apache.tomcat.util.descriptor.web.LoginConfig;
>   import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
>   import org.apache.tomcat.util.http.FastHttpDateFormat;
> +import org.apache.tomcat.util.http.RequestUtil;
>   import org.apache.tomcat.util.res.StringManager;
>  
>   /**
> @@ -237,12 +242,22 @@ public abstract class AuthenticatorBase extends ValveBase
>        */
>       protected SingleSignOn sso = null;
>  
> +    private AllowCorsPreflight allowCorsPreflight = AllowCorsPreflight.NEVER;
> +
>       private volatile String jaspicAppContextID = null;
>       private volatile Optional<AuthConfigProvider> jaspicProvider = null;
>  
>  
>       // ------------------------------------------------------------- Properties
>  
> +    public String getAllowCorsPreflight() {
> +        return allowCorsPreflight.name().toLowerCase();
> +    }
> +
> +    public void setAllowCorsPreflight(String allowCorsPreflight) {
> +        this.allowCorsPreflight = AllowCorsPreflight.valueOf(allowCorsPreflight.trim().toUpperCase());
> +    }
> +

toLowerCase() and toUpperCase() should be locale-agnostic. I bet FILTER
will fail with tr_TR.

>       public boolean getAlwaysUseSession() {
>           return alwaysUseSession;
>       }
> @@ -593,6 +608,14 @@ public abstract class AuthenticatorBase extends ValveBase
>  
>           JaspicState jaspicState = null;
>  
> +        if ((authRequired || constraints != null) && allowCorsPreflightBypass(request)) {
> +            if (log.isDebugEnabled()) {
> +                log.debug(" CORS Preflight request bypassing authentication");
                              ^^
                              Is that space intended?

> +            }
> +            getNext().invoke(request, response);
> +            return;
> +        }
> +
>           if (authRequired) {
>               if (log.isDebugEnabled()) {
>                   log.debug(" Calling authenticate()");
> @@ -648,6 +671,64 @@ public abstract class AuthenticatorBase extends ValveBase
>       }
>  
>  
> +    protected boolean allowCorsPreflightBypass(Request request) {
> +        boolean allowBypass = false;
> +
> +        if (allowCorsPreflight != AllowCorsPreflight.NEVER) {
> +            // First check to see if this is a CORS Preflight request
> +            // This is a subset of the tests in CorsFilter.checkRequestType
> +            if ("OPTIONS".equals(request.getMethod())) {
> +                String originHeader = request.getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
> +                if (originHeader != null &&
> +                        !originHeader.isEmpty() &&
> +                        RequestUtil.isValidOrigin(originHeader) &&
> +                        !RequestUtil.isSameOrigin(request, originHeader)) {
> +                    String accessControlRequestMethodHeader =
> +                            request.getHeader(CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_METHOD);
> +                    if (accessControlRequestMethodHeader != null &&
> +                            !accessControlRequestMethodHeader.isEmpty()) {
> +                        // This appears to be a CORS Preflight request
> +                        if (allowCorsPreflight == AllowCorsPreflight.ALWAYS) {
> +                            allowBypass = true;
> +                        } else if (allowCorsPreflight == AllowCorsPreflight.FILTER) {
> +                            if (DispatcherType.REQUEST == request.getDispatcherType()) {
> +                                // Look at Filter configuration for the Context
> +                                // Can't cache this unless we add a listener to
> +                                // the Context to clear the cache on reload
> +                                for (FilterDef filterDef : request.getContext().findFilterDefs()) {
> +                                    if (CorsFilter.class.getName().equals(filterDef.getFilterClass())) {
> +                                        for (FilterMap filterMap : context.findFilterMaps()) {
> +                                            if (filterMap.getFilterName().equals(filterDef.getFilterName())) {
> +                                                if ((filterMap.getDispatcherMapping() & FilterMap.REQUEST) > 0) {
> +                                                    for (String urlPattern : filterMap.getURLPatterns()) {
> +                                                        if ("/*".equals(urlPattern)) {
So basically, if I have applied the CorsFilter to "/api/* it will
evaluate to false?! This is why I brought up BZ 63938.
You see no other way to make it an exact match a not blanket?

> +                                                            allowBypass = true;
> +                                                            // No need to check other patterns
> +                                                            break;
> +                                                        }
> +                                                    }
> +                                                }
> +                                                // Found mappings for CORS filter.
> +                                                // No need to look further
> +                                                break;
> +                                            }
> +                                        }
> +                                        // Found the CORS filter. No need to look further.
> +                                        break;
> +                                    }
> +                                }
> +                            }
> +                        } else {
> +                            // Unexpected enum type
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +        return allowBypass;
> +    }
> +
> +
>       @Override
>       public boolean authenticate(Request request, HttpServletResponse httpResponse)
>               throws IOException {
> @@ -1301,4 +1382,11 @@ public abstract class AuthenticatorBase extends ValveBase
>           public MessageInfo messageInfo = null;
>           public ServerAuthContext serverAuthContext = null;
>       }
> +
> +
> +    protected enum AllowCorsPreflight {
> +        NEVER,
> +        FILTER,
> +        ALWAYS
> +    }
>   }
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 359f9a4..c7eb551 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -61,6 +61,14 @@
>           changes introduced in 9.0.28. Connections to URLs obtained for JAR
>           resources could not be cast to <code>JarURLConnection</code>. (markt)
>         </fix>
> +      <add>
> +        <bug>63937</bug>: Add a new attribute to the standard
> +        <code>Authenticator</code> implementations,
> +        <code>allowCorsPreflight</code>, that allows the
> +        <code>Authenticator</code>s to be configured to allow CORS preflight
> +        requests to bypass authentication as required by the CORS specification.
> +        (markt)
> +      </add>
>         <fix>
>           <bug>63939</bug>: Correct the same origin check in the CORS filter. An
>           origin with an explicit default port is now considered to be the same as
> diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml
> index 72f0584..ca32e37 100644
> --- a/webapps/docs/config/valve.xml
> +++ b/webapps/docs/config/valve.xml
> @@ -1201,6 +1201,21 @@
>  
>       <attributes>
>  
> +      <attribute name="allowCorsPreflight" required="false">
> +        <p>Are requests that appear to be CORS preflight requests allowed to
> +        bypass the authenticator as required by the CORS specification. The
> +        allowed values are <code>never</code>, <code>filter</code> and
> +        <code>always</code>. <code>never</code> means that a request will never
> +        bypass authentication even if it appears to be a CORS preflight request.
> +        <code>filter</code> means that a request will bypass authentication if
> +        it appears to be a CORS preflight request and the web application the
> +        request maps to has the <a href="filter.html#CORS_Filter">CORS

I have the feeling that some word is either wrong or missing here: ...
and the web application the request maps ...

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight

Mark Thomas-2
On 02/12/2019 19:48, Michael Osipov wrote:
> Am 2019-12-02 um 18:51 schrieb [hidden email]:

<snip/>

>> ------------------------------------------------------------- Properties
>>   +    public String getAllowCorsPreflight() {
>> +        return allowCorsPreflight.name().toLowerCase();
>> +    }
>> +
>> +    public void setAllowCorsPreflight(String allowCorsPreflight) {
>> +        this.allowCorsPreflight =
>> AllowCorsPreflight.valueOf(allowCorsPreflight.trim().toUpperCase());
>> +    }
>> +
>
> toLowerCase() and toUpperCase() should be locale-agnostic.

They should be forced to ENGLISH. I'll get that fixed.

>> +            if (log.isDebugEnabled()) {
>> +                log.debug(" CORS Preflight request bypassing
>> authentication");
>                              ^^
>                              Is that space intended?

Yes. For consistency with surrounding messages. The file is not
consistent though. Removal of all leading spaces in debug messages is
probably the right choice here as that would be consistent with the
majority of debug messages in Tomcat.


>> +                                                        if
>> ("/*".equals(urlPattern)) {
> So basically, if I have applied the CorsFilter to "/api/* it will
> evaluate to false?!

Correct - if you are using "filter". If you use "always" it won't reach
those tests.

> This is why I brought up BZ 63938.
> You see no other way to make it an exact match a not blanket?

Not easily, no. You'd essentially have to recreate large chunks of
ApplicationFilterFactory.

>> --- a/webapps/docs/config/valve.xml
>> +++ b/webapps/docs/config/valve.xml
>> @@ -1201,6 +1201,21 @@
>>         <attributes>
>>   +      <attribute name="allowCorsPreflight" required="false">
>> +        <p>Are requests that appear to be CORS preflight requests
>> allowed to
>> +        bypass the authenticator as required by the CORS
>> specification. The
>> +        allowed values are <code>never</code>, <code>filter</code> and
>> +        <code>always</code>. <code>never</code> means that a request
>> will never
>> +        bypass authentication even if it appears to be a CORS
>> preflight request.
>> +        <code>filter</code> means that a request will bypass
>> authentication if
>> +        it appears to be a CORS preflight request and the web
>> application the
>> +        request maps to has the <a href="filter.html#CORS_Filter">CORS
>
> I have the feeling that some word is either wrong or missing here: ...
> and the web application the request maps ...

Looks fine to me.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: https://bz.apache.org/bugzilla/show_bug.cgi?id=63937 allowCorsPreflight

Michael Osipov
Am 2019-12-02 um 21:07 schrieb Mark Thomas:
> On 02/12/2019 19:48, Michael Osipov wrote:
>> Am 2019-12-02 um 18:51 schrieb [hidden email]:
 >
>> This is why I brought up BZ 63938.
>> You see no other way to make it an exact match a not blanket?
>
> Not easily, no. You'd essentially have to recreate large chunks of
> ApplicationFilterFactory.

Agreed to the tradeoff, I'd rather would see BZ 63938 fixed in that spirit.

>>> --- a/webapps/docs/config/valve.xml
>>> +++ b/webapps/docs/config/valve.xml
>>> @@ -1201,6 +1201,21 @@
>>>          <attributes>
>>>    +      <attribute name="allowCorsPreflight" required="false">
>>> +        <p>Are requests that appear to be CORS preflight requests
>>> allowed to
>>> +        bypass the authenticator as required by the CORS
>>> specification. The
>>> +        allowed values are <code>never</code>, <code>filter</code> and
>>> +        <code>always</code>. <code>never</code> means that a request
>>> will never
>>> +        bypass authentication even if it appears to be a CORS
>>> preflight request.
>>> +        <code>filter</code> means that a request will bypass
>>> authentication if
>>> +        it appears to be a CORS preflight request and the web
>>> application the
>>> +        request maps to has the <a href="filter.html#CORS_Filter">CORS
>>
>> I have the feeling that some word is either wrong or missing here: ...
>> and the web application the request maps ...
>
> Looks fine to me.

Frankly, as a non-English native speaker I do not understand the
sentence. Maybe others won't too.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]