diff --git a/server/src/main/java/com/cloud/network/Ipv6ServiceImpl.java b/server/src/main/java/com/cloud/network/Ipv6ServiceImpl.java index 464f8c90ebb6..410b252fd8e4 100644 --- a/server/src/main/java/com/cloud/network/Ipv6ServiceImpl.java +++ b/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)); diff --git a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java index 4aee5fef48a6..65837f308ae2 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/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); diff --git a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java index 94e79ec6ea75..b6baf1fcf85f 100644 --- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java +++ b/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)); } } diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index ee888a61adc5..0dede6b0471c 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/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); diff --git a/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java b/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java index b24136972ad8..9ac76db95708 100644 --- a/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java +++ b/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); diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index d89d9fa2d93c..33f9f0e362ed 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/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); } diff --git a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java index 5c9d41f90a25..6843960fa7a3 100644 --- a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java +++ b/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)); + } }