Validate ACL, firewall and routing rule protocols against the list, not as a substring

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

@@ -619,7 +619,8 @@ protected void validateProtocol(NetworkACLItemVO networkACLItemVO) {
                 throw new InvalidParameterValueException("Invalid protocol number: " + protoNumber);
             }
         } else {
-            if (!supportedProtocolsForAclRules.contains(protocol.toLowerCase())) {
+            // one of the names, not just part of the string: "cp" or "dp,ic" must not pass
+            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);

server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java

@@ -612,6 +612,32 @@ public void validateProtocolTestProtocolIsStringInvalid() {
         networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
     }
 
+    @Test
+    public void validateProtocolTestProtocolIsPartOfTheSupportedList() {
+        for (String protocol : new String[] {"cp", "t", "dp,ic", ",", "tcp,udp"}) {
+            Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn(protocol);
+            try {
+                networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+                Assert.fail("protocol [" + protocol + "] should have been rejected");
+            } catch (InvalidParameterValueException e) {
+                // expected
+            }
+        }
+    }
+
+    @Test
+    public void validateProtocolTestProtocolNameInAnyCase() {
+        Mockito.doNothing().when(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(null);
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(null);
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(null);
+        Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(null);
+        for (String protocol : new String[] {"TCP", "Udp", "ICMP", "All"}) {
+            Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn(protocol);
+            networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+        }
+    }
+
     @Test
     public void validateProtocolTestProtocolIsStringValid() {
         Mockito.doNothing().when(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);

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));
+    }
 }