Allow port forwarding rules for the same public port but different CIDRs

Author: winterhazel

server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java

@@ -402,9 +402,10 @@ public void detectRulesConflict(FirewallRule newRule) throws NetworkRuleConflict
                 ((rule.getPurpose() == Purpose.Firewall || newRule.getPurpose() == Purpose.Firewall) && ((newRule.getPurpose() != rule.getPurpose()) || (!newRule.getProtocol()
                             .equalsIgnoreCase(rule.getProtocol()))));
 
-            // if both rules are firewall and their cidrs are different, we can skip port ranges verification
-            boolean bothRulesFirewall = (rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall);
+            // if both rules are firewall/port forwarding and their cidrs are different, we can skip port ranges verification
             boolean duplicatedCidrs = false;
+
+            boolean bothRulesFirewall = (rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall);
             if (bothRulesFirewall) {
                 _firewallDao.loadSourceCidrs(rule);
                 _firewallDao.loadSourceCidrs((FirewallRuleVO)newRule);
@@ -418,6 +419,18 @@ public void detectRulesConflict(FirewallRule newRule) throws NetworkRuleConflict
                 duplicatedCidrs = (detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList()) && detectConflictingCidrs(rule.getDestinationCidrList(), newRule.getDestinationCidrList()));
             }
 
+            boolean bothRulesPortForwarding = rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.PortForwarding;
+            if (bothRulesPortForwarding) {
+                _firewallDao.loadSourceCidrs(rule);
+                _firewallDao.loadSourceCidrs((FirewallRuleVO) newRule);
+
+                if (rule.getSourceCidrList() == null || newRule.getSourceCidrList() == null) {
+                    continue;
+                }
+
+                duplicatedCidrs = detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList());
+            }
+
             if (!oneOfRulesIsFirewall) {
                 if (rule.getPurpose() == Purpose.StaticNat && newRule.getPurpose() != Purpose.StaticNat) {
                     throw new NetworkRuleConflictException("There is 1 to 1 Nat rule specified for the ip address id=" + newRule.getSourceIpAddressId());
@@ -452,7 +465,7 @@ public void detectRulesConflict(FirewallRule newRule) throws NetworkRuleConflict
             if (!notNullPorts) {
                 continue;
             } else if (!oneOfRulesIsFirewall &&
-                !(bothRulesFirewall && !duplicatedCidrs) &&
+                !((bothRulesFirewall || bothRulesPortForwarding) && !duplicatedCidrs) &&
                 ((rule.getSourcePortStart().intValue() <= newRule.getSourcePortStart().intValue() &&
                     rule.getSourcePortEnd().intValue() >= newRule.getSourcePortStart().intValue()) ||
                     (rule.getSourcePortStart().intValue() <= newRule.getSourcePortEnd().intValue() &&

systemvm/debian/opt/cloud/bin/cs_forwardingrules.py

@@ -78,4 +78,18 @@ def ruleCompare(ruleA, ruleB):
         return ruleA["public_ip"] == ruleB["public_ip"]
     elif ruleA["type"] == "forward":
         return ruleA["public_ip"] == ruleB["public_ip"] and ruleA["public_ports"] == ruleB["public_ports"] \
-            and ruleA["protocol"] == ruleB["protocol"]
+            and ruleA["protocol"] == ruleB["protocol"] and cidrsConflict(ruleA.get("source_cidr_list"), ruleB.get("source_cidr_list"))
+
+# Same validation as in com.cloud.network.firewall.FirewallManagerImpl.detectConflictingCidrs
+def cidrsConflict(cidrListA, cidrListB):
+    if not cidrListA and not cidrListB:
+        return True
+    if not cidrListA:
+        return False
+    if not cidrListB:
+        return False
+
+    cidrListA = set(cidrListA.split(","))
+    cidrListB = set(cidrListB.split(","))
+
+    return len(cidrListA.intersection(cidrListB)) > 0