Warning, this is a long & complicated email with lots of horrible details :-) I've long been a little confused with the way iptables & bridging interacts, so set out to do some experiments. I added a -j LOG rule to every single chain in both the filter & nat tables, and then tried various traffic patterns, to see which chains were traversed & in which order. There are 2 types of config I considered - virtual networking, and shared physical device. For both these I tried with net.bridge.bridge-nf-call-iptables on & off. This gave 4 scenarios to test with. For the test I simply did 'ping -c 1 <ip addr>', which gives a simple roundtrip with a single packet in each direction. The results were as follows.... Scenario 1: Virtual network =========================== net.bridge.bridge-nf-call-iptables = 0 Host: eth0 -> Internet virbr0 -> MASQUERADE to eth0 Guest: vif1.0 -> virbr0 Traffic: Guest -> Google ------------------------ Out: NAT-PREROUTING IN=virbr0 OUT= SRC=192.168.122.47 DST=64.233.167.99 FORWARD IN=virbr0 OUT=eth0 SRC=192.168.122.47 DST=64.233.167.99 NAT-POSTROUTING IN= OUT=eth0 SRC=192.168.122.47 DST=64.233.167.99 Back: FORWARD IN=eth0 OUT=virbr0 SRC=64.233.167.99 DST=192.168.122.47 Traffic: Guest -> Host ---------------------- Out: NAT-PREROUTING IN=virbr0 OUT= SRC=192.168.122.47 DST=192.168.122.1 INPUT IN=virbr0 OUT= SRC=192.168.122.47 DST=192.168.122.1 Back: OUTPUT IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 Traffic: Host -> Guest ---------------------- Out: NAT-OUTPUT IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 OUTPUT IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 NAT-POSTROUTING IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 Back: INPUT IN=virbr0 OUT= SRC=192.168.122.47 DST=192.168.122.1 Scenario 2: Virtual network =========================== net.bridge.bridge-nf-call-iptables = 1 Host: eth0 -> Internet virbr0 -> MASQUERADE to eth0 Guest: vif1.0 -> virbr0 Traffic: Guest -> Google ------------------------ Out: NAT-PREROUTING IN=virbr0 OUT= PHYSIN=vif1.0 SRC=192.168.122.47 DST=64.233.167.99 FORWARD IN=virbr0 OUT=eth0 PHYSIN=vif1.0 SRC=192.168.122.47 DST=64.233.167.99 NAT-POSTROUTING IN= OUT=eth0 PHYSIN=vif1.0 SRC=192.168.122.47 DST=64.233.167.99 Back: FORWARD IN=eth0 OUT=virbr0 SRC=64.233.167.99 DST=192.168.122.47 Traffic: Guest -> Host ---------------------- Out: NAT-PREROUTING IN=virbr0 OUT= PHYSIN=vif1.0 SRC=192.168.122.47 DST=192.168.122.1 INPUT IN=virbr0 OUT= PHYSIN=vif1.0 SRC=192.168.122.47 DST=192.168.122.1 Back: OUTPUT IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 Traffic: Host -> Guest ---------------------- Out: NAT-OUTPUT IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 OUTPUT IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 NAT-POSTROUTING IN= OUT=virbr0 SRC=192.168.122.1 DST=192.168.122.47 Back: INPUT IN=virbr0 OUT= PHYSIN=vif1.0 SRC=192.168.122.47 DST=192.168.122.1 Scenario 3: Shared physical device ================================== net.bridge.bridge-nf-call-iptables = 0 Host: peth1 -> Internet xenbr0 -> peth1 Guest: vif2.0 -> xenbr0 Traffic: Guest -> Google ------------------------ Nada Traffic: Guest -> Host ---------------------- Out: NAT-PREROUTING IN=eth1 OUT= SRC=192.168.254.120 DST=192.168.254.132 INPUT IN=eth1 OUT= SRC=192.168.254.120 DST=192.168.254.132 Back: OUTPUT IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 Traffic: Host -> Guest ---------------------- Out: NAT-OUTPUT IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 OUTPUT IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 NAT-POSTROUTING IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 Back: INPUT IN=eth1 OUT= SRC=192.168.254.120 DST=192.168.254.132 Scenario 4: Shared physical device ================================== net.bridge.bridge-nf-call-iptables = 1 Host: peth1 -> Internet xenbr0 -> peth1 Guest: vif2.0 -> xenbr0 Traffic: Guest -> Google ------------------------ Out: NAT-PREROUTING IN=xenbr1 OUT= PHYSIN=vif2.0 SRC=192.168.254.120 DST=64.233.167.99 FORWARD IN=xenbr1 OUT=xenbr1 PHYSIN=vif2.0 PHYSOUT=peth1 SRC=192.168.254.120 DST=64.233.167.99 NAT-POSTROUTING IN= OUT=xenbr1 PHYSIN=vif2.0 PHYSOUT=peth1 SRC=192.168.254.120 DST=64.233.167.99 Back: FORWARD IN=xenbr1 OUT=xenbr1 PHYSIN=peth1 PHYSOUT=vif2.0 SRC=64.233.167.99 DST=192.168.254.120 Traffic: Guest -> Host ---------------------- Out: NAT-PREROUTING IN=xenbr1 OUT= PHYSIN=vif2.0 SRC=192.168.254.120 DST=192.168.254.132 FORWARD IN=xenbr1 OUT=xenbr1 PHYSIN=vif2.0 PHYSOUT=vif0.1 SRC=192.168.254.120 DST=192.168.254.132 NAT-POSTROUTING IN= OUT=xenbr1 PHYSIN=vif2.0 PHYSOUT=vif0.1 SRC=192.168.254.120 DST=192.168.254.132 INPUT IN=eth1 OUT= SRC=192.168.254.120 DST=192.168.254.132 Back: OUTPUT IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 FORWARD IN=xenbr1 OUT=xenbr1 PHYSIN=vif0.1 PHYSOUT=vif2.0 SRC=192.168.254.132 DST=192.168.254.120 Traffic: Host -> Guest ---------------------- Out: NAT-OUTPUT IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 OUTPUT IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 NAT-POSTROUTING IN= OUT=eth1 SRC=192.168.254.132 DST=192.168.254.120 FORWARD IN=xenbr1 OUT=xenbr1 PHYSIN=vif0.1 PHYSOUT=vif2.0 SRC=192.168.254.132 DST=192.168.254.120 Back: FORWARD IN=xenbr1 OUT=xenbr1 PHYSIN=vif2.0 PHYSOUT=vif0.1 SRC=192.168.254.120 DST=192.168.254.132 INPUT IN=eth1 OUT= SRC=192.168.254.120 DST=192.168.254.132 Now in this email I'm really only concerned with the first 2 virtual network scernaios. The shared physical device stuff can be ignored henceforth, basically because it 'just works(tm)'. For virtual networks there are basically 3 types of networking config we need to represent in terms of iptables rules, and these need to work for scenrios 1 & 2 - ie regardless of the magic sysctl knob. Here is what we currently implement...... Type 1: Isolated virtual network -------------------------------- - We don't add anything here Type 2: Forwarding to a specific NIC only ----------------------------------------- Chain POSTROUTING (policy ACCEPT 345 packets, 32627 bytes) pkts bytes target prot opt in out source destination 0 0 MASQUERADE all -- * eth1 0.0.0.0/0 0.0.0.0/0 PHYSDEV match ! --physdev-is-bridged Chain FORWARD (policy ACCEPT 29 packets, 2244 bytes) pkts bytes target prot opt in out source destination 0 0 ACCEPT all -- eth1 vnet0 0.0.0.0/0 0.0.0.0/0 state RELATED,ESTABLISHED 0 0 ACCEPT all -- vnet0 eth1 0.0.0.0/0 0.0.0.0/0 0 0 ACCEPT all -- * eth1 0.0.0.0/0 0.0.0.0/0 PHYSDEV match --physdev-in vnet0 Chain INPUT (policy ACCEPT 80483 packets, 382M bytes) pkts bytes target prot opt in out source destination 0 0 ACCEPT udp -- vnet0 * 0.0.0.0/0 0.0.0.0/0 udp dpt:53 0 0 ACCEPT tcp -- vnet0 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:53 0 0 ACCEPT udp -- vnet0 * 0.0.0.0/0 0.0.0.0/0 udp dpt:67 0 0 ACCEPT tcp -- vnet0 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:67 Type 3: Forwarding to any active NIC ------------------------------------ Chain POSTROUTING (policy ACCEPT 360 packets, 33843 bytes) pkts bytes target prot opt in out source destination 2 476 MASQUERADE all -- * * 0.0.0.0/0 0.0.0.0/0 PHYSDEV match ! --physdev-is-bridged Chain FORWARD (policy ACCEPT 29 packets, 2244 bytes) pkts bytes target prot opt in out source destination 0 0 ACCEPT all -- * virbr0 0.0.0.0/0 0.0.0.0/0 state RELATED,ESTABLISHED 0 0 ACCEPT all -- virbr0 * 0.0.0.0/0 0.0.0.0/0 0 0 ACCEPT all -- * * 0.0.0.0/0 0.0.0.0/0 PHYSDEV match --physdev-in virbr0 Chain INPUT (policy ACCEPT 80884 packets, 382M bytes) pkts bytes target prot opt in out source destination 0 0 ACCEPT udp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 udp dpt:53 0 0 ACCEPT tcp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:53 0 0 ACCEPT udp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 udp dpt:67 0 0 ACCEPT tcp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:67 So how do these shape up, given the traversal scenarios & the overall desire to be as restrictive as possible with traffic. Problem: The INPUT rules are missing altogether for the isolated virtual network so potentially DHCP/DNS will be blocked Solution: Add them - simple bug. Problem: The POSTROUTING rule is too generic so it matches pretty much any kind of traffic, from any virtual network, or even from VPN devices setup by VPNC. Solution: Only masquerade traffic whose source address is within the netmask associated with the virtual network in question Problem: The FORWARD rule is too generic, forwarding traffic to/from the virtual network regardless of whether the dest/src IP address is within the netmask associated with the virtual network. Assuming the first problem is setup to only masquerade valid IP addresses from the virtual network, this rule would then allow guests to spoof their IP and have it forwarded off-host. Solution: Only forward packets whose IP address is within the netmask associated with the virtual network Problem: The policy of the FORWARD rule is ACCEPT, and/or later user defined rules may inadvertently match on traffic from the virtual network, again allowing through spoof traffic, or traffic from what should be an isolated virtual network Solution: There needs to be a catch-all REJECT rule associated with every bridge device, in both directions Problem: There is an extra physdev match per bridge device, and per guest device. This is basically unneccessary since the previous rule sets will already have allowed through the traffic. The physdev matches also only work if net.bridge.bridge-nf-call-iptables = 1 Solution: Simply remove the per-device matches Problem: The POSTROUTING rule has a physdev match applied, which only works if net.bridge.bridge-nf-call-iptables = 1. Solution: Remove physdev match & masquerade based on network address associated with the virtual network If we apply all solution outlined here, we'll end up with a set of rules which look like this.......... Type 1: Isolated virtual network -------------------------------- Chain POSTROUTING (policy ACCEPT 273 packets, 26341 bytes) pkts bytes target prot opt in out source destination Chain FORWARD (policy ACCEPT 29 packets, 2244 bytes) pkts bytes target prot opt in out source destination 0 0 REJECT all -- * vnet2 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable 0 0 REJECT all -- vnet2 * 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable Chain INPUT (policy ACCEPT 76724 packets, 366M bytes) pkts bytes target prot opt in out source destination 0 0 ACCEPT udp -- vnet2 * 0.0.0.0/0 0.0.0.0/0 udp dpt:53 0 0 ACCEPT tcp -- vnet2 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:53 0 0 ACCEPT udp -- vnet2 * 0.0.0.0/0 0.0.0.0/0 udp dpt:67 0 0 ACCEPT tcp -- vnet2 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:67 Type 2: Forwarding to a specific NIC only ----------------------------------------- Chain POSTROUTING (policy ACCEPT 273 packets, 26341 bytes) pkts bytes target prot opt in out source destination 0 0 MASQUERADE all -- * eth1 192.168.200.0/24 0.0.0.0/0 Chain FORWARD (policy ACCEPT 29 packets, 2244 bytes) pkts bytes target prot opt in out source destination 0 0 ACCEPT all -- eth1 vnet3 0.0.0.0/0 192.168.200.0/24 state RELATED,ESTABLISHED 0 0 ACCEPT all -- vnet3 eth1 192.168.200.0/24 0.0.0.0/0 0 0 REJECT all -- * vnet3 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable 0 0 REJECT all -- vnet3 * 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable Chain INPUT (policy ACCEPT 76724 packets, 366M bytes) pkts bytes target prot opt in out source destination 0 0 ACCEPT udp -- vnet3 * 0.0.0.0/0 0.0.0.0/0 udp dpt:53 0 0 ACCEPT tcp -- vnet3 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:53 0 0 ACCEPT udp -- vnet3 * 0.0.0.0/0 0.0.0.0/0 udp dpt:67 0 0 ACCEPT tcp -- vnet3 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:67 Type 3: Forwarding to any active NIC ------------------------------------ Chain POSTROUTING (policy ACCEPT 273 packets, 26341 bytes) pkts bytes target prot opt in out source destination 16 1292 MASQUERADE all -- * * 192.168.122.0/24 0.0.0.0/0 Chain FORWARD (policy ACCEPT 29 packets, 2244 bytes) pkts bytes target prot opt in out source destination 44 20200 ACCEPT all -- * virbr0 0.0.0.0/0 192.168.122.0/24 state RELATED,ESTABLISHED 56 3676 ACCEPT all -- virbr0 * 192.168.122.0/24 0.0.0.0/0 0 0 REJECT all -- * virbr0 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable 0 0 REJECT all -- virbr0 * 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable Chain INPUT (policy ACCEPT 76724 packets, 366M bytes) pkts bytes target prot opt in out source destination 28 1728 ACCEPT udp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 udp dpt:53 0 0 ACCEPT tcp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:53 5 1640 ACCEPT udp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 udp dpt:67 0 0 ACCEPT tcp -- virbr0 * 0.0.0.0/0 0.0.0.0/0 tcp dpt:67 So in summary: - Every single network type has the 4 INPUT rules for DHCP/DNS - Every single network type has catch all REJECT rules in FORWARD chain for both directions of traffic - A network forwarding to any device, has ACCEPT rules which allow through traffic associated with the virtual network IP range to/from any device - A network forwarding to a specific device, has ACCEPT rules which allow through traffic associated with the virtual network IP range to/from that specific device. - A network forwarding to any device, has MASQUERADE rule to translate source address which matches the virtual network & destined for any dev - A network forwarding to a specific device, has MASQUERADE rule to translate source address which matches the virutal network & destinaed for that specific device - There are no physdev matches needed. Hopefully at least one person has read this far through the email and still understands what is going on.... I'm attaching a patch which implements all this. BTW, there is also a bug in the vif-bridge script for Xen which adds a per-guest VIF physdev match rule. This needs to be removed too. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Index: qemud/conf.c =================================================================== RCS file: /data/cvs/libvirt/qemud/conf.c,v retrieving revision 1.47 diff -u -p -r1.47 conf.c --- qemud/conf.c 30 Mar 2007 16:23:04 -0000 1.47 +++ qemud/conf.c 5 Apr 2007 00:51:54 -0000 @@ -32,6 +32,8 @@ #include <errno.h> #include <fcntl.h> #include <sys/wait.h> +#include <arpa/inet.h> + #include <libxml/parser.h> #include <libxml/tree.h> @@ -44,7 +46,6 @@ #include "internal.h" #include "conf.h" #include "driver.h" -#include "iptables.h" #include "uuid.h" #include "buf.h" @@ -1127,15 +1128,6 @@ qemudNetworkIfaceConnect(struct qemud_se goto error; } - if (net->type == QEMUD_NET_NETWORK && network->def->forward) { - if ((err = iptablesAddPhysdevForward(server->iptables, ifname))) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "Failed to add iptables rule to allow bridging from '%s' :%s", - ifname, strerror(err)); - goto error; - } - } - snprintf(tapfdstr, sizeof(tapfdstr), "tap,fd=%d,script=,vlan=%d", tapfd, vlan); if (!(retval = strdup(tapfdstr))) @@ -1151,8 +1143,6 @@ qemudNetworkIfaceConnect(struct qemud_se return retval; no_memory: - if (net->type == QEMUD_NET_NETWORK && network->def->forward) - iptablesRemovePhysdevForward(server->iptables, ifname); qemudReportError(server, VIR_ERR_NO_MEMORY, "tapfds"); error: if (retval) @@ -1765,6 +1755,21 @@ static int qemudParseInetXML(struct qemu netmask = NULL; } + if (def->ipAddress[0] && def->netmask[0]) { + struct in_addr inaddress, innetmask; + char *netaddr; + + inet_aton((const char*)def->ipAddress, &inaddress); + inet_aton((const char*)def->netmask, &innetmask); + + inaddress.s_addr &= innetmask.s_addr; + + netaddr = inet_ntoa(inaddress); + + snprintf(def->network,sizeof(def->network)-1, + "%s/%s", netaddr, (const char *)def->netmask); + } + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && @@ -1835,9 +1840,37 @@ static struct qemud_network_def *qemudPa } xmlXPathFreeObject(obj); + /* Parse bridge information */ + obj = xmlXPathEval(BAD_CAST "/network/bridge[1]", ctxt); + if ((obj != NULL) && (obj->type == XPATH_NODESET) && + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr > 0)) { + if (!qemudParseBridgeXML(server, def, obj->nodesetval->nodeTab[0])) { + goto error; + } + } + xmlXPathFreeObject(obj); + + /* Parse IP information */ + obj = xmlXPathEval(BAD_CAST "/network/ip[1]", ctxt); + if ((obj != NULL) && (obj->type == XPATH_NODESET) && + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr > 0)) { + if (!qemudParseInetXML(server, def, obj->nodesetval->nodeTab[0])) { + goto error; + } + } + xmlXPathFreeObject(obj); + + /* IPv4 forwarding setup */ obj = xmlXPathEval(BAD_CAST "count(/network/forward) > 0", ctxt); if ((obj != NULL) && (obj->type == XPATH_BOOLEAN) && obj->boolval) { + if (!def->ipAddress[0] || + !def->netmask[0]) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "Forwarding requested, but no IPv4 address/netmask provided"); + goto error; + } + def->forward = 1; tmp = xmlXPathEval(BAD_CAST "string(/network/forward[1]/@dev)", ctxt); if ((tmp != NULL) && (tmp->type == XPATH_STRING) && @@ -1860,26 +1893,6 @@ static struct qemud_network_def *qemudPa } xmlXPathFreeObject(obj); - /* Parse bridge information */ - obj = xmlXPathEval(BAD_CAST "/network/bridge[1]", ctxt); - if ((obj != NULL) && (obj->type == XPATH_NODESET) && - (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr > 0)) { - if (!qemudParseBridgeXML(server, def, obj->nodesetval->nodeTab[0])) { - goto error; - } - } - xmlXPathFreeObject(obj); - - /* Parse IP information */ - obj = xmlXPathEval(BAD_CAST "/network/ip[1]", ctxt); - if ((obj != NULL) && (obj->type == XPATH_NODESET) && - (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr > 0)) { - if (!qemudParseInetXML(server, def, obj->nodesetval->nodeTab[0])) { - goto error; - } - } - xmlXPathFreeObject(obj); - xmlXPathFreeContext(ctxt); return def; @@ -2622,7 +2635,7 @@ char *qemudGenerateXML(struct qemud_serv char *qemudGenerateNetworkXML(struct qemud_server *server, - struct qemud_network *network ATTRIBUTE_UNUSED, + struct qemud_network *network, struct qemud_network_def *def) { bufferPtr buf = 0; unsigned char *uuid; @@ -2654,11 +2667,17 @@ char *qemudGenerateNetworkXML(struct qem } } - if ((def->bridge != '\0' || def->disableSTP || def->forwardDelay) && - bufferVSprintf(buf, " <bridge name='%s' stp='%s' delay='%d' />\n", - def->bridge, - def->disableSTP ? "off" : "on", - def->forwardDelay) < 0) + bufferAdd(buf, " <bridge", -1); + if (qemudIsActiveNetwork(network)) { + if (bufferVSprintf(buf, " name='%s'", network->bridge) < 0) + goto no_memory; + } else if (def->bridge[0]) { + if (bufferVSprintf(buf, " name='%s'", def->bridge) < 0) + goto no_memory; + } + if (bufferVSprintf(buf, " stp='%s' forwardDelay='%d' />\n", + def->disableSTP ? "off" : "on", + def->forwardDelay) < 0) goto no_memory; if (def->ipAddress[0] || def->netmask[0]) { Index: qemud/internal.h =================================================================== RCS file: /data/cvs/libvirt/qemud/internal.h,v retrieving revision 1.21 diff -u -p -r1.21 internal.h --- qemud/internal.h 16 Mar 2007 15:03:21 -0000 1.21 +++ qemud/internal.h 5 Apr 2007 00:51:54 -0000 @@ -250,6 +250,7 @@ struct qemud_network_def { char ipAddress[BR_INET_ADDR_MAXLEN]; char netmask[BR_INET_ADDR_MAXLEN]; + char network[BR_INET_ADDR_MAXLEN+BR_INET_ADDR_MAXLEN+1]; int nranges; struct qemud_dhcp_range_def *ranges; Index: qemud/iptables.c =================================================================== RCS file: /data/cvs/libvirt/qemud/iptables.c,v retrieving revision 1.9 diff -u -p -r1.9 iptables.c --- qemud/iptables.c 30 Mar 2007 16:25:02 -0000 1.9 +++ qemud/iptables.c 5 Apr 2007 00:51:54 -0000 @@ -656,49 +657,29 @@ iptablesRemoveUdpInput(iptablesContext * return iptablesInput(ctx, iface, port, REMOVE, 0); } -static int -iptablesPhysdevForward(iptablesContext *ctx, - const char *iface, - int action) -{ - return iptablesAddRemoveRule(ctx->forward_filter, - action, - "--match", "physdev", - "--physdev-in", iface, - "--jump", "ACCEPT", - NULL); -} - -int -iptablesAddPhysdevForward(iptablesContext *ctx, - const char *iface) -{ - return iptablesPhysdevForward(ctx, iface, ADD); -} - -int -iptablesRemovePhysdevForward(iptablesContext *ctx, - const char *iface) -{ - return iptablesPhysdevForward(ctx, iface, REMOVE); -} +/* Allow all traffic coming from the bridge, with a valid network address + * to proceed to WAN + */ static int -iptablesInterfaceForward(iptablesContext *ctx, +iptablesForwardAllowOut(iptablesContext *ctx, + const char *network, const char *iface, - const char *target, + const char *physdev, int action) { - if (target && target[0]) { + if (physdev && physdev[0]) { return iptablesAddRemoveRule(ctx->forward_filter, action, + "--source", network, "--in-interface", iface, - "--out-interface", target, + "--out-interface", physdev, "--jump", "ACCEPT", NULL); } else { return iptablesAddRemoveRule(ctx->forward_filter, action, + "--source", network, "--in-interface", iface, "--jump", "ACCEPT", NULL); @@ -706,31 +687,39 @@ iptablesInterfaceForward(iptablesContext } int -iptablesAddInterfaceForward(iptablesContext *ctx, +iptablesAddForwardAllowOut(iptablesContext *ctx, + const char *network, const char *iface, - const char *target) + const char *physdev) { - return iptablesInterfaceForward(ctx, iface, target, ADD); + return iptablesForwardAllowOut(ctx, network, iface, physdev, ADD); } int -iptablesRemoveInterfaceForward(iptablesContext *ctx, +iptablesRemoveForwardAllowOut(iptablesContext *ctx, + const char *network, const char *iface, - const char *target) + const char *physdev) { - return iptablesInterfaceForward(ctx, iface, target, REMOVE); + return iptablesForwardAllowOut(ctx, network, iface, physdev, REMOVE); } + +/* Allow all traffic destined to the bridge, with a valid network address + * and associated with an existing connection + */ static int -iptablesStateForward(iptablesContext *ctx, - const char *iface, - const char *target, - int action) +iptablesForwardAllowIn(iptablesContext *ctx, + const char *network, + const char *iface, + const char *physdev, + int action) { - if (target && target[0]) { + if (physdev && physdev[0]) { return iptablesAddRemoveRule(ctx->forward_filter, action, - "--in-interface", target, + "--destination", network, + "--in-interface", physdev, "--out-interface", iface, "--match", "state", "--state", "ESTABLISHED,RELATED", @@ -739,6 +728,7 @@ iptablesStateForward(iptablesContext *ct } else { return iptablesAddRemoveRule(ctx->forward_filter, action, + "--destination", network, "--out-interface", iface, "--match", "state", "--state", "ESTABLISHED,RELATED", @@ -748,56 +738,126 @@ iptablesStateForward(iptablesContext *ct } int -iptablesAddStateForward(iptablesContext *ctx, +iptablesAddForwardAllowIn(iptablesContext *ctx, + const char *network, + const char *iface, + const char *physdev) +{ + return iptablesForwardAllowIn(ctx, network, iface, physdev, ADD); +} + +int +iptablesRemoveForwardAllowIn(iptablesContext *ctx, + const char *network, + const char *iface, + const char *physdev) +{ + return iptablesForwardAllowIn(ctx, network, iface, physdev, REMOVE); +} + + + +/* Drop all traffic trying to forward from the bridge. + * ie the bridge is the in interface + */ +static int +iptablesForwardRejectOut(iptablesContext *ctx, + const char *iface, + int action) +{ + return iptablesAddRemoveRule(ctx->forward_filter, + action, + "--in-interface", iface, + "--jump", "REJECT", + NULL); +} + +int +iptablesAddForwardRejectOut(iptablesContext *ctx, + const char *iface) +{ + return iptablesForwardRejectOut(ctx, iface, ADD); +} + +int +iptablesRemoveForwardRejectOut(iptablesContext *ctx, + const char *iface) +{ + return iptablesForwardRejectOut(ctx, iface, REMOVE); +} + + + + +/* Drop all traffic trying to forward to the bridge. + * ie the bridge is the out interface + */ +static int +iptablesForwardRejectIn(iptablesContext *ctx, const char *iface, - const char *target) + int action) +{ + return iptablesAddRemoveRule(ctx->forward_filter, + action, + "--out-interface", iface, + "--jump", "REJECT", + NULL); +} + +int +iptablesAddForwardRejectIn(iptablesContext *ctx, + const char *iface) { - return iptablesStateForward(ctx, iface, target, ADD); + return iptablesForwardRejectIn(ctx, iface, ADD); } int -iptablesRemoveStateForward(iptablesContext *ctx, - const char *iface, - const char *target) +iptablesRemoveForwardRejectIn(iptablesContext *ctx, + const char *iface) { - return iptablesStateForward(ctx, iface, target, REMOVE); + return iptablesForwardRejectIn(ctx, iface, REMOVE); } + +/* Masquerade all traffic coming from the network associated + * with the bridge + */ static int -iptablesNonBridgedMasq(iptablesContext *ctx, - const char *target, +iptablesForwardMasquerade(iptablesContext *ctx, + const char *network, + const char *physdev, int action) { - if (target && target[0]) { + if (physdev && physdev[0]) { return iptablesAddRemoveRule(ctx->nat_postrouting, action, - "--out-interface", target, - "--match", "physdev", - "!", "--physdev-is-bridged", + "--source", network, + "--out-interface", physdev, "--jump", "MASQUERADE", NULL); } else { return iptablesAddRemoveRule(ctx->nat_postrouting, action, - "--match", "physdev", - "!", "--physdev-is-bridged", + "--source", network, "--jump", "MASQUERADE", NULL); } } int -iptablesAddNonBridgedMasq(iptablesContext *ctx, - const char *target) +iptablesAddForwardMasquerade(iptablesContext *ctx, + const char *network, + const char *physdev) { - return iptablesNonBridgedMasq(ctx, target, ADD); + return iptablesForwardMasquerade(ctx, network, physdev, ADD); } int -iptablesRemoveNonBridgedMasq(iptablesContext *ctx, - const char *target) +iptablesRemoveForwardMasquerade(iptablesContext *ctx, + const char *network, + const char *physdev) { - return iptablesNonBridgedMasq(ctx, target, REMOVE); + return iptablesForwardMasquerade(ctx, network, physdev, REMOVE); } /* Index: qemud/iptables.h =================================================================== RCS file: /data/cvs/libvirt/qemud/iptables.h,v retrieving revision 1.4 diff -u -p -r1.4 iptables.h --- qemud/iptables.h 30 Mar 2007 16:25:02 -0000 1.4 +++ qemud/iptables.h 5 Apr 2007 00:51:54 -0000 @@ -43,29 +43,40 @@ int iptablesRemoveUdpInput const char *iface, int port); -int iptablesAddPhysdevForward (iptablesContext *ctx, - const char *iface); -int iptablesRemovePhysdevForward (iptablesContext *ctx, - const char *iface); - -int iptablesAddInterfaceForward (iptablesContext *ctx, +int iptablesAddForwardAllowOut (iptablesContext *ctx, + const char *network, const char *iface, - const char *target); -int iptablesRemoveInterfaceForward (iptablesContext *ctx, + const char *physdev); +int iptablesRemoveForwardAllowOut (iptablesContext *ctx, + const char *network, const char *iface, - const char *target); + const char *physdev); -int iptablesAddStateForward (iptablesContext *ctx, +int iptablesAddForwardAllowIn (iptablesContext *ctx, + const char *network, const char *iface, - const char *target); -int iptablesRemoveStateForward (iptablesContext *ctx, + const char *physdev); +int iptablesRemoveForwardAllowIn (iptablesContext *ctx, + const char *network, const char *iface, - const char *target); + const char *physdev); + +int iptablesAddForwardRejectOut (iptablesContext *ctx, + const char *iface); +int iptablesRemoveForwardRejectOut (iptablesContext *ctx, + const char *iface); + +int iptablesAddForwardRejectIn (iptablesContext *ctx, + const char *iface); +int iptablesRemoveForwardRejectIn (iptablesContext *ctx, + const char *iface); -int iptablesAddNonBridgedMasq (iptablesContext *ctx, - const char *target); -int iptablesRemoveNonBridgedMasq (iptablesContext *ctx, - const char *target); +int iptablesAddForwardMasquerade (iptablesContext *ctx, + const char *network, + const char *physdev); +int iptablesRemoveForwardMasquerade (iptablesContext *ctx, + const char *network, + const char *physdev); #endif /* __QEMUD_IPTABLES_H__ */ Index: qemud/qemud.c =================================================================== RCS file: /data/cvs/libvirt/qemud/qemud.c,v retrieving revision 1.38 diff -u -p -r1.38 qemud.c --- qemud/qemud.c 4 Apr 2007 09:32:00 -0000 1.38 +++ qemud/qemud.c 5 Apr 2007 00:51:56 -0000 @@ -45,6 +45,7 @@ #include <getopt.h> #include <ctype.h> + #include <libvirt/virterror.h> #include "internal.h" @@ -1041,26 +1042,8 @@ static int qemudVMData(struct qemud_serv } } -static void -qemudNetworkIfaceDisconnect(struct qemud_server *server, - struct qemud_vm *vm ATTRIBUTE_UNUSED, - struct qemud_vm_net_def *net) { - struct qemud_network *network; - if (net->type != QEMUD_NET_NETWORK) - return; - - if (!(network = qemudFindNetworkByName(server, net->dst.network.name))) { - return; - } else if (network->bridge[0] == '\0') { - return; - } - - iptablesRemovePhysdevForward(server->iptables, net->dst.network.ifname); -} int qemudShutdownVMDaemon(struct qemud_server *server, struct qemud_vm *vm) { - struct qemud_vm_net_def *net; - if (!qemudIsActiveVM(vm)) return 0; @@ -1079,13 +1062,6 @@ int qemudShutdownVMDaemon(struct qemud_s vm->monitor = -1; server->nvmfds -= 2; - net = vm->def->nets; - while (net) { - if (net->type == QEMUD_NET_NETWORK) - qemudNetworkIfaceDisconnect(server, vm, net); - net = net->next; - } - if (waitpid(vm->pid, NULL, WNOHANG) != vm->pid) { kill(vm->pid, SIGKILL); if (waitpid(vm->pid, NULL, 0) != vm->pid) { @@ -1251,27 +1227,20 @@ qemudAddIptablesRules(struct qemud_serve return 1; } - /* allow bridging from the bridge interface itself */ - if ((err = iptablesAddPhysdevForward(server->iptables, network->bridge))) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to allow bridging from '%s' : %s\n", - network->bridge, strerror(err)); - goto err1; - } /* allow DHCP requests through to dnsmasq */ if ((err = iptablesAddTcpInput(server->iptables, network->bridge, 67))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to allow DHCP requests from '%s' : %s\n", network->bridge, strerror(err)); - goto err2; + goto err1; } if ((err = iptablesAddUdpInput(server->iptables, network->bridge, 67))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to allow DHCP requests from '%s' : %s\n", network->bridge, strerror(err)); - goto err3; + goto err2; } /* allow DNS requests through to dnsmasq */ @@ -1279,60 +1248,95 @@ qemudAddIptablesRules(struct qemud_serve qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to allow DNS requests from '%s' : %s\n", network->bridge, strerror(err)); - goto err4; + goto err3; } if ((err = iptablesAddUdpInput(server->iptables, network->bridge, 53))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to allow DNS requests from '%s' : %s\n", network->bridge, strerror(err)); + goto err4; + } + + + /* Catch all rules to block forwarding to/from bridges */ + + if ((err = iptablesAddForwardRejectOut(server->iptables, network->bridge))) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "failed to add iptables rule to block outbound traffic from '%s' : %s\n", + network->bridge, strerror(err)); goto err5; } + if ((err = iptablesAddForwardRejectIn(server->iptables, network->bridge))) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "failed to add iptables rule to block inbound traffic to '%s' : %s\n", + network->bridge, strerror(err)); + goto err6; + } + /* The remaining rules are only needed for IP forwarding */ if (!network->def->forward) return 1; /* allow forwarding packets from the bridge interface */ - if ((err = iptablesAddInterfaceForward(server->iptables, network->bridge, network->def->forwardDev))) { + if ((err = iptablesAddForwardAllowOut(server->iptables, + network->def->network, + network->bridge, + network->def->forwardDev))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to allow forwarding from '%s' : %s\n", network->bridge, strerror(err)); - goto err6; + goto err7; } /* allow forwarding packets to the bridge interface if they are part of an existing connection */ - if ((err = iptablesAddStateForward(server->iptables, network->bridge, network->def->forwardDev))) { + if ((err = iptablesAddForwardAllowIn(server->iptables, + network->def->network, + network->bridge, + network->def->forwardDev))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to allow forwarding to '%s' : %s\n", network->bridge, strerror(err)); - goto err7; + goto err8; } /* enable masquerading */ - if ((err = iptablesAddNonBridgedMasq(server->iptables, network->def->forwardDev))) { + if ((err = iptablesAddForwardMasquerade(server->iptables, + network->def->network, + network->def->forwardDev))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to enable masquerading : %s\n", strerror(err)); - goto err8; + goto err9; } return 1; + err9: + iptablesRemoveForwardAllowIn(server->iptables, + network->def->network, + network->bridge, + network->def->forwardDev); err8: - iptablesRemoveStateForward(server->iptables, network->bridge, network->def->forwardDev); + iptablesRemoveForwardAllowOut(server->iptables, + network->def->network, + network->bridge, + network->def->forwardDev); err7: - iptablesRemoveInterfaceForward(server->iptables, network->bridge, network->def->forwardDev); + iptablesRemoveForwardRejectIn(server->iptables, + network->bridge); err6: - iptablesRemoveUdpInput(server->iptables, network->bridge, 53); + iptablesRemoveForwardRejectOut(server->iptables, + network->bridge); err5: - iptablesRemoveTcpInput(server->iptables, network->bridge, 53); + iptablesRemoveUdpInput(server->iptables, network->bridge, 53); err4: - iptablesRemoveUdpInput(server->iptables, network->bridge, 67); + iptablesRemoveTcpInput(server->iptables, network->bridge, 53); err3: - iptablesRemoveTcpInput(server->iptables, network->bridge, 67); + iptablesRemoveUdpInput(server->iptables, network->bridge, 67); err2: - iptablesRemovePhysdevForward(server->iptables, network->bridge); + iptablesRemoveTcpInput(server->iptables, network->bridge, 67); err1: return 0; } @@ -1341,15 +1345,24 @@ static void qemudRemoveIptablesRules(struct qemud_server *server, struct qemud_network *network) { if (network->def->forward) { - iptablesRemoveNonBridgedMasq(server->iptables, network->def->forwardDev); - iptablesRemoveStateForward(server->iptables, network->bridge, network->def->forwardDev); - iptablesRemoveInterfaceForward(server->iptables, network->bridge, network->def->forwardDev); + iptablesRemoveForwardMasquerade(server->iptables, + network->def->network, + network->def->forwardDev); + iptablesRemoveForwardAllowIn(server->iptables, + network->def->network, + network->bridge, + network->def->forwardDev); + iptablesRemoveForwardAllowOut(server->iptables, + network->def->network, + network->bridge, + network->def->forwardDev); } + iptablesRemoveForwardRejectIn(server->iptables, network->bridge); + iptablesRemoveForwardRejectOut(server->iptables, network->bridge); iptablesRemoveUdpInput(server->iptables, network->bridge, 53); iptablesRemoveTcpInput(server->iptables, network->bridge, 53); iptablesRemoveUdpInput(server->iptables, network->bridge, 67); iptablesRemoveTcpInput(server->iptables, network->bridge, 67); - iptablesRemovePhysdevForward(server->iptables, network->bridge); } static int