network: allow configuring firewalld zone for virtual network bridge device

Since we're setting the zone anyway, it will be useful to allow
setting a different (custom) zone for each network. This will be done
by adding a "zone" attribute to the "bridge" element, e.g.:

   ...
   <bridge name='virbr0' zone='myzone'/>
   ...

If a zone is specified in the config and it can't be honored, this
will be an error.

Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

docs/firewall.html.in

@@ -151,6 +151,11 @@ MASQUERADE all  --  *      *       192.168.122.0/24    !192.168.122.0/24</pre>
       iptables rules regardless of which backend is in use by
       firewalld.
     </p>
+    <p>
+      NB: It is possible to manually set the firewalld zone for a
+      network's interface with the "zone" attribute of the network's
+      "bridge" element.
+    </p>
     <p>
       NB: Prior to libvirt 5.1.0, the firewalld "libvirt" zone did not
       exist, and prior to firewalld 0.7.0 a feature crucial to making

docs/formatnetwork.html.in

@@ -152,6 +152,23 @@
           <span class="since">Since 1.2.11, requires kernel 3.17 or
           newer</span>
         </p>
+
+        <p>
+          The optional <code>zone</code> attribute of
+          the <code>bridge</code> element is used to specify
+          the <a href="https://firewalld.org">firewalld</a>
+          zone for the bridge of a network with <code>forward</code>
+          mode of "nat", "route", "open", or one with
+          no <code>forward</code> specified. By default, the bridges
+          of all virtual networks with these forward modes are placed
+          in the firewalld zone named "libvirt", which permits
+          incoming DNS, DHCP, TFTP, and SSH to the host from guests on
+          the network. This behavior can be changed either by
+          modifying the libvirt zone (using firewalld management
+          tools), or by placing the network in a different zone (which
+          will also be managed using firewalld tools).
+          <span class="since">Since 5.1.0</span>
+        </p>
       </dd>
 
       <dt><code>mtu</code></dt>

docs/schemas/basictypes.rng

@@ -279,6 +279,12 @@
     </data>
   </define>
 
+  <define name="zoneName">
+    <data type="string">
+      <param name="pattern">[a-zA-Z0-9_\-]+</param>
+    </data>
+  </define>
+
   <define name="filePath">
     <data type="string">
       <param name="pattern">.+</param>

docs/schemas/network.rng

@@ -58,6 +58,12 @@
               </attribute>
             </optional>
 
+            <optional>
+              <attribute name="zone">
+                <ref name="zoneName"/>
+              </attribute>
+            </optional>
+
             <optional>
               <attribute name="stp">
                 <ref name="virOnOff"/>

src/conf/network_conf.c

@@ -203,6 +203,7 @@ virNetworkDefFree(virNetworkDefPtr def)
 
     VIR_FREE(def->name);
     VIR_FREE(def->bridge);
+    VIR_FREE(def->bridgeZone);
     VIR_FREE(def->domain);
 
     virNetworkForwardDefClear(&def->forward);
@@ -1684,6 +1685,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
 
     /* Parse bridge information */
     def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt);
+    def->bridgeZone = virXPathString("string(./bridge[1]/@zone)", ctxt);
     stp = virXPathString("string(./bridge[1]/@stp)", ctxt);
     def->stp = (stp && STREQ(stp, "off")) ? false : true;
 
@@ -1920,6 +1922,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
                            def->name);
             goto error;
         }
+        if (def->bridgeZone) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("bridge zone not allowed in %s mode (network '%s')"),
+                           virNetworkForwardTypeToString(def->forward.type),
+                           def->name);
+            goto error;
+        }
         if (def->macTableManager) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("bridge macTableManager setting not allowed "
@@ -1931,9 +1940,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
         ATTRIBUTE_FALLTHROUGH;
 
     case VIR_NETWORK_FORWARD_BRIDGE:
-        if (def->delay || stp) {
+        if (def->delay || stp || def->bridgeZone) {
             virReportError(VIR_ERR_XML_ERROR,
-                           _("bridge delay/stp options only allowed in "
+                           _("bridge delay/stp/zone options only allowed in "
                              "route, nat, and isolated mode, not in %s "
                              "(network '%s')"),
                            virNetworkForwardTypeToString(def->forward.type),
@@ -2508,6 +2517,7 @@ virNetworkDefFormatBuf(virBufferPtr buf,
     if (hasbridge || def->bridge || def->macTableManager) {
         virBufferAddLit(buf, "<bridge");
         virBufferEscapeString(buf, " name='%s'", def->bridge);
+        virBufferEscapeString(buf, " zone='%s'", def->bridgeZone);
         if (hasbridge)
             virBufferAsprintf(buf, " stp='%s' delay='%ld'",
                               def->stp ? "on" : "off", def->delay);

src/conf/network_conf.h

@@ -235,6 +235,7 @@ struct _virNetworkDef {
     int   connections; /* # of guest interfaces connected to this network */
 
     char *bridge;       /* Name of bridge device */
+    char *bridgeZone;  /* name of firewalld zone for bridge */
     int  macTableManager; /* enum virNetworkBridgeMACTableManager */
     char *domain;
     int domainLocalOnly; /* enum virTristateBool: yes disables dns forwarding */

src/network/bridge_driver_linux.c

@@ -671,49 +671,68 @@ int networkAddFirewallRules(virNetworkDefPtr def)
     virFirewallPtr fw = NULL;
     int ret = -1;
 
-    /* if firewalld is active, try to set the "libvirt" zone. This is
+    if (def->bridgeZone) {
-     * desirable (for consistency) if firewalld is using the iptables
-     * backend, but is necessary (for basic network connectivity) if
-     * firewalld is using the nftables backend
-     */
-    if (virFirewallDIsRegistered() == 0) {
 
-        /* if the "libvirt" zone exists, then set it. If not, and
+        /* if a firewalld zone has been specified, fail/log an error
-         * if firewalld is using the nftables backend, then we
+         * if we can't honor it
-         * need to log an error because the combination of
-         * nftables + default zone means that traffic cannot be
-         * forwarded (and even DHCP and DNS from guest to host
-         * will probably no be permitted by the default zone
          */
-        if (virFirewallDZoneExists("libvirt")) {
+        if (virFirewallDIsRegistered() < 0) {
-            if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)
+            virReportError(VIR_ERR_INTERNAL_ERROR,
-                goto cleanup;
+                           _("zone %s requested for network %s "
-        } else {
+                             "but firewalld is not active"),
-            unsigned long version;
+                           def->bridgeZone, def->name);
-            int vresult = virFirewallDGetVersion(&version);
+            goto cleanup;
+        }
 
-            if (vresult < 0)
+        if (virFirewallDInterfaceSetZone(def->bridge, def->bridgeZone) < 0)
-                goto cleanup;
+            goto cleanup;
 
-            /* Support for nftables backend was added in firewalld
+    } else {
-             * 0.6.0. Support for rule priorities (required by the
+
-             * 'libvirt' zone, which should be installed by a
+        /* if firewalld is active, try to set the "libvirt" zone. This is
-             * libvirt package, *not* by firewalld) was not added
+         * desirable (for consistency) if firewalld is using the iptables
-             * until firewalld 0.7.0 (unless it was backported).
+         * backend, but is necessary (for basic network connectivity) if
+         * firewalld is using the nftables backend
+         */
+        if (virFirewallDIsRegistered() == 0) {
+
+            /* if the "libvirt" zone exists, then set it. If not, and
+             * if firewalld is using the nftables backend, then we
+             * need to log an error because the combination of
+             * nftables + default zone means that traffic cannot be
+             * forwarded (and even DHCP and DNS from guest to host
+             * will probably no be permitted by the default zone
              */
-            if (version >= 6000 &&
+            if (virFirewallDZoneExists("libvirt")) {
-                virFirewallDGetBackend() == VIR_FIREWALLD_BACKEND_NFTABLES) {
+                if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    goto cleanup;
-                               _("firewalld is set to use the nftables "
+            } else {
-                                 "backend, but the required firewalld "
+                unsigned long version;
-                                 "'libvirt' zone is missing. Either set "
+                int vresult = virFirewallDGetVersion(&version);
-                                 "the firewalld backend to 'iptables', or "
+
-                                 "ensure that firewalld has a 'libvirt' "
+                if (vresult < 0)
-                                 "zone by upgrading firewalld to a "
+                    goto cleanup;
-                                 "version supporting rule priorities "
+
-                                 "(0.7.0+) and/or rebuilding "
+                /* Support for nftables backend was added in firewalld
-                                 "libvirt with --with-firewalld-zone"));
+                 * 0.6.0. Support for rule priorities (required by the
-                goto cleanup;
+                 * 'libvirt' zone, which should be installed by a
+                 * libvirt package, *not* by firewalld) was not added
+                 * until firewalld 0.7.0 (unless it was backported).
+                 */
+                if (version >= 6000 &&
+                    virFirewallDGetBackend() == VIR_FIREWALLD_BACKEND_NFTABLES) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                   _("firewalld is set to use the nftables "
+                                     "backend, but the required firewalld "
+                                     "'libvirt' zone is missing. Either set "
+                                     "the firewalld backend to 'iptables', or "
+                                     "ensure that firewalld has a 'libvirt' "
+                                     "zone by upgrading firewalld to a "
+                                     "version supporting rule priorities "
+                                     "(0.7.0+) and/or rebuilding "
+                                     "libvirt with --with-firewalld-zone"));
+                    goto cleanup;
+                }
             }
         }
     }

tests/networkxml2xmlin/routed-network.xml

@@ -1,7 +1,7 @@
 <network>
   <name>local</name>
   <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
-  <bridge name="virbr1"/>
+  <bridge name="virbr1" zone="myzone"/>
   <mac address='12:34:56:78:9A:BC'/>
   <forward mode="route" dev="eth1"/>
   <ip address="192.168.122.1" netmask="255.255.255.0">

tests/networkxml2xmlout/routed-network.xml

@@ -4,7 +4,7 @@
   <forward dev='eth1' mode='route'>
     <interface dev='eth1'/>
   </forward>
-  <bridge name='virbr1' stp='on' delay='0'/>
+  <bridge name='virbr1' zone='myzone' stp='on' delay='0'/>
   <mac address='12:34:56:78:9a:bc'/>
   <ip address='192.168.122.1' netmask='255.255.255.0'>
   </ip>