conf: forbid use of multicast mac addresses

A few times libvirt users manually setting mac addresses have
complained of a networking failure that ends up being due to a multicast
mac address being used for a guest interface. This patch prevents that
by logging an error and failing if a multicast mac address is
encountered in each of the three following cases:

1) domain xml <interface> mac address.
2) network xml bridge mac address.
3) network xml dhcp/host mac address.

There are several other places where a mac address can be input that
aren't controlled in this manner because failure to do so has no
consequences (e.g., if the address will be used to search through
existing interfaces for a match).

The RNG has been updated to add multiMacAddr and uniMacAddr along with
the existing macAddr, and macAddr was switched to uniMacAddr where
appropriate.

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 7d7911d0a87cedc92ca55235486c648201241476..9dbda4a6cc77d4bd571a1efbb6a8c0eff8264059 100644 (file)
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -55,9 +55,24 @@
   </define>
 
   <!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC" -->
+  <!-- The lowest bit of the 1st byte is the "multicast" bit. a         -->
+  <!-- uniMacAddr requires that bit to be 0, and a multiMacAddr         -->
+  <!-- requires it to be 1. Plain macAddr will accept either.           -->
+  <!-- Currently there is no use of multiMacAddr in libvirt, it         -->
+  <!-- is included here for documentation/comparison purposes.          -->
+  <define name="uniMacAddr">
+    <data type="string">
+      <param name="pattern">[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}</param>
+    </data>
+  </define>
+  <define name="multiMacAddr">
+    <data type="string">
+      <param name="pattern">[a-fA-F0-9][13579bBdDfF](:[a-fA-F0-9]{2}){5}</param>
+    </data>
+  </define>
   <define name="macAddr">
     <data type="string">
-      <param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param>
+      <param name="pattern">[a-fA-F0-9]{2}(:[a-fA-F0-9]{2}){5}</param>
     </data>
   </define>
 

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5b3e5fa548978ca753bdc929413dd9187b20a2e1..f62969193c4f31b63c2a3e7aed7c37d0df0eef7c 100644 (file)
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1391,7 +1391,7 @@
             <optional>
               <element name="mac">
                 <attribute name="address">
-                  <ref name="macAddr"/>
+                  <ref name="uniMacAddr"/>
                 </attribute>
                 <empty/>
               </element>
@@ -1417,7 +1417,7 @@
             <optional>
               <element name="mac">
                 <attribute name="address">
-                  <ref name="macAddr"/>
+                  <ref name="uniMacAddr"/>
                 </attribute>
                 <empty/>
               </element>
@@ -1498,7 +1498,7 @@
       <optional>
         <element name="mac">
           <attribute name="address">
-            <ref name="macAddr"/>
+            <ref name="uniMacAddr"/>
           </attribute>
           <empty/>
         </element>

diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 6e1002f1b57056033bfe3aa3c77d3bc3e95858c9..2ae879ec47b48a5b700db71378a11e3c34da5a32 100644 (file)
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -57,7 +57,7 @@
         <!-- <mac> element -->
         <optional>
           <element name="mac">
-            <attribute name="address"><ref name="macAddr"/></attribute>
+            <attribute name="address"><ref name="uniMacAddr"/></attribute>
             <empty/>
           </element>
         </optional>
@@ -218,7 +218,7 @@
                 </zeroOrMore>
                 <zeroOrMore>
                   <element name="host">
-                    <attribute name="mac"><ref name="macAddr"/></attribute>
+                    <attribute name="mac"><ref name="uniMacAddr"/></attribute>
                     <attribute name="name"><text/></attribute>
                     <attribute name="ip"><ref name="ipv4Addr"/></attribute>
                   </element>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e6d0f4be010c37d7239a6f1ca3d50dfffaa97b1e..d5def1c816711f113d5032921527d11dba5f369e 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4416,11 +4416,17 @@ virDomainNetDefParseXML(virCapsPtr caps,
 
     if (macaddr) {
         if (virMacAddrParse((const char *)macaddr, def->mac) < 0) {
-            virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+            virDomainReportError(VIR_ERR_XML_ERROR,
                                  _("unable to parse mac address '%s'"),
                                  (const char *)macaddr);
             goto error;
         }
+        if (virMacAddrIsMulticast(def->mac)) {
+            virDomainReportError(VIR_ERR_XML_ERROR,
+                                 _("expected unicast mac address, found multicast '%s'"),
+                                 (const char *)macaddr);
+            goto error;
+        }
     } else {
         virCapabilitiesGenerateMac(caps, def->mac);
     }

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 4341f110733410a8fb33e27070e3ef7638d29e05..17dc0d35390d6d02faafbfa61865b5e135fd4706 100644 (file)
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -419,22 +419,30 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
             def->nranges++;
         } else if (cur->type == XML_ELEMENT_NODE &&
             xmlStrEqual(cur->name, BAD_CAST "host")) {
-            char *mac, *name, *ip;
+            char *mac = NULL, *name = NULL, *ip;
             unsigned char addr[6];
             virSocketAddr inaddr;
 
             mac = virXMLPropString(cur, "mac");
-            if ((mac != NULL) &&
-                (virMacAddrParse(mac, &addr[0]) != 0)) {
-                virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
-                                      _("Cannot parse MAC address '%s' in network '%s'"),
-                                      mac, networkName);
-                VIR_FREE(mac);
-                return -1;
+            if (mac != NULL) {
+                if (virMacAddrParse(mac, &addr[0]) < 0) {
+                    virNetworkReportError(VIR_ERR_XML_ERROR,
+                                          _("Cannot parse MAC address '%s' in network '%s'"),
+                                          mac, networkName);
+                    VIR_FREE(mac);
+                    return -1;
+                }
+                if (virMacAddrIsMulticast(addr)) {
+                    virNetworkReportError(VIR_ERR_XML_ERROR,
+                                         _("expected unicast mac address, found multicast '%s' in network '%s'"),
+                                         (const char *)mac, networkName);
+                    VIR_FREE(mac);
+                    return -1;
+                }
             }
             name = virXMLPropString(cur, "name");
             if ((name != NULL) && (!c_isalpha(name[0]))) {
-                virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
+                virNetworkReportError(VIR_ERR_XML_ERROR,
                                       _("Cannot use name address '%s' in network '%s'"),
                                       name, networkName);
                 VIR_FREE(mac);
@@ -991,6 +999,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
             VIR_FREE(tmp);
             goto error;
         }
+        if (virMacAddrIsMulticast(def->mac)) {
+            virNetworkReportError(VIR_ERR_XML_ERROR,
+                                 _("Invalid multicast bridge mac address '%s' in network '%s'"),
+                                 tmp, def->name);
+            VIR_FREE(tmp);
+            goto error;
+        }
         VIR_FREE(tmp);
         def->mac_specified = true;
     }

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e40c80eed8315f3b4e8780f9051d0aad972c6101..9a718b412f6dd1e9c1e777b1c1efeb3490449e0a 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1208,6 +1208,8 @@ virKeycodeValueTranslate;
 virMacAddrCompare;
 virMacAddrFormat;
 virMacAddrGenerate;
+virMacAddrIsMulticast;
+virMacAddrIsUnicast;
 virMacAddrParse;
 
 

diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
index 70aef56ce14f32172f77f701c0a9d1b29abec2b2..7b403979bd9dcca442591c97d1fdb08847deba5c 100644 (file)
--- a/src/util/virmacaddr.c
+++ b/src/util/virmacaddr.c
@@ -126,3 +126,16 @@ void virMacAddrGenerate(const unsigned char *prefix,
     addr[4] = virRandomBits(8);
     addr[5] = virRandomBits(8);
 }
+
+/* The low order bit of the first byte is the "multicast" bit. */
+bool
+virMacAddrIsMulticast(const unsigned char *addr)
+{
+    return !!(addr[0] & 1);
+}
+
+bool
+virMacAddrIsUnicast(const unsigned char *addr)
+{
+    return !(addr[0] & 1);
+}

diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h
index 278f41ee0c48d1c340255e7d12d2d5bc9104a1a9..f8d3e0c53510afc546ae30874c3c041e42476092 100644 (file)
--- a/src/util/virmacaddr.h
+++ b/src/util/virmacaddr.h
@@ -37,5 +37,6 @@ void virMacAddrGenerate(const unsigned char *prefix,
                         unsigned char *addr);
 int virMacAddrParse(const char* str,
                     unsigned char *addr) ATTRIBUTE_RETURN_CHECK;
-
+bool virMacAddrIsUnicast(const unsigned char *addr);
+bool virMacAddrIsMulticast(const unsigned char *addr);
 #endif /* __VIR_MACADDR_H__ */