This is an automated email from the ASF dual-hosted git repository. hanicz pushed a commit to branch v3.0.0 in repository https://gitbox.apache.org/repos/asf/knox.git
commit f4ce36819f537d1ad470b44f330c858f1698a224 Author: hanicz <[email protected]> AuthorDate: Wed Aug 12 15:26:33 2026 +0200 KNOX-3416: KnoxSSO redirects to untrusted site (#1346) * KNOX-3416: KnoxSSO redirects to untrusted site * KNOX-3416: Clarify error message for userInfo (cherry picked from commit c3dee56346cfb2670a114d478db0ba2b51aa01f7) --- .../gateway/service/knoxsso/KnoxSSOMessages.java | 4 ++ .../gateway/service/knoxsso/WebSSOResource.java | 25 ++++++++----- .../service/knoxsso/WebSSOResourceTest.java | 43 ++++++++++++++++++++++ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/KnoxSSOMessages.java b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/KnoxSSOMessages.java index 3e642219b..b9a87cfbe 100644 --- a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/KnoxSSOMessages.java +++ b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/KnoxSSOMessages.java @@ -61,6 +61,10 @@ public interface KnoxSSOMessages { "not valid according to the configured whitelist: {1}. See documentation for KnoxSSO Whitelisting.") void whiteListMatchFail(String original, String whitelist); + @Message( level = MessageLevel.ERROR, text = "The original URL: {0} for redirecting back after authentication " + + "embeds userinfo (e.g. user@host) and is therefore rejected.") + void userInfoInOriginalURL(String original); + @Message( level = MessageLevel.INFO, text = "Knox Token service ({0}) stored state for token {1} ({2})") void storedToken(String topologyName, String tokenDisplayText, String tokenId); } 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 16e676240..cf73bc987 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 @@ -264,20 +264,27 @@ public class WebSSOResource { boolean validRedirect = true; - // 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) { - try { + try { + // A redirect target embedding userinfo (e.g. https://knox-host:8443@evil/) is + // never legitimate; reject it before the host-only whitelist check. + if (Urls.containsUserInfo(original)) { + validRedirect = false; + LOGGER.userInfoInOriginalURL(Log4jAuditor.maskTokenFromURL(original)); + } else if (whitelist != null) { + // If there is a whitelist defined, then the original URL must be validated against it. + // If there is no whitelist, then everything is valid. validRedirect = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, original); - } catch (MalformedURLException e) { - throw new WebApplicationException("Malformed original URL: " + original, - Response.Status.BAD_REQUEST); + if (!validRedirect) { + LOGGER.whiteListMatchFail(Log4jAuditor.maskTokenFromURL(original), whitelist); + } } + } catch (MalformedURLException e) { + throw new WebApplicationException("Malformed original URL: " + original, + Response.Status.BAD_REQUEST); } if (!validRedirect) { - LOGGER.whiteListMatchFail(Log4jAuditor.maskTokenFromURL(original), whitelist); - throw new WebApplicationException("Original URL not valid according to the configured whitelist.", + throw new WebApplicationException("Original URL not valid for redirect.", Response.Status.BAD_REQUEST); } } else { 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 f0e119478..12d84ab0c 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 @@ -473,6 +473,49 @@ public class WebSSOResourceTest { } } + @Test + public void testUserInfoOriginalURLRejected() throws Exception { + ServletContext context = EasyMock.createNiceMock(ServletContext.class); + EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(expectGatewayConfig()).anyTimes(); + + HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); + EasyMock.expect(request.getParameter("originalUrl")).andReturn( + "https://localhost:[email protected]/";); + EasyMock.expect(request.getAttribute("targetServiceRole")).andReturn("KNOXSSO").anyTimes(); + EasyMock.expect(request.getParameterMap()).andReturn(Collections.emptyMap()); + EasyMock.expect(request.getServletContext()).andReturn(context).anyTimes(); + EasyMock.expect(request.getServerName()).andReturn("localhost").anyTimes(); + + Principal principal = EasyMock.createNiceMock(Principal.class); + EasyMock.expect(principal.getName()).andReturn("alice").anyTimes(); + EasyMock.expect(request.getUserPrincipal()).andReturn(principal).anyTimes(); + + GatewayServices services = EasyMock.createNiceMock(GatewayServices.class); + EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(services); + + AliasService aliasService = EasyMock.createNiceMock(AliasService.class); + EasyMock.expect(services.getService(ServiceType.ALIAS_SERVICE)).andReturn(aliasService).anyTimes(); + EasyMock.expect(aliasService.getPasswordFromAliasForGateway(TokenUtils.SIGNING_HMAC_SECRET_ALIAS)).andReturn(null).anyTimes(); + + JWTokenAuthority authority = new TestJWTokenAuthority(gatewayPublicKey, gatewayPrivateKey); + EasyMock.expect(services.getService(ServiceType.TOKEN_SERVICE)).andReturn(authority); + + HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class); + ServletOutputStream outputStream = EasyMock.createNiceMock(ServletOutputStream.class); + CookieResponseWrapper responseWrapper = new CookieResponseWrapper(response, outputStream); + + EasyMock.replay(principal, services, context, request); + + WebSSOResource webSSOResponse = new WebSSOResource(); + webSSOResponse.request = request; + webSSOResponse.response = responseWrapper; + webSSOResponse.context = context; + webSSOResponse.init(); + + WebApplicationException e = Assert.assertThrows(WebApplicationException.class, webSSOResponse::doGet); + assertEquals(HttpStatus.SC_BAD_REQUEST, e.getResponse().getStatus()); + } + private GatewayConfig expectGatewayConfig() { return expectGatewayConfig(true); }
