[libvirt] [libvirt PATCHv3 03/10] reverse sense of address matching
Stefan Berger
stefanb at linux.vnet.ibm.com
Mon Oct 17 15:47:40 UTC 2011
On 10/12/2011 03:50 PM, David L Stevens wrote:
> This patch changes rules of the form:
>
> if ! addr drop
> accept
> to:
> if addr return
> ...
> drop
>
> The patch adds a "mac" chain to do a mac address list and separates the "arp"
> chain into separate "arpmac" and "arpip" chains that can check multiple MAC
> or IP addresses in any combination. This patch itself does not support multiple
> addresses via the MAC and IP variables, but only changes the form of the rules
> to allow multiple addresses in the future.
>
> Signed-off-by: David L Stevens<dlstevens at us.ibm.com>
> ---
> examples/xml/nwfilter/Makefile.am | 4 ++
> examples/xml/nwfilter/allow-arp.xml | 5 ++-
> examples/xml/nwfilter/allow-arpip.xml | 3 ++
> examples/xml/nwfilter/allow-arpmac.xml | 3 ++
> examples/xml/nwfilter/clean-traffic.xml | 6 ++--
> examples/xml/nwfilter/no-arp-spoofing.xml | 19 ++--------
> examples/xml/nwfilter/no-arpip-spoofing.xml | 12 ++++++
> examples/xml/nwfilter/no-arpmac-spoofing.xml | 7 ++++
> examples/xml/nwfilter/no-ip-spoofing.xml | 6 ++-
> examples/xml/nwfilter/no-mac-spoofing.xml | 12 ++++--
> examples/xml/nwfilter/no-other-l2-traffic.xml | 13 +++++--
> src/conf/nwfilter_conf.c | 4 ++-
> src/conf/nwfilter_conf.h | 4 ++-
> src/nwfilter/nwfilter_ebiptables_driver.c | 48 +++++++++++++++++-------
> 14 files changed, 99 insertions(+), 47 deletions(-)
> create mode 100644 examples/xml/nwfilter/allow-arpip.xml
> create mode 100644 examples/xml/nwfilter/allow-arpmac.xml
> create mode 100644 examples/xml/nwfilter/no-arpip-spoofing.xml
> create mode 100644 examples/xml/nwfilter/no-arpmac-spoofing.xml
>
> diff --git a/examples/xml/nwfilter/Makefile.am b/examples/xml/nwfilter/Makefile.am
> index 23fd753..84aaa3c 100644
> --- a/examples/xml/nwfilter/Makefile.am
> +++ b/examples/xml/nwfilter/Makefile.am
> @@ -3,12 +3,16 @@
>
> FILTERS = \
> allow-arp.xml \
> + allow-arpip.xml \
> + allow-arpmac.xml \
> allow-dhcp-server.xml \
> allow-dhcp.xml \
> allow-incoming-ipv4.xml \
> allow-ipv4.xml \
> clean-traffic.xml \
> no-arp-spoofing.xml \
> + no-arpmac-spoofing.xml \
> + no-arpip-spoofing.xml \
> no-ip-multicast.xml \
> no-ip-spoofing.xml \
> no-mac-broadcast.xml \
> diff --git a/examples/xml/nwfilter/allow-arp.xml b/examples/xml/nwfilter/allow-arp.xml
> index 63a92b2..6271ae4 100644
> --- a/examples/xml/nwfilter/allow-arp.xml
> +++ b/examples/xml/nwfilter/allow-arp.xml
> @@ -1,3 +1,4 @@
> -<filter name='allow-arp' chain='arp'>
> -<rule direction='inout' action='accept'/>
> +<filter name='allow-arp' chain='arpmac'>
> +<filterref filter='allow-arpmac.xml'/>
> +<filterref filter='allow-arpip.xml'/>
> </filter>
> diff --git a/examples/xml/nwfilter/allow-arpip.xml b/examples/xml/nwfilter/allow-arpip.xml
> new file mode 100644
> index 0000000..6ddb6fe
> --- /dev/null
> +++ b/examples/xml/nwfilter/allow-arpip.xml
> @@ -0,0 +1,3 @@
> +<filter name='allow-arpip' chain='arpip'>
> +<rule direction='inout' action='accept'/>
> +</filter>
> diff --git a/examples/xml/nwfilter/allow-arpmac.xml b/examples/xml/nwfilter/allow-arpmac.xml
> new file mode 100644
> index 0000000..54f6714
> --- /dev/null
> +++ b/examples/xml/nwfilter/allow-arpmac.xml
> @@ -0,0 +1,3 @@
> +<filter name='allow-arpmac' chain='arpmac'>
> +<rule direction='inout' action='accept'/>
> +</filter>
> diff --git a/examples/xml/nwfilter/clean-traffic.xml b/examples/xml/nwfilter/clean-traffic.xml
> index 40f0ecb..9cee799 100644
> --- a/examples/xml/nwfilter/clean-traffic.xml
> +++ b/examples/xml/nwfilter/clean-traffic.xml
> @@ -11,10 +11,10 @@
> <!-- preventing ARP spoofing/poisoning -->
> <filterref filter='no-arp-spoofing'/>
>
> -<!-- preventing any other traffic than IPv4 and ARP -->
> -<filterref filter='no-other-l2-traffic'/>
> -
> <!-- allow qemu to send a self-announce upon migration end -->
> <filterref filter='qemu-announce-self'/>
>
> +<!-- preventing any other traffic than IPv4 and ARP -->
> +<filterref filter='no-other-l2-traffic'/>
> +
> </filter>
> diff --git a/examples/xml/nwfilter/no-arp-spoofing.xml b/examples/xml/nwfilter/no-arp-spoofing.xml
> index 3c83acd..1979b20 100644
> --- a/examples/xml/nwfilter/no-arp-spoofing.xml
> +++ b/examples/xml/nwfilter/no-arp-spoofing.xml
> @@ -1,17 +1,4 @@
> -<filter name='no-arp-spoofing' chain='arp'>
> -<uuid>f88f1932-debf-4aa1-9fbe-f10d3aa4bc95</uuid>
> -<rule action='drop' direction='out' priority='300'>
> -<mac match='no' srcmacaddr='$MAC'/>
> -</rule>
> -
> -<!-- no arp spoofing -->
> -<!-- drop if ipaddr or macaddr does not belong to guest -->
> -<rule action='drop' direction='out' priority='350'>
> -<arp match='no' arpsrcmacaddr='$MAC'/>
> -</rule>
> -<rule action='drop' direction='out' priority='400'>
> -<arp match='no' arpsrcipaddr='$IP' />
> -</rule>
> -<!-- allow everything else -->
> -<rule action='accept' direction='in' priority='425' />
> +<filter name='no-arp-spoofing'>
> +<filterref filter='no-arpmac-spoofing' />
> +<filterref filter='no-arpip-spoofing' />
> </filter>
> diff --git a/examples/xml/nwfilter/no-arpip-spoofing.xml b/examples/xml/nwfilter/no-arpip-spoofing.xml
> new file mode 100644
> index 0000000..ee42d40
> --- /dev/null
> +++ b/examples/xml/nwfilter/no-arpip-spoofing.xml
> @@ -0,0 +1,12 @@
> +<filter name='no-arpip-spoofing' chain='arpip'>
> +<!-- no arp spoofing -->
> +<!-- drop if ipaddr does not belong to guest -->
> +<rule action='return' direction='out' priority='400'>
> +<arp match='yes' arpsrcipaddr='$IP' />
> +</rule>
> +<rule action='return' direction='out' priority='410'>
> +<arp match='yes' arpsrcipaddr='0.0.0.0' />
> +</rule>
> +<!-- drop everything else -->
> +<rule action='drop' direction='out' priority='1000' />
> +</filter>
> diff --git a/examples/xml/nwfilter/no-arpmac-spoofing.xml b/examples/xml/nwfilter/no-arpmac-spoofing.xml
> new file mode 100644
> index 0000000..90499d3
> --- /dev/null
> +++ b/examples/xml/nwfilter/no-arpmac-spoofing.xml
> @@ -0,0 +1,7 @@
> +<filter name='no-arpmac-spoofing' chain='arpmac'>
> +<rule action='return' direction='out' priority='350'>
> +<arp match='yes' arpsrcmacaddr='$MAC'/>
> +</rule>
> +<!-- drop everything else -->
> +<rule action='drop' direction='out' priority='1000' />
> +</filter>
> diff --git a/examples/xml/nwfilter/no-ip-spoofing.xml b/examples/xml/nwfilter/no-ip-spoofing.xml
> index b8c94c8..84e8a5e 100644
> --- a/examples/xml/nwfilter/no-ip-spoofing.xml
> +++ b/examples/xml/nwfilter/no-ip-spoofing.xml
> @@ -1,7 +1,9 @@
> <filter name='no-ip-spoofing' chain='ipv4'>
>
> <!-- drop if srcipaddr is not the IP address of the guest -->
> -<rule action='drop' direction='out'>
> -<ip match='no' srcipaddr='$IP' />
> +<rule action='return' direction='out'>
> +<ip match='yes' srcipaddr='$IP' />
> </rule>
> +<!-- drop any that don't match the source IP list -->
> +<rule action='drop' direction='out' />
> </filter>
> diff --git a/examples/xml/nwfilter/no-mac-spoofing.xml b/examples/xml/nwfilter/no-mac-spoofing.xml
> index f210623..aee56c7 100644
> --- a/examples/xml/nwfilter/no-mac-spoofing.xml
> +++ b/examples/xml/nwfilter/no-mac-spoofing.xml
> @@ -1,5 +1,9 @@
> -<filter name='no-mac-spoofing' chain='ipv4'>
> -<rule action='drop' direction='out' priority='10'>
> -<mac match='no' srcmacaddr='$MAC' />
> -</rule>
> +<filter name='no-mac-spoofing' chain='mac'>
> +<!-- no mac spoofing -->
> +<!-- drop if macaddr does not belong to guest -->
> +<rule action='return' direction='out' priority='350'>
> +<mac match='yes' srcmacaddr='$MAC'/>
> +</rule>
> +<!-- drop everything else -->
> +<rule action='drop' direction='out' priority='1000' />
> </filter>
> diff --git a/examples/xml/nwfilter/no-other-l2-traffic.xml b/examples/xml/nwfilter/no-other-l2-traffic.xml
> index 8bad86e..0501b1a 100644
> --- a/examples/xml/nwfilter/no-other-l2-traffic.xml
> +++ b/examples/xml/nwfilter/no-other-l2-traffic.xml
> @@ -1,7 +1,12 @@
> -<filter name='no-other-l2-traffic'>
> +<filter name='no-other-l2-traffic' chain='root'>
>
> -<!-- drop all other l2 traffic than for which rules have been
> - written for; i.e., drop all other than arp and ipv4 traffic -->
> -<rule action='drop' direction='inout' priority='1000'/>
> +<!-- drop all other than arp and ipv4 traffic -->
> +<rule action='accept' direction='inout'>
> +<mac protocolid='0x800' />
> +</rule>
> +<rule action='accept' direction='inout'>
> +<mac protocolid='0x806' />
> +</rule>
The hex numbers can be replaced with their string representation. Afaics
the parser/formatter consults the macProtoMap in nwfilter_conf.c and
translates it into the protocol number for safer (internal) use.
> +<rule action='drop' direction='inout' priority='1000' />
>
> </filter>
> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
> index e0c2fb6..31199cb 100644
> --- a/src/conf/nwfilter_conf.c
> +++ b/src/conf/nwfilter_conf.c
> @@ -82,7 +82,9 @@ VIR_ENUM_IMPL(virNWFilterEbtablesTable, VIR_NWFILTER_EBTABLES_TABLE_LAST,
>
> VIR_ENUM_IMPL(virNWFilterChainSuffix, VIR_NWFILTER_CHAINSUFFIX_LAST,
> "root",
> - "arp",
> + "mac",
> + "arpmac",
> + "arpip",
> "rarp",
> "ipv4",
> "ipv6");
> diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
> index c96851a..17e954e 100644
> --- a/src/conf/nwfilter_conf.h
> +++ b/src/conf/nwfilter_conf.h
> @@ -428,7 +428,9 @@ struct _virNWFilterEntry {
>
> enum virNWFilterChainSuffixType {
> VIR_NWFILTER_CHAINSUFFIX_ROOT = 0,
> - VIR_NWFILTER_CHAINSUFFIX_ARP,
> + VIR_NWFILTER_CHAINSUFFIX_MAC,
> + VIR_NWFILTER_CHAINSUFFIX_ARPMAC,
> + VIR_NWFILTER_CHAINSUFFIX_ARPIP,
> VIR_NWFILTER_CHAINSUFFIX_RARP,
> VIR_NWFILTER_CHAINSUFFIX_IPv4,
> VIR_NWFILTER_CHAINSUFFIX_IPv6,
> diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
> index f87cfa1..3c6fca7 100644
> --- a/src/nwfilter/nwfilter_ebiptables_driver.c
> +++ b/src/nwfilter/nwfilter_ebiptables_driver.c
> @@ -130,20 +130,24 @@ struct ushort_map {
>
>
> enum l3_proto_idx {
> - L3_PROTO_IPV4_IDX = 0,
> - L3_PROTO_IPV6_IDX,
> - L3_PROTO_ARP_IDX,
> + L3_PROTO_MAC_IDX = 0,
> + L3_PROTO_ARPMAC_IDX,
> + L3_PROTO_ARPIP_IDX,
> L3_PROTO_RARP_IDX,
> + L3_PROTO_IPV4_IDX,
> + L3_PROTO_IPV6_IDX,
> L3_PROTO_LAST_IDX
> };
>
The ordering here was meant to create the chains used to evaluating the
ARP protocol after IPv4 for faster processing of IPv4, which would be
the bulk and efficiency would matter most for IPv4. MAC can be first.
Also don't just remove the ARP table. It can stay, even if example
filters were to be changed. Others may have written filters accessing
the arp chain and still need it.
> #define USHORTMAP_ENTRY_IDX(IDX, ATT, VAL) [IDX] = { .attr = ATT, .val = VAL }
>
> static const struct ushort_map l3_protocols[] = {
> - USHORTMAP_ENTRY_IDX(L3_PROTO_IPV4_IDX, ETHERTYPE_IP , "ipv4"),
> - USHORTMAP_ENTRY_IDX(L3_PROTO_IPV6_IDX, ETHERTYPE_IPV6 , "ipv6"),
> - USHORTMAP_ENTRY_IDX(L3_PROTO_ARP_IDX , ETHERTYPE_ARP , "arp"),
> - USHORTMAP_ENTRY_IDX(L3_PROTO_RARP_IDX, ETHERTYPE_REVARP, "rarp"),
> + USHORTMAP_ENTRY_IDX(L3_PROTO_MAC_IDX, 0 , "mac"),
> + USHORTMAP_ENTRY_IDX(L3_PROTO_IPV4_IDX, ETHERTYPE_IP , "ipv4"),
> + USHORTMAP_ENTRY_IDX(L3_PROTO_IPV6_IDX, ETHERTYPE_IPV6 , "ipv6"),
> + USHORTMAP_ENTRY_IDX(L3_PROTO_ARPMAC_IDX,ETHERTYPE_ARP , "arpmac"),
> + USHORTMAP_ENTRY_IDX(L3_PROTO_ARPIP_IDX, ETHERTYPE_ARP , "arpip"),
> + USHORTMAP_ENTRY_IDX(L3_PROTO_RARP_IDX, ETHERTYPE_REVARP, "rarp"),
> USHORTMAP_ENTRY_IDX(L3_PROTO_LAST_IDX, 0 , NULL),
> };
>
> @@ -1947,7 +1951,7 @@ ebtablesCreateRuleInstance(char chainPrefix,
>
> virBufferAsprintf(&buf, " -p 0x%x",
> (rule->prtclType == VIR_NWFILTER_RULE_PROTOCOL_ARP)
> - ? l3_protocols[L3_PROTO_ARP_IDX].attr
> + ? l3_protocols[L3_PROTO_ARPMAC_IDX].attr
> : l3_protocols[L3_PROTO_RARP_IDX].attr);
>
> if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataHWType)) {
> @@ -2775,15 +2779,22 @@ ebtablesCreateTmpSubChain(virBufferPtr buf,
> char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
> char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
> : CHAINPREFIX_HOST_OUT_TEMP;
> + char protostr[16];
>
> PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
> PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val);
>
> + if (l3_protocols[protoidx].attr)
> + snprintf(protostr, sizeof(protostr), "-p 0x%04x ",
> + l3_protocols[protoidx].attr);
> + else
> + protostr[0] = '\0';
> +
> virBufferAsprintf(buf,
> CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
> CMD_EXEC
> "%s"
> - CMD_DEF("%s -t %s -A %s -p 0x%x -j %s") CMD_SEPARATOR
> + CMD_DEF("%s -t %s -A %s %s -j %s") CMD_SEPARATOR
> CMD_EXEC
> "%s",
>
> @@ -2792,7 +2803,7 @@ ebtablesCreateTmpSubChain(virBufferPtr buf,
> CMD_STOPONERR(stopOnError),
>
> ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
> - rootchain, l3_protocols[protoidx].attr, chain,
> + rootchain, protostr, chain,
>
> CMD_STOPONERR(stopOnError));
>
I believe this and the preceding hunk would be necessary with the
patches I posted today.
> @@ -3365,6 +3376,11 @@ ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED,
> if (chains_out != 0)
> ebtablesCreateTmpRootChain(&buf, 0, ifname, 1);
>
> + if (chains_in& (1<< VIR_NWFILTER_CHAINSUFFIX_MAC))
> + ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_MAC_IDX, 1);
> + if (chains_out& (1<< VIR_NWFILTER_CHAINSUFFIX_MAC))
> + ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_MAC_IDX, 1);
> +
> if (chains_in& (1<< VIR_NWFILTER_CHAINSUFFIX_IPv4))
> ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV4_IDX, 1);
> if (chains_out& (1<< VIR_NWFILTER_CHAINSUFFIX_IPv4))
> @@ -3376,10 +3392,14 @@ ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED,
> ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV6_IDX, 1);
>
> /* keep arp,rarp as last */
> - if (chains_in& (1<< VIR_NWFILTER_CHAINSUFFIX_ARP))
> - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARP_IDX, 1);
> - if (chains_out& (1<< VIR_NWFILTER_CHAINSUFFIX_ARP))
> - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARP_IDX, 1);
> + if (chains_in& (1<< VIR_NWFILTER_CHAINSUFFIX_ARPMAC))
> + ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARPMAC_IDX, 1);
> + if (chains_in& (1<< VIR_NWFILTER_CHAINSUFFIX_ARPIP))
> + ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARPIP_IDX, 1);
> + if (chains_out& (1<< VIR_NWFILTER_CHAINSUFFIX_ARPMAC))
> + ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARPMAC_IDX, 1);
> + if (chains_out& (1<< VIR_NWFILTER_CHAINSUFFIX_ARPIP))
> + ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARPIP_IDX, 1);
> if (chains_in& (1<< VIR_NWFILTER_CHAINSUFFIX_RARP))
> ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_RARP_IDX, 1);
> if (chains_out& (1<< VIR_NWFILTER_CHAINSUFFIX_RARP))
these two could go.
More information about the libvir-list
mailing list