Feature/forge 633

common/src/main/java/org/onehippo/forge/ipfilter/common/AuthObject.java

@@ -43,7 +43,9 @@ public class AuthObject {
     private final Set<String> hosts;
     private final Set<String> ranges;
     private final Set<String> ignoredPaths;
+    private final Set<String> trustedProxies;
     private final Set<IpMatcher> ipMatchers;
+    private final Set<IpMatcher> trustedProxyMatchers;
     private final Map<String, Set<String>> ignoredHeaders;
     private final List<Pattern> hostPatterns;
     private final List<Pattern> ignoredPathPatterns;
@@ -55,30 +57,36 @@ private AuthObject() {
         this.ignoredPaths = Collections.emptySet();
         this.hosts = Collections.emptySet();
         this.ranges = Collections.emptySet();
+        this.trustedProxies = Collections.emptySet();
         this.ignoredHeaders = Collections.emptyMap();
         this.ignoredPathPatterns = Collections.emptyList();
         this.hostPatterns = Collections.emptyList();
         this.ipMatchers = Collections.emptySet();
+        this.trustedProxyMatchers = Collections.emptySet();
         this.allowCmsUsers = false;
         this.mustMatchAll = false;
         this.forwardedForHeader = IpFilterConstants.HEADER_X_FORWARDED_FOR;
     }
 
     public AuthObject(final Set<String> ignoredPaths, final Set<String> hosts,
                       final Set<String> ranges, final Map<String, Set<String>> ignoredHeaders,
-                      final boolean allowCmsUsers, final String forwardHeader, final boolean cacheEnabled, final boolean mustMatchAll) {
+                      final boolean allowCmsUsers, final String forwardHeader,
+                      final Set<String> trustedProxies,
+                      final boolean cacheEnabled, final boolean mustMatchAll) {
         this.valid = true;
         this.cacheEnabled = cacheEnabled;
         this.ignoredPaths = ignoredPaths;
         this.hosts = hosts;
         this.ranges = ranges;
+        this.trustedProxies = trustedProxies;
         this.allowCmsUsers = allowCmsUsers;
         this.mustMatchAll = mustMatchAll;
         this.forwardedForHeader = forwardHeader;
         this.ignoredHeaders = ImmutableMap.copyOf(ignoredHeaders);
         this.ignoredPathPatterns = parsePatterns();
         this.hostPatterns = parseHostPatterns();
         this.ipMatchers = parseIpMatchers();
+        this.trustedProxyMatchers = parseTrustedProxyMatchers();
     }
 
     public boolean isValid() {
@@ -111,6 +119,10 @@ public String getForwardedForHeader() {
         return forwardedForHeader;
     }
 
+    public Set<IpMatcher> getTrustedProxyMatchers() {
+        return trustedProxyMatchers;
+    }
+
 
     public Map<String, Set<String>> getIgnoredHeaders() {
         return ignoredHeaders;
@@ -156,6 +168,21 @@ private Set<IpMatcher> parseIpMatchers() {
         return ImmutableSet.copyOf(ipMatchers);
     }
 
+    private Set<IpMatcher> parseTrustedProxyMatchers() {
+        final Set<IpMatcher> matchers = new HashSet<>();
+        if (trustedProxies != null) {
+            for (String proxy : trustedProxies) {
+                final IpMatcher matcher = IpMatcher.valueOf(proxy);
+                if (matcher != null) {
+                    matchers.add(matcher);
+                } else {
+                    log.warn("Skipping invalid trusted proxy value: {}", proxy);
+                }
+            }
+        }
+        return ImmutableSet.copyOf(matchers);
+    }
+
     private List<Pattern> parsePatterns() {
         final List<Pattern> patterns = new ArrayList<>();
         for (String ignored : ignoredPaths) {

common/src/main/java/org/onehippo/forge/ipfilter/common/BaseIpFilter.java

@@ -158,7 +158,7 @@ private Status allowed(final HttpServletRequest request) {
             return Status.OK;
         }
 
-        final String ip = IpFilterUtils.getIp(request, authObject.getForwardedForHeader());
+        final String ip = IpFilterUtils.getIp(request, authObject.getForwardedForHeader(), authObject.getTrustedProxyMatchers());
         if (Strings.isNullOrEmpty(ip)) {
             // shouldn't happen
             log.warn("{}: IP was null or empty. Host is {}", this.getClass().getSimpleName(), host);

common/src/main/java/org/onehippo/forge/ipfilter/common/IpFilterConfigLoader.java

@@ -359,7 +359,11 @@ private AuthObject parse(final Node node, final Multimap<String, String> globalS
         final boolean matchAll = JcrUtils.getBooleanProperty(node, IpFilterConstants.CONFIG_MATCH_ALL, false);
         final boolean cacheEnabled = JcrUtils.getBooleanProperty(node, IpFilterConstants.CONFIG_CACHE_ENABLED, true);
 
-        return new AuthObject(ignoredPathSet, hostSet, rangesSet, ignoredHeaders, allowCmsUsers, forwardHeader, cacheEnabled, matchAll);
+        final Set<String> trustedProxiesSet = new HashSet<>();
+        final String[] trustedProxies = JcrUtils.getMultipleStringProperty(node, IpFilterConstants.CONFIG_TRUSTED_PROXIES, ArrayUtils.EMPTY_STRING_ARRAY);
+        Collections.addAll(trustedProxiesSet, trustedProxies);
+
+        return new AuthObject(ignoredPathSet, hostSet, rangesSet, ignoredHeaders, allowCmsUsers, forwardHeader, trustedProxiesSet, cacheEnabled, matchAll);
     }
 
     private Map<String, Set<String>> parseHeaders(final Node root) throws RepositoryException {

common/src/main/java/org/onehippo/forge/ipfilter/common/IpFilterConstants.java

@@ -28,6 +28,7 @@ public final class IpFilterConstants {
     public static final String CONFIG_CACHE_ENABLED = "cache-enabled";
     public static final String CONFIG_FORWARDED_FOR_HEADER = "forwarded-for-header";
     public static final String CONFIG_FORWARDED_HOST_HEADER = "forwarded-host-header";
+    public static final String CONFIG_TRUSTED_PROXIES = "trusted-proxies";
 
     public static final int BASIC_AUTH_PREFIX_LENGTH = "Basic ".length();
 

common/src/main/java/org/onehippo/forge/ipfilter/common/IpFilterUtils.java

@@ -25,6 +25,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.Set;
 
 public final class IpFilterUtils {
 
@@ -47,19 +48,38 @@ public static String getParameter(final FilterConfig filterConfig, final String
     }
 
 
-    public static String getIp(HttpServletRequest request, final String name) {
+    public static String getIp(final HttpServletRequest request, final String name, final Set<IpMatcher> trustedProxies) {
+        final String remoteAddr = request.getRemoteAddr();
         final String headerName = Strings.isNullOrEmpty(name) ? IpFilterConstants.HEADER_X_FORWARDED_FOR : name;
         final String header = request.getHeader(headerName);
         if (Strings.isNullOrEmpty(header)) {
             log.debug("Header: {} was empty", headerName);
-            return request.getRemoteAddr();
+            return remoteAddr;
+        }
+        if (trustedProxies.isEmpty()) {
+            log.warn("Header '{}' is present but no trusted-proxies configured; using remote address to prevent IP spoofing. " +
+                    "Configure '{}' to enable X-Forwarded-For support.", headerName, IpFilterConstants.CONFIG_TRUSTED_PROXIES);
+            return remoteAddr;
+        }
+        if (!isTrustedProxy(remoteAddr, trustedProxies)) {
+            log.debug("Remote address '{}' is not a trusted proxy; ignoring header '{}'", remoteAddr, headerName);
+            return remoteAddr;
         }
         final Iterable<String> ipAddresses = COMMA_SPLITTER.split(header);
         final Iterator<String> iterator = ipAddresses.iterator();
         if (iterator.hasNext()) {
             return iterator.next();
         }
-        return request.getRemoteAddr();
+        return remoteAddr;
+    }
+
+    private static boolean isTrustedProxy(final String remoteAddr, final Set<IpMatcher> trustedProxies) {
+        for (final IpMatcher matcher : trustedProxies) {
+            if (matcher.matches(remoteAddr)) {
+                return true;
+            }
+        }
+        return false;
     }
 
 

common/src/test/java/org/onehippo/forge/ipfilter/common/IpFilterUtilsTest.java

@@ -25,26 +25,100 @@
 import static org.easymock.EasyMock.*;
 import static org.junit.Assert.*;
 
+
+
 public class IpFilterUtilsTest {
 
 
     @Test
     public void testGetIp() {
         final Set<String> E = Collections.emptySet();
-        final AuthObject object = new AuthObject(E, E, E, Collections.emptyMap(), true, null,true, true);
+        final AuthObject object = new AuthObject(E, E, E, Collections.emptyMap(), true, null, Collections.emptySet(), true, true);
         HttpServletRequest request = createMock(HttpServletRequest.class);
         expect(request.getRemoteAddr()).andReturn("127.0.0.1").anyTimes();
         expect(request.getHeader(IpFilterConstants.HEADER_X_FORWARDED_FOR)).andReturn(null).anyTimes();
         replay(request);
-        String header = IpFilterUtils.getIp(request, object.getForwardedForHeader());
+        String header = IpFilterUtils.getIp(request, object.getForwardedForHeader(), object.getTrustedProxyMatchers());
         assertEquals("127.0.0.1", header);
-        // recreate
+        // recreate: XFF present but no trusted proxies configured -> remoteAddr is returned (spoofing prevention)
         request = createMock(HttpServletRequest.class);
+        expect(request.getRemoteAddr()).andReturn("127.0.0.1").anyTimes();
         expect(request.getHeader(IpFilterConstants.HEADER_X_FORWARDED_FOR)).andReturn("localhost").anyTimes();
         replay(request);
-        header = IpFilterUtils.getIp(request, object.getForwardedForHeader());
-        assertEquals("localhost", header);
+        header = IpFilterUtils.getIp(request, object.getForwardedForHeader(), object.getTrustedProxyMatchers());
+        assertEquals("127.0.0.1", header);
+    }
+
+    @Test
+    public void testGetIpXffIgnoredWithNoTrustedProxies() {
+        // XFF present, no trusted proxies configured -> remoteAddr returned to prevent spoofing
+        final HttpServletRequest request = createMock(HttpServletRequest.class);
+        expect(request.getRemoteAddr()).andReturn("192.0.2.100").anyTimes();
+        expect(request.getHeader(IpFilterConstants.HEADER_X_FORWARDED_FOR)).andReturn("203.0.113.50").anyTimes();
+        replay(request);
+
+        final String ip = IpFilterUtils.getIp(request, IpFilterConstants.HEADER_X_FORWARDED_FOR, Collections.emptySet());
+        assertEquals("192.0.2.100", ip);
+    }
+
+    @Test
+    public void testGetIpXffTrustedWhenRemoteAddrIsTrustedProxy() {
+        // XFF present, remoteAddr is a configured trusted proxy -> XFF IP returned
+        final IpMatcher proxyMatcher = IpMatcher.valueOf("10.0.0.1");
+        final Set<IpMatcher> trustedProxies = Collections.singleton(proxyMatcher);
+
+        final HttpServletRequest request = createMock(HttpServletRequest.class);
+        expect(request.getRemoteAddr()).andReturn("10.0.0.1").anyTimes();
+        expect(request.getHeader(IpFilterConstants.HEADER_X_FORWARDED_FOR)).andReturn("203.0.113.50").anyTimes();
+        replay(request);
+
+        final String ip = IpFilterUtils.getIp(request, IpFilterConstants.HEADER_X_FORWARDED_FOR, trustedProxies);
+        assertEquals("203.0.113.50", ip);
+    }
+
+    @Test
+    public void testGetIpXffIgnoredWhenRemoteAddrIsNotTrustedProxy() {
+        // XFF present, remoteAddr is NOT a trusted proxy -> remoteAddr returned (prevent spoofing)
+        final IpMatcher proxyMatcher = IpMatcher.valueOf("10.0.0.1");
+        final Set<IpMatcher> trustedProxies = Collections.singleton(proxyMatcher);
+
+        final HttpServletRequest request = createMock(HttpServletRequest.class);
+        expect(request.getRemoteAddr()).andReturn("192.0.2.100").anyTimes();
+        expect(request.getHeader(IpFilterConstants.HEADER_X_FORWARDED_FOR)).andReturn("203.0.113.50").anyTimes();
+        replay(request);
+
+        final String ip = IpFilterUtils.getIp(request, IpFilterConstants.HEADER_X_FORWARDED_FOR, trustedProxies);
+        assertEquals("192.0.2.100", ip);
+    }
+
+    @Test
+    public void testGetIpXffTrustedWhenRemoteAddrMatchesTrustedProxyCidrRange() {
+        // XFF present, remoteAddr falls within a trusted proxy CIDR range -> XFF IP returned
+        final IpMatcher cidrMatcher = IpMatcher.valueOf("10.0.0.0/24");
+        final Set<IpMatcher> trustedProxies = Collections.singleton(cidrMatcher);
+
+        final HttpServletRequest request = createMock(HttpServletRequest.class);
+        expect(request.getRemoteAddr()).andReturn("10.0.0.5").anyTimes();
+        expect(request.getHeader(IpFilterConstants.HEADER_X_FORWARDED_FOR)).andReturn("203.0.113.50").anyTimes();
+        replay(request);
+
+        final String ip = IpFilterUtils.getIp(request, IpFilterConstants.HEADER_X_FORWARDED_FOR, trustedProxies);
+        assertEquals("203.0.113.50", ip);
+    }
+
+    @Test
+    public void testGetIpXffIgnoredWhenRemoteAddrOutsideTrustedProxyCidrRange() {
+        // XFF present, remoteAddr outside the trusted proxy CIDR range -> remoteAddr returned
+        final IpMatcher cidrMatcher = IpMatcher.valueOf("10.0.0.0/24");
+        final Set<IpMatcher> trustedProxies = Collections.singleton(cidrMatcher);
+
+        final HttpServletRequest request = createMock(HttpServletRequest.class);
+        expect(request.getRemoteAddr()).andReturn("10.0.1.1").anyTimes();
+        expect(request.getHeader(IpFilterConstants.HEADER_X_FORWARDED_FOR)).andReturn("203.0.113.50").anyTimes();
+        replay(request);
 
+        final String ip = IpFilterUtils.getIp(request, IpFilterConstants.HEADER_X_FORWARDED_FOR, trustedProxies);
+        assertEquals("10.0.1.1", ip);
     }
 
     @Test

demo/pom.xml

@@ -60,7 +60,7 @@
 
   <properties>
 
-    <bloomreach.forge.ipfilter.version>5.0.2-SNAPSHOT</bloomreach.forge.ipfilter.version>
+    <bloomreach.forge.ipfilter.version>5.0.3-SNAPSHOT</bloomreach.forge.ipfilter.version>
     <development-module-deploy-dir>shared/lib</development-module-deploy-dir>
 
     <docker.postgres.bind.1>${project.basedir}/target/postgres-data:/var/lib/postgresql/data</docker.postgres.bind.1>

demo/repository-data/application/src/main/resources/hcm-config/configuration/modules/ipfilter.yaml

@@ -3,5 +3,7 @@ definitions:
     /hippo:configuration/hippo:modules/ipfilter/hippo:moduleconfig:
       forwarded-host-header: [X-Forwarded-Host-2]
     /hippo:configuration/hippo:modules/ipfilter/hippo:moduleconfig/localhost:
-      match-all: true
-      cache-enabled: false
+      hostnames: [.*]
+      match-all: false
+      cache-enabled: false
+      allowed-ip-ranges: [10.0.0.100, 203.0.113.50, 127.0.0.1]