You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by am...@apache.org on 2023/05/02 11:55:04 UTC

[knox] branch master updated: KNOX-2906 - SSO whitelist checker should cut off path segments and query params (#754)

This is an automated email from the ASF dual-hosted git repository.

amagyar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new 4ee5073bc KNOX-2906 - SSO whitelist checker should cut off path segments and query params (#754)
4ee5073bc is described below

commit 4ee5073bcc8068fa7989558d8b6bd8079684bd10
Author: Attila Magyar <m....@gmail.com>
AuthorDate: Tue May 2 13:54:58 2023 +0200

    KNOX-2906 - SSO whitelist checker should cut off path segments and query params (#754)
---
 .../gateway/service/knoxsso/WebSSOResource.java    | 12 +----------
 .../service/knoxsso/WebSSOResourceTest.java        | 23 ++++++++++++++++++++
 .../gateway/dispatch/GatewayDispatchFilter.java    | 25 ++--------------------
 .../org/apache/knox/gateway/util/RegExUtils.java   | 22 +++++++++++++++++++
 4 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
index afbca9289..531524d11 100644
--- a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
+++ b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
@@ -22,11 +22,8 @@ import static javax.ws.rs.core.MediaType.APPLICATION_XML;
 import static org.apache.knox.gateway.services.GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE;
 
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -252,14 +249,7 @@ public class WebSSOResource {
       // If there is a whitelist defined, then the original URL must be validated against it.
       // If there is no whitelist, then everything is valid.
       if (whitelist != null) {
-        String decodedOriginal = null;
-        try {
-          decodedOriginal = URLDecoder.decode(original, StandardCharsets.UTF_8.name());
-        } catch (UnsupportedEncodingException e) {
-          //
-        }
-
-        validRedirect = RegExUtils.checkWhitelist(whitelist, (decodedOriginal != null ? decodedOriginal : original));
+        validRedirect = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, original);
       }
 
       if (!validRedirect) {
diff --git a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
index 1d8940b41..33be1c79c 100644
--- a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
+++ b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
@@ -148,6 +148,29 @@ public class WebSSOResourceTest {
         "/local/resource/"));
   }
 
+  @Test
+  public void testWhitelistMatchingAgainstBaseURL() {
+    Assert.assertTrue("Failed to match whitelist",
+            RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+                    "https://KNOX_GW_DOMAIN"));
+    Assert.assertTrue("Failed to match whitelist",
+            RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+                    "https://KNOX_GW_DOMAIN?a=1&b=2"));
+    Assert.assertTrue("Failed to match whitelist",
+            RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+                    "https://KNOX_GW_DOMAIN?a=1&b=2"));
+    Assert.assertTrue("Failed to match whitelist",
+            RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+                    "https://KNOX_GW_DOMAIN/path1/path2/path/3?a=1&b=2"));
+    Assert.assertFalse("Inappropriately matched whitelist",
+            RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+            "https://google.com?https://KNOX_GW_DOMAIN"));
+    Assert.assertFalse("Inappropriately matched whitelist",
+            RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+                    "https://google.com/https://KNOX_GW_DOMAIN"));
+  }
+
+
   private void configureCommonExpectations(Map<String, String> contextExpectations) throws Exception {
     configureCommonExpectations(contextExpectations, false, false, true);
   }
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
index f54c63902..2b56a0660 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
@@ -32,12 +32,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
-import java.net.MalformedURLException;
 import java.net.URISyntaxException;
-import java.net.URL;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Locale;
@@ -145,26 +140,10 @@ public class GatewayDispatchFilter extends AbstractGatewayFilter {
       }
 
       if (whitelist != null) {
-        String requestURI = request.getRequestURI();
-
-        String decodedURL = requestURI;
-        try {
-          decodedURL = URLDecoder.decode(requestURI, StandardCharsets.UTF_8.name());
-        } catch (UnsupportedEncodingException e) {
-          //
-        }
-        String baseUrl;
-        try {
-          URL url = new URL(decodedURL);
-          baseUrl = new URL(url.getProtocol(), url.getHost(), url.getPort(), "").toString();
-        } catch (MalformedURLException e) {
-          throw new RuntimeException(e);
-        }
-
-        isAllowed = RegExUtils.checkWhitelist(whitelist, baseUrl);
+        isAllowed = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, request.getRequestURI());
 
         if (!isAllowed) {
-          LOG.dispatchDisallowed(requestURI);
+          LOG.dispatchDisallowed(request.getRequestURI());
         }
     }
 
diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
index 9ebd1d742..7a644fdc0 100644
--- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
+++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
@@ -17,6 +17,11 @@
  */
 package org.apache.knox.gateway.util;
 
+import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -40,4 +45,21 @@ public class RegExUtils {
     return false;
   }
 
+  public static boolean checkBaseUrlAgainstWhitelist(String whitelist, String fullUrl) {
+    String decodedURL = fullUrl;
+    try {
+      decodedURL = URLDecoder.decode(fullUrl, StandardCharsets.UTF_8.name());
+    } catch (UnsupportedEncodingException e) {
+      //
+    }
+    String baseUrl;
+    try {
+      URL url = new URL(decodedURL);
+      baseUrl = new URL(url.getProtocol(), url.getHost(), url.getPort(), "").toString();
+    } catch (MalformedURLException e) {
+      throw new RuntimeException(e);
+    }
+    return checkWhitelist(whitelist, baseUrl);
+  }
+
 }