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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-20 at 13:58 +0200, Sylvain Munaut wrote:
> Hi,
> 
> 
> >> 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.
> >>  - You can use ::0/0 or 0.0.0.0/0 to allow v6 or v4 globally but filter
> >>    the other one.
> >
> > Sounds sensible. Can you give examples of the rulesets create in each
> > case?
> 
> Yes, see below.

Thanks. Like with the v4 ones could you also include these in the commit
log.

> 
> I also added the eui64 idea from jacek:
>  - The ICMP Link Local messages are locked to the LL address derived
> from the mac address
>  - The 'ip' parameters also allow the special 'eui64' token to allow
> any address with the lower 64 bits set to the EUI64 corresponding to
> the MAC. (Filtering on the network part is not done and must be done
> globally by the user if needed, or just manually specify the address
> completely).
> 
> 
> * ip=""
> 
> iptables:
>  ACCEPT  all  0.0.0.0/0  0.0.0.0/0  PHYSDEV match --physdev-out
> vif91.0 --physdev-is-bridged
>  ACCEPT  all  0.0.0.0/0  0.0.0.0/0  PHYSDEV match --physdev-in
> vif91.0 --physdev-is-bridged

> ip6tables:
>  DROP    udp    ::/0  ::/0  PHYSDEV match --physdev-in  vif91.0
> --physdev-is-bridged udp spt:547 dpt:546

So here we deny DHCP whereas for v4 we don't? Why is that? And in other
cases for v4 we explicitly allow it? I see you called this out in the
commit message, but I must confess I don't know v6 well enough to guess
why. Is allowing a guest to send DHCP responses more dangerous for v6
than with v4?


>  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 mat

216:3eff:fed0:da2d here is related to the mac address and therefore to
the eui64 option?

BTW, please can you update docs/misc/xl-network-configuration.markdown
to reflect the ipv6 behaviour.

> ch --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).
> 
> 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
> 
> 
> 
> 
> >> By default, domains aren't allows to send Router Advertisement
> >> or DHCP responses, see the ipv6_allow_ra to enable them.
> >
> > How does one go about setting this?
> 
> Well ... I thought it would work just like  accel=  (which I'm using
> in prod but that's with 'xm' on 4.1).
> Turns out after a bit of googling that 'accel' might work just by luck
> because somehow at some point passing that option was added to 'xm' in
> 2007 ...
> 
> I just retried now with 'xl' under 4.4 and indeed that doesn't work :(
> 
> So if I want to pass custom parameters to the vif-script, I have to
> add it to libxl ?

At the moment, yes.

> And if yes, is that acceptable ? What would be the best way : add each
> parameter independently, or add a single 'hotplug_extra' parameter
> that would need to be parsed in the hotplug scripts themselves ?

TBH I'm not sure. I'll reply separately retitling this to be more
obvious and ccing a few folks.

> (I'll wait until I fixed this before reposting obviously)

I think if you note it in the commit message that it is pending a
resolution to this then it would be reasonable to repost. I *think* that
the implementation of consuming these options from xenstored should be
largely orthogonal to the mechanism of how they get there.

BTW, as a hack/workaround for testing you could create vif-sylvain which
hacks things into the right places and then execs the real script.

> 
> >> +##
> >> +# Check if the given IP is IPv6 or not
> >> +#
> >> +is_ipv6()
> >> +{
> >> +        echo "$1" | perl -wane '/:/ && print "yes"'
> >
> > Annoyingly I don't think we currently require Perl in the runtime
> > environment (it is used at build time). Luckily I think you can
> > implement this as
> >         case $addr in
> >                 *:*) ipv6_addrs="$addrs $ipv6_addrs";;
> >                 *) ipv4.... ;;
> >         esac
> >
> > (probably inline in the handle_iptable function, no need for this helper
> > in that case)
> 
> 
> I fixed that using only 'awk'. I left it as a helper because it's used
> in vif-route as well later and I find it nicer to have the test in a
> single place.

Ack, thanks.



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