diff --git a/doc/licenses/ipaddress-5.4.0/README b/doc/licenses/ipaddress-5.4.0/README new file mode 100644 index 000000000..48580a5e9 --- /dev/null +++ b/doc/licenses/ipaddress-5.4.0/README @@ -0,0 +1,8 @@ +IPAddress (https://seancfoley.github.io/IPAddress/) +--------------------------------------------------- + + Version: 5.4.0 + From: 'Sean C Foley' (https://seancfoley.github.io/) + License(s): + Apache v2.0 + diff --git a/doc/licenses/ipaddress-5.4.0/dep-coordinates.txt b/doc/licenses/ipaddress-5.4.0/dep-coordinates.txt new file mode 100644 index 000000000..30920cc31 --- /dev/null +++ b/doc/licenses/ipaddress-5.4.0/dep-coordinates.txt @@ -0,0 +1 @@ +com.github.seancfoley:ipaddress:jar:5.4.0 diff --git a/doc/licenses/spring-framework-5.3.27/README b/doc/licenses/spring-framework-5.3.27/README deleted file mode 100644 index c534a2b6b..000000000 --- a/doc/licenses/spring-framework-5.3.27/README +++ /dev/null @@ -1,8 +0,0 @@ -Spring Framework (https://spring.io/projects/spring-framework) --------------------------------------------------------------- - - Version: 5.3.27 - From: 'Spring' (https://spring.io/) - License(s): - Apache v2.0 - diff --git a/doc/licenses/spring-framework-5.3.27/dep-coordinates.txt b/doc/licenses/spring-framework-5.3.27/dep-coordinates.txt deleted file mode 100644 index 8afba931c..000000000 --- a/doc/licenses/spring-framework-5.3.27/dep-coordinates.txt +++ /dev/null @@ -1,7 +0,0 @@ -org.springframework:spring-aop:jar:5.3.27 -org.springframework:spring-beans:jar:5.3.27 -org.springframework:spring-context:jar:5.3.27 -org.springframework:spring-core:jar:5.3.27 -org.springframework:spring-expression:jar:5.3.27 -org.springframework:spring-jcl:jar:5.3.27 -org.springframework:spring-web:jar:5.3.27 diff --git a/doc/licenses/spring-security-5.8.3/NOTICE b/doc/licenses/spring-security-5.8.3/NOTICE deleted file mode 100644 index 7aac7fe5c..000000000 --- a/doc/licenses/spring-security-5.8.3/NOTICE +++ /dev/null @@ -1,2 +0,0 @@ -This product includes software developed by Spring Security -Project (https://www.springframework.org/security). diff --git a/doc/licenses/spring-security-5.8.3/README b/doc/licenses/spring-security-5.8.3/README deleted file mode 100644 index 46b0ae1b1..000000000 --- a/doc/licenses/spring-security-5.8.3/README +++ /dev/null @@ -1,8 +0,0 @@ -Spring Security (https://spring.io/projects/spring-security) ------------------------------------------------------------- - - Version: 5.8.3 - From: 'Spring' (https://spring.io/) - License(s): - Apache v2.0 - diff --git a/doc/licenses/spring-security-5.8.3/dep-coordinates.txt b/doc/licenses/spring-security-5.8.3/dep-coordinates.txt deleted file mode 100644 index b1b573395..000000000 --- a/doc/licenses/spring-security-5.8.3/dep-coordinates.txt +++ /dev/null @@ -1,3 +0,0 @@ -org.springframework.security:spring-security-core:jar:5.8.3 -org.springframework.security:spring-security-crypto:jar:5.8.3 -org.springframework.security:spring-security-web:jar:5.8.3 diff --git a/extensions/guacamole-auth-json/pom.xml b/extensions/guacamole-auth-json/pom.xml index 9cd7adf3e..daab34da8 100644 --- a/extensions/guacamole-auth-json/pom.xml +++ b/extensions/guacamole-auth-json/pom.xml @@ -73,11 +73,18 @@ provided - + - org.springframework.security - spring-security-web - 5.8.3 + com.github.seancfoley + ipaddress + 5.4.0 + + + + + junit + junit + test diff --git a/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java b/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java index d39c78443..248f8b2c9 100644 --- a/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java +++ b/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java @@ -20,13 +20,13 @@ package org.apache.guacamole.auth.json; import com.google.inject.Inject; +import inet.ipaddr.IPAddressString; import java.util.ArrayList; import java.util.Collection; import javax.servlet.http.HttpServletRequest; import org.apache.guacamole.GuacamoleException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.security.web.util.matcher.IpAddressMatcher; /** * Service for testing the validity of received HTTP requests. @@ -45,6 +45,17 @@ public class RequestValidationService { @Inject private ConfigurationService confService; + /** + * Constructor that enables passing of an instance of + * ConfigurationService. (Only used for unit testing) + * + * @param confService + * The (mock) instance of ConfigurationService + */ + public RequestValidationService(ConfigurationService confService) { + this.confService = confService; + } + /** * Returns whether the given request can be used for authentication, taking * into account restrictions specified within guacamole.properties. @@ -77,16 +88,11 @@ public class RequestValidationService { return true; } - // Build matchers for each trusted network - Collection matchers = new ArrayList<>(trustedNetworks.size()); - for (String network : trustedNetworks) - matchers.add(new IpAddressMatcher(network)); - - // Otherwise ensure at least one subnet matches - for (IpAddressMatcher matcher : matchers) { + // Otherwise ensure that the remote address is part of a trusted network + for (String network : trustedNetworks) { // Request is allowed if any subnet matches - if (matcher.matches(request)) { + if (new IPAddressString(network).contains(new IPAddressString(request.getRemoteAddr()))) { logger.debug("Authentication request from \"{}\" is ALLOWED (matched subnet).", request.getRemoteAddr()); return true; } diff --git a/extensions/guacamole-auth-json/src/test/java/org/apache/guacamole/auth/json/RequestValidationServiceTest.java b/extensions/guacamole-auth-json/src/test/java/org/apache/guacamole/auth/json/RequestValidationServiceTest.java new file mode 100644 index 000000000..ed5d77ce3 --- /dev/null +++ b/extensions/guacamole-auth-json/src/test/java/org/apache/guacamole/auth/json/RequestValidationServiceTest.java @@ -0,0 +1,452 @@ +/* + * 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.guacamole.auth.json; + +import java.io.BufferedReader; +import java.security.Principal; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Enumeration; +import java.util.Locale; +import java.util.Map; +import java.util.regex.Pattern; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; +import javax.servlet.ServletInputStream; +import javax.servlet.RequestDispatcher; +import org.junit.Test; +import static org.junit.Assert.*; + +/** + * Unit test for RequestValidationService. Verifies that only requests + * from trusted hosts are allowed to authenticate. + */ +public class RequestValidationServiceTest { + + /** + * This class is used to mock ConfigurationService. + */ + private class MockConfigurationService extends ConfigurationService { + + /** + * List of networks to be trusted + */ + private Collection trustedNetworks; + + /** + * Constructor that enables passing of a comma-separated list of + * trusted networks. + * + * @param trustedNetworks + * The comma-separated list of trusted networks + */ + private MockConfigurationService(String trustedNetworks) { + if (trustedNetworks == null || trustedNetworks.isEmpty()) + this.trustedNetworks = Collections.emptyList(); + else + this.trustedNetworks = Arrays.asList(Pattern.compile(",\\s*").split(trustedNetworks)); + } + + @Override + public Collection getTrustedNetworks() { + return trustedNetworks; + } + + } + + /** + * The instance of RequestValidationService to be tested. + */ + private RequestValidationService requestService; + + /** + * Method that returns a (mock) HttpServletRequest with the provided + * remote address. + * + * @param remoteAddr + * The remote address of the request + */ + private static HttpServletRequest mockHttpServletRequest(String remoteAddr) { + + return new HttpServletRequest() { + + @Override + public Object getAttribute(String name) { + return null; + } + + @Override + public Enumeration getAttributeNames() { + return null; + } + + @Override + public String getAuthType() { + return null; + } + + @Override + public String getCharacterEncoding() { + return null; + } + + @Override + public int getContentLength() { + return 0; + } + + @Override + public String getContentType() { + return null; + } + + @Override + public String getContextPath() { + return null; + } + + @Override + public Cookie[] getCookies() { + return null; + } + + @Override + public long getDateHeader(String name) { + return 0; + } + + @Override + public String getHeader(String name) { + return null; + } + + @Override + public Enumeration getHeaderNames() { + return null; + } + + @Override + public Enumeration getHeaders(String name) { + return null; + } + + @Override + public ServletInputStream getInputStream() { + return null; + } + + @Override + public int getIntHeader(String name) { + return 0; + } + + @Override + public String getLocalAddr() { + return null; + } + + @Override + public Locale getLocale() { + return null; + } + + @Override + public Enumeration getLocales() { + return null; + } + + @Override + public String getLocalName() { + return null; + } + + @Override + public int getLocalPort() { + return 0; + } + + @Override + public String getMethod() { + return null; + } + + @Override + public String getParameter(String name) { + return null; + } + + @Override + public Map getParameterMap() { + return null; + } + + @Override + public Enumeration getParameterNames() { + return null; + } + + @Override + public String[] getParameterValues(String name) { + return null; + } + + @Override + public String getPathInfo() { + return null; + } + + @Override + public String getPathTranslated() { + return null; + } + + @Override + public String getProtocol() { + return null; + } + + @Override + public String getQueryString() { + return null; + } + + @Override + public BufferedReader getReader() { + return null; + } + + @Override + @Deprecated + public String getRealPath(String path) { + return null; + } + + @Override + public String getRemoteAddr() { + return remoteAddr; + } + + @Override + public String getRemoteHost() { + return null; + } + + @Override + public int getRemotePort() { + return 0; + } + + @Override + public String getRemoteUser() { + return null; + } + + @Override + public RequestDispatcher getRequestDispatcher(String path) { + return null; + } + + @Override + public String getRequestedSessionId() { + return null; + } + + @Override + public String getRequestURI() { + return null; + } + + @Override + public StringBuffer getRequestURL() { + return null; + } + + @Override + public String getScheme() { + return null; + } + + @Override + public String getServerName() { + return null; + } + + @Override + public int getServerPort() { + return 0; + } + + @Override + public String getServletPath() { + return null; + } + + @Override + public HttpSession getSession() { + return null; + } + + @Override + public HttpSession getSession(boolean create) { + return null; + } + + @Override + public Principal getUserPrincipal() { + return null; + } + + @Override + public boolean isRequestedSessionIdFromCookie() { + return false; + } + + @Override + @Deprecated + public boolean isRequestedSessionIdFromUrl() { + return false; + } + + @Override + public boolean isRequestedSessionIdFromURL() { + return false; + } + + @Override + public boolean isRequestedSessionIdValid() { + return false; + } + + @Override + public boolean isSecure() { + return false; + } + + @Override + public boolean isUserInRole(String role) { + return false; + } + + @Override + public void removeAttribute(String name) { + return; + } + + @Override + public void setAttribute(String name, Object o) { + return; + } + + @Override + public void setCharacterEncoding(String env) { + return; + } + + }; + + }; + + /** + * Verifies that all hosts are allowed to authenticate when no + * trusted networks are specified. + */ + @Test + public void testNoTrustedNetwork() { + + requestService = new RequestValidationService(new MockConfigurationService(null)); + + try { + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("1.1.1.1"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("10.10.10.10"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("100.100.100.100"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("1:1:1:1:1:1:1:1"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("10:10:10:10:10:10:10:10"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("100:100:100:100:100:100:100:100"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("1000:1000:1000:1000:1000:1000:1000:1000"))); + } + catch (AssertionError e) { + fail("A network was denied to authenticate even though no trusted networks were specified."); + } + + } + + /** + * Verifies that hosts from trusted networks are allowed to + * authenticate: + */ + @Test + public void testTrustedNetwork() { + + requestService = new RequestValidationService(new MockConfigurationService("10.0.0.0/8,127.0.0.0/8,172.16.0.0/12,192.168.0.0/16,1.2.3.4/32,::1/128,fc00::/7")); + + try { + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("10.0.0.0"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("10.255.255.255"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("127.0.0.0"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("127.255.255.255"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("172.16.0.0"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("172.31.255.255"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("192.168.0.0"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("192.168.255.255"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("1.2.3.4"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("::1"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("fc00::"))); + assertTrue(requestService.isAuthenticationAllowed(mockHttpServletRequest("fdff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"))); + } + catch (AssertionError e) { + fail("A trusted network was denied to authenticate."); + } + + } + + /** + * Verifies that hosts outside trusted networks are not allowed to + * authenticate. + */ + @Test + public void testUntrustedNetwork() { + + requestService = new RequestValidationService(new MockConfigurationService("10.0.0.0/8,127.0.0.0/8,172.16.0.0/12,192.168.0.0/16,1.2.3.4/32,::1/128,fc00::/7")); + + try { + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("9.255.255.255"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("11.0.0.0"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("126.255.255.255"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("128.0.0.0"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("172.15.255.255"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("172.32.0.0"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("192.167.255.255"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("192.169.0.0"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("1.2.3.3"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("1.2.3.5"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("::0"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("::2"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("fbff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"))); + assertFalse(requestService.isAuthenticationAllowed(mockHttpServletRequest("fe00::"))); + } + catch (AssertionError e) { + fail("An untrusted network was allowed to authenticate."); + } + + } + +}