Re: [Xen-devel] [PATCH 3/5] hotplug/linux: Add IPv6 support to the iptables logic

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-20 at 16:56 +0200, Sylvain Munaut wrote:
> This adds the same functions for ip6tables as the one for iptables.
> The 'ip' variable can now contain ipv6s for the domain and add
> appropriate rules
> 
>  - If the 'ip' var is empty then both full IPv4 and IPv6 are allowed.
>  - If only IPv4 ips are present, then IPv6 will be completely disallowed.
>  - If only IPv6 ips are present, then IPv4 will be completely disallowed.

The respective examples for these cases show "[nothing]" in the tables
for the "wrong" one. I think that is correct, but I would describe it
here as "will have no rules created" since "completely disallowed" would
depend on the default rules being set to drop.

>  - You can use ::0/0 or 0.0.0.0/0 to allow v6 or v4 globally but filter
>    the other one.
> 
>  (see below for examples of rules generated after this patch)
> 
> This gracefully handles if the dom0 doesn't have IPv6. If
> the call to ip6tables doesn't succeed, it just ignores any
> IPv6 stuff.
> 
> By default, domains aren't allows to send Router Advertisement
> or DHCP responses, see the ipv6_allow_ra to enable them. This
> is to limit the risk of a rogue VM sending RA and DHCPv6 response
> to try and intercept traffic by taking advantage of unconfigured
> VMs (where IPv6 autoconfig is enabled by default but not used)

I'm still not entirely sure about differing from v4 in this way. Does
anyone else have any thoughts, Ian? (See 
http://bugs.xenproject.org/xen/mid/%3C1400594068.6946.66.camel@xxxxxxxxxxxxxxxxxxxxxx%3E
for my previous comments)

Other than that the proposed new rules sets look good to me.

> Examples of rules added :
> 
>  * ip=""
> 
>  iptables:
> 
>   ACCEPT  all  0.0.0.0/0  0.0.0.0/0  PHYSDEV match --physdev-in
>    vif91.0 --physdev-is-bridged
>   ACCEPT  all  0.0.0.0/0  0.0.0.0/0  PHYSDEV match --physdev-out
>    vif91.0 --physdev-is-bridged
> 
>  ip6tables:
> 
>   DROP    udp    ::/0  ::/0  PHYSDEV match --physdev-in  vif91.0
>    --physdev-is-bridged udp spt:547 dpt:546
>   DROP    icmpv6 ::/0  ::/0  PHYSDEV match --physdev-in  vif91.0
>    --physdev-is-bridged ipv6-icmptype 134
>   ACCEPT  all    ::/0  ::/0  PHYSDEV match --physdev-out vif91.0
>    --physdev-is-bridged
>   ACCEPT  all    ::/0  ::/0  PHYSDEV match --physdev-in  vif91.0
>    --physdev-is-bridged
> 
>  * ip="192.168.0.254"
> 
>  iptables:
> 
>   ACCEPT  udp  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-in
>    vif92.0 --physdev-is-bridged udp spt:68 dpt:67
>   ACCEPT  all  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-out
>    vif92.0 --physdev-is-bridged
>   ACCEPT  all  192.168.0.254  0.0.0.0/0  PHYSDEV match --physdev-in
>    vif92.0 --physdev-is-bridged
> 
>  ip6tables:
> 
>   [nothing]
> 
>  * ip="2001:aaaa:bbbb:cccc::1 eui64"
> 
>  iptables:
> 
>   [nothing]
> 
>  ip6tables:
> 
>   DROP    udp    ::/0                 ::/0
>    PHYSDEV match --physdev-in  vif94.0 --physdev-is-bridged udp spt:547
>  dpt:546
>   DROP    icmpv6 ::/0                 ::/0
>    PHYSDEV match --physdev-in  vif94.0 --physdev-is-bridged ipv6-icmptype
>  134
>   ACCEPT  udp    ::/0                 ::/0
>    PHYSDEV match --physdev-in  vif94.0 --physdev-is-bridged udp spt:546
>    dpt:547
>   ACCEPT  all    fe80::216:3eff:fed0:da2d/128  ::/0
>    PHYSDEV match --physdev-in  vif94.0 --physdev-is-bridged
>   ACCEPT  all    ::/0                 ::/0
>    PHYSDEV match --physdev-out vif94.0 --physdev-is-bridged
>   ACCEPT  all    2001:aaaa:bbbb:cccc::1/128  ::/0
>    PHYSDEV match --physdev-in  vif94.0 --physdev-is-bridged
>   ACCEPT  all    ::216:3eff:fed0:da2d/::ffff:ffff:ffff:ffff  ::/0
>    PHYSDEV match --physdev-in  vif94.0 --physdev-is-bridged
> 
>  * ip="192.168.0.254 2001:aaaa:bbbb:cccc::1" (either ipv4 or ipv6 can
>  be replaced by the 0.0.0.0/0 or ::0/0 address to allow any, the
>  dhcp/nd rules might be redudant then).

"redundant"

> 
>  iptables:
> 
>   ACCEPT  udp  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-in
>    vif95.0 --physdev-is-bridged udp spt:68 dpt:67
>   ACCEPT  all  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-out
>    vif95.0 --physdev-is-bridged
>   ACCEPT  all  192.168.0.254  0.0.0.0/0  PHYSDEV match --physdev-in
>    vif95.0 --physdev-is-bridged
> 
>  ip6tables:
> 
>   DROP    udp    ::/0                 ::/0           PHYSDEV match
>    --physdev-in  vif95.0 --physdev-is-bridged udp spt:547 dpt:546
>   DROP    icmpv6 ::/0                 ::/0           PHYSDEV match
>    --physdev-in  vif95.0 --physdev-is-bridged ipv6-icmptype 134
>   ACCEPT  udp    ::/0                 ::/0           PHYSDEV match
>    --physdev-in  vif95.0 --physdev-is-bridged udp spt:546 dpt:547
>   ACCEPT  all    fe80::216:3eff:fed0:da2d/128  ::/0  PHYSDEV match
>    --physdev-in  vif95.0 --physdev-is-bridged
>   ACCEPT  all    ::/0                 ::/0           PHYSDEV match
>    --physdev-out vif95.0 --physdev-is-bridged
> 
> WARNING: There is currently no way to set the ipv6_allow_ra option
>          Support for this will come in a later patch.

This is pending replies to
http://marc.info/?l=xen-devel&m=140058920523168

> 
> Signed-off-by: Sylvain Munaut <s.munaut@xxxxxxxxxxxxxxxxxxxx>
> ---
>  tools/hotplug/Linux/vif-common.sh |   95 
> +++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/tools/hotplug/Linux/vif-common.sh 
> b/tools/hotplug/Linux/vif-common.sh
> index b098630..6cbb963 100644
> --- a/tools/hotplug/Linux/vif-common.sh
> +++ b/tools/hotplug/Linux/vif-common.sh
> @@ -121,6 +121,8 @@ fi
>  ip=${ip:-}
>  ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
>  
> +ipv6_allow_ra=$(xenstore_read_default "$XENBUS_PATH/ipv6_allow_ra" "false")
> +
>  frob_iptable()
>  {
>    local has_err="no"
> @@ -167,6 +169,79 @@ frob_iptable()
>      log err "iptables setup failed. This may affect guest networking."
>    fi
>  }
> +
> +frob_ip6table()
> +{
> +  local has_err="no"
> +  local has_any="no"
> +  local mac=$(xenstore_read "$XENBUS_PATH/mac")
> +  local eui64=$(echo $mac | awk '{split($1,i,":"); print xor(i[1],2) i[2] 
> ":" i[3] "ff:fe" i[4] ":" i[5] i[6] }')

Yowza! I think a comment with a link to
http://en.wikipedia.org/wiki/IPv6_address#Modified_EUI-64 might help
here.

> +
> +  # Add or remove
> +  if [ "$command" == "online" -o "$command" == "add" ]
> +  then
> +    local c="-I"
> +  else
> +    local c="-D"
> +  fi
> +
> +  # Add rules for each address
> +  local addr
> +
> +  for addr in $@; do
> +    if [ "$addr" = "any" ]; then
> +      addr="::0/0"
> +      has_any="yes"
> +    elif [ "$addr" = "eui64" ]; then
> +      addr="::$eui64/::ffff:ffff:ffff:ffff"
> +    fi
> +
> +    ip6tables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in 
> "$dev" \
> +      -s "$addr" -j ACCEPT 2>/dev/null || has_err="yes"
> +  done
> +
> +  # Always Allow all packets _to_ the domain
> +  ip6tables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-out 
> "$dev" \
> +    -j ACCEPT 2>/dev/null || has_err="yes"
> +
> +  # If 'any' isn't allowed, we needs to allow a few more things
> +  if [ "$has_any" != "yes" ]
> +  then
> +
> +    # Always allow ICMP messages from link-local addresses (for ND)
> +    ip6tables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in 
> "$dev" \
> +      -s "fe80::$eui64" -j ACCEPT 2>/dev/null || has_err="yes"
> +
> +    # Always allow the domain to talk to a DHCP server
> +    ip6tables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in 
> "$dev" \
> +      -p udp --sport 546 --dport 547 -j ACCEPT 2>/dev/null || has_err="yes"
> +
> +  fi
> +
> +  # Filter out RA and DHCP server responses if not explicitely allowed

"explicitly".

> +  if [ "$ipv6_allow_ra" != "true" ]
> +  then
> +    ip6tables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in 
> "$dev" \
> +      -p icmpv6 --icmpv6-type router-advertisement -j DROP 2>/dev/null || 
> has_err="yes"
> +
> +    ip6tables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in 
> "$dev" \
> +      -p udp --sport 547 --dport 546 -j DROP 2>/dev/null || has_err="yes"
> +  fi
> +
> +  # Error handling
> +  if [ \( "$command" == "online" -o "$command" == "add" \) -a "$has_err" == 
> "yes" ]
> +  then
> +    log err "ip6tables setup failed. This may affect guest networking."
> +  fi
> +}
> +
> +
> +##
> +# Check if the given IP is IPv6 or not
> +#
> +is_ipv6()
> +{
> +  echo "$1" | awk '/:|eui64/ { print "yes" }'
>  }
>  
> 
> @@ -188,19 +263,34 @@ handle_iptable()
>      return
>    fi
>  
> +  # User has a working IPv4 iptables, but maybe no IPv6 support ...
> +  local do_ipv6="yes"
> +
> +  if ! ip6tables -L -n >&/dev/null
> +  then
> +    do_ipv6="no"
> +  fi
> +
>    # Scan through the addresses
>    local ipv4_addrs
> +  local ipv6_addrs
>  
>    if [ "$ip" != "" ]
>    then
>      local addr
>      for addr in $ip
>      do
> +      result=$(is_ipv6 "$addr")
> +      if [ -z "$result" ] ; then
>          ipv4_addrs="$addr $ipv4_addrs"
> +      else
> +        ipv6_addrs="$addr $ipv6_addrs"
> +      fi
>      done
>    else
>      # No IP addresses have been specified, so allow anything.
>      ipv4_addrs="any"
> +    ipv6_addrs="any"
>    fi
>  
>    # Actually add the rules
> @@ -211,6 +301,11 @@ handle_iptable()
>      frob_iptable $ipv4_addrs
>    fi
>  
> +  if [ "$ipv6_addrs" != "" -a "$do_ipv6" = "yes" ]

I think if ipv6_addrs != "" or "any" (or $ip !+ "" if you prefer) and
do_ipv6 == no then a warning would be appropriate, something similar to
the "ip6tables setup failed. This may affect guest networking." one
(although best make it different to be distinct. "ip6table not present.
This may ..." ?)

> +  then
> +    frob_ip6table $ipv6_addrs
> +  fi
> +
>    release_lock "iptables"
>  }
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel