From 8d1a9fdf428a2971a57924cdf589fbca6af87efe Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Tue, 26 Sep 2023 17:15:38 -0400 Subject: [PATCH 1/4] GUACAMOLE-1855: Implement property for tracking list of IP Addresses. --- extensions/guacamole-auth-json/pom.xml | 1 + guacamole-ext/pom.xml | 7 ++ .../properties/IPAddressListProperty.java | 113 ++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 guacamole-ext/src/main/java/org/apache/guacamole/properties/IPAddressListProperty.java diff --git a/extensions/guacamole-auth-json/pom.xml b/extensions/guacamole-auth-json/pom.xml index a986b627f..1065b32ae 100644 --- a/extensions/guacamole-auth-json/pom.xml +++ b/extensions/guacamole-auth-json/pom.xml @@ -78,6 +78,7 @@ com.github.seancfoley ipaddress 5.5.0 + provided diff --git a/guacamole-ext/pom.xml b/guacamole-ext/pom.xml index 161fbe648..a24375625 100644 --- a/guacamole-ext/pom.xml +++ b/guacamole-ext/pom.xml @@ -110,6 +110,13 @@ jackson-databind + + + com.github.seancfoley + ipaddress + 5.5.0 + + diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/properties/IPAddressListProperty.java b/guacamole-ext/src/main/java/org/apache/guacamole/properties/IPAddressListProperty.java new file mode 100644 index 000000000..1e0f6edfa --- /dev/null +++ b/guacamole-ext/src/main/java/org/apache/guacamole/properties/IPAddressListProperty.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.guacamole.properties; + +import inet.ipaddr.IPAddress; +import inet.ipaddr.IPAddressString; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.regex.Pattern; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleServerException; + +/** + * A GuacamoleProperty implementation that parses a String for a comma-separated + * list of IP addresses and/or IP subnets, both IPv4 and IPv6, and returns the + * list of those valid IP addresses/subnets. + */ +public abstract class IPAddressListProperty implements GuacamoleProperty> { + + /** + * A pattern which matches against the delimiters between values. This is + * currently simply a comma and any following whitespace. Parts of the + * input string which match this pattern will not be included in the parsed + * result. + */ + private static final Pattern DELIMITER_PATTERN = Pattern.compile(",\\s*"); + + @Override + public List parseValue(String values) throws GuacamoleException { + + // Null for null + if (values == null) + return null; + + // Not null, just empty + if (values.isEmpty()) + return Collections.emptyList(); + + // Split the string into an array + List addrStrings = Arrays.asList(DELIMITER_PATTERN.split(values)); + List ipAddresses = new ArrayList<>(); + + // Loop through each string + for (String addrString : addrStrings) { + + // Convert the string to an IPAddressString for validation + IPAddressString ipString = new IPAddressString(addrString); + + // If this isn't a valid address, subnet, etc., throw an exception + if (!ipString.isIPAddress()) + throw new GuacamoleServerException("Invalid IP address specified: " + addrString); + + // Add the address to the list. + ipAddresses.add(ipString.getAddress()); + } + + // Return our list of valid IP addresses and/or subnets + return ipAddresses; + + } + + /** + * Return true if the provided address list contains the client address, + * or false if no match is found. + * + * @param addrList + * The address list to check for matches. + * + * @param ipAddr + * The client address to look for in the list. + * + * @return + * True if the client address is in the provided list, otherwise + * false. + */ + public static boolean addressListContains(List addrList, IPAddress ipAddr) { + + // If either is null, return false + if (ipAddr == null || addrList == null) + return false; + + for (IPAddress ipEntry : addrList) + + // If version matches and entry contains it, return true + if (ipEntry.getIPVersion().equals(ipAddr.getIPVersion()) + && ipEntry.contains(ipAddr)) + return true; + + // No match, so return false + return false; + + } + +} From 9139bdef02b2c7334f9853db3240fdede1c61831 Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Tue, 26 Sep 2023 17:16:02 -0400 Subject: [PATCH 2/4] GUACAMOLE-1855: Implement bypass and enforcement options in the Duo 2FA module. --- extensions/guacamole-auth-duo/pom.xml | 8 ++ .../auth/duo/UserVerificationService.java | 66 ++++++++++++++-- .../auth/duo/conf/ConfigurationService.java | 76 +++++++++++++++++++ 3 files changed, 143 insertions(+), 7 deletions(-) diff --git a/extensions/guacamole-auth-duo/pom.xml b/extensions/guacamole-auth-duo/pom.xml index 95afee441..f7631bb22 100644 --- a/extensions/guacamole-auth-duo/pom.xml +++ b/extensions/guacamole-auth-duo/pom.xml @@ -130,6 +130,14 @@ ${kotlin.version} + + + com.github.seancfoley + ipaddress + 5.5.0 + provided + + diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java index 2333e21ef..b763f6f30 100644 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java +++ b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java @@ -23,10 +23,13 @@ import com.duosecurity.Client; import com.duosecurity.exception.DuoException; import com.duosecurity.model.Token; import com.google.inject.Inject; +import inet.ipaddr.IPAddress; +import inet.ipaddr.IPAddressString; import java.net.URI; import java.net.URISyntaxException; import java.util.Collections; import java.util.concurrent.TimeUnit; +import java.util.List; import javax.servlet.http.HttpServletRequest; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; @@ -107,9 +110,63 @@ public class UserVerificationService { public void verifyAuthenticatedUser(AuthenticatedUser authenticatedUser) throws GuacamoleException { - // Ignore anonymous users (unverifiable) + // Pull the original HTTP request used to authenticate + Credentials credentials = authenticatedUser.getCredentials(); + HttpServletRequest request = credentials.getRequest(); + IPAddress clientAddr = new IPAddressString(request.getRemoteAddr()).getAddress(); + + // Ignore anonymous users String username = authenticatedUser.getIdentifier(); - if (username.equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER)) + if (username == null || username.equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER)) + return; + + // We enforce by default + boolean enforceHost = true; + + // Check for a list of addresses that should be bypassed and iterate + List bypassAddresses = confService.getBypassHosts(); + for (IPAddress bypassAddr : bypassAddresses) { + + // If the address contains current client address, flip enforce flag + // and break out + if (clientAddr != null && clientAddr.isIPAddress() + && bypassAddr.getIPVersion().equals(clientAddr.getIPVersion()) + && bypassAddr.contains(clientAddr)) { + enforceHost = false; + break; + } + } + + // Check for a list of addresses that should be enforced and iterate + List enforceAddresses = confService.getEnforceHosts(); + + // Only continue processing if the list is not empty + if (!enforceAddresses.isEmpty()) { + + // If client address is not available or invalid, MFA will + // be enforced. + if (clientAddr == null || !clientAddr.isIPAddress()) { + enforceHost = true; + } + + else { + // With addresses set, this default changes to false. + enforceHost = false; + + for (IPAddress enforceAddr : enforceAddresses) { + + // If there's a match, flip the enforce flag and break out of the loop + if (enforceAddr.getIPVersion().equals(clientAddr.getIPVersion()) + && enforceAddr.contains(clientAddr)) { + enforceHost = true; + break; + } + } + } + } + + // If the enforce flag has been changed, exit, bypassing Duo MFA. + if (!enforceHost) return; // Obtain a Duo client for redirecting the user to the Duo service and @@ -137,11 +194,6 @@ public class UserVerificationService { + "not currently available (failed health check).", e); } - // Pull the original HTTP request used to authenticate, as well as any - // associated credentials - Credentials credentials = authenticatedUser.getCredentials(); - HttpServletRequest request = credentials.getRequest(); - // Retrieve signed Duo authentication code and session state from the // request (these will be absent if this is an initial authentication // attempt and not a redirect back from Duo) diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/conf/ConfigurationService.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/conf/ConfigurationService.java index 5ed7d7a21..43a2d98ed 100644 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/conf/ConfigurationService.java +++ b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/conf/ConfigurationService.java @@ -20,10 +20,14 @@ package org.apache.guacamole.auth.duo.conf; import com.google.inject.Inject; +import inet.ipaddr.IPAddress; import java.net.URI; +import java.util.Collections; +import java.util.List; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.properties.IntegerGuacamoleProperty; +import org.apache.guacamole.properties.IPAddressListProperty; import org.apache.guacamole.properties.StringGuacamoleProperty; import org.apache.guacamole.properties.URIGuacamoleProperty; @@ -105,6 +109,40 @@ public class ConfigurationService { public String getName() { return "duo-auth-timeout"; } }; + + /** + * The optional property that contains a comma-separated list of IP addresses + * or CIDRs for which the MFA requirement should be bypassed. If the Duo + * extension is installed, any/all users authenticating from clients that + * match this list will be able to successfully log in without fulfilling + * the MFA requirement. If this option is omitted or is empty, and the + * Duo module is installed, all users from all hosts will have Duo MFA + * enforced. + */ + private static final IPAddressListProperty DUO_BYPASS_HOSTS = + new IPAddressListProperty() { + + @Override + public String getName() { return "duo-bypass-hosts"; } + + }; + + /** + * The optional property that contains a comma-separated list of IP addresses + * or CIDRs for which the MFA requirement should be explicitly enforced. If + * the Duo module is enabled and this property is specified, users that log + * in from hosts that match the items in this list will have Duo MFA required, + * and all users from hosts that do not match this list will be able to log + * in without the MFA requirement. If this option is missing or empty and + * the Duo module is installed, MFA will be enforced for all users. + */ + private static final IPAddressListProperty DUO_ENFORCE_HOSTS = + new IPAddressListProperty() { + + @Override + public String getName() { return "duo-enforce-hosts"; } + + }; /** * Returns the hostname of the Duo API endpoint to be used to verify user @@ -188,5 +226,43 @@ public class ConfigurationService { public int getAuthenticationTimeout() throws GuacamoleException { return environment.getProperty(DUO_AUTH_TIMEOUT, 5); } + + /** + * Returns the list of IP addresses and subnets defined in guacamole.properties + * for which Duo MFA should _not_ be enforced. Users logging in from hosts + * contained in this list will be logged in without the MFA requirement. + * + * @return + * A list of IP addresses and subnets for which Duo MFA should not be + * enforced. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed, or if an invalid IP address + * or subnet is specified. + */ + public List getBypassHosts() throws GuacamoleException { + return environment.getProperty(DUO_BYPASS_HOSTS, Collections.emptyList()); + } + + /** + * Returns the list of IP addresses and subnets defined in guacamole.properties + * for which Duo MFA should explicitly be enforced, while logins from all + * other hosts should not enforce MFA. Users logging in from hosts + * contained in this list will be required to complete the Duo MFA authentication, + * while users from all other hosts will be logged in without the MFA requirement. + * + * @return + * A list of IP addresses and subnets for which Duo MFA should be + * explicitly enforced. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed, or if an invalid IP address + * or subnet is specified. + */ + public List getEnforceHosts() throws GuacamoleException { + return environment.getProperty(DUO_ENFORCE_HOSTS, Collections.emptyList()); + } + + } From 614cd550bd1c1d11fef48604d5e3d87ced07009d Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Tue, 26 Sep 2023 17:16:25 -0400 Subject: [PATCH 3/4] GUACAMOLE-1855: Implement bypass and enforcement options in the TOTP module. --- extensions/guacamole-auth-totp/pom.xml | 8 +++ .../auth/totp/conf/ConfigurationService.java | 68 +++++++++++++++++++ .../totp/user/UserVerificationService.java | 66 ++++++++++++++++-- 3 files changed, 138 insertions(+), 4 deletions(-) diff --git a/extensions/guacamole-auth-totp/pom.xml b/extensions/guacamole-auth-totp/pom.xml index 36279940f..711b7ba98 100644 --- a/extensions/guacamole-auth-totp/pom.xml +++ b/extensions/guacamole-auth-totp/pom.xml @@ -177,6 +177,14 @@ 2.1.1 provided + + + + com.github.seancfoley + ipaddress + 5.5.0 + provided + diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/conf/ConfigurationService.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/conf/ConfigurationService.java index 06984ce40..311291484 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/conf/ConfigurationService.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/conf/ConfigurationService.java @@ -20,10 +20,14 @@ package org.apache.guacamole.auth.totp.conf; import com.google.inject.Inject; +import inet.ipaddr.IPAddress; +import java.util.Collections; +import java.util.List; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.properties.EnumGuacamoleProperty; +import org.apache.guacamole.properties.IPAddressListProperty; import org.apache.guacamole.properties.IntegerGuacamoleProperty; import org.apache.guacamole.properties.StringGuacamoleProperty; import org.apache.guacamole.totp.TOTPGenerator; @@ -88,6 +92,36 @@ public class ConfigurationService { public String getName() { return "totp-mode"; } }; + + /** + * A property that contains a list of IP addresses and/or subnets for which + * MFA via the TOTP module should be bypassed. Users logging in from addresses + * contained in this list will not be prompted for a second authentication + * factor. If this property is empty or not defined, and the TOTP module + * is installed, all users will be prompted for MFA. + */ + private static final IPAddressListProperty TOTP_BYPASS_HOSTS = + new IPAddressListProperty() { + + @Override + public String getName() { return "totp-bypass-hosts"; } + + }; + + /** + * A property that contains a list of IP addresses and/or subnets for which + * MFA via the TOTP module should explicitly be enabled. If this property is defined, + * and the TOTP module is installed, users logging in from hosts contained + * in this list will be prompted for MFA, and users logging in from all + * other hosts will not be prompted for MFA. + */ + private static final IPAddressListProperty TOTP_ENFORCE_HOSTS = + new IPAddressListProperty() { + + @Override + public String getName() { return "totp-enforce-hosts"; } + + }; /** * Returns the human-readable name of the entity issuing user accounts. If @@ -158,5 +192,39 @@ public class ConfigurationService { public TOTPGenerator.Mode getMode() throws GuacamoleException { return environment.getProperty(TOTP_MODE, TOTPGenerator.Mode.SHA1); } + + /** + * Return the list of IP addresses and/or subnets for which MFA authentication via the + * TOTP module should be bypassed, allowing users from those addresses to log in + * without the MFA requirement. + * + * @return + * A list of IP addresses and/or subnets for which MFA authentication + * should be bypassed. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed, or an invalid IP address + * or subnet is specified. + */ + public List getBypassHosts() throws GuacamoleException { + return environment.getProperty(TOTP_BYPASS_HOSTS, Collections.emptyList()); + } + + /** + * Return the list of IP addresses and/or subnets for which MFA authentication via the TOTP + * module should be explicitly enabled, requiring users logging in from hosts specified in + * the list to complete MFA. + * + * @return + * A list of IP addresses and/or subnets for which MFA authentication + * should be explicitly enabled. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed, or an invalid IP address + * or subnet is specified. + */ + public List getEnforceHosts() throws GuacamoleException { + return environment.getProperty(TOTP_ENFORCE_HOSTS, Collections.emptyList()); + } } diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java index 30573fcbf..2420944f9 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java @@ -22,9 +22,12 @@ package org.apache.guacamole.auth.totp.user; import com.google.common.io.BaseEncoding; import com.google.inject.Inject; import com.google.inject.Provider; +import inet.ipaddr.IPAddress; +import inet.ipaddr.IPAddressString; import java.security.InvalidKeyException; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -311,6 +314,65 @@ public class UserVerificationService { public void verifyIdentity(UserContext context, AuthenticatedUser authenticatedUser) throws GuacamoleException { + // Pull the original HTTP request used to authenticate + Credentials credentials = authenticatedUser.getCredentials(); + HttpServletRequest request = credentials.getRequest(); + + // Get the current client address + IPAddressString clientAddr = new IPAddressString(request.getRemoteAddr()); + + // Ignore anonymous users + if (authenticatedUser.getIdentifier().equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER)) + return; + + // We enforce by default + boolean enforceHost = true; + + // Check for a list of addresses that should be bypassed and iterate + List bypassAddresses = confService.getBypassHosts(); + for (IPAddress bypassAddr : bypassAddresses) { + // If the address contains current client address, flip enforce flag + // and break out + if (clientAddr != null && clientAddr.isIPAddress() + && bypassAddr.getIPVersion().equals(clientAddr.getIPVersion()) + && bypassAddr.contains(clientAddr.getAddress())) { + enforceHost = false; + break; + } + } + + // Check for a list of addresses that should be enforced and iterate + List enforceAddresses = confService.getEnforceHosts(); + + // Only continue processing if the list is not empty + if (!enforceAddresses.isEmpty()) { + + if (clientAddr == null || !clientAddr.isIPAddress()) { + logger.warn("Client address is not valid, " + + "MFA will be enforced."); + enforceHost = true; + } + + else { + // With addresses set, this default changes to false. + enforceHost = false; + + for (IPAddress enforceAddr : enforceAddresses) { + + // If there's a match, flip the enforce flag and break out of the loop + if (enforceAddr.getIPVersion().equals(clientAddr.getIPVersion()) + && enforceAddr.contains(clientAddr.getAddress())) { + enforceHost = true; + break; + } + } + } + } + + // If the enforce flag has been changed, exit, bypassing TOTP MFA. + if (!enforceHost) + return; + // Ignore anonymous users String username = authenticatedUser.getIdentifier(); if (username.equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER)) @@ -325,10 +387,6 @@ public class UserVerificationService { if (key == null) return; - // Pull the original HTTP request used to authenticate - Credentials credentials = authenticatedUser.getCredentials(); - HttpServletRequest request = credentials.getRequest(); - // Retrieve TOTP from request String code = request.getParameter(AuthenticationCodeField.PARAMETER_NAME); From 2ecad02fe1a90575892f59550c4233a6318db5f9 Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Sat, 18 Nov 2023 19:14:02 -0500 Subject: [PATCH 4/4] GUACAMOLE-1855: Use common code for checking for IP in list. --- .../auth/duo/UserVerificationService.java | 47 +++++----------- .../totp/user/UserVerificationService.java | 53 ++++++------------- 2 files changed, 30 insertions(+), 70 deletions(-) diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java index b763f6f30..d2a6fa74d 100644 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java +++ b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java @@ -40,6 +40,7 @@ import org.apache.guacamole.language.TranslatableMessage; import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.credentials.CredentialsInfo; +import org.apache.guacamole.properties.IPAddressListProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -120,52 +121,30 @@ public class UserVerificationService { if (username == null || username.equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER)) return; - // We enforce by default - boolean enforceHost = true; - - // Check for a list of addresses that should be bypassed and iterate + // Pull address lists to check from configuration. Note that the enforce + // list will override the bypass list, which means that, if the client + // address happens to be in both lists, Duo MFA will be enforced. List bypassAddresses = confService.getBypassHosts(); - for (IPAddress bypassAddr : bypassAddresses) { - - // If the address contains current client address, flip enforce flag - // and break out - if (clientAddr != null && clientAddr.isIPAddress() - && bypassAddr.getIPVersion().equals(clientAddr.getIPVersion()) - && bypassAddr.contains(clientAddr)) { - enforceHost = false; - break; - } - } - - // Check for a list of addresses that should be enforced and iterate List enforceAddresses = confService.getEnforceHosts(); + // Check if the bypass list contains the client address, and set the + // enforce flag to the opposite. + boolean enforceHost = !(IPAddressListProperty.addressListContains(bypassAddresses, clientAddr)); + // Only continue processing if the list is not empty if (!enforceAddresses.isEmpty()) { // If client address is not available or invalid, MFA will // be enforced. - if (clientAddr == null || !clientAddr.isIPAddress()) { + if (clientAddr == null || !clientAddr.isIPAddress()) enforceHost = true; - } - else { - // With addresses set, this default changes to false. - enforceHost = false; - - for (IPAddress enforceAddr : enforceAddresses) { - - // If there's a match, flip the enforce flag and break out of the loop - if (enforceAddr.getIPVersion().equals(clientAddr.getIPVersion()) - && enforceAddr.contains(clientAddr)) { - enforceHost = true; - break; - } - } - } + // Check the enforce list for the client address and set enforcement flag. + else + enforceHost = IPAddressListProperty.addressListContains(enforceAddresses, clientAddr); } - // If the enforce flag has been changed, exit, bypassing Duo MFA. + // If the enforce flag is not true, bypass Duo MFA. if (!enforceHost) return; diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java index 2420944f9..e08042551 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java @@ -47,6 +47,7 @@ import org.apache.guacamole.net.auth.User; import org.apache.guacamole.net.auth.UserContext; import org.apache.guacamole.net.auth.UserGroup; import org.apache.guacamole.net.auth.credentials.CredentialsInfo; +import org.apache.guacamole.properties.IPAddressListProperty; import org.apache.guacamole.totp.TOTPGenerator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -319,57 +320,37 @@ public class UserVerificationService { HttpServletRequest request = credentials.getRequest(); // Get the current client address - IPAddressString clientAddr = new IPAddressString(request.getRemoteAddr()); + IPAddress clientAddr = new IPAddressString(request.getRemoteAddr()).getAddress(); // Ignore anonymous users if (authenticatedUser.getIdentifier().equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER)) return; - // We enforce by default - boolean enforceHost = true; - - // Check for a list of addresses that should be bypassed and iterate + // Pull address lists to check from configuration. Note that the enforce + // list will override the bypass list, which means that, if the client + // address happens to be in both lists, Duo MFA will be enforced. List bypassAddresses = confService.getBypassHosts(); - for (IPAddress bypassAddr : bypassAddresses) { - // If the address contains current client address, flip enforce flag - // and break out - if (clientAddr != null && clientAddr.isIPAddress() - && bypassAddr.getIPVersion().equals(clientAddr.getIPVersion()) - && bypassAddr.contains(clientAddr.getAddress())) { - enforceHost = false; - break; - } - } - - // Check for a list of addresses that should be enforced and iterate List enforceAddresses = confService.getEnforceHosts(); + // Check the bypass list for the client address, and set the enforce + // flag to the opposite. + boolean enforceHost = !(IPAddressListProperty.addressListContains(bypassAddresses, clientAddr)); + // Only continue processing if the list is not empty if (!enforceAddresses.isEmpty()) { - if (clientAddr == null || !clientAddr.isIPAddress()) { - logger.warn("Client address is not valid, " - + "MFA will be enforced."); + // If client address is not available or invalid, MFA will + // be enforced. + if (clientAddr == null || !clientAddr.isIPAddress()) enforceHost = true; - } - else { - // With addresses set, this default changes to false. - enforceHost = false; - - for (IPAddress enforceAddr : enforceAddresses) { - - // If there's a match, flip the enforce flag and break out of the loop - if (enforceAddr.getIPVersion().equals(clientAddr.getIPVersion()) - && enforceAddr.contains(clientAddr.getAddress())) { - enforceHost = true; - break; - } - } - } + // Check the enforce list and set the flag if the client address + // is found in the list. + else + enforceHost = IPAddressListProperty.addressListContains(enforceAddresses, clientAddr); } - // If the enforce flag has been changed, exit, bypassing TOTP MFA. + // If the enforce flag is not true, bypass TOTP MFA. if (!enforceHost) return;