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

enable to create VPC portfowarding rules with source cidr (#7081) 

Co-authored-by: Lopez <rodrigo@scclouds.com.br>
Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>
This commit is contained in:
Rodrigo D. Lopez 2024-11-28 13:53:07 -03:00 committed by GitHub
parent a8cb7abca3
commit 4189bac8e0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
25 changed files with 396 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
api/src/main/java/com/cloud/agent/api/to/PortForwardingRuleTO.java
View File

@ -19,6 +19,9 @@ package com.cloud.agent.api.to;
import com.cloud.network.rules.FirewallRule;
import com.cloud.network.rules.PortForwardingRule;
import com.cloud.utils.net.NetUtils;
import org.apache.commons.lang3.StringUtils;
import java.util.List;
/**
* PortForwardingRuleTO specifies one port forwarding rule.
@ -29,6 +32,8 @@ public class PortForwardingRuleTO extends FirewallRuleTO {
String dstIp;
int[] dstPortRange;
List<String> sourceCidrList;
protected PortForwardingRuleTO() {
super();
}
@ -37,6 +42,7 @@ public class PortForwardingRuleTO extends FirewallRuleTO {
super(rule, srcVlanTag, srcIp);
this.dstIp = rule.getDestinationIpAddress().addr();
this.dstPortRange = new int[] {rule.getDestinationPortStart(), rule.getDestinationPortEnd()};
this.sourceCidrList = rule.getSourceCidrList();
}
public PortForwardingRuleTO(long id, String srcIp, int srcPortStart, int srcPortEnd, String dstIp, int dstPortStart, int dstPortEnd, String protocol,
@ -58,4 +64,11 @@ public class PortForwardingRuleTO extends FirewallRuleTO {
return NetUtils.portRangeToString(dstPortRange);
}
public String getSourceCidrListAsString() {
if (sourceCidrList != null) {
return StringUtils.join(sourceCidrList, ",");
}
return null;
}
}

5
api/src/main/java/com/cloud/network/rules/RulesService.java
View File

@ -26,6 +26,7 @@ import com.cloud.exception.ResourceUnavailableException;
import com.cloud.user.Account;
import com.cloud.utils.Pair;
import com.cloud.utils.net.Ip;
import org.apache.cloudstack.api.command.user.firewall.UpdatePortForwardingRuleCmd;
public interface RulesService {
Pair<List<? extends FirewallRule>, Integer> searchStaticNatRules(Long ipId, Long id, Long vmId, Long start, Long size, String accountName, Long domainId,
@ -81,6 +82,8 @@ public interface RulesService {
boolean disableStaticNat(long ipId) throws ResourceUnavailableException, NetworkRuleConflictException, InsufficientAddressCapacityException;
PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Integer privateEndPort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay);
PortForwardingRule updatePortForwardingRule(UpdatePortForwardingRuleCmd cmd);
void validatePortForwardingSourceCidrList(List<String> sourceCidrList);
}

1
api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
View File

@ -427,6 +427,7 @@ public class ApiConstants {
public static final String SNAPSHOT_TYPE = "snapshottype";
public static final String SNAPSHOT_QUIESCEVM = "quiescevm";
public static final String SUPPORTS_STORAGE_SNAPSHOT = "supportsstoragesnapshot";
public static final String SOURCE_CIDR_LIST = "sourcecidrlist";
public static final String SOURCE_ZONE_ID = "sourcezoneid";
public static final String START_DATE = "startdate";
public static final String START_ID = "startid";

22
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java
View File

@ -107,8 +107,12 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
description = "the ID of the virtual machine for the port forwarding rule")
private Long virtualMachineId;
@Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, description = "the cidr list to forward traffic from. Multiple entries must be separated by a single comma character (,). This parameter is deprecated. Do not use.")
private List<String> cidrlist;
@Parameter(name = ApiConstants.CIDR_LIST,
type = CommandType.LIST,
collectionType = CommandType.STRING,
description = " the source CIDR list to allow traffic from; all other CIDRs will be blocked. " +
"Multiple entries must be separated by a single comma character (,). This param will be used only for VPC tiers. By default, all CIDRs are allowed.")
private List<String> sourceCidrList;
@Parameter(name = ApiConstants.OPEN_FIREWALL, type = CommandType.BOOLEAN, description = "if true, firewall rule for source/end public port is automatically created; "
+ "if false - firewall rule has to be created explicitly. If not specified 1) defaulted to false when PF"
@ -157,11 +161,7 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
@Override
public List<String> getSourceCidrList() {
if (cidrlist != null) {
throw new InvalidParameterValueException("Parameter cidrList is deprecated; if you need to open firewall "
+ "rule for the specific cidr, please refer to createFirewallRule command");
}
return null;
return sourceCidrList;
}
public Boolean getOpenFirewall() {
@ -334,12 +334,6 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
@Override
public void create() {
// cidr list parameter is deprecated
if (cidrlist != null) {
throw new InvalidParameterValueException(
"Parameter cidrList is deprecated; if you need to open firewall rule for the specific cidr, please refer to createFirewallRule command");
}
Ip privateIp = getVmSecondaryIp();
if (privateIp != null) {
if (!NetUtils.isValidIp4(privateIp.toString())) {
@ -347,6 +341,8 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
}
}
_rulesService.validatePortForwardingSourceCidrList(sourceCidrList);
try {
PortForwardingRule result = _rulesService.createPortForwardingRule(this, virtualMachineId, privateIp, getOpenFirewall(), isDisplay());
setEntityId(result.getId());

15
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java
View File

@ -33,6 +33,8 @@ import com.cloud.network.rules.PortForwardingRule;
import com.cloud.user.Account;
import com.cloud.utils.net.Ip;
import java.util.List;
@APICommand(name = "updatePortForwardingRule",
responseObject = FirewallRuleResponse.class,
description = "Updates a port forwarding rule. Only the private port and the virtual machine can be updated.", entityType = {PortForwardingRule.class},
@ -65,6 +67,13 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd {
@Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {RoleType.Admin})
private Boolean display;
@Parameter(name = ApiConstants.CIDR_LIST,
type = CommandType.LIST,
collectionType = CommandType.STRING,
description = " the source CIDR list to allow traffic from; all other CIDRs will be blocked. " +
"Multiple entries must be separated by a single comma character (,). This param will be used only for VPC tiers. By default, all CIDRs are allowed.")
private List<String> sourceCidrList;
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
@ -96,6 +105,10 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd {
return display;
}
public List<String> getSourceCidrList() {
return sourceCidrList;
}
/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
@ -132,7 +145,7 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd {
@Override
public void execute() {
PortForwardingRule rule = _rulesService.updatePortForwardingRule(getId(), getPrivatePort(), getPrivateEndPort(), getVirtualMachineId(), getVmGuestIp(), getCustomId(), getDisplay());
PortForwardingRule rule = _rulesService.updatePortForwardingRule(this);
FirewallRuleResponse fwResponse = new FirewallRuleResponse();
if (rule != null) {
fwResponse = _responseGenerator.createPortForwardingRuleResponse(rule);

2
api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java
View File

@ -106,7 +106,7 @@ public class CreateLoadBalancerRuleCmd extends BaseAsyncCreateCmd /*implements L
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "the domain ID associated with the load balancer")
private Long domainId;
@Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, since = "4.18.0.0", description = "the CIDR list to allow traffic, "
@Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, since = "4.18.0.0", description = "the source CIDR list to allow traffic from; "
+ "all other CIDRs will be blocked. Multiple entries must be separated by a single comma character (,). By default, all CIDRs are allowed.")
private List<String> cidrlist;

2
core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetPortForwardingRulesConfigItem.java
View File

@ -41,7 +41,7 @@ public class SetPortForwardingRulesConfigItem extends AbstractConfigItemFacade {
for (final PortForwardingRuleTO rule : command.getRules()) {
final ForwardingRule fwdRule = new ForwardingRule(rule.revoked(), rule.getProtocol().toLowerCase(), rule.getSrcIp(), rule.getStringSrcPortRange(), rule.getDstIp(),
rule.getStringDstPortRange());
rule.getStringDstPortRange(), rule.getSourceCidrListAsString());
rules.add(fwdRule);
}

9
core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/ForwardingRule.java
View File

@ -26,18 +26,21 @@ public class ForwardingRule {
private String sourcePortRange;
private String destinationIpAddress;
private String destinationPortRange;
private String sourceCidrList;
public ForwardingRule() {
// Empty constructor for (de)serialization
}
public ForwardingRule(boolean revoke, String protocol, String sourceIpAddress, String sourcePortRange, String destinationIpAddress, String destinationPortRange) {
public ForwardingRule(boolean revoke, String protocol, String sourceIpAddress, String sourcePortRange, String destinationIpAddress, String destinationPortRange,
String sourceCidrList) {
this.revoke = revoke;
this.protocol = protocol;
this.sourceIpAddress = sourceIpAddress;
this.sourcePortRange = sourcePortRange;
this.destinationIpAddress = destinationIpAddress;
this.destinationPortRange = destinationPortRange;
this.sourceCidrList = sourceCidrList;
}
public boolean isRevoke() {
@ -88,4 +91,8 @@ public class ForwardingRule {
this.destinationPortRange = destinationPortRange;
}
public String getSourceCidrList() {
return sourceCidrList;
}
}

2
engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDao.java
View File

@ -29,4 +29,6 @@ public interface FirewallRulesCidrsDao extends GenericDao<FirewallRulesCidrsVO,
@DB
List<FirewallRulesCidrsVO> listByFirewallRuleId(long firewallRuleId);
void updateSourceCidrsForRule(Long firewallRuleId, List<String> sourceCidrList);
}

19
engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java
View File

@ -20,6 +20,7 @@ import java.util.ArrayList;
import java.util.List;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@ -65,9 +66,27 @@ public class FirewallRulesCidrsDaoImpl extends GenericDaoBase<FirewallRulesCidrs
return results;
}
@Override
public void updateSourceCidrsForRule(Long firewallRuleId, List<String> sourceCidrList) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
txn.start();
SearchCriteria<FirewallRulesCidrsVO> sc = CidrsSearch.create();
sc.setParameters("firewallRuleId", firewallRuleId);
remove(sc);
persist(firewallRuleId, sourceCidrList);
txn.commit();
}
@Override
@DB
public void persist(long firewallRuleId, List<String> sourceCidrs) {
if (CollectionUtils.isEmpty(sourceCidrs)) {
return;
}
TransactionLegacy txn = TransactionLegacy.currentTxn();
txn.start();

3
engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java
View File

@ -243,9 +243,6 @@ public class FirewallRulesDaoImpl extends GenericDaoBase<FirewallRuleVO, Long> i
}
public void saveSourceCidrs(FirewallRuleVO firewallRule, List<String> cidrList) {
if (cidrList == null) {
return;
}
_firewallRulesCidrsDao.persist(firewallRule.getId(), cidrList);
}

23
engine/schema/src/main/java/com/cloud/network/rules/PortForwardingRuleVO.java
View File

@ -25,6 +25,7 @@ import javax.persistence.EnumType;
import javax.persistence.Enumerated;
import javax.persistence.PrimaryKeyJoinColumn;
import javax.persistence.Table;
import javax.persistence.Transient;
import com.cloud.utils.net.Ip;
@ -47,21 +48,20 @@ public class PortForwardingRuleVO extends FirewallRuleVO implements PortForwardi
@Column(name = "instance_id")
private long virtualMachineId;
@Transient
List<String> sourceCidrs;
public PortForwardingRuleVO() {
}
public PortForwardingRuleVO(String xId, long srcIpId, int srcPortStart, int srcPortEnd, Ip dstIp, int dstPortStart, int dstPortEnd, String protocol, long networkId,
long accountId, long domainId, long instanceId) {
super(xId, srcIpId, srcPortStart, srcPortEnd, protocol, networkId, accountId, domainId, Purpose.PortForwarding, null, null, null, null, null);
long accountId, long domainId, long instanceId, List<String> sourceCidrs) {
super(xId, srcIpId, srcPortStart, srcPortEnd, protocol, networkId, accountId, domainId, Purpose.PortForwarding, sourceCidrs, null, null, null, null);
this.destinationIpAddress = dstIp;
this.virtualMachineId = instanceId;
this.destinationPortStart = dstPortStart;
this.destinationPortEnd = dstPortEnd;
}
public PortForwardingRuleVO(String xId, long srcIpId, int srcPort, Ip dstIp, int dstPort, String protocol, List<String> sourceCidrs, long networkId, long accountId,
long domainId, long instanceId) {
this(xId, srcIpId, srcPort, srcPort, dstIp, dstPort, dstPort, protocol.toLowerCase(), networkId, accountId, domainId, instanceId);
this.sourceCidrs = sourceCidrs;
}
@Override
@ -106,4 +106,13 @@ public class PortForwardingRuleVO extends FirewallRuleVO implements PortForwardi
return null;
}
public void setSourceCidrList(List<String> sourceCidrs) {
this.sourceCidrs = sourceCidrs;
}
@Override
public List<String> getSourceCidrList() {
return sourceCidrs;
}
}

45
engine/schema/src/main/java/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java
View File

@ -20,6 +20,9 @@ import java.util.List;
import javax.inject.Inject;
import com.cloud.utils.db.Transaction;
import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionLegacy;
import org.springframework.stereotype.Component;
import com.cloud.network.dao.FirewallRulesCidrsDao;
@ -41,7 +44,7 @@ public class PortForwardingRulesDaoImpl extends GenericDaoBase<PortForwardingRul
protected final SearchBuilder<PortForwardingRuleVO> ActiveRulesSearchByAccount;
@Inject
protected FirewallRulesCidrsDao _portForwardingRulesCidrsDao;
protected FirewallRulesCidrsDao portForwardingRulesCidrsDao;
protected PortForwardingRulesDaoImpl() {
super();
@ -170,4 +173,44 @@ public class PortForwardingRulesDaoImpl extends GenericDaoBase<PortForwardingRul
sc.setParameters("dstIp", secondaryIp);
return findOneBy(sc);
}
@Override
public PortForwardingRuleVO persist(PortForwardingRuleVO portForwardingRule) {
return Transaction.execute((TransactionCallback<PortForwardingRuleVO>) transactionStatus -> {
PortForwardingRuleVO dbPfRule = super.persist(portForwardingRule);
portForwardingRulesCidrsDao.persist(portForwardingRule.getId(), portForwardingRule.getSourceCidrList());
List<String> cidrList = portForwardingRulesCidrsDao.getSourceCidrs(portForwardingRule.getId());
portForwardingRule.setSourceCidrList(cidrList);
return dbPfRule;
});
}
@Override
public boolean update(Long id, PortForwardingRuleVO entity) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
txn.start();
boolean success = super.update(id, entity);
if (!success) {
return false;
}
portForwardingRulesCidrsDao.updateSourceCidrsForRule(entity.getId(), entity.getSourceCidrList());
txn.commit();
return true;
}
@Override
public PortForwardingRuleVO findById(Long id) {
PortForwardingRuleVO rule = super.findById(id);
List<String> sourceCidrList = portForwardingRulesCidrsDao.getSourceCidrs(id);
rule.setSourceCidrList(sourceCidrList);
return rule;
}
}

2
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
View File

@ -472,7 +472,7 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
sourcePort, sourcePort,
vmIp,
destPort, destPort,
"tcp", networkId, accountId, domainId, vmId);
"tcp", networkId, accountId, domainId, vmId, null);
newRule.setDisplay(true);
newRule.setState(FirewallRule.State.Add);
newRule = portForwardingRulesDao.persist(newRule);

75
server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
View File

@ -33,6 +33,7 @@ import org.apache.cloudstack.api.command.user.ipv6.ListIpv6FirewallRulesCmd;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@ -391,20 +392,34 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
((rule.getPurpose() == Purpose.Firewall || newRule.getPurpose() == Purpose.Firewall) && ((newRule.getPurpose() != rule.getPurpose()) || (!newRule.getProtocol()
.equalsIgnoreCase(rule.getProtocol()))));
// if both rules are firewall and their cidrs are different, we can skip port ranges verification
boolean bothRulesFirewall = (rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall);
// if both rules are firewall/port forwarding and their cidrs are different, we can skip port ranges verification
boolean duplicatedCidrs = false;
boolean bothRulesFirewall = (rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall);
if (bothRulesFirewall) {
_firewallDao.loadSourceCidrs(rule);
_firewallDao.loadSourceCidrs((FirewallRuleVO)newRule);
if (ObjectUtils.anyNull(rule.getSourceCidrList(), newRule.getSourceCidrList())) {
continue;
}
_firewallDao.loadDestinationCidrs(rule);
_firewallDao.loadDestinationCidrs((FirewallRuleVO) newRule);
if (rule.getSourceCidrList() == null || newRule.getSourceCidrList() == null) {
duplicatedCidrs = detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList()) && detectConflictingCidrs(rule.getDestinationCidrList(), newRule.getDestinationCidrList());
}
boolean bothRulesPortForwarding = rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.PortForwarding;
if (bothRulesPortForwarding) {
_firewallDao.loadSourceCidrs(rule);
_firewallDao.loadSourceCidrs((FirewallRuleVO) newRule);
if (ObjectUtils.anyNull(rule.getSourceCidrList(), newRule.getSourceCidrList())) {
continue;
}
duplicatedCidrs = (detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList()) && detectConflictingCidrs(rule.getDestinationCidrList(), newRule.getDestinationCidrList()));
duplicatedCidrs = detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList());
}
if (!oneOfRulesIsFirewall) {
@ -440,18 +455,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
if (!notNullPorts) {
continue;
} else if (!oneOfRulesIsFirewall &&
!(bothRulesFirewall && !duplicatedCidrs) &&
((rule.getSourcePortStart().intValue() <= newRule.getSourcePortStart().intValue() &&
rule.getSourcePortEnd().intValue() >= newRule.getSourcePortStart().intValue()) ||
(rule.getSourcePortStart().intValue() <= newRule.getSourcePortEnd().intValue() &&
rule.getSourcePortEnd().intValue() >= newRule.getSourcePortEnd().intValue()) ||
(newRule.getSourcePortStart().intValue() <= rule.getSourcePortStart().intValue() &&
newRule.getSourcePortEnd().intValue() >= rule.getSourcePortStart().intValue()) ||
(newRule.getSourcePortStart().intValue() <= rule.getSourcePortEnd().intValue() &&
newRule.getSourcePortEnd().intValue() >= rule.getSourcePortEnd().intValue()))) {
//Above else if conditions checks for the conflicting port ranges.
} else if (checkIfRulesHaveConflictingPortRanges(newRule, rule, oneOfRulesIsFirewall, bothRulesFirewall, bothRulesPortForwarding, duplicatedCidrs)) {
// we allow port forwarding rules with the same parameters but different protocols
boolean allowPf =
(rule.getPurpose() == Purpose.PortForwarding && newRule.getPurpose() == Purpose.PortForwarding && !newRule.getProtocol().equalsIgnoreCase(
@ -478,6 +482,45 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
}
}
protected boolean checkIfRulesHaveConflictingPortRanges(FirewallRule newRule, FirewallRule rule, boolean oneOfRulesIsFirewall, boolean bothRulesFirewall, boolean bothRulesPortForwarding, boolean duplicatedCidrs) {
String rulesAsString = String.format("[%s] and [%s]", rule, newRule);
if (oneOfRulesIsFirewall) {
s_logger.debug(String.format("Only one of the rules (%s) is firewall; therefore, their port ranges will not conflict.",
rulesAsString));
return false;
}
if ((bothRulesFirewall || bothRulesPortForwarding) && !duplicatedCidrs) {
s_logger.debug(String.format("Both rules (%s) are firewall/port forwarding, but they do not have duplicated CIDRs; therefore, their port ranges will not conflict.",
rulesAsString));
return false;
}
if (rule.getSourcePortStart() <= newRule.getSourcePortStart() && rule.getSourcePortEnd() >= newRule.getSourcePortStart()) {
s_logger.debug(String.format("Rules (%s) have conflicting port ranges.", rulesAsString));
return true;
}
if (rule.getSourcePortStart() <= newRule.getSourcePortEnd() && rule.getSourcePortEnd() >= newRule.getSourcePortEnd()) {
s_logger.debug(String.format("Rules (%s) have conflicting port ranges.", rulesAsString));
return true;
}
if (newRule.getSourcePortStart() <= rule.getSourcePortStart() && newRule.getSourcePortEnd() >= rule.getSourcePortStart()) {
s_logger.debug(String.format("Rules (%s) have conflicting port ranges.", rulesAsString));
return true;
}
if (newRule.getSourcePortStart() <= rule.getSourcePortEnd() && newRule.getSourcePortEnd() >= rule.getSourcePortEnd()) {
s_logger.debug(String.format("Rules (%s) have conflicting port ranges.", rulesAsString));
return true;
}
s_logger.debug(String.format("Rules (%s) do not have conflicting port ranges.", rulesAsString));
return false;
}
@Override
public void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer portStart, Integer portEnd, String proto, Purpose purpose, FirewallRuleType type,
Long networkId, FirewallRule.TrafficType trafficType) {

2
server/src/main/java/com/cloud/network/router/CommandSetupHelper.java
View File

@ -27,6 +27,7 @@ import java.util.Set;
import javax.inject.Inject;
import com.cloud.network.rules.PortForwardingRuleVO;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@ -396,6 +397,7 @@ public class CommandSetupHelper {
final List<PortForwardingRuleTO> rulesTO = new ArrayList<PortForwardingRuleTO>();
if (rules != null) {
for (final PortForwardingRule rule : rules) {
_rulesDao.loadSourceCidrs((PortForwardingRuleVO) rule);
final IpAddress sourceIp = _networkModel.getIp(rule.getSourceIpAddressId());
final PortForwardingRuleTO ruleTO = new PortForwardingRuleTO(rule, null, sourceIp.getAddress().addr());
rulesTO.add(ruleTO);

52
server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
View File

@ -33,6 +33,7 @@ import com.cloud.vm.VirtualMachineProfile;
import com.cloud.vm.VirtualMachineProfileImpl;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.api.command.user.firewall.ListPortForwardingRulesCmd;
import org.apache.cloudstack.api.command.user.firewall.UpdatePortForwardingRuleCmd;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.log4j.Logger;
@ -103,6 +104,7 @@ import com.cloud.vm.dao.NicSecondaryIpDao;
import com.cloud.vm.dao.NicSecondaryIpVO;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;
import org.apache.commons.collections.CollectionUtils;
public class RulesManagerImpl extends ManagerBase implements RulesManager, RulesService {
private static final Logger s_logger = Logger.getLogger(RulesManagerImpl.class);
@ -114,7 +116,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
@Inject
PortForwardingRulesDao _portForwardingDao;
@Inject
FirewallRulesCidrsDao _firewallCidrsDao;
FirewallRulesCidrsDao firewallCidrsDao;
@Inject
FirewallRulesDao _firewallDao;
@Inject
@ -249,6 +251,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
final Long accountId = ipAddress.getAllocatedToAccountId();
final Long domainId = ipAddress.getAllocatedInDomainId();
List<String> sourceCidrList = rule.getSourceCidrList();
// start port can't be bigger than end port
if (rule.getDestinationPortStart() > rule.getDestinationPortEnd()) {
@ -312,9 +315,8 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
final IPAddressVO ipAddressFinal = ipAddress;
return Transaction.execute((TransactionCallbackWithException<PortForwardingRuleVO, NetworkRuleConflictException>) status -> {
PortForwardingRuleVO newRule =
new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIpFinal,
rule.getDestinationPortStart(), rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId);
new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIpFinal,
rule.getDestinationPortStart(), rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId, sourceCidrList);
if (forDisplay != null) {
newRule.setDisplay(forDisplay);
}
@ -898,6 +900,10 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
_accountMgr.checkAccess(caller, null, true, rules.toArray(new PortForwardingRuleVO[rules.size()]));
}
for (PortForwardingRuleVO rule : rules) {
rule.setSourceCidrList(firewallCidrsDao.getSourceCidrs(rule.getId()));
}
try {
if (!_firewallMgr.applyRules(rules, continueOnError, true)) {
return false;
@ -951,6 +957,10 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
_accountMgr.checkAccess(caller, null, true, rules.toArray(new PortForwardingRuleVO[rules.size()]));
}
for (PortForwardingRuleVO rule: rules) {
rule.setSourceCidrList(firewallCidrsDao.getSourceCidrs(rule.getId()));
}
try {
if (!_firewallMgr.applyRules(rules, continueOnError, true)) {
return false;
@ -1570,12 +1580,22 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
@Override
@ActionEvent(eventType = EventTypes.EVENT_NET_RULE_MODIFY, eventDescription = "updating forwarding rule", async = true)
public PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Integer privateEndPort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay) {
Account caller = CallContext.current().getCallingAccount();
public PortForwardingRule updatePortForwardingRule(UpdatePortForwardingRuleCmd cmd) {
long id = cmd.getId();
Integer privatePort = cmd.getPrivatePort();
Integer privateEndPort = cmd.getPrivateEndPort();
Long virtualMachineId = cmd.getVirtualMachineId();
Ip vmGuestIp = cmd.getVmGuestIp();
String customId = cmd.getCustomId();
Boolean forDisplay = cmd.getDisplay();
List<String> sourceCidrList = cmd.getSourceCidrList();
PortForwardingRuleVO rule = _portForwardingDao.findById(id);
if (rule == null) {
throw new InvalidParameterValueException("Unable to find " + id);
}
Account caller = CallContext.current().getCallingAccount();
_accountMgr.checkAccess(caller, null, true, rule);
if (customId != null) {
@ -1636,6 +1656,8 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
}
}
validatePortForwardingSourceCidrList(sourceCidrList);
// revoke old rules at first
List<PortForwardingRuleVO> rules = new ArrayList<PortForwardingRuleVO>();
rule.setState(State.Revoke);
@ -1663,6 +1685,11 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
rule.setVirtualMachineId(virtualMachineId);
rule.setDestinationIpAddress(dstIp);
}
if (sourceCidrList != null) {
rule.setSourceCidrList(sourceCidrList);
}
_portForwardingDao.update(id, rule);
//apply new rules
@ -1672,4 +1699,17 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules
return _portForwardingDao.findById(id);
}
@Override
public void validatePortForwardingSourceCidrList(List<String> sourceCidrList) {
if (CollectionUtils.isEmpty(sourceCidrList)) {
return;
}
for (String cidr : sourceCidrList) {
if (!NetUtils.isValidCidrList(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
throw new InvalidParameterValueException(String.format("The given source CIDR [%s] is invalid.", cidr));
}
}
}
}

100
server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java
View File

@ -27,7 +27,6 @@ import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.element.FirewallServiceProvider;
import com.cloud.network.element.VirtualRouterElement;
import com.cloud.network.element.VpcVirtualRouterElement;
import com.cloud.network.rules.FirewallManager;
import com.cloud.network.rules.FirewallRule;
import com.cloud.network.rules.FirewallRule.Purpose;
import com.cloud.network.rules.FirewallRuleVO;
@ -35,9 +34,9 @@ import com.cloud.network.vpc.VpcManager;
import com.cloud.user.AccountManager;
import com.cloud.user.DomainManager;
import com.cloud.utils.component.ComponentContext;
import junit.framework.Assert;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.log4j.Logger;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
@ -45,7 +44,8 @@ import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import java.util.ArrayList;
import java.util.Arrays;
@ -108,12 +108,35 @@ public class FirewallManagerTest {
@Mock
FirewallRulesDao _firewallDao;
@Spy
@InjectMocks
FirewallManager _firewallMgr = new FirewallManagerImpl();
FirewallManagerImpl _firewallMgr;
FirewallRule fwRule50to150;
FirewallRule fwRule100to200;
FirewallRule fwRule151to200;
FirewallRule pfRule50to150;
FirewallRule pfRule100to200;
FirewallRule pfRule151to200;
@Before
public void initMocks() {
MockitoAnnotations.initMocks(this);
fwRule50to150 = createFirewallRule(50, 150, Purpose.Firewall);
fwRule100to200 = createFirewallRule(100, 150, Purpose.Firewall);
fwRule151to200 = createFirewallRule(151, 200, Purpose.Firewall);
pfRule50to150 = createFirewallRule(50, 150, Purpose.PortForwarding);
pfRule100to200 = createFirewallRule(100, 150, Purpose.PortForwarding);
pfRule151to200 = createFirewallRule(151, 200, Purpose.PortForwarding);
}
private FirewallRule createFirewallRule(int startPort, int endPort, Purpose purpose) {
return new FirewallRuleVO("xid", 1L, startPort, endPort, "TCP", 2, 3, 4, purpose, new ArrayList<>(),
new ArrayList<>(), 5, 6, null, FirewallRule.TrafficType.Ingress);
}
@Ignore("Requires database to be set up")
@ -210,6 +233,75 @@ public class FirewallManagerTest {
}
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestOnlyOneRuleIsFirewallReturnsFalse()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150, pfRule50to150, true, false, false, true);
Assert.assertFalse(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallButNoDuplicateCidrsReturnsFalse()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150, fwRule50to150, false, true, false, false);
Assert.assertFalse(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingButNoDuplicateCidrsReturnsFalse()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150, pfRule50to150, false, false, true, false);
Assert.assertFalse(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallAndDuplicatedCidrsAndNewRuleSourceStartPortIsInsideExistingRangeReturnsTrue()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule100to200, fwRule50to150, false, true, false, true);
Assert.assertTrue(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallAndDuplicatedCidrsAndNewRuleSourceEndPortIsInsideExistingRangeReturnsTrue()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150, fwRule100to200, false, true, false, true);
Assert.assertTrue(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingAndDuplicatedCidrsAndNewRuleSourceStartPortIsInsideExistingRangeReturnsTrue()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150, pfRule100to200, false, false, true, true);
Assert.assertTrue(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingAndDuplicatedCidrsAndNewRuleSourceEndPortIsInsideExistingRangeReturnsTrue()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150, pfRule100to200, false, false, true, true);
Assert.assertTrue(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallAndDuplicatedCidrsAndNoRangeConflictReturnsFalse()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150, fwRule151to200, false, true, false, true);
Assert.assertFalse(result);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingAndDuplicatedCidrsAndNoRangeConflictReturnsFalse()
{
boolean result = _firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150, pfRule151to200, false, false, true, true);
Assert.assertFalse(result);
}
}

10
systemvm/debian/opt/cloud/bin/configure.py
View File

@ -1217,7 +1217,10 @@ class CsForwardingRules(CsDataBag):
self.fw.append(["filter", "", fw7])
def forward_vpc(self, rule):
fw_prerout_rule = "-A PREROUTING -d %s/32 " % (rule["public_ip"])
fw_prerout_rule = "-A PREROUTING"
if "source_cidr_list" in rule and rule["source_cidr_list"]:
fw_prerout_rule += " -s %s" % rule["source_cidr_list"]
fw_prerout_rule += " -d %s/32" % rule["public_ip"]
if not rule["protocol"] == "any":
fw_prerout_rule += " -m %s -p %s" % (rule["protocol"], rule["protocol"])
if not rule["public_ports"] == "any":
@ -1226,7 +1229,10 @@ class CsForwardingRules(CsDataBag):
if not rule["internal_ports"] == "any":
fw_prerout_rule += ":" + self.portsToString(rule["internal_ports"], "-")
fw_output_rule = "-A OUTPUT -d %s/32" % rule["public_ip"]
fw_output_rule = "-A OUTPUT"
if "source_cidr_list" in rule and rule["source_cidr_list"]:
fw_output_rule += " -s %s" % rule["source_cidr_list"]
fw_output_rule += " -d %s/32" % rule["public_ip"]
if not rule["protocol"] == "any":
fw_output_rule += " -m %s -p %s" % (rule["protocol"], rule["protocol"])
if not rule["public_ports"] == "any":

2
systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py
View File

@ -327,7 +327,7 @@ class CsNetfilter(object):
return self.rule
def to_str(self, delete=False):
""" Convert the rule back into aynactically correct iptables command """
""" Convert the rule back into syntactically correct iptables command """
# Order is important
order = ['-A', '-s', '-d', '!_-d', '-i', '!_-i', '-p', '-m', '-m2', '--icmp-type', '--state',
'--dport', '--destination-port', '-o', '!_-o', '-j', '--set-xmark', '--checksum',

22
systemvm/debian/opt/cloud/bin/cs_forwardingrules.py
View File

@ -16,6 +16,8 @@
# under the License.
import logging
def merge(dbag, rules):
for rule in rules["rules"]:
source_ip = rule["source_ip_address"]
@ -33,6 +35,8 @@ def merge(dbag, rules):
newrule["public_ports"] = rule["source_port_range"]
newrule["internal_ports"] = rule["destination_port_range"]
newrule["protocol"] = rule["protocol"]
if "source_cidr_list" in rule:
newrule["source_cidr_list"] = rule["source_cidr_list"]
if not revoke:
if rules["type"] == "staticnatrules":
@ -59,7 +63,7 @@ def merge(dbag, rules):
for forward in dbag[source_ip]:
if ruleCompare(forward, newrule):
index = dbag[source_ip].index(forward)
print "removing index %s" % str(index)
logging.info("Removing forwarding rule [%s] at index [%s].", forward, index)
if not index == -1:
del dbag[source_ip][index]
@ -74,4 +78,18 @@ def ruleCompare(ruleA, ruleB):
return ruleA["public_ip"] == ruleB["public_ip"]
elif ruleA["type"] == "forward":
return ruleA["public_ip"] == ruleB["public_ip"] and ruleA["public_ports"] == ruleB["public_ports"] \
and ruleA["protocol"] == ruleB["protocol"]
and ruleA["protocol"] == ruleB["protocol"] and cidrsConflict(ruleA.get("source_cidr_list"), ruleB.get("source_cidr_list"))
# Same validation as in com.cloud.network.firewall.FirewallManagerImpl.detectConflictingCidrs
def cidrsConflict(cidrListA, cidrListB):
if not cidrListA and not cidrListB:
return True
if not cidrListA:
return False
if not cidrListB:
return False
cidrListA = set(cidrListA.split(","))
cidrListB = set(cidrListB.split(","))
return len(cidrListA.intersection(cidrListB)) > 0

1
ui/public/locales/en.json
View File

@ -1946,6 +1946,7 @@
"label.source": "Select Import-Export Source Hypervisor",
"label.source.based": "SourceBased",
"label.sourcecidr": "Source CIDR",
"label.sourcecidrlist": "Source CIDR list",
"label.sourcehost": "Source host",
"label.sourceipaddress": "Source IP address",
"label.sourceipaddressnetworkid": "Network ID of source IP address",

3
ui/public/locales/pt_BR.json
View File

@ -349,7 +349,7 @@
"label.choose.saml.identity": "Escolha um provedor de identidade SAML",
"label.cidr": "CIDR",
"label.cidr.destination.network": "CIDR da rede de destino",
"label.cidrlist": "Lista de CIDR ",
"label.cidrlist": "Lista de CIDRs",
"label.cisco.nexus1000v.ip.address": "Endere\u00e7o IP do Nexus 1000v",
"label.cisco.nexus1000v.password": "Senha do Nexus 1000v",
"label.cisco.nexus1000v.username": "Usu\u00e1rio do Nexus 1000v",
@ -1494,6 +1494,7 @@
"label.sockettimeout": "Tempo limite no socket",
"label.source.based": "SourceBased",
"label.sourcecidr": "CIDR de origem",
"label.sourcecidrlist": "Lista de CIDRs de origem",
"label.sourceipaddress": "Endere\u00e7o IP de origem",
"label.sourcenat": "Source NAT",
"label.sourcenatsupported": "Suporte \u00e0 source NAT",

6
ui/src/views/network/LoadBalancing.vue
View File

@ -37,7 +37,7 @@
</div>
<div class="form">
<div class="form__item" ref="newCidrList">
<tooltip-label :title="$t('label.cidrlist')" bold :tooltip="createLoadBalancerRuleParams.cidrlist.description" :tooltip-placement="'right'"/>
<tooltip-label :title="$t('label.sourcecidrlist')" bold :tooltip="createLoadBalancerRuleParams.cidrlist.description" :tooltip-placement="'right'"/>
<a-input v-model:value="newRule.cidrlist"></a-input>
</div>
<div class="form__item">
@ -126,7 +126,7 @@
:rowKey="record => record.id">
<template #bodyCell="{ column, record }">
<template v-if="column.key === 'cidrlist'">
<span style="white-space: pre-line"> {{ record.cidrlist?.replaceAll(" ", "\n") }}</span>
<span style="white-space: pre-line"> {{ record.cidrlist?.replaceAll(",", "\n") }}</span>
</template>
<template v-if="column.key === 'algorithm'">
{{ returnAlgorithmName(record.algorithm) }}
@ -808,7 +808,7 @@ export default {
},
{
key: 'cidrlist',
title: this.$t('label.cidrlist')
title: this.$t('label.sourcecidrlist')
},
{
key: 'protocol',

27
ui/src/views/network/PortForwarding.vue
View File

@ -72,6 +72,12 @@
<a-select-option value="udp" :label="$t('label.udp')">{{ $t('label.udp') }}</a-select-option>
</a-select>
</div>
<div v-if="isVPC()">
<div class="form__item" ref="newCidrList">
<tooltip-label :title="$t('label.sourcecidrlist')" bold :tooltip="apiParams.cidrlist.description" :tooltip-placement="'right'"/>
<a-input v-model:value="newRule.cidrlist"></a-input>
</div>
</div>
<div class="form__item" style="margin-left: auto;">
<div class="form__label">{{ $t('label.add.vm') }}</div>
<a-button :disabled="!('createPortForwardingRule' in $store.getters.apis)" type="primary" @click="openAddVMModal">{{ $t('label.add') }}</a-button>
@ -108,6 +114,9 @@
<template v-if="column.key === 'protocol'">
{{ getCapitalise(record.protocol) }}
</template>
<template v-if="column.key === 'cidrlist'">
<span style="white-space: pre-line"> {{ record.cidrlist?.replaceAll(",", "\n") }}</span>
</template>
<template v-if="column.key === 'vm'">
<div><desktop-outlined/>
<router-link
@ -334,9 +343,11 @@ import Status from '@/components/widgets/Status'
import TooltipButton from '@/components/widgets/TooltipButton'
import BulkActionView from '@/components/view/BulkActionView'
import eventBus from '@/config/eventBus'
import TooltipLabel from '@/components/widgets/TooltipLabel.vue'
export default {
components: {
TooltipLabel,
Status,
TooltipButton,
BulkActionView
@ -399,6 +410,11 @@ export default {
key: 'protocol',
title: this.$t('label.protocol')
},
{
key: 'cidrlist',
title: this.$t('label.sourcecidrlist'),
hidden: !this.isVPC()
},
{
title: this.$t('label.state'),
dataIndex: 'state'
@ -411,7 +427,7 @@ export default {
key: 'actions',
title: this.$t('label.actions')
}
],
].filter(item => !item.hidden),
tiers: {
loading: false,
data: []
@ -450,7 +466,8 @@ export default {
vmPage: 1,
vmPageSize: 10,
vmCount: 0,
searchQuery: null
searchQuery: null,
cidrlist: ''
}
},
computed: {
@ -458,6 +475,9 @@ export default {
return this.selectedRowKeys.length > 0
}
},
beforeCreate () {
this.apiParams = this.$getApiParams('createPortForwardingRule')
},
created () {
console.log(this.resource)
this.initForm()
@ -830,6 +850,9 @@ export default {
onSearch (value) {
this.searchQuery = value
this.fetchVirtualMachines()
},
isVPC () {
return 'vpcid' in this.resource
}
}
}