Validate firewall and routed rule protocols against the list too

The firewall (port forwarding, static NAT, firewall), IPv6 firewall and
routing firewall rule checks test the protocol against the provider's
supported protocols with the same substring test, so "cp" or "dp,ic"
passed them as well. Add NetUtils.isProtocolInList(), which matches a
whole entry of the comma-separated list in any case, trimming the entries
since some providers list them with a space after the comma ("tcp,udp,
icmp, all"), and use it for these checks and the network ACL one.

Signed-off-by: Brad House <bhouse@nexthop.ai>

Author: bhouse-nexthop

server/src/main/java/com/cloud/network/Ipv6ServiceImpl.java

@@ -583,7 +583,7 @@ public FirewallRule createIpv6FirewallRule(CreateIpv6FirewallRuleCmd cmd) throws
                 supportedProtocols = caps.get(Network.Capability.SupportedProtocols).toLowerCase();
             }
 
-            if (!supportedProtocols.contains(protocol.toLowerCase())) {
+            if (!NetUtils.isProtocolInList(supportedProtocols, protocol)) {
                 throw new InvalidParameterValueException(String.format("Protocol %s is not supported in zone", protocol));
             } else if (!supportedTrafficTypes.contains(trafficType.toString().toLowerCase())) {
                 throw new InvalidParameterValueException(String.format("Traffic Type %s is currently supported by Firewall in network %s", trafficType, network));

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

@@ -603,7 +603,7 @@ public void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer
                 supportedProtocols = caps.get(Capability.SupportedProtocols).toLowerCase();
             }
 
-            if (!supportedProtocols.contains(proto.toLowerCase())) {
+            if (!NetUtils.isProtocolInList(supportedProtocols, proto)) {
                 throw new InvalidParameterValueException("Protocol " + proto + " is not supported in zone " + network.getDataCenterId());
             } else if (proto.equalsIgnoreCase(NetUtils.ICMP_PROTO) && purpose != Purpose.Firewall) {
                 throw new InvalidParameterValueException("Protocol " + proto + " is currently supported only for rules with purpose " + Purpose.Firewall);

server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java

@@ -17,7 +17,6 @@
 package com.cloud.network.vpc;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
@@ -621,7 +620,7 @@ protected void validateProtocol(NetworkACLItemVO networkACLItemVO) {
             }
         } else {
             // one of the names, not just part of the string: "cp" or "dp,ic" must not pass
-            if (!Arrays.asList(supportedProtocolsForAclRules.split(",")).contains(protocol.toLowerCase(Locale.ROOT))) {
+            if (!NetUtils.isProtocolInList(supportedProtocolsForAclRules, protocol)) {
                 throw new InvalidParameterValueException(String.format("Invalid protocol [%s]. Expected one of: [%s]", protocol, supportedProtocolsForAclRules));
             }
         }

server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java

@@ -893,7 +893,7 @@ public FirewallRule createRoutingFirewallRule(CreateRoutingFirewallRuleCmd creat
                 supportedProtocols = caps.get(Network.Capability.SupportedProtocols).toLowerCase();
             }
 
-            if (!supportedProtocols.contains(protocol.toLowerCase())) {
+            if (!NetUtils.isProtocolInList(supportedProtocols, protocol)) {
                 throw new InvalidParameterValueException(String.format("Protocol %s is not supported in zone", protocol));
             } else if (!supportedTrafficTypes.contains(trafficType.toString().toLowerCase())) {
                 throw new InvalidParameterValueException("Traffic Type " + trafficType + " is currently supported by Firewall in network " + networkId);

utils/src/main/java/com/cloud/utils/net/NetUtils.java

@@ -38,6 +38,7 @@
 import java.util.Formatter;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Random;
 import java.util.Set;
 import java.util.SortedSet;
@@ -991,6 +992,24 @@ public static long getCidrSize(final String netmask) {
         return MAX_CIDR - count;
     }
 
+    /**
+     * Whether the protocol is one of the comma-separated protocols, as a whole entry and in any case.
+     * The lists come from provider capabilities, some of which have spaces after their commas; the
+     * protocol itself is taken as given.
+     */
+    public static boolean isProtocolInList(final String protocols, final String protocol) {
+        if (protocols == null || protocol == null) {
+            return false;
+        }
+        final String p = protocol.toLowerCase(Locale.ROOT);
+        for (final String entry : protocols.split(",")) {
+            if (entry.trim().toLowerCase(Locale.ROOT).equals(p)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     public static boolean isValidPort(final int p) {
         return !(p > PORT_RANGE_MAX || p < PORT_RANGE_MIN);
     }

utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java

@@ -955,4 +955,19 @@ public void testVpnIpRange() {
         String cidr = NetUtils.transformCidr(startIp + "/" + cidrSize);
         Assert.assertEquals("10.1.2.0/28", cidr);
     }
+
+    @Test
+    public void testIsProtocolInList() {
+        assertTrue(NetUtils.isProtocolInList("tcp,udp,icmp,all", "tcp"));
+        assertTrue(NetUtils.isProtocolInList("tcp,udp,icmp,all", "ICMP"));
+        // provider capabilities with a space after the comma
+        assertTrue(NetUtils.isProtocolInList("tcp,udp,icmp, all", "all"));
+        assertTrue(NetUtils.isProtocolInList("tcp, udp", "udp"));
+        // part of an entry, or of the list, is not an entry
+        for (String protocol : new String[] {"cp", "t", "dp,ic", ",", "tcp,udp", "", "proxy", " all"}) {
+            assertFalse(protocol, NetUtils.isProtocolInList("tcp,udp,icmp,all,tcp-proxy", protocol));
+        }
+        assertFalse(NetUtils.isProtocolInList(null, "tcp"));
+        assertFalse(NetUtils.isProtocolInList("tcp", null));
+    }
 }