diff --git a/api/src/com/cloud/agent/api/to/FirewallRuleTO.java b/api/src/com/cloud/agent/api/to/FirewallRuleTO.java index edfb0aa7c17..25e7b5f60b3 100644 --- a/api/src/com/cloud/agent/api/to/FirewallRuleTO.java +++ b/api/src/com/cloud/agent/api/to/FirewallRuleTO.java @@ -48,6 +48,7 @@ public class FirewallRuleTO implements InternalIdentity { boolean revoked; boolean alreadyAdded; private List sourceCidrList; + private List destCidrList; FirewallRule.Purpose purpose; private Integer icmpType; private Integer icmpCode; @@ -170,6 +171,7 @@ public class FirewallRuleTO implements InternalIdentity { rule.getSourceCidrList(), rule.getIcmpType(), rule.getIcmpCode()); + this.destCidrList = rule.getDestinationCidrList(); this.trafficType = trafficType; this.defaultEgressPolicy = defaultEgressPolicy; } @@ -257,6 +259,10 @@ public class FirewallRuleTO implements InternalIdentity { return sourceCidrList; } + public List getDestCidrList(){ + return destCidrList; + } + public boolean isAlreadyAdded() { return alreadyAdded; } diff --git a/api/src/com/cloud/network/rules/FirewallRule.java b/api/src/com/cloud/network/rules/FirewallRule.java index 4346e2f665d..6a46c7da03d 100644 --- a/api/src/com/cloud/network/rules/FirewallRule.java +++ b/api/src/com/cloud/network/rules/FirewallRule.java @@ -79,6 +79,8 @@ public interface FirewallRule extends ControlledEntity, Identity, InternalIdenti List getSourceCidrList(); + List getDestinationCidrList(); + Long getRelated(); FirewallRuleType getType(); diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index 5784c9d0bc2..a16a327bae0 100644 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -48,6 +48,7 @@ public class ApiConstants { public static final String CIDR = "cidr"; public static final String IP6_CIDR = "ip6cidr"; public static final String CIDR_LIST = "cidrlist"; + public static final String DEST_CIDR_LIST = "destcidrlist"; public static final String CLEANUP = "cleanup"; public static final String MAKEREDUNDANTE = "makeredundant"; public static final String CLUSTER_ID = "clusterid"; diff --git a/api/src/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java index 0303445af77..1f14abf4a5a 100644 --- a/api/src/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java @@ -77,6 +77,9 @@ public class CreateEgressFirewallRuleCmd extends BaseAsyncCreateCmd implements F @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, description = "the cidr list to forward traffic from") private List cidrlist; + @Parameter(name = ApiConstants.DEST_CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, description = "the cidr list to forward traffic to") + private List destCidrList; + @Parameter(name = ApiConstants.ICMP_TYPE, type = CommandType.INTEGER, description = "type of the icmp message being sent") private Integer icmpType; @@ -113,6 +116,11 @@ public class CreateEgressFirewallRuleCmd extends BaseAsyncCreateCmd implements F } } + @Override + public List getDestinationCidrList(){ + return destCidrList; + } + public Long getVpcId() { Network network = _networkService.getNetwork(getNetworkId()); if (network == null) { @@ -136,6 +144,10 @@ public class CreateEgressFirewallRuleCmd extends BaseAsyncCreateCmd implements F cidrlist = cidrs; } + public void setDestCidrList(List cidrs){ + destCidrList = cidrs; + } + @Override public void execute() throws ResourceUnavailableException { CallContext callerContext = CallContext.current(); @@ -245,6 +257,16 @@ public class CreateEgressFirewallRuleCmd extends BaseAsyncCreateCmd implements F } } } + + //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.isValidCIDR(cidr)) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Destination cidrs formatting error" + cidr); + } + } + } + if (getProtocol().equalsIgnoreCase(NetUtils.ALL_PROTO)) { if (getSourcePortStart() != null && getSourcePortEnd() != null) { throw new InvalidParameterValueException("Do not pass ports to protocol ALL, protocol ALL do not require ports. Unable to create " + diff --git a/api/src/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java index ee62aa5ad4a..548b81447fe 100644 --- a/api/src/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java @@ -349,6 +349,11 @@ public class CreateFirewallRuleCmd extends BaseAsyncCreateCmd implements Firewal } } + @Override + public List getDestinationCidrList(){ + return null; + } + @Override public Class getEntityType() { return FirewallRule.class; diff --git a/api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java index 9a0dffe1fdc..69360608df1 100644 --- a/api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java @@ -429,6 +429,11 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P return null; } + @Override + public List getDestinationCidrList(){ + return null; + } + @Override public boolean isDisplay() { if (display != null) { diff --git a/api/src/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java index d85030e3cd9..1b367a9fa7b 100644 --- a/api/src/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java @@ -323,6 +323,11 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Sta return true; } + @Override + public List getDestinationCidrList(){ + return null; + } + @Override public Class getEntityType() { return FirewallRule.class; diff --git a/api/src/org/apache/cloudstack/api/response/FirewallResponse.java b/api/src/org/apache/cloudstack/api/response/FirewallResponse.java index 462bd1bd1b0..eabc935f424 100644 --- a/api/src/org/apache/cloudstack/api/response/FirewallResponse.java +++ b/api/src/org/apache/cloudstack/api/response/FirewallResponse.java @@ -79,6 +79,10 @@ public class FirewallResponse extends BaseResponse { @Param(description = "is rule for display to the regular user", since = "4.4", authorized = {RoleType.Admin}) private Boolean forDisplay; + @SerializedName(ApiConstants.DEST_CIDR_LIST) + @Param(description = "the cidr list to forward traffic to") + private String destCidr; + public void setId(String id) { this.id = id; } @@ -130,4 +134,8 @@ public class FirewallResponse extends BaseResponse { public void setForDisplay(Boolean forDisplay) { this.forDisplay = forDisplay; } + + public void setDestCidr(String cidrList){ + this.destCidr = cidrList; + } } diff --git a/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java b/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java index 1f8d4cfd188..707888502eb 100644 --- a/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java @@ -56,13 +56,13 @@ public class SetFirewallRulesCommand extends NetworkElementCommand { if (fwTO.revoked()) { StringBuilder sb = new StringBuilder(); /* This entry is added just to make sure atleast there will one entry in the list to get the ipaddress */ - sb.append(fwTO.getSrcIp()).append(":reverted:0:0:0:"); + sb.append(fwTO.getSrcIp()).append(":reverted:0:0:0:0:").append(fwTO.getId()).append(":"); String fwRuleEntry = sb.toString(); toAdd.add(fwRuleEntry); continue; } - List cidr; + List sCidr, dCidr; StringBuilder sb = new StringBuilder(); sb.append(fwTO.getSrcIp()).append(":").append(fwTO.getProtocol()).append(":"); if ("icmp".compareTo(fwTO.getProtocol()) == 0) { @@ -73,12 +73,13 @@ public class SetFirewallRulesCommand extends NetworkElementCommand { else sb.append(fwTO.getStringSrcPortRange()).append(":"); - cidr = fwTO.getSourceCidrList(); - if (cidr == null || cidr.isEmpty()) { - sb.append("0.0.0.0/0"); + sCidr = fwTO.getSourceCidrList(); + dCidr = fwTO.getDestCidrList(); + if (sCidr == null || sCidr.isEmpty()) { + sb.append("0.0.0.0/0"); //check if this is necessary because we are providing the source cidr by default??? } else { boolean firstEntry = true; - for (String tag : cidr) { + for (String tag : sCidr) { if (!firstEntry) sb.append("-"); sb.append(tag); @@ -86,8 +87,23 @@ public class SetFirewallRulesCommand extends NetworkElementCommand { } } sb.append(":"); - String fwRuleEntry = sb.toString(); + if(dCidr == null || dCidr.isEmpty()){ + sb.append(""); + } + else{ + boolean firstEntry = true; + for(String cidr : dCidr){ + if(!firstEntry) + sb.append("-"); + sb.append(cidr); + firstEntry = false; + } + } + sb.append(":"); + sb.append(fwTO.getId()); + sb.append(":"); + String fwRuleEntry = sb.toString(); toAdd.add(fwRuleEntry); } diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/facade/SetFirewallRulesConfigItem.java b/core/src/com/cloud/agent/resource/virtualnetwork/facade/SetFirewallRulesConfigItem.java index 3327afa9eb7..bdf8fc46263 100644 --- a/core/src/com/cloud/agent/resource/virtualnetwork/facade/SetFirewallRulesConfigItem.java +++ b/core/src/com/cloud/agent/resource/virtualnetwork/facade/SetFirewallRulesConfigItem.java @@ -40,7 +40,7 @@ public class SetFirewallRulesConfigItem extends AbstractConfigItemFacade{ final List rules = new ArrayList(); for (final FirewallRuleTO rule : command.getRules()) { final FirewallRule fwRule = new FirewallRule(rule.getId(), rule.getSrcVlanTag(), rule.getSrcIp(), rule.getProtocol(), rule.getSrcPortRange(), rule.revoked(), - rule.isAlreadyAdded(), rule.getSourceCidrList(), rule.getPurpose().toString(), rule.getIcmpType(), rule.getIcmpCode(), rule.getTrafficType().toString(), + rule.isAlreadyAdded(), rule.getSourceCidrList(), rule.getDestCidrList(), rule.getPurpose().toString(), rule.getIcmpType(), rule.getIcmpCode(), rule.getTrafficType().toString(), rule.getGuestCidr(), rule.isDefaultEgressPolicy()); rules.add(fwRule); } diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/model/FirewallRule.java b/core/src/com/cloud/agent/resource/virtualnetwork/model/FirewallRule.java index 0543094795c..44ec9bef76b 100644 --- a/core/src/com/cloud/agent/resource/virtualnetwork/model/FirewallRule.java +++ b/core/src/com/cloud/agent/resource/virtualnetwork/model/FirewallRule.java @@ -30,6 +30,7 @@ public class FirewallRule { private boolean revoked; private boolean alreadyAdded; private List sourceCidrList; + private List destCidrList; private String purpose; private Integer icmpType; private Integer icmpCode; @@ -43,7 +44,7 @@ public class FirewallRule { } public FirewallRule(long id, String srcVlanTag, String srcIp, String protocol, int[] srcPortRange, boolean revoked, boolean alreadyAdded, List sourceCidrList, - String purpose, Integer icmpType, Integer icmpCode, String trafficType, String guestCidr, boolean defaultEgressPolicy) { + List destCidrList, String purpose, Integer icmpType, Integer icmpCode, String trafficType, String guestCidr, boolean defaultEgressPolicy) { this.id = id; this.srcVlanTag = srcVlanTag; this.srcIp = srcIp; @@ -58,6 +59,7 @@ public class FirewallRule { this.trafficType = trafficType; this.guestCidr = guestCidr; this.defaultEgressPolicy = defaultEgressPolicy; + this.destCidrList = destCidrList; } public long getId() { diff --git a/engine/components-api/src/com/cloud/network/rules/StaticNatRuleImpl.java b/engine/components-api/src/com/cloud/network/rules/StaticNatRuleImpl.java index d21159bdff6..96950a12c84 100644 --- a/engine/components-api/src/com/cloud/network/rules/StaticNatRuleImpl.java +++ b/engine/components-api/src/com/cloud/network/rules/StaticNatRuleImpl.java @@ -149,6 +149,11 @@ public class StaticNatRuleImpl implements StaticNatRule { return forDisplay; } + @Override + public List getDestinationCidrList(){ + return null; + } + @Override public Class getEntityType() { return FirewallRule.class; diff --git a/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java index 5395f12b217..597ea6733e9 100644 --- a/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java @@ -449,6 +449,7 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl final Command[] cmds = checkForCommandsAndTag(commands); + //check what agent is returned. final AgentAttache agent = getAttache(hostId); if (agent == null || agent.isClosed()) { throw new AgentUnavailableException("agent not logged into this management server", hostId); diff --git a/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml index f2be2b4c919..117370720da 100644 --- a/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml +++ b/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml @@ -158,8 +158,9 @@ - + + diff --git a/engine/schema/src/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java b/engine/schema/src/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java index bfaf5627a5c..f6185309972 100644 --- a/engine/schema/src/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java +++ b/engine/schema/src/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java @@ -71,8 +71,8 @@ public class FirewallRulesCidrsDaoImpl extends GenericDaoBase { List listByIpAndPurposeWithState(Long addressId, FirewallRule.Purpose purpose, FirewallRule.State state); void loadSourceCidrs(FirewallRuleVO rule); + + void loadDestinationCidrs(FirewallRuleVO rule); } diff --git a/engine/schema/src/com/cloud/network/dao/FirewallRulesDaoImpl.java b/engine/schema/src/com/cloud/network/dao/FirewallRulesDaoImpl.java index 45ab1f84512..3ac860b08c5 100644 --- a/engine/schema/src/com/cloud/network/dao/FirewallRulesDaoImpl.java +++ b/engine/schema/src/com/cloud/network/dao/FirewallRulesDaoImpl.java @@ -54,6 +54,8 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i @Inject protected FirewallRulesCidrsDao _firewallRulesCidrsDao; @Inject + protected FirewallRulesDcidrsDao _firewallRulesDcidrsDao; + @Inject ResourceTagDao _tagsDao; @Inject IPAddressDao _ipDao; @@ -224,8 +226,17 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i txn.start(); FirewallRuleVO dbfirewallRule = super.persist(firewallRule); + + //Fill the firewall_rules_cidrs table saveSourceCidrs(firewallRule, firewallRule.getSourceCidrList()); + + //Fill the firewall_ruls_dcidrs table + saveDestinationCidrs(firewallRule, firewallRule.getDestinationCidrList()); + + //Add the source and dest cidrs into the dbfirewall rule to be returned. + //Have to read again from DB as the fields are transient. loadSourceCidrs(dbfirewallRule); + loadDestinationCidrs(dbfirewallRule); txn.commit(); return dbfirewallRule; @@ -238,6 +249,14 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i _firewallRulesCidrsDao.persist(firewallRule.getId(), cidrList); } + public void saveDestinationCidrs(FirewallRuleVO firewallRule, List cidrList){ + if(cidrList == null){ + return; + } + _firewallRulesDcidrsDao.persist(firewallRule.getId(), cidrList); + + } + @Override public List listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose) { @@ -360,4 +379,11 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i List sourceCidrs = _firewallRulesCidrsDao.getSourceCidrs(rule.getId()); rule.setSourceCidrList(sourceCidrs); } + + @Override + public void loadDestinationCidrs(FirewallRuleVO rule){ + List destCidrs = _firewallRulesDcidrsDao.getDestCidrs(rule.getId()); + rule.setDestinationCidrsList(destCidrs); + } + } diff --git a/engine/schema/src/com/cloud/network/dao/FirewallRulesDcidrsDao.java b/engine/schema/src/com/cloud/network/dao/FirewallRulesDcidrsDao.java new file mode 100644 index 00000000000..bc7efc4651c --- /dev/null +++ b/engine/schema/src/com/cloud/network/dao/FirewallRulesDcidrsDao.java @@ -0,0 +1,28 @@ +// 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 com.cloud.network.dao; + +import com.cloud.utils.db.GenericDao; + +import java.util.List; + +public interface FirewallRulesDcidrsDao extends GenericDao { + + void persist(long firewallRuleId, List destCidrs); + + List getDestCidrs(long firewallId); +} \ No newline at end of file diff --git a/engine/schema/src/com/cloud/network/dao/FirewallRulesDcidrsDaoImpl.java b/engine/schema/src/com/cloud/network/dao/FirewallRulesDcidrsDaoImpl.java new file mode 100644 index 00000000000..24d142a2d5e --- /dev/null +++ b/engine/schema/src/com/cloud/network/dao/FirewallRulesDcidrsDaoImpl.java @@ -0,0 +1,76 @@ +// 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 com.cloud.network.dao; + +import com.cloud.utils.db.DB; +import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; +import com.cloud.utils.db.Transaction; +import com.cloud.utils.db.TransactionCallbackNoReturn; +import com.cloud.utils.db.TransactionStatus; +import org.springframework.stereotype.Component; + +import javax.ejb.Local; +import java.util.ArrayList; +import java.util.List; + +@Component +@Local(value = FirewallRulesDcidrsDao.class) +public class FirewallRulesDcidrsDaoImpl extends GenericDaoBase implements FirewallRulesDcidrsDao { + + protected final SearchBuilder cidrsSearch; + + protected FirewallRulesDcidrsDaoImpl(){ + cidrsSearch = createSearchBuilder(); + cidrsSearch.and("firewallRuleId", cidrsSearch.entity().getFirewallRuleId(), SearchCriteria.Op.EQ); + cidrsSearch.done(); + + } + + @Override + @DB + public List getDestCidrs(long firewallRuleId){ + SearchCriteria sc =cidrsSearch.create(); + sc.setParameters("firewallRuleId", firewallRuleId); + + List results = search(sc, null); + + List cidrs = new ArrayList(results.size()); + for (FirewallRulesDestCidrsVO result : results) { + cidrs.add(result.getCidr()); + } + + return cidrs; + } + + @Override + @DB + public void persist(final long firewallRuleId, final List destCidrs){ + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + for(String cidr: destCidrs){ + FirewallRulesDestCidrsVO vo = new FirewallRulesDestCidrsVO(firewallRuleId, cidr); + persist(vo); + } + } + }); + } + + +} diff --git a/engine/schema/src/com/cloud/network/dao/FirewallRulesDestCidrsVO.java b/engine/schema/src/com/cloud/network/dao/FirewallRulesDestCidrsVO.java new file mode 100644 index 00000000000..ce00e0ecfb9 --- /dev/null +++ b/engine/schema/src/com/cloud/network/dao/FirewallRulesDestCidrsVO.java @@ -0,0 +1,63 @@ +// 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 com.cloud.network.dao; + +import org.apache.cloudstack.api.InternalIdentity; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; + +@Entity +@Table(name = ("firewall_rules_dcidrs")) +public class FirewallRulesDestCidrsVO implements InternalIdentity{ + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private Long id; + + @Column(name = "firewall_rule_id") + private long firewallRuleId; + + @Column(name = "destination_cidr") + private String destCidr; + + public FirewallRulesDestCidrsVO(){ + + } + + public FirewallRulesDestCidrsVO(long firewallRuleId, String destCidr){ + this.firewallRuleId = firewallRuleId; + this.destCidr = destCidr; + } + + public long getFirewallRuleId(){ + return firewallRuleId; + } + + public String getCidr(){ + return destCidr; + } + + @Override + public long getId() { + return id; + } +} \ No newline at end of file diff --git a/engine/schema/src/com/cloud/network/rules/FirewallRuleVO.java b/engine/schema/src/com/cloud/network/rules/FirewallRuleVO.java index 169102af8c7..282fa7403e7 100644 --- a/engine/schema/src/com/cloud/network/rules/FirewallRuleVO.java +++ b/engine/schema/src/com/cloud/network/rules/FirewallRuleVO.java @@ -110,6 +110,9 @@ public class FirewallRuleVO implements FirewallRule { @Transient List sourceCidrs; + @Transient + List destinationCidrs; + @Column(name = "uuid") String uuid; @@ -117,6 +120,15 @@ public class FirewallRuleVO implements FirewallRule { this.sourceCidrs = sourceCidrs; } + public void setDestinationCidrsList(List destinationCidrs){ + this.destinationCidrs = destinationCidrs; + } + + @Override + public List getDestinationCidrList(){ + return destinationCidrs; + } + @Override public List getSourceCidrList() { return sourceCidrs; @@ -213,6 +225,9 @@ public class FirewallRuleVO implements FirewallRule { this.icmpType = icmpType; this.sourceCidrs = sourceCidrs; + this.destinationCidrs = null; + + if (related != null) { assert (purpose == Purpose.Firewall) : "related field can be set for rule of purpose " + Purpose.Firewall + " only"; } @@ -224,7 +239,7 @@ public class FirewallRuleVO implements FirewallRule { } public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId, - Purpose purpose, List sourceCidrs, Integer icmpCode, Integer icmpType, Long related, TrafficType trafficType, FirewallRuleType type) { + Purpose purpose, List sourceCidrs, List destinationCidrs, Integer icmpCode, Integer icmpType, Long related, TrafficType trafficType, FirewallRuleType type) { this(xId, ipAddressId, portStart, portEnd, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType, related, trafficType); this.type = type; } @@ -234,6 +249,13 @@ public class FirewallRuleVO implements FirewallRule { this(xId, ipAddressId, port, port, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType, related, null); } + + public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId, + Purpose purpose, List sourceCidrs, List destCidrs, Integer icmpCode, Integer icmpType, Long related, TrafficType trafficType) { + this(xId,ipAddressId, portStart, portEnd, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType, related, trafficType); + this.destinationCidrs = destCidrs; + } + @Override public String toString() { return new StringBuilder("Rule[").append(id).append("-").append(purpose).append("-").append(state).append("]").toString(); diff --git a/engine/schema/src/com/cloud/upgrade/dao/Upgrade4930to41000.java b/engine/schema/src/com/cloud/upgrade/dao/Upgrade4930to41000.java index 8f14c5403c2..53a910eb449 100644 --- a/engine/schema/src/com/cloud/upgrade/dao/Upgrade4930to41000.java +++ b/engine/schema/src/com/cloud/upgrade/dao/Upgrade4930to41000.java @@ -81,6 +81,7 @@ public class Upgrade4930to41000 implements DbUpgrade { public void performDataMigration(Connection conn) { updateSystemVmTemplates(conn); populateGuestOsDetails(conn); + updateSourceCidrs(conn); } @SuppressWarnings("serial") @@ -493,4 +494,17 @@ public class Upgrade4930to41000 implements DbUpgrade { } } + + private void updateSourceCidrs(Connection conn){ + //with ipset the value for source cidr 0.0.0.0/0 can't be added in ipset. So changing it to network cidr. + try(PreparedStatement pstmt = conn.prepareStatement("UPDATE `cloud`.`firewall_rules_cidrs` AS s, (SELECT IFNULL(networks.network_cidr,networks.cidr) cidr," + + "`firewall_rules_cidrs`.`id`, `firewall_rules`.`traffic_type` "+ + "FROM `cloud`.`networks`, `cloud`.`firewall_rules`,`cloud`.`firewall_rules_cidrs` WHERE `cloud`.`networks`.`id`=`cloud`.`firewall_rules`.`network_id` " + + "AND `cloud`.`firewall_rules`.`id` = `cloud`.`firewall_rules_cidrs`.`firewall_rule_id`) AS p " + + "SET `s`.`source_cidr` = `p`.`cidr` WHERE `s`.`source_cidr`=\"0.0.0.0/0\" AND `s`.`id`=`p`.`id` AND `p`.`traffic_type`=\"Egress\" ;")){ + pstmt.execute(); + }catch (SQLException e) { + throw new CloudRuntimeException("updateSourceCidrs:Exception:" + e.getMessage(), e); + } + } } diff --git a/server/src/com/cloud/api/ApiDBUtils.java b/server/src/com/cloud/api/ApiDBUtils.java index a3e0439a0a0..a0e74aae012 100644 --- a/server/src/com/cloud/api/ApiDBUtils.java +++ b/server/src/com/cloud/api/ApiDBUtils.java @@ -311,6 +311,7 @@ import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.dao.VMSnapshotDao; import com.cloud.user.AccountManager; +import com.cloud.network.dao.FirewallRulesDcidrsDao; public class ApiDBUtils { private static ManagementServer s_ms; @@ -371,6 +372,7 @@ public class ApiDBUtils { static ConfigurationDao s_configDao; static ConsoleProxyDao s_consoleProxyDao; static FirewallRulesCidrsDao s_firewallCidrsDao; + static FirewallRulesDcidrsDao s_firewallDcidrsDao; static VMInstanceDao s_vmDao; static ResourceLimitService s_resourceLimitMgr; static ProjectService s_projectMgr; @@ -543,6 +545,8 @@ public class ApiDBUtils { @Inject private FirewallRulesCidrsDao firewallCidrsDao; @Inject + private FirewallRulesDcidrsDao firewalDcidrsDao; + @Inject private VMInstanceDao vmDao; @Inject private ResourceLimitService resourceLimitMgr; @@ -718,6 +722,7 @@ public class ApiDBUtils { s_configDao = configDao; s_consoleProxyDao = consoleProxyDao; s_firewallCidrsDao = firewallCidrsDao; + s_firewallDcidrsDao = firewalDcidrsDao; s_vmDao = vmDao; s_resourceLimitMgr = resourceLimitMgr; s_projectMgr = projectMgr; @@ -1303,6 +1308,10 @@ public class ApiDBUtils { return s_firewallCidrsDao.getSourceCidrs(id); } + public static List findFirewallDestCidrs(long id){ + return s_firewallDcidrsDao.getDestCidrs(id); + } + public static Account getProjectOwner(long projectId) { return s_projectMgr.getProjectOwner(projectId); } diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index eb9f3c66daa..7d7f0fdcdae 100644 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -2216,6 +2216,11 @@ public class ApiResponseHelper implements ResponseGenerator { List cidrs = ApiDBUtils.findFirewallSourceCidrs(fwRule.getId()); response.setCidrList(StringUtils.join(cidrs, ",")); + if(fwRule.getTrafficType() == FirewallRule.TrafficType.Egress){ + List destCidrs = ApiDBUtils.findFirewallDestCidrs(fwRule.getId()); + response.setDestCidr(StringUtils.join(destCidrs,",")); + } + if (fwRule.getTrafficType() == FirewallRule.TrafficType.Ingress) { IpAddress ip = ApiDBUtils.findIpAddressById(fwRule.getSourceIpAddressId()); response.setPublicIpAddressId(ip.getUuid()); diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index 2905fc38eb2..a8f4b1714bd 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -22,10 +22,12 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.Collections; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.dao.FirewallRulesDcidrsDao; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -111,6 +113,8 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, @Inject FirewallRulesCidrsDao _firewallCidrsDao; @Inject + FirewallRulesDcidrsDao _firewallDcidrsDao; + @Inject AccountManager _accountMgr; @Inject NetworkOrchestrationService _networkMgr; @@ -172,7 +176,11 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, throw new InvalidParameterValueException("Egress firewall rules are not supported for " + network.getGuestType() + " networks"); } - return createFirewallRule(null, caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), + List sourceCidrs = rule.getSourceCidrList(); + if (sourceCidrs != null && !sourceCidrs.isEmpty()) + Collections.replaceAll(sourceCidrs, "0.0.0.0/0", network.getCidr()); + + return createFirewallRule(null, caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), sourceCidrs, rule.getDestinationCidrList(), rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), rule.getTrafficType(), rule.isDisplay()); } @@ -183,12 +191,12 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, Long sourceIpAddressId = rule.getSourceIpAddressId(); return createFirewallRule(sourceIpAddressId, caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), - rule.getSourceCidrList(), rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), rule.getTrafficType(), rule.isDisplay()); + rule.getSourceCidrList(), null, rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), rule.getTrafficType(), rule.isDisplay()); } - + //Destination CIDR capability is currently implemented for egress rules only. For others, the field is passed as null. @DB protected FirewallRule createFirewallRule(final Long ipAddrId, Account caller, final String xId, final Integer portStart, final Integer portEnd, - final String protocol, final List sourceCidrList, final Integer icmpCode, final Integer icmpType, final Long relatedRuleId, + final String protocol, final List sourceCidrList, final List destCidrList, final Integer icmpCode, final Integer icmpType, final Long relatedRuleId, final FirewallRule.FirewallRuleType type, final Long networkId, final FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws NetworkRuleConflictException { @@ -234,24 +242,24 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, @Override public FirewallRuleVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { FirewallRuleVO newRule = - new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountIdFinal, domainIdFinal, Purpose.Firewall, - sourceCidrList, icmpCode, icmpType, relatedRuleId, trafficType); - newRule.setType(type); + new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountIdFinal, domainIdFinal, Purpose.Firewall, + sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, trafficType); + newRule.setType(type); if (forDisplay != null) { newRule.setDisplay(forDisplay); } - newRule = _firewallDao.persist(newRule); + newRule = _firewallDao.persist(newRule); - if (type == FirewallRuleType.User) - detectRulesConflict(newRule); + if (type == FirewallRuleType.User) + detectRulesConflict(newRule); - if (!_firewallDao.setStateToAdd(newRule)) { - throw new CloudRuntimeException("Unable to update the state to add for " + newRule); - } - CallContext.current().setEventDetails("Rule Id: " + newRule.getId()); + if (!_firewallDao.setStateToAdd(newRule)) { + throw new CloudRuntimeException("Unable to update the state to add for " + newRule); + } + CallContext.current().setEventDetails("Rule Id: " + newRule.getId()); - return newRule; - } + return newRule; + } }); } @@ -340,6 +348,17 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, return new Pair, Integer>(result.first(), result.second()); } + //Intermediate funciton used in detectRulesConflict to check for the conflicting cidrs in the rules already applied and the newRule being applied. + boolean detectConflictingCidrs(List cidrList1, List cidrList2){ + if(cidrList1.isEmpty() && cidrList2.isEmpty()){ + return true; + } + + Collection similar = new HashSet(cidrList1); + similar.retainAll(cidrList2); + return (similar.size()>0); + } + @Override public void detectRulesConflict(FirewallRule newRule) throws NetworkRuleConflictException { List rules; @@ -366,23 +385,16 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, boolean bothRulesFirewall = (rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall); boolean duplicatedCidrs = false; if (bothRulesFirewall) { - // Verify that the rules have different cidrs _firewallDao.loadSourceCidrs(rule); _firewallDao.loadSourceCidrs((FirewallRuleVO)newRule); - List ruleCidrList = rule.getSourceCidrList(); - List newRuleCidrList = newRule.getSourceCidrList(); + _firewallDao.loadDestinationCidrs(rule); + _firewallDao.loadDestinationCidrs((FirewallRuleVO) newRule); - if (ruleCidrList == null || newRuleCidrList == null) { + if (rule.getSourceCidrList() == null || newRule.getSourceCidrList() == null) { continue; } - - Collection similar = new HashSet(ruleCidrList); - similar.retainAll(newRuleCidrList); - - if (similar.size() > 0) { - duplicatedCidrs = true; - } + duplicatedCidrs = (detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList()) && detectConflictingCidrs(rule.getDestinationCidrList(), newRule.getDestinationCidrList())); } if (!oneOfRulesIsFirewall) { @@ -393,10 +405,12 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, } } + // Checking if the rule applied is to the same network that is passed in the rule. if (rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) { throw new NetworkRuleConflictException("New rule is for a different network than what's specified in rule " + rule.getXid()); } + //Check for the ICMP protocol. This has to be done separately from other protocols as we need to check the ICMP codes and ICMP type also. if (newRule.getProtocol().equalsIgnoreCase(NetUtils.ICMP_PROTO) && newRule.getProtocol().equalsIgnoreCase(rule.getProtocol())) { if (newRule.getIcmpCode().longValue() == rule.getIcmpCode().longValue() && newRule.getIcmpType().longValue() == rule.getIcmpType().longValue() && newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()) && duplicatedCidrs) { @@ -408,10 +422,12 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, (newRule.getSourcePortStart() != null && newRule.getSourcePortEnd() != null && rule.getSourcePortStart() != null && rule.getSourcePortEnd() != null); boolean nullPorts = (newRule.getSourcePortStart() == null && newRule.getSourcePortEnd() == null && rule.getSourcePortStart() == null && rule.getSourcePortEnd() == null); - if(nullPorts && duplicatedCidrs && (rule.getProtocol().equalsIgnoreCase(newRule.getProtocol())) && !newRule.getProtocol().equalsIgnoreCase(NetUtils.ICMP_PROTO)) - { + + // If ports are not specified and cidrs are same and protocol is also same(NOT ICMP as it is separately checked above) + if(nullPorts && duplicatedCidrs && (rule.getProtocol().equalsIgnoreCase(newRule.getProtocol())) && !newRule.getProtocol().equalsIgnoreCase(NetUtils.ICMP_PROTO)) { throw new NetworkRuleConflictException("There is already a firewall rule specified with protocol = " +newRule.getProtocol()+ " and no ports"); } + if (!notNullPorts) { continue; } else if (!oneOfRulesIsFirewall && @@ -424,6 +440,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, 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. // we allow port forwarding rules with the same parameters but different protocols boolean allowPf = @@ -657,6 +674,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, for (FirewallRuleVO rule : rules) { // load cidrs if any rule.setSourceCidrList(_firewallCidrsDao.getSourceCidrs(rule.getId())); + rule.setDestinationCidrsList(_firewallDcidrsDao.getDestCidrs(rule.getId())); } if (caller != null) { @@ -682,10 +700,14 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, NetworkVO network = _networkDao.findById(networkId); List sourceCidr = new ArrayList(); + List destCidr = new ArrayList(); + + + sourceCidr.add(network.getCidr()); + destCidr.add(NetUtils.ALL_CIDRS); - sourceCidr.add(NetUtils.ALL_CIDRS); FirewallRuleVO ruleVO = - new FirewallRuleVO(null, null, null, null, "all", networkId, network.getAccountId(), network.getDomainId(), Purpose.Firewall, sourceCidr, null, null, null, + new FirewallRuleVO(null, null, null, null, "all", networkId, network.getAccountId(), network.getDomainId(), Purpose.Firewall, sourceCidr, destCidr, null, null, null, FirewallRule.TrafficType.Egress, FirewallRuleType.System); ruleVO.setState(add ? State.Add : State.Revoke); List rules = new ArrayList(); @@ -884,7 +906,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, List oneCidr = new ArrayList(); oneCidr.add(NetUtils.ALL_CIDRS); - return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, oneCidr, icmpCode, icmpType, relatedRuleId, FirewallRule.FirewallRuleType.User, + return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, oneCidr, null, icmpCode, icmpType, relatedRuleId, FirewallRule.FirewallRuleType.User, networkId, FirewallRule.TrafficType.Ingress, true); } @@ -998,7 +1020,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, if (rule.getSourceCidrList() == null && (rule.getPurpose() == Purpose.Firewall || rule.getPurpose() == Purpose.NetworkACL)) { _firewallDao.loadSourceCidrs(rule); } - createFirewallRule(ip.getId(), acct, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), + createFirewallRule(ip.getId(), acct, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(),null, rule.getIcmpCode(), rule.getIcmpType(), rule.getRelated(), FirewallRuleType.System, rule.getNetworkId(), rule.getTrafficType(), true); } catch (Exception e) { s_logger.debug("Failed to add system wide firewall rule, due to:" + e.toString()); diff --git a/server/src/com/cloud/network/router/CommandSetupHelper.java b/server/src/com/cloud/network/router/CommandSetupHelper.java index c2b4a1ae3f2..d00c9a8861b 100644 --- a/server/src/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/com/cloud/network/router/CommandSetupHelper.java @@ -441,6 +441,7 @@ public class CommandSetupHelper { } for (final FirewallRule rule : rules) { _rulesDao.loadSourceCidrs((FirewallRuleVO) rule); + _rulesDao.loadDestinationCidrs((FirewallRuleVO)rule); final FirewallRule.TrafficType traffictype = rule.getTrafficType(); if (traffictype == FirewallRule.TrafficType.Ingress) { final IpAddress sourceIp = _networkModel.getIp(rule.getSourceIpAddressId()); diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 97f3d9483d9..55c09983ca8 100644 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -1948,10 +1948,13 @@ Configurable, StateListener sourceCidr = new ArrayList(); + final List destCidr = new ArrayList(); + + sourceCidr.add(network.getCidr()); + destCidr.add(NetUtils.ALL_CIDRS); - sourceCidr.add(NetUtils.ALL_CIDRS); final FirewallRule rule = new FirewallRuleVO(null, null, null, null, "all", networkId, network.getAccountId(), network.getDomainId(), Purpose.Firewall, sourceCidr, - null, null, null, FirewallRule.TrafficType.Egress, FirewallRule.FirewallRuleType.System); + destCidr, null, null, null, FirewallRule.TrafficType.Egress, FirewallRule.FirewallRuleType.System); rules.add(rule); } else { diff --git a/server/test/com/cloud/network/firewall/FirewallManagerTest.java b/server/test/com/cloud/network/firewall/FirewallManagerTest.java index 823b495ecdd..a0bc8976c32 100644 --- a/server/test/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/test/com/cloud/network/firewall/FirewallManagerTest.java @@ -25,6 +25,7 @@ import static org.mockito.Mockito.when; import static org.mockito.Mockito.spy; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import com.cloud.exception.NetworkRuleConflictException; @@ -189,9 +190,17 @@ public class FirewallManagerTest { FirewallRuleVO rule2 = spy(new FirewallRuleVO("rule2", 3, 1701, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null)); FirewallRuleVO rule3 = spy(new FirewallRuleVO("rule3", 3, 4500, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null)); + List sString = Arrays.asList("10.1.1.1/24","192.168.1.1/24"); + List dString1 = Arrays.asList("10.1.1.1/25"); + List dString2 = Arrays.asList("10.1.1.128/25"); + + FirewallRuleVO rule4 = spy(new FirewallRuleVO("rule4", 3L, 10, 20, "TCP", 1, 2, 1, Purpose.Firewall, sString, dString1, null, null, + null, FirewallRule.TrafficType.Egress)); + ruleList.add(rule1); ruleList.add(rule2); ruleList.add(rule3); + ruleList.add(rule4); FirewallManagerImpl firewallMgr = (FirewallManagerImpl)_firewallMgr; @@ -199,15 +208,19 @@ public class FirewallManagerTest { when(rule1.getId()).thenReturn(1L); when(rule2.getId()).thenReturn(2L); when(rule3.getId()).thenReturn(3L); + when(rule4.getId()).thenReturn(4L); FirewallRule newRule1 = new FirewallRuleVO("newRule1", 3, 500, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null); FirewallRule newRule2 = new FirewallRuleVO("newRule2", 3, 1701, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null); FirewallRule newRule3 = new FirewallRuleVO("newRule3", 3, 4500, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null); + FirewallRule newRule4 = new FirewallRuleVO("newRule4", 3L, 15, 25, "TCP", 1, 2, 1, Purpose.Firewall, sString, dString2, null, null, + null, FirewallRule.TrafficType.Egress); try { firewallMgr.detectRulesConflict(newRule1); firewallMgr.detectRulesConflict(newRule2); firewallMgr.detectRulesConflict(newRule3); + firewallMgr.detectRulesConflict(newRule4); } catch (NetworkRuleConflictException ex) { Assert.fail(); diff --git a/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java b/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java index 0a38158565a..92d421c0efb 100644 --- a/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java +++ b/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.networkoffering; import java.io.IOException; +import com.cloud.network.dao.FirewallRulesDcidrsDaoImpl; import com.cloud.storage.StorageManager; import org.mockito.Mockito; import org.springframework.context.annotation.Bean; @@ -131,7 +132,7 @@ import com.cloud.vm.dao.VMInstanceDaoImpl; DiskOfferingDaoImpl.class, DataCenterDaoImpl.class, DataCenterIpAddressDaoImpl.class, DataCenterVnetDaoImpl.class, PodVlanDaoImpl.class, DataCenterDetailsDaoImpl.class, NicSecondaryIpDaoImpl.class, UserIpv6AddressDaoImpl.class, UserDaoImpl.class, NicDaoImpl.class, NetworkDomainDaoImpl.class, HostDetailsDaoImpl.class, HostTagsDaoImpl.class, ClusterDaoImpl.class, FirewallRulesDaoImpl.class, - FirewallRulesCidrsDaoImpl.class, PhysicalNetworkDaoImpl.class, PhysicalNetworkTrafficTypeDaoImpl.class, PhysicalNetworkServiceProviderDaoImpl.class, + FirewallRulesCidrsDaoImpl.class, FirewallRulesDcidrsDaoImpl.class, PhysicalNetworkDaoImpl.class, PhysicalNetworkTrafficTypeDaoImpl.class, PhysicalNetworkServiceProviderDaoImpl.class, LoadBalancerDaoImpl.class, NetworkServiceMapDaoImpl.class, PrimaryDataStoreDaoImpl.class, StoragePoolDetailsDaoImpl.class, PortableIpRangeDaoImpl.class, RegionDaoImpl.class, PortableIpDaoImpl.class, AccountGuestVlanMapDaoImpl.class, ImageStoreDaoImpl.class, ImageStoreDetailsDaoImpl.class}, includeFilters = {@Filter(value = ChildTestConfiguration.Library.class, type = FilterType.CUSTOM)}, diff --git a/setup/db/db/schema-4930to41000.sql b/setup/db/db/schema-4930to41000.sql index 6abab2b4628..59c452fba63 100644 --- a/setup/db/db/schema-4930to41000.sql +++ b/setup/db/db/schema-4930to41000.sql @@ -246,3 +246,12 @@ CREATE TABLE `cloud`.`guest_os_details` ( ) ENGINE=InnoDB DEFAULT CHARSET=utf8; ALTER TABLE `user_ip_address` ADD COLUMN `rule_state` VARCHAR(32) COMMENT 'static rule state while removing'; +CREATE TABLE `cloud`.`firewall_rules_dcidrs`( + `id` BIGINT(20) unsigned NOT NULL AUTO_INCREMENT, + `firewall_rule_id` BIGINT(20) unsigned NOT NULL, + `destination_cidr` VARCHAR(18) DEFAULT NULL, + PRIMARY KEY (id), + UNIQUE KEY `unique_rule_dcidrs` (`firewall_rule_id`, `destination_cidr`), + KEY `fk_firewall_dcidrs_firewall_rules` (`firewall_rule_id`), + CONSTRAINT `fk_firewall_dcidrs_firewall_rules` FOREIGN KEY (`firewall_rule_id`) REFERENCES `firewall_rules` (`id`) ON DELETE CASCADE +)ENGINE=InnoDB DEFAULT CHARSET=utf8; \ No newline at end of file diff --git a/systemvm/patches/debian/config/opt/cloud/bin/configure.py b/systemvm/patches/debian/config/opt/cloud/bin/configure.py index 73a1b4f4670..9efc07c7188 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/configure.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/configure.py @@ -100,18 +100,26 @@ class CsAcl(CsDataBag): self.rule['allowed'] = True self.rule['action'] = "ACCEPT" - if self.rule['type'] == 'all' and not obj['source_cidr_list']: - self.rule['cidr'] = ['0.0.0.0/0'] + if self.rule['type'] == 'all' and obj['source_cidr_list']: + self.rule['cidr'] = [] else: self.rule['cidr'] = obj['source_cidr_list'] + if self.direction == 'egress': + try: + if not obj['dest_cidr_list']: + self.rule['dcidr'] = [] + else: + self.rule['dcidr'] = obj['dest_cidr_list'] + except Exception: + self.rule['dcidr'] = [] + logging.debug("AclIP created for rule ==> %s", self.rule) def create(self): - for cidr in self.rule['cidr']: - self.add_rule(cidr) + self.add_rule() - def add_rule(self, cidr): + def add_rule(self): icmp_type = '' rule = self.rule icmp_type = "any" @@ -126,24 +134,48 @@ class CsAcl(CsDataBag): if "first_port" in self.rule.keys() and \ self.rule['first_port'] != self.rule['last_port']: rnge = " --dport %s:%s" % (rule['first_port'], rule['last_port']) - if self.direction == 'ingress': - if rule['protocol'] == "icmp": - self.fw.append(["mangle", "front", - " -A FIREWALL_%s" % self.ip + - " -s %s " % cidr + - " -p %s " % rule['protocol'] + - " -m %s " % rule['protocol'] + - " --icmp-type %s -j %s" % (icmp_type, self.rule['action'])]) - else: - self.fw.append(["mangle", "front", - " -A FIREWALL_%s" % self.ip + - " -s %s " % cidr + - " -p %s " % rule['protocol'] + - " -m %s " % rule['protocol'] + - " %s -j RETURN" % rnge]) logging.debug("Current ACL IP direction is ==> %s", self.direction) + + if self.direction == 'ingress': + for cidr in self.rule['cidr']: + if rule['protocol'] == "icmp": + self.fw.append(["mangle", "front", + " -A FIREWALL_%s" % self.ip + + " -s %s " % cidr + + " -p %s " % rule['protocol'] + + " --icmp-type %s -j %s" % (icmp_type, self.rule['action'])]) + else: + self.fw.append(["mangle", "front", + " -A FIREWALL_%s" % self.ip + + " -s %s " % cidr + + " -p %s " % rule['protocol'] + + " %s -j RETURN" % rnge]) + + sflag=False + dflag=False if self.direction == 'egress': + ruleId = self.rule['id'] + sourceIpsetName = 'sourceCidrIpset-%d' %ruleId + destIpsetName = 'destCidrIpset-%d' %ruleId + + #create source cidr ipset + srcIpset = 'ipset create '+sourceIpsetName + ' hash:net ' + dstIpset = 'ipset create '+destIpsetName + ' hash:net ' + + CsHelper.execute(srcIpset) + CsHelper.execute(dstIpset) + for cidr in self.rule['cidr']: + ipsetAddCmd = 'ipset add '+ sourceIpsetName + ' '+cidr + CsHelper.execute(ipsetAddCmd) + sflag = True + + logging.debug("egress rule ####==> %s", self.rule) + for cidr in self.rule['dcidr']: + ipsetAddCmd = 'ipset add '+ destIpsetName + ' '+cidr + CsHelper.execute(ipsetAddCmd) + dflag = True + self.fw.append(["filter", "", " -A FW_OUTBOUND -j FW_EGRESS_RULES"]) fwr = " -I FW_EGRESS_RULES" @@ -165,16 +197,23 @@ class CsAcl(CsDataBag): else: self.rule['action'] = "ACCEPT" + egressIpsetStr='' + if sflag == True and dflag == True: + egressIpsetStr = " -m set --match-set %s src " % sourceIpsetName + \ + " -m set --match-set %s dst " % destIpsetName + elif sflag == True: + egressIpsetStr = " -m set --match-set %s src " % sourceIpsetName + elif dflag == True: + egressIpsetStr = " -m set --match-set %s dst " % destIpsetName + if rule['protocol'] == "icmp": - fwr += " -s %s " % cidr + \ - " -p %s " % rule['protocol'] + \ + fwr += egressIpsetStr + " -p %s " % rule['protocol'] + " -m %s " % rule['protocol'] + \ " --icmp-type %s" % icmp_type elif rule['protocol'] != "all": - fwr += " -s %s " % cidr + \ - " -p %s " % rule['protocol'] + \ - " %s" % rnge + fwr += egressIpsetStr + " -p %s " % rule['protocol'] + " -m %s " % rule['protocol'] + \ + " %s" % rnge elif rule['protocol'] == "all": - fwr += " -s %s " % cidr + fwr += egressIpsetStr self.fw.append(["filter", "", "%s -j %s" % (fwr, rule['action'])]) logging.debug("EGRESS rule configured for protocol ==> %s, action ==> %s", rule['protocol'], rule['action']) @@ -265,6 +304,9 @@ class CsAcl(CsDataBag): # Ensure that FW_EGRESS_RULES chain exists CsHelper.execute("iptables-save | grep '^:FW_EGRESS_RULES' || iptables -t filter -N FW_EGRESS_RULES") CsHelper.execute("iptables-save | grep '^-A FW_EGRESS_RULES -j ACCEPT$' | sed 's/^-A/iptables -t filter -D/g' | bash") + CsHelper.execute("iptables -F FW_EGRESS_RULES") + CsHelper.execute("ipset -L | grep Name: | awk {'print $2'} | ipset flush") + CsHelper.execute("ipset -L | grep Name: | awk {'print $2'} | ipset destroy") def process(self): for item in self.dbag: diff --git a/tools/appliance/definitions/systemvmtemplate/install_systemvm_packages.sh b/tools/appliance/definitions/systemvmtemplate/install_systemvm_packages.sh index 665f64ca5a3..227bfc2adce 100644 --- a/tools/appliance/definitions/systemvmtemplate/install_systemvm_packages.sh +++ b/tools/appliance/definitions/systemvmtemplate/install_systemvm_packages.sh @@ -67,6 +67,7 @@ function install_packages() { xenstore-utils libxenstore3.0 \ conntrackd ipvsadm libnetfilter-conntrack3 libnl-3-200 libnl-genl-3-200 \ ipcalc \ + ipset \ iptables-persistent \ libtcnative-1 libssl-dev libapr1-dev \ python-flask \ diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css index 636f7532d64..0477c4e9a9f 100644 --- a/ui/css/cloudstack3.css +++ b/ui/css/cloudstack3.css @@ -8223,6 +8223,7 @@ div.container div.panel div#details-tab-addloadBalancer.detail-group div.loadBal .multi-edit th.add-rule, .multi-edit td.add-rule { + border-right: 1px solid #cdcccc; } .multi-edit .data-table td.add-vm:hover { diff --git a/ui/l10n/en.js b/ui/l10n/en.js index b30e7b1b3b5..c71c7c103aa 100644 --- a/ui/l10n/en.js +++ b/ui/l10n/en.js @@ -512,6 +512,7 @@ var dictionary = {"ICMP.code":"ICMP Code", "label.cidr":"CIDR", "label.cidr.account":"CIDR or Account/Security Group", "label.cidr.list":"Source CIDR", + "label.cidr.destination.list":"Destination CIDR", "label.cisco.nexus1000v.ip.address":"Nexus 1000v IP Address", "label.cisco.nexus1000v.password":"Nexus 1000v Password", "label.cisco.nexus1000v.username":"Nexus 1000v Username", diff --git a/ui/scripts/network.js b/ui/scripts/network.js index 1f85a6309a2..6071befb0ab 100755 --- a/ui/scripts/network.js +++ b/ui/scripts/network.js @@ -1397,6 +1397,11 @@ label: 'label.cidr.list', isOptional: true }, + 'destcidrlist': { + edit: true, + label: 'label.cidr.destination.list', + isOptional: true + }, 'protocol': { label: 'label.protocol', select: function(args) { @@ -1414,6 +1419,7 @@ var name = $(this).attr('rel'); return name != 'cidrlist' && + name != 'destcidrlist' && name != 'icmptype' && name != 'icmpcode' && name != 'protocol' && @@ -1482,6 +1488,7 @@ var data = { protocol: args.data.protocol, cidrlist: args.data.cidrlist, + destcidrlist: args.data.destcidrlist, networkid: args.context.networks[0].id }; @@ -1582,7 +1589,9 @@ rule.endport = ' '; } } - + if(!rule.destcidrlist){ + rule.destcidrlist = ' '; + } return rule; }) });