[libvirt] [PATCH] Add V6LOCAL parameter to nwfilter rules.

Currently, adding any sort of IPv6 nwfilter rules is rather difficult. There are no standard rules, and you end up doing a lot of things by hand. This patch makes the $V6LOCAL variable available within rules. This is the generated from the interface's mac address using the modified EUI-64 format, which matches what the guest should be using. This is part of what information is needed to correctly filter guest IPv6 traffic. Since this changes with the MAC address, it is significantly easier if libvirt populates it (rather then requring the user to enter it) --- docs/formatnwfilter.html.in | 9 ++++++--- src/conf/nwfilter_params.h | 1 + src/nwfilter/nwfilter_gentech_driver.c | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) mode change 100644 => 100755 src/nwfilter/nwfilter_gentech_driver.c diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index 45b97f7..aa1ff9f 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in @@ -239,9 +239,9 @@ <h3><a name="nwfconceptsvars">Usage of variables in filters</a></h3> <p> - Two variables names have so far been reserved for usage by the - network traffic filtering subsystem: <code>MAC</code> and - <code>IP</code>. + Three variables names have so far been reserved for usage by the + network traffic filtering subsystem: <code>MAC</code>, + <code>IP</code>, and <code>V6LOCAL</code> <br/><br/> <code>MAC</code> is the MAC address of the network interface. A filtering rule that references this variable @@ -251,6 +251,9 @@ parameter similar to the IP parameter above, it is discouraged since libvirt knows what MAC address an interface will be using. <br/><br/> + <code>V6LOCAL</code> is the computed IPv6 link-local address. + This is based on the MAC variable + <br/><br/> The parameter <code>IP</code> represents the IP address that the operating system inside the virtual machine is expected to use on the given interface. The <code>IP</code> parameter diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h index 5e9777b..f61250f 100644 --- a/src/conf/nwfilter_params.h +++ b/src/conf/nwfilter_params.h @@ -98,6 +98,7 @@ bool virNWFilterHashTableEqual(virNWFilterHashTablePtr a, # define NWFILTER_VARNAME_IP "IP" # define NWFILTER_VARNAME_MAC "MAC" +# define NWFILTER_VARNAME_V6LOCAL "V6LOCAL" # define NWFILTER_VARNAME_CTRL_IP_LEARNING "CTRL_IP_LEARNING" # define NWFILTER_VARNAME_DHCPSERVER "DHCPSERVER" diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c old mode 100644 new mode 100755 index 1ce5e70..a86dae8 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -45,6 +45,7 @@ VIR_LOG_INIT("nwfilter.nwfilter_gentech_driver"); #define NWFILTER_STD_VAR_MAC NWFILTER_VARNAME_MAC #define NWFILTER_STD_VAR_IP NWFILTER_VARNAME_IP +#define NWFILTER_STD_VAR_V6LOCAL NWFILTER_VARNAME_V6LOCAL #define NWFILTER_DFLT_LEARN "any" @@ -163,6 +164,28 @@ virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table, "%s", _("Could not add variable 'MAC' to hashmap")); return -1; } + + virMacAddr parsedMac; + if (virMacAddrParse(macaddr, &parsedMac) == 0) + { + parsedMac.addr[0] ^= 2; + + char euiMacAddr[26]; + snprintf(euiMacAddr, sizeof(euiMacAddr), "fe80::%x%x:%xff:fe%x:%x%x", parsedMac.addr[0], parsedMac.addr[1], parsedMac.addr[2], + parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]); + + val = virNWFilterVarValueCreateSimpleCopyValue(euiMacAddr); + if (!val) + return -1; + + if (virHashAddEntry(table->hashTable, + NWFILTER_STD_VAR_V6LOCAL, + val) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not add variable 'V6LOCAL' to hashmap")); + return -1; + } + } } if (ipaddr) { -- 1.7.1

On 04/02/2014 01:40 PM, Brian Rak wrote:
Currently, adding any sort of IPv6 nwfilter rules is rather difficult. There are no standard rules,
Long lines; we tend to keep commit messages wrapped around 72 columns or so ('git log' adds indentation, and commits start to look stupid in the terminal if they wrap while reading 'git log').
and you end up doing a lot of things by hand. This patch makes the $V6LOCAL variable available within rules. This is the generated from the interface's mac address using the modified EUI-64 format, which matches what the guest should be using.
An example in the commit message of what the variable expands to would be nice.
This is part of what information is needed to correctly filter guest IPv6 traffic. Since this changes with the MAC address, it is significantly easier if libvirt populates it (rather then requring the
s/requring/requiring/
user to enter it)
--- docs/formatnwfilter.html.in | 9 ++++++--- src/conf/nwfilter_params.h | 1 + src/nwfilter/nwfilter_gentech_driver.c | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) mode change 100644 => 100755 src/nwfilter/nwfilter_gentech_driver.c
@@ -251,6 +251,9 @@ parameter similar to the IP parameter above, it is discouraged since libvirt knows what MAC address an interface will be using. <br/><br/> + <code>V6LOCAL</code> is the computed IPv6 link-local address. + This is based on the MAC variable
Also worth an example of what this will contain (such as fe80::5254:00ff:fe1a:0a6d). And definitely needs a "Since" tag (in the appropriate <div> markup) mentioning this was added in 1.2.4.
+ + virMacAddr parsedMac; + if (virMacAddrParse(macaddr, &parsedMac) == 0) + {
Style - this { belongs on the same line as the if.
+ parsedMac.addr[0] ^= 2; + + char euiMacAddr[26]; + snprintf(euiMacAddr, sizeof(euiMacAddr), "fe80::%x%x:%xff:fe%x:%x%x", parsedMac.addr[0], parsedMac.addr[1], parsedMac.addr[2], + parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]);
Long line; please wrap to 80 columns. You MEANT to use %02x; your code misbehaves on zero bytes. Why do you need to open-code the snprintf; would it be any cleaner to just use functions from util/virsocketaddr.h for formatting an IPv6 value that you construct from the MAC address? The idea probably has merit, so I'm looking forward to v2. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 04/02/2014 01:56 PM, Eric Blake wrote:
On 04/02/2014 01:40 PM, Brian Rak wrote:
Currently, adding any sort of IPv6 nwfilter rules is rather difficult. There are no standard rules,
Also worth an example of what this will contain (such as fe80::5254:00ff:fe1a:0a6d).
+ parsedMac.addr[0] ^= 2;
Oh, and forgot to mention, should this be |= instead of ^=, since by default, libvirt assigns MAC addresses with bit 2 already set? My understanding is that bit 2 is the locally-administered bit, and that the V6LOCAL address always wants it set (^= only works if the MAC address is not also locally administered, but libvirt's generated MAC addresses of 52:54:00:xx:yy:zz fall foul of that) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 4/2/2014 4:11 PM, Eric Blake wrote:
On 04/02/2014 01:40 PM, Brian Rak wrote:
Currently, adding any sort of IPv6 nwfilter rules is rather difficult. There are no standard rules, Also worth an example of what this will contain (such as fe80::5254:00ff:fe1a:0a6d). + parsedMac.addr[0] ^= 2; Oh, and forgot to mention, should this be |= instead of ^=, since by default, libvirt assigns MAC addresses with bit 2 already set? My understanding is that bit 2 is the locally-administered bit, and that
On 04/02/2014 01:56 PM, Eric Blake wrote: the V6LOCAL address always wants it set (^= only works if the MAC address is not also locally administered, but libvirt's generated MAC addresses of 52:54:00:xx:yy:zz fall foul of that)
For link-local addresses, you want to invert the bit, not ensure that it's always set. This matches what linux is doing doing: http://lxr.free-electrons.com/source/net/ipv6/addrconf.c#L1724

On 4/2/2014 3:56 PM, Eric Blake wrote:
On 04/02/2014 01:40 PM, Brian Rak wrote:
Currently, adding any sort of IPv6 nwfilter rules is rather difficult. There are no standard rules, Long lines; we tend to keep commit messages wrapped around 72 columns or so ('git log' adds indentation, and commits start to look stupid in the terminal if they wrap while reading 'git log').
and you end up doing a lot of things by hand. This patch makes the $V6LOCAL variable available within rules. This is the generated from the interface's mac address using the modified EUI-64 format, which matches what the guest should be using. An example in the commit message of what the variable expands to would be nice.
This is part of what information is needed to correctly filter guest IPv6 traffic. Since this changes with the MAC address, it is significantly easier if libvirt populates it (rather then requring the s/requring/requiring/
user to enter it)
--- docs/formatnwfilter.html.in | 9 ++++++--- src/conf/nwfilter_params.h | 1 + src/nwfilter/nwfilter_gentech_driver.c | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) mode change 100644 => 100755 src/nwfilter/nwfilter_gentech_driver.c
@@ -251,6 +251,9 @@ parameter similar to the IP parameter above, it is discouraged since libvirt knows what MAC address an interface will be using. <br/><br/> + <code>V6LOCAL</code> is the computed IPv6 link-local address. + This is based on the MAC variable Also worth an example of what this will contain (such as fe80::5254:00ff:fe1a:0a6d). And definitely needs a "Since" tag (in the appropriate <div> markup) mentioning this was added in 1.2.4.
+ + virMacAddr parsedMac; + if (virMacAddrParse(macaddr, &parsedMac) == 0) + { Style - this { belongs on the same line as the if. Thanks, will make those changes
+ parsedMac.addr[0] ^= 2; + + char euiMacAddr[26]; + snprintf(euiMacAddr, sizeof(euiMacAddr), "fe80::%x%x:%xff:fe%x:%x%x", parsedMac.addr[0], parsedMac.addr[1], parsedMac.addr[2], + parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]); Long line; please wrap to 80 columns. You MEANT to use %02x; your code misbehaves on zero bytes. Why do you need to open-code the snprintf; would it be any cleaner to just use functions from util/virsocketaddr.h for formatting an IPv6 value that you construct from the MAC address? In my opinion, manually formatting the address here is a lot simpler to understand then constructing a virSockAddr and using virSocketAddrFormat on it. It's definitely shorter code this way. I'm not sure which way makes more sense.

On Wed, Apr 02, 2014 at 03:40:14PM -0400, Brian Rak wrote:
@@ -163,6 +164,28 @@ virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table, "%s", _("Could not add variable 'MAC' to hashmap")); return -1; } + + virMacAddr parsedMac; + if (virMacAddrParse(macaddr, &parsedMac) == 0) + { + parsedMac.addr[0] ^= 2; + + char euiMacAddr[26]; + snprintf(euiMacAddr, sizeof(euiMacAddr), "fe80::%x%x:%xff:fe%x:%x%x", parsedMac.addr[0], parsedMac.addr[1], parsedMac.addr[2], + parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]);
Unfortunately this IPv6 addressing scheme isn't portable. Windows Vista will create link local addresses which are completely random, not based on the MAC address. http://technet.microsoft.com/en-us/magazine/2007.08.cableguy.aspx I wonder if there's a way todo something more clever for IPv6 to learn the addresses, we as do for IPv4 address learning, or snoop route advertisment traffic as we do for DHCP Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Apr 02, 2014 at 03:40:14PM -0400, Brian Rak wrote:
@@ -163,6 +164,28 @@ virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table, "%s", _("Could not add variable 'MAC' to hashmap")); return -1; } + + virMacAddr parsedMac; + if (virMacAddrParse(macaddr, &parsedMac) == 0) + { + parsedMac.addr[0] ^= 2; + + char euiMacAddr[26]; + snprintf(euiMacAddr, sizeof(euiMacAddr), "fe80::%x%x:%xff:fe%x:%x%x", parsedMac.addr[0], parsedMac.addr[1], parsedMac.addr[2], + parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]); Unfortunately this IPv6 addressing scheme isn't portable. Windows Vista will create link local addresses which are completely random, not based on the MAC address.
http://technet.microsoft.com/en-us/magazine/2007.08.cableguy.aspx
I wonder if there's a way todo something more clever for IPv6 to learn the addresses, we as do for IPv4 address learning, or snoop route advertisment traffic as we do for DHCP
Regards, Daniel Vista can be configured to use the EUI64 format though (as per that
On 4/4/2014 4:48 AM, Daniel P. Berrange wrote: link). I don't think that we can really trust that the guest is not malicious, so I'm not sure that trying to learn the link-local IPv6 address would be secure. I'm not sure if there's other security issues or not, but a malicious guest using another guest's link local address would definitely cause some problems.

On Fri, Apr 04, 2014 at 10:08:26AM -0400, Brian Rak wrote:
On 4/4/2014 4:48 AM, Daniel P. Berrange wrote:
On Wed, Apr 02, 2014 at 03:40:14PM -0400, Brian Rak wrote:
@@ -163,6 +164,28 @@ virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table, "%s", _("Could not add variable 'MAC' to hashmap")); return -1; } + + virMacAddr parsedMac; + if (virMacAddrParse(macaddr, &parsedMac) == 0) + { + parsedMac.addr[0] ^= 2; + + char euiMacAddr[26]; + snprintf(euiMacAddr, sizeof(euiMacAddr), "fe80::%x%x:%xff:fe%x:%x%x", parsedMac.addr[0], parsedMac.addr[1], parsedMac.addr[2], + parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]); Unfortunately this IPv6 addressing scheme isn't portable. Windows Vista will create link local addresses which are completely random, not based on the MAC address.
http://technet.microsoft.com/en-us/magazine/2007.08.cableguy.aspx
I wonder if there's a way todo something more clever for IPv6 to learn the addresses, we as do for IPv4 address learning, or snoop route advertisment traffic as we do for DHCP
Vista can be configured to use the EUI64 format though (as per that link). I don't think that we can really trust that the guest is not malicious, so I'm not sure that trying to learn the link-local IPv6 address would be secure.
I'm not sure if there's other security issues or not, but a malicious guest using another guest's link local address would definitely cause some problems.
NB you have to decide what the threat scenario is. The learning code is obviously vulnerable if your threat is a guest that is malicious from the time it is first booted & we weren't attempting to address that scenario though. The learning code is intended to protect against the more limited scenario where your guest is initially trusted, but gets compromised while running. This is good enough for an out of the box config. If you care about stronger security then you only want to rely on learning from a trusted DHCP server response, or use a hardcoded IP address variable in the guest XML config. These cease to be zero-config though since the host admin must set the IP addr of the trusted DHCP server, or set the guest IP addr(s). Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Brian Rak
-
Daniel P. Berrange
-
Eric Blake