From dev-return-203913-archive-asf-public=cust-asf.ponee.io@tomcat.apache.org Sat Nov 30 12:08:28 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 5E6B0180638 for ; Sat, 30 Nov 2019 13:08:28 +0100 (CET) Received: (qmail 52244 invoked by uid 500); 30 Nov 2019 12:08:25 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 52085 invoked by uid 99); 30 Nov 2019 12:08:24 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 30 Nov 2019 12:08:24 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 5ACDD8B692; Sat, 30 Nov 2019 12:08:24 +0000 (UTC) Date: Sat, 30 Nov 2019 12:08:25 +0000 To: "dev@tomcat.apache.org" Subject: [tomcat] 02/03: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63939 same origin MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: markt@apache.org In-Reply-To: <157511570359.6559.4149223517350162253@gitbox.apache.org> References: <157511570359.6559.4149223517350162253@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: tomcat X-Git-Refname: refs/heads/7.0.x X-Git-Reftype: branch X-Git-Rev: dd818eb9667a9a55eb81374202a690c4568ff9e8 X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20191130120824.5ACDD8B692@gitbox.apache.org> This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 7.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git commit dd818eb9667a9a55eb81374202a690c4568ff9e8 Author: Mark Thomas AuthorDate: Fri Nov 29 23:19:00 2019 +0000 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63939 same origin Refactor isSameOrigin test into utility class (for re-use in AuthenticatorBase) and fix two bugs --- java/org/apache/catalina/filters/CorsFilter.java | 33 +----- java/org/apache/tomcat/util/http/RequestUtil.java | 51 ++++++++++ .../tomcat/util/http/TestRequestUtilNormalize.java | 2 +- .../util/http/TestRequestUtilSameOrigin.java | 113 +++++++++++++++++++++ 4 files changed, 167 insertions(+), 32 deletions(-) diff --git a/java/org/apache/catalina/filters/CorsFilter.java b/java/org/apache/catalina/filters/CorsFilter.java index 308c1b4..4f107ce 100644 --- a/java/org/apache/catalina/filters/CorsFilter.java +++ b/java/org/apache/catalina/filters/CorsFilter.java @@ -38,6 +38,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.http.RequestUtil; import org.apache.tomcat.util.http.ResponseUtil; import org.apache.tomcat.util.res.StringManager; @@ -641,7 +642,7 @@ public class CorsFilter implements Filter { requestType = CORSRequestType.INVALID_CORS; } else if (!isValidOrigin(originHeader)) { requestType = CORSRequestType.INVALID_CORS; - } else if (isLocalOrigin(request, originHeader)) { + } else if (RequestUtil.isSameOrigin(request, originHeader)) { return CORSRequestType.NOT_CORS; } else { String method = request.getMethod(); @@ -684,36 +685,6 @@ public class CorsFilter implements Filter { } - private boolean isLocalOrigin(HttpServletRequest request, String origin) { - - // Build scheme://host:port from request - StringBuilder target = new StringBuilder(); - String scheme = request.getScheme(); - if (scheme == null) { - return false; - } else { - scheme = scheme.toLowerCase(Locale.ENGLISH); - } - target.append(scheme); - target.append("://"); - - String host = request.getServerName(); - if (host == null) { - return false; - } - target.append(host); - - int port = request.getServerPort(); - if ("http".equals(scheme) && port != 80 || - "https".equals(scheme) && port != 443) { - target.append(':'); - target.append(port); - } - - return origin.equalsIgnoreCase(target.toString()); - } - - /* * Return the lower case, trimmed value of the media type from the content * type. diff --git a/java/org/apache/tomcat/util/http/RequestUtil.java b/java/org/apache/tomcat/util/http/RequestUtil.java index 28922c4..cfa9c57 100644 --- a/java/org/apache/tomcat/util/http/RequestUtil.java +++ b/java/org/apache/tomcat/util/http/RequestUtil.java @@ -16,6 +16,10 @@ */ package org.apache.tomcat.util.http; +import java.util.Locale; + +import javax.servlet.http.HttpServletRequest; + public class RequestUtil { private RequestUtil() { @@ -113,4 +117,51 @@ public class RequestUtil { // Return the normalized path that we have completed return normalized; } + + + public static boolean isSameOrigin(HttpServletRequest request, String origin) { + // Build scheme://host:port from request + StringBuilder target = new StringBuilder(); + String scheme = request.getScheme(); + if (scheme == null) { + return false; + } else { + scheme = scheme.toLowerCase(Locale.ENGLISH); + } + target.append(scheme); + target.append("://"); + + String host = request.getServerName(); + if (host == null) { + return false; + } + target.append(host); + + int port = request.getServerPort(); + // Origin may or may not include the (default) port. + // At this point target doesn't include a port. + if (target.length() == origin.length()) { + // origin and target can only be equal if both are using default + // ports. Therefore only append the port to the target if a + // non-default port is used. + if (("http".equals(scheme) || "ws".equals(scheme)) && port != 80 || + ("https".equals(scheme) || "wss".equals(scheme)) && port != 443) { + target.append(':'); + target.append(port); + } + } else { + // origin and target can only be equal if: + // a) origin includes an explicit default port + // b) origin is using a non-default port + // Either way, add the port to the target so it can be compared + target.append(':'); + target.append(port); + } + + + // Both scheme and host are case-insensitive but the CORS spec states + // this check should be case-sensitive + return origin.equals(target.toString()); + } + } diff --git a/test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java b/test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java index b642868..ab7ebaf 100644 --- a/test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java +++ b/test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java @@ -31,7 +31,7 @@ public class TestRequestUtilNormalize { @Parameterized.Parameters(name = "{index}: input[{0}]") public static Collection parameters() { - List parameterSets = new ArrayList<>(); + List parameterSets = new ArrayList(); parameterSets.add(new String[] { "//something", "/something" }); parameterSets.add(new String[] { "some//thing", "/some/thing" }); diff --git a/test/org/apache/tomcat/util/http/TestRequestUtilSameOrigin.java b/test/org/apache/tomcat/util/http/TestRequestUtilSameOrigin.java new file mode 100644 index 0000000..42de243 --- /dev/null +++ b/test/org/apache/tomcat/util/http/TestRequestUtilSameOrigin.java @@ -0,0 +1,113 @@ +/* + * 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.tomcat.util.http; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; + +import org.apache.catalina.connector.Request; + +@RunWith(Parameterized.class) +public class TestRequestUtilSameOrigin { + + @Parameterized.Parameters(name = "{index}: request[{0}], origin[{1}]") + public static Collection parameters() { + List parameterSets = new ArrayList(); + + TesterRequest request1 = new TesterRequest("http", "example.com", 80); + TesterRequest request2 = new TesterRequest("ws", "example.com", 80); + TesterRequest request3 = new TesterRequest("http", "example.com", 443); + TesterRequest request4 = new TesterRequest("http", "example.com", 8080); + + parameterSets.add(new Object[] { request1, "http://example.com", Boolean.TRUE }); + parameterSets.add(new Object[] { request1, "http://example.com:80", Boolean.TRUE }); + parameterSets.add(new Object[] { request1, "http://example.com:8080", Boolean.FALSE}); + + parameterSets.add(new Object[] { request2, "ws://example.com", Boolean.TRUE }); + parameterSets.add(new Object[] { request2, "ws://example.com:80", Boolean.TRUE }); + parameterSets.add(new Object[] { request2, "ws://example.com:8080", Boolean.FALSE}); + + parameterSets.add(new Object[] { request3, "http://example.com", Boolean.FALSE }); + parameterSets.add(new Object[] { request3, "http://example.com:80", Boolean.FALSE }); + parameterSets.add(new Object[] { request3, "http://example.com:443", Boolean.TRUE}); + + parameterSets.add(new Object[] { request4, "http://example.com", Boolean.FALSE }); + parameterSets.add(new Object[] { request4, "http://example.com:80", Boolean.FALSE }); + parameterSets.add(new Object[] { request4, "http://example.com:8080", Boolean.TRUE}); + + return parameterSets; + } + + + @Parameter(0) + public HttpServletRequest request; + @Parameter(1) + public String origin; + @Parameter(2) + public Boolean same; + + + @Test + public void testSameOrigin() { + Assert.assertEquals(same, Boolean.valueOf(RequestUtil.isSameOrigin(request, origin))); + } + + + private static class TesterRequest extends HttpServletRequestWrapper { + + private final String scheme; + private final String host; + private final int port; + + public TesterRequest(String scheme, String host, int port) { + super(new Request()); + this.scheme = scheme; + this.host = host; + this.port = port; + } + + @Override + public String getScheme() { + return scheme; + } + + @Override + public String getServerName() { + return host; + } + + @Override + public int getServerPort() { + return port; + } + + @Override + public String toString() { + return scheme + "://" + host + ":" + port; + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org