This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 4a0ca2071d3 An ICMP ACL rule should not be able to have code and type
null (#8464)
4a0ca2071d3 is described below
commit 4a0ca2071d3b1aef055e6661de4707ee5a0b34fd
Author: Gabriel Pordeus Santos <[email protected]>
AuthorDate: Wed Feb 14 13:59:29 2024 -0300
An ICMP ACL rule should not be able to have code and type null (#8464)
---
.../cloud/network/vpc/NetworkACLServiceImpl.java | 35 ++++++--
.../network/vpc/NetworkACLServiceImplTest.java | 98 +++++++++++++++++++---
2 files changed, 113 insertions(+), 20 deletions(-)
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 92784e27d48..94b5ea91ce2 100644
--- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
+++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
@@ -860,14 +860,7 @@ public class NetworkACLServiceImpl extends ManagerBase
implements NetworkACLServ
if (!isPartialUpgrade || StringUtils.isNotBlank(protocol)) {
networkACLItemVo.setProtocol(protocol);
}
- Integer icmpCode = updateNetworkACLItemCmd.getIcmpCode();
- if (!isPartialUpgrade || icmpCode != null) {
- networkACLItemVo.setIcmpCode(icmpCode);
- }
- Integer icmpType = updateNetworkACLItemCmd.getIcmpType();
- if (!isPartialUpgrade || icmpType != null) {
- networkACLItemVo.setIcmpType(icmpType);
- }
+ updateIcmpCodeAndType(isPartialUpgrade, updateNetworkACLItemCmd,
networkACLItemVo);
String action = updateNetworkACLItemCmd.getAction();
if (!isPartialUpgrade || StringUtils.isNotBlank(action)) {
Action aclRuleAction =
validateAndCreateNetworkAclRuleAction(action);
@@ -891,6 +884,32 @@ public class NetworkACLServiceImpl extends ManagerBase
implements NetworkACLServ
}
}
+ protected void updateIcmpCodeAndType (boolean isPartialUpgrade,
UpdateNetworkACLItemCmd updateNetworkACLItemCmd, NetworkACLItemVO
networkACLItemVo) {
+ Integer icmpCode = updateNetworkACLItemCmd.getIcmpCode();
+ Integer icmpType = updateNetworkACLItemCmd.getIcmpType();
+
+ if (!isPartialUpgrade) {
+ updateIcmpCodeAndTypeFullUpgrade(icmpCode, icmpType,
networkACLItemVo);
+ return;
+ }
+ if (icmpCode != null) {
+ networkACLItemVo.setIcmpCode(icmpCode);
+ }
+ if (icmpType != null) {
+ networkACLItemVo.setIcmpType(icmpType);
+ }
+ }
+
+ private void updateIcmpCodeAndTypeFullUpgrade (Integer icmpCode, Integer
icmpType, NetworkACLItemVO networkACLItemVo) {
+ if
(networkACLItemVo.getProtocol().equalsIgnoreCase(NetUtils.ICMP_PROTO)) {
+ networkACLItemVo.setIcmpCode(icmpCode != null ? icmpCode : -1);
+ networkACLItemVo.setIcmpType(icmpType != null ? icmpType : -1);
+ } else {
+ networkACLItemVo.setIcmpCode(null);
+ networkACLItemVo.setIcmpType(null);
+ }
+ }
+
/**
* We validate the network ACL rule ID provided. If not ACL rule is found
with the given Id an {@link InvalidParameterValueException} is thrown.
* If an ACL rule is found, we return the clone of the rule to avoid
messing up with CGlib enhanced objects that might be linked to database entries.
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 cd1745cc2ef..bce081d4319 100644
--- a/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
+++ b/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
@@ -31,6 +31,7 @@ import java.util.Map;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.network.vpc.dao.VpcDao;
+import com.cloud.utils.net.NetUtils;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
@@ -772,8 +773,6 @@ public class NetworkACLServiceImplTest {
Mockito.when(updateNetworkACLItemCmdMock.getSourcePortEnd()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getSourceCidrList()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getProtocol()).thenReturn(null);
-
Mockito.when(updateNetworkACLItemCmdMock.getIcmpCode()).thenReturn(null);
-
Mockito.when(updateNetworkACLItemCmdMock.getIcmpType()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getAction()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getTrafficType()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getCustomId()).thenReturn(null);
@@ -789,8 +788,7 @@ public class NetworkACLServiceImplTest {
Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setSourcePortEnd(Mockito.anyInt());
Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setSourceCidrList(Mockito.anyList());
Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setProtocol(Mockito.anyString());
- Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setIcmpCode(Mockito.anyInt());
- Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setIcmpType(Mockito.anyInt());
+
Mockito.verify(networkAclServiceImpl).updateIcmpCodeAndType(Mockito.any(Boolean.class),
Mockito.any(), Mockito.any());
Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setAction(Mockito.any(Action.class));
Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setTrafficType(Mockito.any(TrafficType.class));
Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setUuid(Mockito.anyString());
@@ -808,14 +806,13 @@ public class NetworkACLServiceImplTest {
Mockito.when(updateNetworkACLItemCmdMock.getSourcePortEnd()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getSourceCidrList()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getProtocol()).thenReturn(null);
-
Mockito.when(updateNetworkACLItemCmdMock.getIcmpCode()).thenReturn(null);
-
Mockito.when(updateNetworkACLItemCmdMock.getIcmpType()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getAction()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getTrafficType()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getCustomId()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.getReason()).thenReturn(null);
Mockito.when(updateNetworkACLItemCmdMock.isDisplay()).thenReturn(false);
+ Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("");
networkAclServiceImpl.transferDataToNetworkAclRulePojo(updateNetworkACLItemCmdMock,
networkAclItemVoMock, networkAclMock);
@@ -824,8 +821,7 @@ public class NetworkACLServiceImplTest {
Mockito.verify(networkAclItemVoMock,
Mockito.times(1)).setSourcePortEnd(nullable(Integer.class));
Mockito.verify(networkAclItemVoMock,
Mockito.times(1)).setSourceCidrList(nullable(List.class));
Mockito.verify(networkAclItemVoMock,
Mockito.times(1)).setProtocol(nullable(String.class));
- Mockito.verify(networkAclItemVoMock,
Mockito.times(1)).setIcmpCode(nullable(Integer.class));
- Mockito.verify(networkAclItemVoMock,
Mockito.times(1)).setIcmpType(nullable(Integer.class));
+
Mockito.verify(networkAclServiceImpl).updateIcmpCodeAndType(Mockito.any(Boolean.class),
Mockito.any(), Mockito.any());
Mockito.verify(networkAclItemVoMock,
Mockito.times(1)).setAction(nullable(Action.class));
Mockito.verify(networkAclItemVoMock,
Mockito.times(1)).setTrafficType(nullable(TrafficType.class));
Mockito.verify(networkAclItemVoMock,
Mockito.times(0)).setUuid(nullable(String.class));
@@ -845,14 +841,13 @@ public class NetworkACLServiceImplTest {
Mockito.when(updateNetworkACLItemCmdMock.getSourceCidrList()).thenReturn(cidrsList);
Mockito.when(updateNetworkACLItemCmdMock.getProtocol()).thenReturn("all");
- Mockito.when(updateNetworkACLItemCmdMock.getIcmpCode()).thenReturn(5);
- Mockito.when(updateNetworkACLItemCmdMock.getIcmpType()).thenReturn(6);
Mockito.when(updateNetworkACLItemCmdMock.getAction()).thenReturn("deny");
Mockito.when(updateNetworkACLItemCmdMock.getTrafficType()).thenReturn(TrafficType.Egress);
Mockito.when(updateNetworkACLItemCmdMock.getCustomId()).thenReturn("customUuid");
Mockito.when(updateNetworkACLItemCmdMock.getReason()).thenReturn("reason");
Mockito.when(updateNetworkACLItemCmdMock.isDisplay()).thenReturn(true);
+ Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("");
networkAclServiceImpl.transferDataToNetworkAclRulePojo(updateNetworkACLItemCmdMock,
networkAclItemVoMock, networkAclMock);
@@ -861,8 +856,7 @@ public class NetworkACLServiceImplTest {
Mockito.verify(networkAclItemVoMock).setSourcePortEnd(24);
Mockito.verify(networkAclItemVoMock).setSourceCidrList(cidrsList);
Mockito.verify(networkAclItemVoMock).setProtocol("all");
- Mockito.verify(networkAclItemVoMock).setIcmpCode(5);
- Mockito.verify(networkAclItemVoMock).setIcmpType(6);
+
Mockito.verify(networkAclServiceImpl).updateIcmpCodeAndType(Mockito.any(Boolean.class),
Mockito.any(), Mockito.any());
Mockito.verify(networkAclItemVoMock).setAction(Action.Deny);
Mockito.verify(networkAclItemVoMock).setTrafficType(TrafficType.Egress);
Mockito.verify(networkAclItemVoMock).setUuid("customUuid");
@@ -871,6 +865,86 @@ public class NetworkACLServiceImplTest {
Mockito.verify(networkAclServiceImpl).validateAndCreateNetworkAclRuleAction("deny");
}
+ private void setUpdateICMPCodeAndTypeTest(String protocol, Integer
icmpCode, Integer icmpType) {
+ Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn(protocol);
+
Mockito.when(updateNetworkACLItemCmdMock.getIcmpCode()).thenReturn(icmpCode);
+
Mockito.when(updateNetworkACLItemCmdMock.getIcmpType()).thenReturn(icmpType);
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestIsPartialUpgradeIsICMPNotNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest(NetUtils.ICMP_PROTO, 5, 5);
+ networkAclServiceImpl.updateIcmpCodeAndType(true,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock).setIcmpCode(5);
+ Mockito.verify(networkAclItemVoMock).setIcmpType(5);
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestIsPartialUpgradeIsICMPNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest(NetUtils.ICMP_PROTO, null, null);
+ networkAclServiceImpl.updateIcmpCodeAndType(true,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock,
Mockito.never()).setIcmpCode(nullable(Integer.class));
+ Mockito.verify(networkAclItemVoMock,
Mockito.never()).setIcmpType(nullable(Integer.class));
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestIsPartialUpgradeNotICMPNotNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest("", 5, 5);
+ networkAclServiceImpl.updateIcmpCodeAndType(true,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock).setIcmpCode(5);
+ Mockito.verify(networkAclItemVoMock).setIcmpType(5);
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestIsPartialUpgradeNotICMPNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest("", null, null);
+ networkAclServiceImpl.updateIcmpCodeAndType(true,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock,
Mockito.never()).setIcmpCode(Mockito.any());
+ Mockito.verify(networkAclItemVoMock,
Mockito.never()).setIcmpType(Mockito.any());
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestNotPartialUpgradeIsICMPNotNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest(NetUtils.ICMP_PROTO, 5, 5);
+ networkAclServiceImpl.updateIcmpCodeAndType(false,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock).setIcmpCode(5);
+ Mockito.verify(networkAclItemVoMock).setIcmpType(5);
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestNotPartialUpgradeIsICMPNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest(NetUtils.ICMP_PROTO, null, null);
+ networkAclServiceImpl.updateIcmpCodeAndType(false,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock).setIcmpCode(-1);
+ Mockito.verify(networkAclItemVoMock).setIcmpType(-1);
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestNotPartialUpgradeNotICMPNotNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest("", 5, 5);
+
+ networkAclServiceImpl.updateIcmpCodeAndType(false,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock).setIcmpCode(null);
+ Mockito.verify(networkAclItemVoMock).setIcmpType(null);
+ }
+
+ @Test
+ public void
updateICMPCodeAndTypeTestNotPartialUpgradeNotICMPNullCodeAndType() {
+ setUpdateICMPCodeAndTypeTest("", null, null);
+
+ networkAclServiceImpl.updateIcmpCodeAndType(false,
updateNetworkACLItemCmdMock, networkAclItemVoMock);
+
+ Mockito.verify(networkAclItemVoMock).setIcmpCode(null);
+ Mockito.verify(networkAclItemVoMock).setIcmpType(null);
+ }
+
@Test
public void updateNetworkACLTestParametersNotNull() {
String name = "name";
