GUACAMOLE-990: Limit maximum number of tracked addresses.

extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java

@@ -53,9 +53,9 @@ public class AuthenticationFailureStatus {
     private final long duration;
 
     /**
-     * Creates an AuthenticationFailureStatus that represents a single failure
-     * and is subject to the given restrictions. Additional failures may be
-     * flagged after creation with {@link #notifyFailed()}.
+     * Creates an AuthenticationFailureStatus that is initialized to zero
+     * failures and is subject to the given restrictions. Additional failures
+     * may be flagged after creation with {@link #notifyFailed()}.
      *
      * @param maxAttempts
      *     The maximum number of failures that may occur before the
@@ -67,7 +67,7 @@ public class AuthenticationFailureStatus {
      */
     public AuthenticationFailureStatus(int maxAttempts, int duration) {
         this.lastFailure = System.nanoTime();
-        this.failureCount = new AtomicInteger(1);
+        this.failureCount = new AtomicInteger(0);
         this.maxAttempts = maxAttempts;
         this.duration = TimeUnit.SECONDS.toNanos(duration);
     }

extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java

@@ -19,8 +19,8 @@
 
 package org.apache.guacamole.auth.ban;
 
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
 import javax.servlet.http.HttpServletRequest;
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.GuacamoleServerException;
@@ -44,8 +44,7 @@ public class AuthenticationFailureTracker {
      * All authentication failures currently being tracked, stored by the
      * associated IP address.
      */
-    private final ConcurrentMap<String, AuthenticationFailureStatus> failures =
-            new ConcurrentHashMap<>();
+    private final Cache<String, AuthenticationFailureStatus> failures;
 
     /**
      * The maximum number of failed authentication attempts allowed before an
@@ -70,8 +69,14 @@ public class AuthenticationFailureTracker {
      * @param banDuration
      *     The length of time that each address should be banned after reaching
      *     the maximum number of failed authentication attempts, in seconds.
+     *
+     * @param maxAddresses
+     *     The maximum number of unique IP addresses that should be tracked
+     *     before discarding older tracked failures.
      */
-    public AuthenticationFailureTracker(int maxAttempts, int banDuration) {
+    public AuthenticationFailureTracker(int maxAttempts, int banDuration,
+            long maxAddresses) {
+
         this.maxAttempts = maxAttempts;
         this.banDuration = banDuration;
 
@@ -93,6 +98,14 @@ public class AuthenticationFailureTracker {
                     banDuration, maxAttempts);
         }
 
+        // Limit maximum number of tracked addresses to configured upper bound
+        this.failures = Caffeine.newBuilder()
+                .maximumSize(maxAddresses)
+                .build();
+
+        logger.info("Up to {} unique addresses will be tracked/banned at any "
+                + " given time.", maxAddresses);
+
     }
 
     /**
@@ -147,10 +160,8 @@ public class AuthenticationFailureTracker {
      */
     private AuthenticationFailureStatus getAuthenticationFailure(String address) {
 
-        AuthenticationFailureStatus newFailure = new AuthenticationFailureStatus(maxAttempts, banDuration);
-        AuthenticationFailureStatus status = failures.putIfAbsent(address, newFailure);
-        if (status == null)
-            return newFailure;
+        AuthenticationFailureStatus status = failures.get(address,
+                (addr) -> new AuthenticationFailureStatus(maxAttempts, banDuration));
 
         status.notifyFailed();
         return status;
@@ -199,7 +210,7 @@ public class AuthenticationFailureTracker {
                     address, status.getFailures(), maxAttempts);
         }
         else
-            status = failures.get(address);
+            status = failures.getIfPresent(address);
 
         if (status != null) {
 
@@ -216,7 +227,7 @@ public class AuthenticationFailureTracker {
             // relevant (all failures are sufficiently old)
             else if (!status.isValid()) {
                 logger.debug("Removing address \"{}\" from tracking as there are no recent authentication failures.", address);
-                failures.remove(address);
+                failures.invalidate(address);
             }
 
         }

extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java

@@ -20,6 +20,7 @@
 package org.apache.guacamole.auth.ban;
 
 import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
 import org.apache.guacamole.environment.Environment;
 import org.apache.guacamole.environment.LocalEnvironment;
 import org.apache.guacamole.net.auth.AbstractAuthenticationProvider;
@@ -27,6 +28,7 @@ import org.apache.guacamole.net.auth.AuthenticatedUser;
 import org.apache.guacamole.net.auth.Credentials;
 import org.apache.guacamole.net.auth.UserContext;
 import org.apache.guacamole.properties.IntegerGuacamoleProperty;
+import org.apache.guacamole.properties.LongGuacamoleProperty;
 
 /**
  * AuthenticationProvider implementation that blocks further authentication
@@ -61,6 +63,20 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide
 
     };
 
+    /**
+     * The maximum number of failed authentication attempts tracked at any
+     * given time. Once this number of addresses is exceeded, the oldest
+     * authentication attempts are rotated off on an LRU basis.
+     */
+    private static final LongGuacamoleProperty MAX_ADDRESSES = new LongGuacamoleProperty() {
+
+        @Override
+        public String getName() {
+            return "ban-max-addresses";
+        }
+
+    };
+
     /**
      * The default maximum number of failed authentication attempts allowed
      * before an address is temporarily banned.
@@ -74,6 +90,13 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide
      */
     private static final int DEFAULT_IP_BAN_DURATION = 300;
 
+    /**
+     * The maximum number of failed authentication attempts tracked at any
+     * given time. Once this number of addresses is exceeded, the oldest
+     * authentication attempts are rotated off on an LRU basis.
+     */
+    private static final long DEFAULT_MAX_ADDRESSES = 10485760;
+
     /**
      * Shared tracker of addresses that have repeatedly failed authentication.
      */
@@ -95,8 +118,15 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide
         Environment environment = LocalEnvironment.getInstance();
         int maxAttempts = environment.getProperty(MAX_ATTEMPTS, DEFAULT_MAX_ATTEMPTS);
         int banDuration = environment.getProperty(IP_BAN_DURATION, DEFAULT_IP_BAN_DURATION);
+        long maxAddresses = environment.getProperty(MAX_ADDRESSES, DEFAULT_MAX_ADDRESSES);
 
-        tracker = new AuthenticationFailureTracker(maxAttempts, banDuration);
+        if (maxAddresses <= 0)
+            throw new GuacamoleServerException("The maximum number of "
+                    + "addresses tracked, as specified by the "
+                    + "\"" +  MAX_ADDRESSES.getName() + "\" property, must be "
+                    + "greater than zero.");
+
+        tracker = new AuthenticationFailureTracker(maxAttempts, banDuration, maxAddresses);
         BanningAuthenticationListener.setAuthenticationFailureTracker(tracker);
 
     }