From 52a1b2decbbec5d1c3650c306bc17edd33012619 Mon Sep 17 00:00:00 2001 From: Edison Su Date: Fri, 25 Feb 2011 18:32:05 -0500 Subject: [PATCH] security group is per bridge --- .../computing/LibvirtComputingResource.java | 71 ++++++++++++---- api/src/com/cloud/agent/api/to/NetworkTO.java | 12 ++- api/src/com/cloud/agent/api/to/NicTO.java | 1 + api/src/com/cloud/vm/NicProfile.java | 9 ++ scripts/vm/network/security_group.py | 83 +++++++++++-------- .../src/com/cloud/configuration/Config.java | 1 - .../ConfigurationManagerImpl.java | 4 - .../cloud/hypervisor/HypervisorGuruBase.java | 1 + .../com/cloud/network/NetworkManagerImpl.java | 1 + .../cloud/network/guru/DirectNetworkGuru.java | 5 ++ .../cloud/network/guru/PublicNetworkGuru.java | 9 +- 11 files changed, 132 insertions(+), 65 deletions(-) diff --git a/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java b/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java index 9f90da879b7..674cc986503 100644 --- a/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java +++ b/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java @@ -1402,12 +1402,14 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv private Answer execute(SecurityIngressRulesCmd cmd) { String vif = null; + String brname = null; try { Connect conn = LibvirtConnection.getConnection(); List nics = getInterfaces(conn, cmd.getVmName()); - vif = nics.get(0).getBrName(); + vif = nics.get(0).getDevName(); + brname = nics.get(0).getBrName(); } catch (LibvirtException e) { - + return new SecurityIngressRuleAnswer(cmd, false, e.toString()); } boolean result = add_network_rules(cmd.getVmName(), @@ -1415,7 +1417,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv cmd.getGuestIp(),cmd.getSignature(), Long.toString(cmd.getSeqNum()), cmd.getGuestMac(), - cmd.stringifyRules(), vif); + cmd.stringifyRules(), vif, brname); if (!result) { s_logger.warn("Failed to program network rules for vm " + cmd.getVmName()); @@ -2053,9 +2055,17 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv s_logger.debug("starting " + vmName + ": " + vm.toString()); startDomain(conn, vmName, vm.toString()); - if (vmSpec.getType() != VirtualMachine.Type.User) { - default_network_rules_for_systemvm(vmName); - } + + NicTO[] nics = vmSpec.getNics(); + for (NicTO nic : nics) { + if (nic.isSecurityGroupEnabled()) { + if (vmSpec.getType() != VirtualMachine.Type.User) { + default_network_rules_for_systemvm(conn, vmName); + } else { + default_network_rules(conn, vmName, nic, vmSpec.getId()); + } + } + } // Attach each data volume to the VM, if there is a deferred attached disk for (DiskDef disk : vm.getDevices().getDisks()) { @@ -3129,6 +3139,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv if (!_can_bridge_firewall) { return false; } + Script cmd = new Script(_securityGroupPath, _timeout, s_logger); cmd.add("destroy_network_rules_for_vm"); cmd.add("--vmname"); @@ -3140,16 +3151,28 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return true; } - private boolean default_network_rules(String vmName, long vmId, String rules) { + private boolean default_network_rules(Connect conn, String vmName, NicTO nic, Long vmId) { if (!_can_bridge_firewall) { return false; } + + List intfs = getInterfaces(conn, vmName); + if (intfs.size() < nic.getDeviceId()) { + return false; + } + + InterfaceDef intf = intfs.get(nic.getDeviceId()); + String brname = intf.getBrName(); + String vif = intf.getDevName(); + Script cmd = new Script(_securityGroupPath, _timeout, s_logger); cmd.add("default_network_rules"); cmd.add("--vmname", vmName); - cmd.add("--rules", rules); - cmd.add("--vmid", Long.toString(vmId)); - + cmd.add("--vmid", vmId.toString()); + cmd.add("--vmip", nic.getIp()); + cmd.add("--vmmac", nic.getMac()); + cmd.add("--vif", vif); + cmd.add("--brname", brname); String result = cmd.execute(); if (result != null) { return false; @@ -3157,14 +3180,22 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return true; } - private boolean default_network_rules_for_systemvm(String vmName) { + private boolean default_network_rules_for_systemvm(Connect conn, String vmName) { if (!_can_bridge_firewall) { return false; } + List intfs = getInterfaces(conn, vmName); + if (intfs.size() < 1) { + return false; + } + /*FIX ME: */ + InterfaceDef intf = intfs.get(intfs.size() - 1); + String brname = intf.getBrName(); + Script cmd = new Script(_securityGroupPath, _timeout, s_logger); cmd.add("default_network_rules_systemvm"); - cmd.add("--vmname"); - cmd.add(vmName); + cmd.add("--vmname", vmName); + cmd.add("--brname", brname); String result = cmd.execute(); if (result != null) { return false; @@ -3172,10 +3203,11 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return true; } - private boolean add_network_rules(String vmName, String vmId, String guestIP, String sig, String seq, String mac, String rules, String vif) { + private boolean add_network_rules(String vmName, String vmId, String guestIP, String sig, String seq, String mac, String rules, String vif, String brname) { if (!_can_bridge_firewall) { return false; } + String newRules = rules.replace(" ", ";"); Script cmd = new Script(_securityGroupPath, _timeout, s_logger); cmd.add("add_network_rules"); @@ -3186,6 +3218,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv cmd.add("--seq", seq); cmd.add("--vmmac", mac); cmd.add("--vif", vif); + cmd.add("--brname", brname); if (rules != null) cmd.add("--rules", newRules); String result = cmd.execute(); @@ -3302,8 +3335,14 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv private Answer execute(NetworkRulesSystemVmCommand cmd) { boolean success = false; - - success = default_network_rules_for_systemvm(cmd.getVmName()); + Connect conn; + try { + conn = LibvirtConnection.getConnection(); + success = default_network_rules_for_systemvm(conn, cmd.getVmName()); + } catch (LibvirtException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } return new Answer(cmd, success, ""); } diff --git a/api/src/com/cloud/agent/api/to/NetworkTO.java b/api/src/com/cloud/agent/api/to/NetworkTO.java index e0d1b92b614..ed446da126e 100644 --- a/api/src/com/cloud/agent/api/to/NetworkTO.java +++ b/api/src/com/cloud/agent/api/to/NetworkTO.java @@ -37,6 +37,7 @@ public class NetworkTO { protected TrafficType type; protected URI broadcastUri; protected URI isolationUri; + protected boolean isSecurityGroupEnabled; public NetworkTO() { } @@ -84,7 +85,11 @@ public class NetworkTO { public void setType(TrafficType type) { this.type = type; } - + + public void setSecurityGroupEnabled(boolean enabled) { + this.isSecurityGroupEnabled = enabled; + } + /** * This constructor is usually for hosts where the other information are not important. * @@ -160,4 +165,9 @@ public class NetworkTO { public void setIsolationuri(URI isolationUri) { this.isolationUri = isolationUri; } + + public boolean isSecurityGroupEnabled() { + return this.isSecurityGroupEnabled; + } + } diff --git a/api/src/com/cloud/agent/api/to/NicTO.java b/api/src/com/cloud/agent/api/to/NicTO.java index 3b73ca1f0b0..addb290d171 100644 --- a/api/src/com/cloud/agent/api/to/NicTO.java +++ b/api/src/com/cloud/agent/api/to/NicTO.java @@ -9,6 +9,7 @@ public class NicTO extends NetworkTO { Integer networkRateMulticastMbps; boolean defaultNic; + public NicTO() { super(); } diff --git a/api/src/com/cloud/vm/NicProfile.java b/api/src/com/cloud/vm/NicProfile.java index ae84bdda519..1ef08db3710 100644 --- a/api/src/com/cloud/vm/NicProfile.java +++ b/api/src/com/cloud/vm/NicProfile.java @@ -34,6 +34,7 @@ public class NicProfile { String dns1; String dns2; int networkRate; + boolean isSecurityGroupEnabled; public String getDns1() { return dns1; @@ -238,6 +239,14 @@ public class NicProfile { this.reservationId = reservationId; } + public boolean isSecurityGroupEnabled() { + return this.isSecurityGroupEnabled; + } + + public void setSecurityGroupEnabled(boolean enabled) { + this.isSecurityGroupEnabled = enabled; + } + public void deallocate() { this.gateway = null; this.mode = null; diff --git a/scripts/vm/network/security_group.py b/scripts/vm/network/security_group.py index 81f3e5a4e0a..93fed8407af 100755 --- a/scripts/vm/network/security_group.py +++ b/scripts/vm/network/security_group.py @@ -54,34 +54,13 @@ def can_bridge_firewall(privnic): print "failed to turn on bridge netfilter" exit(3) -''' - try: - execute("iptables -N BRIDGE-FIREWALL") - execute("iptables -N BRIDGE-FIREWALL") - execute("iptables -I BRIDGE-FIREWALL -m state --state RELATED,ESTABLISHED -j ACCEPT") - execute("iptables -D FORWARD -j RH-Firewall-1-INPUT") - except: - logging.exception('Chain BRIDGE-FIREWALL already exists: ') - - result = 0 - try: - execute("iptables -n -L FORWARD | grep BRIDGE-FIREWALL") - except: - try: - execute("iptables -I FORWARD -m physdev --physdev-is-bridged -j BRIDGE-FIREWALL") - execute("iptables -A FORWARD -m physdev --physdev-is-bridged --physdev-out " + privnic + " -j ACCEPT") - execute("iptables -A FORWARD -j DROP") - except: - result = 1 -''' - if not os.path.exists('/var/run/cloud'): os.makedirs('/var/run/cloud') cleanup_rules_for_dead_vms() cleanup_rules() - return result + return True ''' def ipset(ipsetname, proto, start, end, ips): try: @@ -195,10 +174,14 @@ def default_ebtables_rules(vm_name, vm_ip, vm_mac, vif): return 'false' -def default_network_rules_systemvm(vm_name): +def default_network_rules_systemvm(vm_name, brname): + if not addFWFramework(brname): + return False + vifs = getVifs(vm_name) domid = getvmId(vm_name) vmchain = vm_name + brfw = "BRIDGE-FIREWALL-" + brname delete_rules_for_vm_in_bridge_firewall_chain(vm_name) @@ -210,8 +193,8 @@ def default_network_rules_systemvm(vm_name): for vif in vifs: try: - execute("iptables -A BRIDGE-FIREWALL -m physdev --physdev-is-bridged --physdev-out " + vif + " -j " + vmchain) - execute("iptables -A BRIDGE-FIREWALL -m physdev --physdev-is-bridged --physdev-in " + vif + " -j " + vmchain) + execute("iptables -A " + brfw + " -m physdev --physdev-is-bridged --physdev-out " + vif + " -j " + vmchain) + execute("iptables -A " + brfw + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -j " + vmchain) except: logging.debug("Failed to program default rules") return 'false' @@ -223,8 +206,12 @@ def default_network_rules_systemvm(vm_name): return 'true' -def default_network_rules(vm_name, vm_id, vm_ip, vm_mac, vif): +def default_network_rules(vm_name, vm_id, vm_ip, vm_mac, vif, brname): + if not addFWFramework(brname): + return False + vmName = vm_name + brfw = "BRIDGE-FIREWALL-" + brname domID = getvmId(vm_name) delete_rules_for_vm_in_bridge_firewall_chain(vmName) vmchain = vm_name @@ -243,8 +230,8 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_mac, vif): execute("iptables -F " + vmchain_default) try: - execute("iptables -A BRIDGE-FIREWALL -m physdev --physdev-is-bridged --physdev-out " + vif + " -j " + vmchain_default) - execute("iptables -A BRIDGE-FIREWALL -m physdev --physdev-is-bridged --physdev-in " + vif + " -j " + vmchain_default) + execute("iptables -A " + brfw + " -m physdev --physdev-is-bridged --physdev-out " + vif + " -j " + vmchain_default) + execute("iptables -A " + brfw + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -j " + vmchain_default) execute("iptables -A " + vmchain_default + " -m state --state RELATED,ESTABLISHED -j ACCEPT") #allow dhcp execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -p udp --dport 67 --sport 68 -j ACCEPT") @@ -272,7 +259,7 @@ def delete_rules_for_vm_in_bridge_firewall_chain(vmName): vmchain = vm_name - delcmd = "iptables -S BRIDGE-FIREWALL | grep " + vmchain + " | sed 's/-A/-D/'" + delcmd = "iptables -S | grep " + vmchain + " | grep physdev-is-bridged | sed 's/-A/-D/'" delcmds = execute(delcmd).split('\n') delcmds.pop() for cmd in delcmds: @@ -419,7 +406,7 @@ def remove_rule_log_for_vm(vmName): return result -def add_network_rules(vm_name, vm_id, vm_ip, signature, seqno, vmMac, rules, vif): +def add_network_rules(vm_name, vm_id, vm_ip, signature, seqno, vmMac, rules, vif, brname): try: vmName = vm_name domId = getvmId(vmName) @@ -437,7 +424,7 @@ def add_network_rules(vm_name, vm_id, vm_ip, signature, seqno, vmMac, rules, vif return 'true' if changes[0] or changes[2]: - default_network_rules(vmName, vm_id, vm_ip, vmMac, vif) + default_network_rules(vmName, vm_id, vm_ip, vmMac, vif, brname) lines = rules.split(';')[:-1] @@ -513,6 +500,32 @@ def getvmId(vmName): cmd = "virsh list |grep " + vmName + " | awk '{print $1}'" return bash("-c", cmd).stdout.strip() +def addFWFramework(brname): + brfw = "BRIDGE-FIREWALL-" + brname + try: + execute("iptables -L " + brfw) + except: + execute("iptables -N " + brfw) + + try: + refs = execute("iptables -n -L " + brfw + " |grep " + brfw + " | cut -d \( -f2 | awk '{print $1}'").strip() + if refs == "0": + execute("iptables -A FORWARD -i " + brname + " -m physdev --physdev-is-bridged -j " + brfw) + execute("iptables -A FORWARD -o " + brname + " -m physdev --physdev-is-bridged -j " + brfw) + phydev = execute("brctl show |grep " + brname + " | awk '{print $4}'").strip() + execute("iptables -A " + brfw + " -m physdev --physdev-is-bridged --physdev-out " + phydev + " -j ACCEPT") + execute("iptables -A " + brfw + " -m state --state RELATED,ESTABLISHED -j ACCEPT") + execute("iptables -A FORWARD -i " + brname + " -j DROP") + execute("iptables -A FORWARD -o " + brname + " -j DROP") + + return True + except: + try: + execute("iptables -F " + brfw) + except: + return False + return False + if __name__ == '__main__': logging.basicConfig(filename="/var/log/cloud/security_group.log", format="%(asctime)s - %(message)s", level=logging.DEBUG) parser = OptionParser() @@ -524,20 +537,20 @@ if __name__ == '__main__': parser.add_option("--sig", dest="sig") parser.add_option("--seq", dest="seq") parser.add_option("--rules", dest="rules") - parser.add_option("--phynic", dest="phynic") + parser.add_option("--brname", dest="brname") (option, args) = parser.parse_args() cmd = args[0] if cmd == "can_bridge_firewall": can_bridge_firewall(args[1]) elif cmd == "default_network_rules": - default_network_rules(option.vmName, option.vmID, option.vmIP, option.vmMAC, option.vif) + default_network_rules(option.vmName, option.vmID, option.vmIP, option.vmMAC, option.vif, option.brname) elif cmd == "destroy_network_rules_for_vm": destroy_network_rules_for_vm(option.vmName) elif cmd == "default_network_rules_systemvm": - default_network_rules_systemvm(option.vmName) + default_network_rules_systemvm(option.vmName, option.brname) elif cmd == "get_rule_logs_for_vms": get_rule_logs_for_vms() elif cmd == "add_network_rules": - add_network_rules(option.vmName, option.vmID, option.vmIP, option.sig, option.seq, option.vmMAC, option.rules, option.vif) + add_network_rules(option.vmName, option.vmID, option.vmIP, option.sig, option.seq, option.vmMAC, option.rules, option.vif, option.brname) elif cmd == "cleanup_rules": cleanup_rules() diff --git a/server/src/com/cloud/configuration/Config.java b/server/src/com/cloud/configuration/Config.java index 95e5aa24d63..b4731739839 100755 --- a/server/src/com/cloud/configuration/Config.java +++ b/server/src/com/cloud/configuration/Config.java @@ -152,7 +152,6 @@ public enum Config { SecStorageAllowedInternalDownloadSites("Advanced", ManagementServer.class, String.class, "secstorage.allowed.internal.sites", null, "Comma separated list of cidrs internal to the datacenter that can host template download servers", null), SecStorageEncryptCopy("Advanced", ManagementServer.class, Boolean.class, "secstorage.encrypt.copy", "false", "Use SSL method used to encrypt copy traffic between zones", "true,false"), SecStorageSecureCopyCert("Advanced", ManagementServer.class, String.class, "secstorage.ssl.cert.domain", "realhostip.com", "SSL certificate used to encrypt copy traffic between zones", null), - DirectAttachSecurityGroupsEnabled("Advanced", ManagementServer.class, Boolean.class, "direct.attach.security.groups.enabled", "false", "Ec2-style distributed firewall for direct-attach VMs", "true,false"), DirectAttachNetworkEnabled("Advanced", ManagementServer.class, Boolean.class, "direct.attach.network.externalIpAllocator.enabled", "false", "Direct-attach VMs using external DHCP server", "true,false"), DirectAttachNetworkExternalAPIURL("Advanced", ManagementServer.class, String.class, "direct.attach.network.externalIpAllocator.url", null, "Direct-attach VMs using external DHCP server (API url)", null), CheckPodCIDRs("Advanced", ManagementServer.class, String.class, "check.pod.cidrs", "true", "If true, different pods must belong to different CIDR subnets.", "true,false"), diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java index e3c1944e4ee..b550aa2c99d 100755 --- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -1135,10 +1135,6 @@ public class ConfigurationManagerImpl implements ConfigurationManager, Configura //Create deafult networks createDefaultNetworks(zone.getId(), isSecurityGroupEnabled); - - if (isSecurityGroupEnabled) { - _configDao.update(Config.DirectAttachSecurityGroupsEnabled.key(), "true"); - } txn.commit(); return zone; } catch (Exception ex) { diff --git a/server/src/com/cloud/hypervisor/HypervisorGuruBase.java b/server/src/com/cloud/hypervisor/HypervisorGuruBase.java index 84944322ee1..75b28a9665d 100644 --- a/server/src/com/cloud/hypervisor/HypervisorGuruBase.java +++ b/server/src/com/cloud/hypervisor/HypervisorGuruBase.java @@ -49,6 +49,7 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis to.setBroadcastUri(profile.getBroadCastUri()); to.setIsolationuri(profile.getIsolationUri()); to.setNetworkRateMbps(profile.getNetworkRate()); + to.setSecurityGroupEnabled(profile.isSecurityGroupEnabled()); return to; } diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 1d33cc7798d..49ff594b945 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -1172,6 +1172,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } element.prepare(network, profile, vmProfile, dest, context); } + profile.setSecurityGroupEnabled(network.isSecurityGroupEnabled()); concierge.updateNicProfile(profile, network); vmProfile.addNic(profile); } diff --git a/server/src/com/cloud/network/guru/DirectNetworkGuru.java b/server/src/com/cloud/network/guru/DirectNetworkGuru.java index 5f881155a59..9c62b29c350 100644 --- a/server/src/com/cloud/network/guru/DirectNetworkGuru.java +++ b/server/src/com/cloud/network/guru/DirectNetworkGuru.java @@ -172,6 +172,11 @@ public class DirectNetworkGuru extends AdapterBase implements NetworkGuru { } getIp(nic, dc, vm, network); + + /*It's public ip, don't release it*/ + if (network.isSecurityGroupEnabled() && nic.getIp4Address() != null) { + nic.setStrategy(ReservationStrategy.Create); + } return nic; } diff --git a/server/src/com/cloud/network/guru/PublicNetworkGuru.java b/server/src/com/cloud/network/guru/PublicNetworkGuru.java index 6ca1e73d57d..75d713531e7 100644 --- a/server/src/com/cloud/network/guru/PublicNetworkGuru.java +++ b/server/src/com/cloud/network/guru/PublicNetworkGuru.java @@ -84,14 +84,7 @@ public class PublicNetworkGuru extends AdapterBase implements NetworkGuru { protected void getIp(NicProfile nic, DataCenter dc, VirtualMachineProfile vm, Network network) throws InsufficientVirtualNetworkCapcityException, InsufficientAddressCapacityException, ConcurrentOperationException { if (nic.getIp4Address() == null) { - VlanType type = VlanType.VirtualNetwork; - Long networkId = null; - if (network.isSecurityGroupEnabled()) { - type = VlanType.DirectAttached; - networkId = network.getId(); - } - - PublicIp ip = _networkMgr.assignPublicIpAddress(dc.getId(), null, vm.getOwner(), type, networkId); + PublicIp ip = _networkMgr.assignPublicIpAddress(dc.getId(), null, vm.getOwner(), VlanType.VirtualNetwork, null); nic.setIp4Address(ip.getAddress().toString()); nic.setGateway(ip.getGateway()); nic.setNetmask(ip.getNetmask());