etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

When creating a new firewall egress rule, the source and destination CIDRs were not being trimmed, this commit changes this to trim both CIDRs from the user input. (#5867) 

Co-authored-by: Joao <JoaoJandre@gitlab.com>
This commit is contained in:
JoaoJandre 2022-02-11 14:06:01 -03:00 committed by GitHub
parent bcd0979a5a
commit 2d8f070ea4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 360 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java
View File

@ -20,6 +20,7 @@ package org.apache.cloudstack.api.command.user.firewall;
import java.util.ArrayList;
import java.util.List;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.RoleType;
@ -242,42 +243,17 @@ public class CreateEgressFirewallRuleCmd extends BaseAsyncCreateCmd implements F
@Override
public void create() {
if (getSourceCidrList() != null) {
String guestCidr = _networkService.getNetwork(getNetworkId()).getCidr();
validateCidrs();
for (String cidr : getSourceCidrList()) {
if (!NetUtils.isValidIp4Cidr(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Source cidrs formatting error " + cidr);
}
if (cidr.equals(NetUtils.ALL_IP4_CIDRS)) {
continue;
}
if (!NetUtils.isNetworkAWithinNetworkB(cidr, guestCidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, cidr + " is not within the guest cidr " + guestCidr);
}
}
}
//Destination CIDR formatting check. Since it's optional param, no need to set a default as in the case of source.
if(destCidrList != null){
for(String cidr : destCidrList){
if(!NetUtils.isValidIp4Cidr(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Destination cidrs formatting error" + cidr);
}
}
}
if (getProtocol().equalsIgnoreCase(NetUtils.ALL_PROTO)) {
if (NetUtils.ALL_PROTO.equalsIgnoreCase(getProtocol())) {
if (getSourcePortStart() != null && getSourcePortEnd() != null) {
throw new InvalidParameterValueException("Do not pass ports to protocol ALL, protocol ALL do not require ports. Unable to create " +
"firewall rule for the network id=" + networkId);
throw new InvalidParameterValueException(String.format("Unable to create firewall rule for the network id=[%d] due to: ports must not be informed to protocol ALL, as it does not require them.", networkId));
}
}
if (getVpcId() != null) {
throw new InvalidParameterValueException("Unable to create firewall rule for the network id=" + networkId +
" as firewall egress rule can be created only for non vpc networks.");
}
throw new InvalidParameterValueException(String.format("Unable to create firewall rule for the network id=[%d] as firewall egress rule can be created only for non VPC networks.", networkId));
}
try {
FirewallRule result = _firewallService.createEgressFirewallRule(this);
@ -286,12 +262,47 @@ public class CreateEgressFirewallRuleCmd extends BaseAsyncCreateCmd implements F
setEntityUuid(result.getUuid());
}
} catch (NetworkRuleConflictException ex) {
s_logger.info("Network rule conflict: " + ex.getMessage());
s_logger.trace("Network Rule Conflict: ", ex);
String message = "Network rule conflict: ";
if (!s_logger.isTraceEnabled()) {
s_logger.info(message + ex.getMessage());
} else {
s_logger.trace(message, ex);
}
throw new ServerApiException(ApiErrorCode.NETWORK_RULE_CONFLICT_ERROR, ex.getMessage());
}
}
protected void validateCidrs() {
if (getSourceCidrList() != null) {
String guestCidr = _networkService.getNetwork(getNetworkId()).getCidr();
for (String cidr : getSourceCidrList()) {
cidr = StringUtils.trim(cidr);
isValidIpCidr(cidr, "Source");
if (cidr.equals(NetUtils.ALL_IP4_CIDRS)) {
continue;
}
if (!NetUtils.isNetworkAWithinNetworkB(cidr, guestCidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR,String.format("[%s] is not within the guest CIDR [%s]. ", cidr, guestCidr));
}
}
}
//Destination CIDR formatting check. Since it's optional param, no need to set a default as in the case of source.
if(destCidrList != null){
for(String cidr : destCidrList){
cidr = StringUtils.trim(cidr);
isValidIpCidr(cidr, "Destination");
}
}
}
private void isValidIpCidr(String cidr, String sourceOrDestination) {
if (!NetUtils.isValidIp4Cidr(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR,String.format("%s CIDRs formatting error [%s].", sourceOrDestination, cidr));
}
}
@Override
public String getEventType() {
return EventTypes.EVENT_FIREWALL_EGRESS_OPEN;

317
api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmdTest.java Normal file
View File

@ -0,0 +1,317 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package org.apache.cloudstack.api.command.user.firewall;
import com.cloud.network.Network;
import com.cloud.network.NetworkService;
import com.cloud.utils.net.NetUtils;
import org.apache.cloudstack.api.ServerApiException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import java.util.ArrayList;
@PrepareForTest(NetUtils.class)
@RunWith(PowerMockRunner.class)
public class CreateEgressFirewallRuleCmdTest {
@Mock
NetworkService networkServiceMock;
@Spy
@InjectMocks
CreateEgressFirewallRuleCmd cmdMock = new CreateEgressFirewallRuleCmd();
@Test
public void validateCidrsTestValidCidrs(){
ArrayList<String> listMock = new ArrayList<>();
String cidrMock = "10.1.1.0/24";
listMock.add(cidrMock);
cmdMock.setSourceCidrList(listMock);
cmdMock.setDestCidrList(listMock);
long networkIdMock = 1234;
Mockito.doReturn(networkIdMock).when(cmdMock).getNetworkId();
Network networkMock = Mockito.mock(Network.class);
Mockito.doReturn(networkMock).when(networkServiceMock).getNetwork(networkIdMock);
Mockito.doReturn(cidrMock).when(networkMock).getCidr();
PowerMockito.mockStatic(NetUtils.class);
PowerMockito.when(NetUtils.isValidIp4Cidr(cidrMock)).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp6Cidr(cidrMock)).thenReturn(false);
PowerMockito.when(NetUtils.isNetworkAWithinNetworkB(cidrMock,cidrMock)).thenReturn(true);
cmdMock.validateCidrs();
Mockito.verify(cmdMock, Mockito.atLeast(2)).getSourceCidrList();
Mockito.verify(cmdMock).getNetworkId();
Mockito.verify(networkServiceMock).getNetwork(networkIdMock);
Mockito.verify(networkMock).getCidr();
PowerMockito.verifyStatic(NetUtils.class, Mockito.times(2));
NetUtils.isValidIp4Cidr(cidrMock);
NetUtils.isValidIp6Cidr(cidrMock);
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isNetworkAWithinNetworkB(cidrMock,cidrMock);
}
@Test
public void validateCidrsTestCidrsBeginningWithWhiteSpace(){
ArrayList<String> listMock = new ArrayList<>();
String cidrMock = " 10.1.1.0/24";
listMock.add(cidrMock);
cmdMock.setSourceCidrList(listMock);
cmdMock.setDestCidrList(listMock);
long networkIdMock = 1234;
Mockito.doReturn(networkIdMock).when(cmdMock).getNetworkId();
Network networkMock = Mockito.mock(Network.class);
Mockito.doReturn(networkMock).when(networkServiceMock).getNetwork(networkIdMock);
Mockito.doReturn("10.1.1.0/24").when(networkMock).getCidr();
PowerMockito.mockStatic(NetUtils.class);
PowerMockito.when(NetUtils.isValidIp4Cidr(Mockito.eq("10.1.1.0/24"))).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp6Cidr(Mockito.eq("10.1.1.0/24"))).thenReturn(false);
PowerMockito.when(NetUtils.isNetworkAWithinNetworkB(Mockito.eq("10.1.1.0/24"), Mockito.eq("10.1.1.0/24"))).thenReturn(true);
cmdMock.validateCidrs();
Mockito.verify(cmdMock, Mockito.atLeast(2)).getSourceCidrList();
Mockito.verify(cmdMock).getNetworkId();
Mockito.verify(networkServiceMock).getNetwork(networkIdMock);
Mockito.verify(networkMock).getCidr();
PowerMockito.verifyStatic(NetUtils.class, Mockito.times(2));
NetUtils.isValidIp4Cidr(Mockito.eq("10.1.1.0/24"));
NetUtils.isValidIp6Cidr(Mockito.eq("10.1.1.0/24"));
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isNetworkAWithinNetworkB(Mockito.eq("10.1.1.0/24"), Mockito.eq("10.1.1.0/24"));
}
@Test (expected = ServerApiException.class)
public void validateCidrsTestInvalidSourceCidr(){
ArrayList<String> listMock = new ArrayList<>();
String sourceCidrMock = "aaaa";
listMock.add(sourceCidrMock);
cmdMock.setSourceCidrList(listMock);
cmdMock.setDestCidrList(listMock);
long networkIdMock = 1234;
Mockito.doReturn(networkIdMock).when(cmdMock).getNetworkId();
Network networkMock = Mockito.mock(Network.class);
Mockito.doReturn(networkMock).when(networkServiceMock).getNetwork(networkIdMock);
Mockito.doReturn("10.1.1.0/24").when(networkMock).getCidr();
PowerMockito.mockStatic(NetUtils.class);
PowerMockito.when(NetUtils.isValidIp4Cidr(sourceCidrMock)).thenReturn(false);
PowerMockito.when(NetUtils.isValidIp6Cidr(sourceCidrMock)).thenReturn(false);
cmdMock.validateCidrs();
Mockito.verify(cmdMock, Mockito.atLeast(2)).getSourceCidrList();
Mockito.verify(cmdMock).getNetworkId();
Mockito.verify(networkServiceMock).getNetwork(networkIdMock);
Mockito.verify(networkMock).getCidr();
PowerMockito.verifyStatic(NetUtils.class, Mockito.times(1));
NetUtils.isValidIp4Cidr(sourceCidrMock);
NetUtils.isValidIp6Cidr(sourceCidrMock);
}
@Test (expected = ServerApiException.class)
public void validateCidrsTestInvalidDestinationCidr(){
ArrayList<String> listSourceMock = new ArrayList<>();
String sourceCidrMock = "10.1.1.0/24";
listSourceMock.add(sourceCidrMock);
cmdMock.setSourceCidrList(listSourceMock);
ArrayList<String> listDestMock = new ArrayList<>();
String destCidrMock = "aaaa";
listDestMock.add(destCidrMock);
cmdMock.setDestCidrList(listDestMock);
long networkIdMock = 1234;
Mockito.doReturn(networkIdMock).when(cmdMock).getNetworkId();
Network networkMock = Mockito.mock(Network.class);
Mockito.doReturn(networkMock).when(networkServiceMock).getNetwork(networkIdMock);
Mockito.doReturn(sourceCidrMock).when(networkMock).getCidr();
PowerMockito.mockStatic(NetUtils.class);
PowerMockito.when(NetUtils.isValidIp4Cidr(Mockito.eq(sourceCidrMock))).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp6Cidr(Mockito.eq(sourceCidrMock))).thenReturn(false);
PowerMockito.when(NetUtils.isNetworkAWithinNetworkB(sourceCidrMock, sourceCidrMock)).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp4Cidr(Mockito.eq(destCidrMock))).thenReturn(false);
PowerMockito.when(NetUtils.isValidIp6Cidr(Mockito.eq(destCidrMock))).thenReturn(false);
cmdMock.validateCidrs();
Mockito.verify(cmdMock, Mockito.atLeast(2)).getSourceCidrList();
Mockito.verify(cmdMock).getNetworkId();
Mockito.verify(networkServiceMock).getNetwork(networkIdMock);
Mockito.verify(networkMock).getCidr();
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isValidIp4Cidr(sourceCidrMock);
NetUtils.isValidIp6Cidr(sourceCidrMock);
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isNetworkAWithinNetworkB(sourceCidrMock,sourceCidrMock);
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isValidIp4Cidr(destCidrMock);
NetUtils.isValidIp6Cidr(destCidrMock);
}
@Test
public void validateCidrsTestSourceCidrEqualsAllIp4(){
ArrayList<String> listSourceMock = new ArrayList<>();
String sourceCidrMock = "0.0.0.0/0";
listSourceMock.add(sourceCidrMock);
cmdMock.setSourceCidrList(listSourceMock);
ArrayList<String> listDestMock = new ArrayList<>();
String destCidrMock = "10.1.1.0/24";
listDestMock.add(destCidrMock);
cmdMock.setDestCidrList(listDestMock);
long networkIdMock = 1234;
Mockito.doReturn(networkIdMock).when(cmdMock).getNetworkId();
Network networkMock = Mockito.mock(Network.class);
Mockito.doReturn(networkMock).when(networkServiceMock).getNetwork(networkIdMock);
Mockito.doReturn(sourceCidrMock).when(networkMock).getCidr();
PowerMockito.mockStatic(NetUtils.class);
PowerMockito.when(NetUtils.isValidIp4Cidr(Mockito.eq(sourceCidrMock))).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp6Cidr(Mockito.eq(sourceCidrMock))).thenReturn(false);
PowerMockito.when(NetUtils.isValidIp4Cidr(Mockito.eq(destCidrMock))).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp6Cidr(Mockito.eq(destCidrMock))).thenReturn(false);
cmdMock.validateCidrs();
Mockito.verify(cmdMock, Mockito.atLeast(2)).getSourceCidrList();
Mockito.verify(cmdMock).getNetworkId();
Mockito.verify(networkServiceMock).getNetwork(networkIdMock);
Mockito.verify(networkMock).getCidr();
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isValidIp4Cidr(sourceCidrMock);
NetUtils.isValidIp6Cidr(sourceCidrMock);
PowerMockito.verifyStatic(NetUtils.class, Mockito.never());
NetUtils.isNetworkAWithinNetworkB(sourceCidrMock,sourceCidrMock);
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isValidIp4Cidr(destCidrMock);
NetUtils.isValidIp6Cidr(destCidrMock);
}
@Test(expected = ServerApiException.class)
public void validateCidrsTestSourceCidrNotWithinNetwork() {
ArrayList<String> listMock = new ArrayList<>();
String cidrMock = "10.1.1.0/24";
listMock.add(cidrMock);
cmdMock.setSourceCidrList(listMock);
cmdMock.setDestCidrList(listMock);
long networkIdMock = 1234;
Mockito.doReturn(networkIdMock).when(cmdMock).getNetworkId();
Network networkMock = Mockito.mock(Network.class);
Mockito.doReturn(networkMock).when(networkServiceMock).getNetwork(networkIdMock);
Mockito.doReturn(cidrMock).when(networkMock).getCidr();
PowerMockito.mockStatic(NetUtils.class);
PowerMockito.when(NetUtils.isValidIp4Cidr(cidrMock)).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp6Cidr(cidrMock)).thenReturn(false);
PowerMockito.when(NetUtils.isNetworkAWithinNetworkB(cidrMock, cidrMock)).thenReturn(false);
cmdMock.validateCidrs();
Mockito.verify(cmdMock, Mockito.atLeast(2)).getSourceCidrList();
Mockito.verify(cmdMock).getNetworkId();
Mockito.verify(networkServiceMock).getNetwork(networkIdMock);
Mockito.verify(networkMock).getCidr();
PowerMockito.verifyStatic(NetUtils.class, Mockito.times(1));
NetUtils.isValidIp4Cidr(cidrMock);
NetUtils.isValidIp6Cidr(cidrMock);
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isNetworkAWithinNetworkB(cidrMock, cidrMock);
}
@Test
public void validateCidrsTestNullDestinationCidr(){
ArrayList<String> listSourceMock = new ArrayList<>();
String sourceCidrMock = "10.1.1.0/24";
listSourceMock.add(sourceCidrMock);
cmdMock.setSourceCidrList(listSourceMock);
long networkIdMock = 1234;
Mockito.doReturn(networkIdMock).when(cmdMock).getNetworkId();
Network networkMock = Mockito.mock(Network.class);
Mockito.doReturn(networkMock).when(networkServiceMock).getNetwork(networkIdMock);
Mockito.doReturn(sourceCidrMock).when(networkMock).getCidr();
PowerMockito.mockStatic(NetUtils.class);
PowerMockito.when(NetUtils.isValidIp4Cidr(Mockito.eq(sourceCidrMock))).thenReturn(true);
PowerMockito.when(NetUtils.isValidIp6Cidr(Mockito.eq(sourceCidrMock))).thenReturn(false);
PowerMockito.when(NetUtils.isNetworkAWithinNetworkB(sourceCidrMock, sourceCidrMock)).thenReturn(true);
cmdMock.validateCidrs();
Mockito.verify(cmdMock, Mockito.atLeast(2)).getSourceCidrList();
Mockito.verify(cmdMock).getNetworkId();
Mockito.verify(networkServiceMock).getNetwork(networkIdMock);
Mockito.verify(networkMock).getCidr();
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isValidIp4Cidr(sourceCidrMock);
NetUtils.isValidIp6Cidr(sourceCidrMock);
PowerMockito.verifyStatic(NetUtils.class);
NetUtils.isNetworkAWithinNetworkB(sourceCidrMock,sourceCidrMock);
PowerMockito.verifyStatic(NetUtils.class, Mockito.never());
NetUtils.isValidIp4Cidr(null);
NetUtils.isValidIp6Cidr(null);
}
}