From 949ad3f4c48f62b53f80e9dd05a3b7fc18939d67 Mon Sep 17 00:00:00 2001 From: Abhinandan Prateek Date: Wed, 10 Aug 2011 13:52:42 +0530 Subject: [PATCH] bug 10561: readding source cidr changes to firewall rules --- .../commands/CreateIpForwardingRuleCmd.java | 5 ++++ .../cloud/network/lb/LoadBalancingRule.java | 5 ++++ .../com/cloud/network/rules/FirewallRule.java | 2 ++ .../src/com/cloud/network/LoadBalancerVO.java | 2 +- .../network/dao/FirewallRulesDaoImpl.java | 21 ++++++++++++++++ .../network/firewall/FirewallManagerImpl.java | 17 +++++++++---- .../cloud/network/rules/FirewallManager.java | 2 +- .../cloud/network/rules/FirewallRuleVO.java | 24 +++++++++++++++---- .../network/rules/PortForwardingRuleVO.java | 2 +- .../cloud/network/rules/RulesManagerImpl.java | 4 ++-- .../network/rules/StaticNatRuleImpl.java | 5 ++++ 11 files changed, 76 insertions(+), 13 deletions(-) diff --git a/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java b/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java index 1b48d4ba59d..6d2ddc2ddd4 100644 --- a/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java +++ b/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java @@ -263,5 +263,10 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Sta public Integer getIcmpType() { return null; } + + @Override + public List getSourceCidrList() { + return null; + } } diff --git a/api/src/com/cloud/network/lb/LoadBalancingRule.java b/api/src/com/cloud/network/lb/LoadBalancingRule.java index 48b82a8341e..2103badfb2d 100644 --- a/api/src/com/cloud/network/lb/LoadBalancingRule.java +++ b/api/src/com/cloud/network/lb/LoadBalancingRule.java @@ -164,4 +164,9 @@ public class LoadBalancingRule implements FirewallRule, LoadBalancer{ public Integer getIcmpType() { return null; } + + @Override + public List getSourceCidrList() { + return null; + } } diff --git a/api/src/com/cloud/network/rules/FirewallRule.java b/api/src/com/cloud/network/rules/FirewallRule.java index 21593aa31b9..aa08fd1cfa9 100644 --- a/api/src/com/cloud/network/rules/FirewallRule.java +++ b/api/src/com/cloud/network/rules/FirewallRule.java @@ -73,5 +73,7 @@ public interface FirewallRule extends ControlledEntity { Integer getIcmpCode(); Integer getIcmpType(); + + List getSourceCidrList(); } diff --git a/server/src/com/cloud/network/LoadBalancerVO.java b/server/src/com/cloud/network/LoadBalancerVO.java index 800af92af83..1f211699f71 100644 --- a/server/src/com/cloud/network/LoadBalancerVO.java +++ b/server/src/com/cloud/network/LoadBalancerVO.java @@ -55,7 +55,7 @@ public class LoadBalancerVO extends FirewallRuleVO implements LoadBalancer { } public LoadBalancerVO(String xId, String name, String description, long srcIpId, int srcPort, int dstPort, String algorithm, long networkId, long accountId, long domainId) { - super(xId, srcIpId, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, null, null); + super(xId, srcIpId, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, null, null, null); this.name = name; this.description = description; this.algorithm = algorithm; diff --git a/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java b/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java index 78b403b5d26..ac56fa18487 100644 --- a/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java +++ b/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java @@ -176,6 +176,27 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i return listBy(sc); } + @Override @DB + public FirewallRuleVO persist(FirewallRuleVO firewallRule) { + Transaction txn = Transaction.currentTxn(); + txn.start(); + + FirewallRuleVO dbfirewallRule = super.persist(firewallRule); + saveSourceCidrs(firewallRule); + + txn.commit(); + return dbfirewallRule; + } + + + public void saveSourceCidrs(FirewallRuleVO firewallRule) { + List cidrlist = firewallRule.getSourceCidrList(); + if (cidrlist == null) { + return; + } + _firewallRulesCidrsDao.persist(firewallRule.getId(), cidrlist); + } + @Override public List listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose) { diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index b9281832db7..f02a29c73ca 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -99,13 +99,13 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma public FirewallRule createFirewallRule(FirewallRule rule) throws NetworkRuleConflictException { Account caller = UserContext.current().getCaller(); - return createFirewallRule(rule.getSourceIpAddressId(), caller, rule.getXid(), rule.getSourcePortStart() ,rule.getSourcePortEnd(), rule.getProtocol(), rule.getIcmpCode(), rule.getIcmpType()); + return createFirewallRule(rule.getSourceIpAddressId(), caller, rule.getXid(), rule.getSourcePortStart() ,rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), rule.getIcmpCode(), rule.getIcmpType()); } @DB @Override @ActionEvent(eventType = EventTypes.EVENT_FIREWALL_OPEN, eventDescription = "creating firewll rule", create = true) - public FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart,Integer portEnd, String protocol, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException{ + public FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart,Integer portEnd, String protocol, List sourceCidrList, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException{ IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); // Validate ip address @@ -128,7 +128,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma Transaction txn = Transaction.currentTxn(); txn.start(); - FirewallRuleVO newRule = new FirewallRuleVO (xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountId, domainId, Purpose.Firewall, icmpCode, icmpType); + FirewallRuleVO newRule = new FirewallRuleVO (xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountId, domainId, Purpose.Firewall, sourceCidrList, icmpCode, icmpType); newRule = _firewallDao.persist(newRule); detectRulesConflict(newRule, ipAddress); @@ -334,6 +334,12 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma return true; } + for (FirewallRuleVO rule: rules){ + // load cidrs if any + rule.setSourceCidrList(_firewallCidrsDao.getSourceCidrs(rule.getId())); + } + + if (caller != null) { _accountMgr.checkAccess(caller, rules.toArray(new FirewallRuleVO[rules.size()])); } @@ -457,7 +463,10 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma if (!rules.isEmpty()) { return rules.get(0); } - return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, icmpCode, icmpType); + + List oneCidr = new ArrayList(); + oneCidr.add(NetUtils.ALL_CIDRS); + return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, oneCidr, icmpCode, icmpType); } @Override diff --git a/server/src/com/cloud/network/rules/FirewallManager.java b/server/src/com/cloud/network/rules/FirewallManager.java index bac3975cd16..c7faf66b6ac 100644 --- a/server/src/com/cloud/network/rules/FirewallManager.java +++ b/server/src/com/cloud/network/rules/FirewallManager.java @@ -48,7 +48,7 @@ public interface FirewallManager extends FirewallService{ */ boolean revokeFirewallRule(long ruleId, boolean apply, Account caller, long userId); - FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart, Integer portEnd, String protocol, Integer icmpCode, Integer icmpType) + FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart, Integer portEnd, String protocol, List sourceCidrList, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException; FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer startPort, Integer endPort, String protocol, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException; diff --git a/server/src/com/cloud/network/rules/FirewallRuleVO.java b/server/src/com/cloud/network/rules/FirewallRuleVO.java index 448ca3030f2..cf73e52d75a 100644 --- a/server/src/com/cloud/network/rules/FirewallRuleVO.java +++ b/server/src/com/cloud/network/rules/FirewallRuleVO.java @@ -89,7 +89,22 @@ public class FirewallRuleVO implements FirewallRule { @Column(name="icmp_type") Integer icmpType; - + + // This is a delayed load value. If the value is null, + // then this field has not been loaded yet. + // Call firewallrules dao to load it. + @Transient + List sourceCidrs; + + + public void setSourceCidrList(List sourceCidrs) { + this.sourceCidrs=sourceCidrs; + } + + @Override + public List getSourceCidrList() { + return sourceCidrs; + } @Override public long getAccountId() { @@ -157,7 +172,7 @@ public class FirewallRuleVO implements FirewallRule { protected FirewallRuleVO() { } - public FirewallRuleVO(String xId, long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId, Purpose purpose, Integer icmpCode, Integer icmpType) { + 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) { this.xId = xId; if (xId == null) { this.xId = UUID.randomUUID().toString(); @@ -173,10 +188,11 @@ public class FirewallRuleVO implements FirewallRule { this.state = State.Staged; this.icmpCode = icmpCode; this.icmpType = icmpType; + this.sourceCidrs = sourceCidrs; } - public FirewallRuleVO(String xId, long ipAddressId, int port, String protocol, long networkId, long accountId, long domainId, Purpose purpose, Integer icmpCode, Integer icmpType) { - this(xId, ipAddressId, port, port, protocol, networkId, accountId, domainId, purpose, icmpCode, icmpType); + public FirewallRuleVO(String xId, long ipAddressId, int port, String protocol, long networkId, long accountId, long domainId, Purpose purpose, List sourceCidrs, Integer icmpCode, Integer icmpType) { + this(xId, ipAddressId, port, port, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType); } @Override diff --git a/server/src/com/cloud/network/rules/PortForwardingRuleVO.java b/server/src/com/cloud/network/rules/PortForwardingRuleVO.java index 3278b68304f..69d8230a307 100644 --- a/server/src/com/cloud/network/rules/PortForwardingRuleVO.java +++ b/server/src/com/cloud/network/rules/PortForwardingRuleVO.java @@ -53,7 +53,7 @@ public class PortForwardingRuleVO extends FirewallRuleVO implements PortForwardi } 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); + super(xId, srcIpId, srcPortStart, srcPortEnd, protocol, networkId, accountId, domainId, Purpose.PortForwarding, null, null, null); this.destinationIpAddress = dstIp; this.virtualMachineId = instanceId; this.destinationPortStart = dstPortStart; diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index f9492d60182..4afe37eb0e3 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -265,7 +265,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } FirewallRuleVO newRule = new FirewallRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(), - networkId, accountId, domainId, rule.getPurpose(), null, null); + networkId, accountId, domainId, rule.getPurpose(), null, null, null); newRule = _firewallDao.persist(newRule); try { @@ -904,7 +904,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { _firewallMgr.createRuleForAllCidrs(ip.getId(), caller, ports[i], ports[i], protocol, null, null); } - rules[i] = new FirewallRuleVO(null, ip.getId(), ports[i], protocol, ip.getAssociatedWithNetworkId(), ip.getAllocatedToAccountId(), ip.getAllocatedInDomainId(), purpose, null, null); + rules[i] = new FirewallRuleVO(null, ip.getId(), ports[i], protocol, ip.getAssociatedWithNetworkId(), ip.getAllocatedToAccountId(), ip.getAllocatedInDomainId(), purpose, null, null, null); rules[i] = _firewallDao.persist(rules[i]); } txn.commit(); diff --git a/server/src/com/cloud/network/rules/StaticNatRuleImpl.java b/server/src/com/cloud/network/rules/StaticNatRuleImpl.java index dec3a208a00..58e8cda4384 100644 --- a/server/src/com/cloud/network/rules/StaticNatRuleImpl.java +++ b/server/src/com/cloud/network/rules/StaticNatRuleImpl.java @@ -117,5 +117,10 @@ public class StaticNatRuleImpl implements StaticNatRule{ public Integer getIcmpType() { return null; } + + @Override + public List getSourceCidrList() { + return null; + } }