ZCS-8286 Add whitelist (blacklist exceptions) for FeedManager. (#993)

* Improvements on FeedManager blacklist logging.
Zimbra · Dec 9, 2019 · 37e5d58 · 37e5d58
1 parent 19b2681
commit 37e5d58
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 14 deletions.
diff --git a/common/src/java/com/zimbra/common/localconfig/LC.java b/common/src/java/com/zimbra/common/localconfig/LC.java
@@ -1307,6 +1307,9 @@ public enum PUBLIC_SHARE_VISIBILITY { samePrimaryDomain, all, none };
     // Feed Manager comma-separated blacklist. addresses can be CIDR notation, or single ip. no wild-cards (default blacklist private networks)
     public static final KnownKey zimbra_feed_manager_blacklist = KnownKey.newKey("10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,fd00::/8");
 
+    // Feed Manager comma-separated whitelist (exceptions to the blacklist). addresses can be CIDR notation, or single ip. no wild-cards
+    public static final KnownKey zimbra_feed_manager_whitelist = KnownKey.newKey("");
+
     // Zimbra valid class list to de-serialize
     public static final KnownKey zimbra_deserialize_classes = KnownKey.newKey("");
 

diff --git a/store/src/java-test/com/zimbra/cs/service/FeedManagerTest.java b/store/src/java-test/com/zimbra/cs/service/FeedManagerTest.java
@@ -29,6 +29,7 @@
 import org.apache.http.client.utils.URIBuilder;
 import org.eclipse.jetty.http.HttpStatus;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -43,6 +44,12 @@
 
 public class FeedManagerTest {
 
+    @Before
+    public void setUp() {
+        LC.zimbra_feed_manager_blacklist.setDefault("10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,fd00::/8");
+        LC.zimbra_feed_manager_whitelist.setDefault("");
+    }
+
     @BeforeClass
     public static void init() throws Exception {
         MailboxTestUtil.initServer();
@@ -248,6 +255,87 @@ public void testIsBlockedFeedAddressDefaultBlacklist() throws Exception {
         Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.15.150.140/feed")));
     }
 
+    @Test
+    public void testIsBlockedFeedAddressDefaultBlacklistWithWhitelistedIp() throws Exception {
+        LC.zimbra_feed_manager_whitelist.setDefault("192.168.1.106");
+
+        Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.1.106/feed")));
+        Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.1.106:8080/feed")));
+        Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://user:[email protected]/feed")));
+
+        // loopback
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://localhost/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://localhost:8085/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://127.0.0.1/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://127.0.0.1:8085/feed")));
+
+        // private
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://172.16.150.140/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://172.25.150.140/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://user:[email protected]/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.5.1:8080/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.166.150/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.166.150:8081/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.0.0.1/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.15.150.140/feed")));
+    }
+
+    @Test
+    public void testIsBlockedFeedAddressDefaultBlacklistWithWhitelistedRange() throws Exception {
+        LC.zimbra_feed_manager_whitelist.setDefault("192.168.100.0/25");
+
+        Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.100.0/feed")));
+        for (int i = 1; i < 128; i++) {
+            Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.100." + i + "/feed")));
+        }
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.100.128/feed")));
+
+        // loopback
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://localhost/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://localhost:8085/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://127.0.0.1/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://127.0.0.1:8085/feed")));
+
+        // private
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://172.16.150.140/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://172.25.150.140/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://user:[email protected]/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.5.1:8080/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.166.150/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.166.150:8081/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.0.0.1/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.15.150.140/feed")));
+    }
+
+    @Test
+    public void testIsBlockedFeedAddressDefaultBlacklistWithWhitelistedMultiple() throws Exception {
+        LC.zimbra_feed_manager_whitelist.setDefault("192.168.100.0/25,192.168.105.122,10.12.150.101");
+
+        Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.105.122/feed")));
+        Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.12.150.101/feed")));
+        Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.100.0/feed")));
+        for (int i = 1; i < 128; i++) {
+            Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.100." + i + "/feed")));
+        }
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.100.128/feed")));
+
+        // loopback
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://localhost/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://localhost:8085/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://127.0.0.1/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://127.0.0.1:8085/feed")));
+
+        // private
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://172.16.150.140/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://172.25.150.140/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://user:[email protected]/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.5.1:8080/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.166.150/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://192.168.166.150:8081/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.0.0.1/feed")));
+        Assert.assertTrue(FeedManager.isBlockedFeedAddress(new URIBuilder("http://10.15.150.140/feed")));
+    }
+
     @Test
     public void testIsBlockedFeedAddressPublicBlacklisted() throws Exception {
         Assert.assertFalse(FeedManager.isBlockedFeedAddress(new URIBuilder("http://198.51.100.230:8081/feed")));

diff --git a/store/src/java/com/zimbra/cs/service/FeedManager.java b/store/src/java/com/zimbra/cs/service/FeedManager.java
@@ -274,27 +274,39 @@ protected static boolean isAddressInRange(InetAddress targetAddress, String pref
     }
 
     /**
-     * Returns true if target address is link-local, loopback, or blacklisted.
+     * Returns true if target address is not whitelisted,
+     * and is link-local, loopback, or blacklisted.
      * @param url The target
      * @return True if address is blocked for feed manager
      */
     protected static boolean isBlockedFeedAddress(URIBuilder url) {
-        String blacklistString = LC.zimbra_feed_manager_blacklist.value();
-        List<String> blacklist = new ArrayList<String>();
-        if (!StringUtil.isNullOrEmpty(blacklistString)) {
-            blacklist.addAll(Arrays.asList(blacklistString.split(",")));
-        }
+        InetAddress targetAddress;
         try {
-            InetAddress targetAddress = InetAddress.getByName(url.getHost());
-            return targetAddress.isAnyLocalAddress()
-                || targetAddress.isLinkLocalAddress()
-                || targetAddress.isLoopbackAddress()
-                || blacklist.stream()
-                    .anyMatch(ip -> isAddressInRange(targetAddress, ip));
+            targetAddress = InetAddress.getByName(url.getHost());
         } catch (UnknownHostException e) {
             ZimbraLog.misc.warn("unable to identify feed manager target url host: %s", url);
+            return false;
+        }
+
+        String whitelistString = LC.zimbra_feed_manager_whitelist.value();
+        List<String> whitelist = new ArrayList<String>();
+        if (!StringUtil.isNullOrEmpty(whitelistString)) {
+            whitelist.addAll(Arrays.asList(whitelistString.trim().split(",")));
+        }
+        if (whitelist.stream().anyMatch(ip -> isAddressInRange(targetAddress, ip))) {
+            return false;
+        }
+
+        String blacklistString = LC.zimbra_feed_manager_blacklist.value();
+        List<String> blacklist = new ArrayList<String>();
+        if (!StringUtil.isNullOrEmpty(blacklistString)) {
+            blacklist.addAll(Arrays.asList(blacklistString.trim().split(",")));
         }
-        return false;
+        return targetAddress.isAnyLocalAddress()
+            || targetAddress.isLinkLocalAddress()
+            || targetAddress.isLoopbackAddress()
+            || blacklist.stream()
+                .anyMatch(ip -> isAddressInRange(targetAddress, ip));
     }
 
     private static RemoteDataInfo retrieveRemoteData(String url, Folder.SyncData fsd)
@@ -342,7 +354,12 @@ private static RemoteDataInfo retrieveRemoteData(String url, Folder.SyncData fsd
 
                 // validate target address (also handles followed `location` header addresses)
                 if (isBlockedFeedAddress(httpurl)) {
-                    throw ServiceException.INVALID_REQUEST(String.format("invalid url for feed: %s", url), null);
+                    ZimbraLog.misc.info(
+                        "Feed ip address blocked: %s. See localconfig for comma-separated ip list configuration: "
+                        + "zimbra_feed_manager_blacklist, zimbra_feed_manager_whitelist",
+                        url);
+                    throw ServiceException.INVALID_REQUEST(
+                        String.format("Address for feed is blocked: %s. See mailbox logs for details.", url), null);
                 }
                 // username and password are encoded in the URL as http://user:pass@host/...
                 if (url.indexOf('@') != -1) {