From 69de85ec80efd714528955e9c0ab67ee6811c824 Mon Sep 17 00:00:00 2001

Message-Id: <69de85ec80efd714528955e9c0ab67ee6811c824@dist-git>

From: Laine Stump <laine@laine.org>

Date: Fri, 1 Feb 2019 20:29:32 -0500

Subject: [PATCH] network: allow configuring firewalld zone for virtual network

 bridge device

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

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>

(cherry picked from commit 30a6f9168634f8ce269f1ef294c4a18d9c95939c)

Conflicts:

   src/conf/network_conf.c - upstream added a new bool called

     hasBridge that is the equivalent of all the comparisons in the

     if() just following the line that adds "zone='blah'" to the xml

     string.

https://bugzilla.redhat.com/1650320

Signed-off-by: Laine Stump <laine@laine.org>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 docs/firewall.html.in                      |  5 ++

 docs/formatnetwork.html.in                 | 17 ++++

 docs/schemas/basictypes.rng                |  6 ++

 docs/schemas/network.rng                   |  6 ++

 src/conf/network_conf.c                    | 14 +++-

 src/conf/network_conf.h                    |  1 +

 src/network/bridge_driver_linux.c          | 95 +++++++++++++---------

 tests/networkxml2xmlin/routed-network.xml  |  2 +-

 tests/networkxml2xmlout/routed-network.xml |  2 +-

 9 files changed, 106 insertions(+), 42 deletions(-)

diff --git a/docs/firewall.html.in b/docs/firewall.html.in

index 5d584e582e..e86ab0d974 100644

--- a/docs/firewall.html.in

+++ b/docs/firewall.html.in

@@ -151,6 +151,11 @@ MASQUERADE all  --  *      *       192.168.122.0/24    !192.168.122.0/24

       iptables rules regardless of which backend is in use by

       firewalld.

+    

+      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.

+    

       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

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in

index 363a72bbc9..7ddcfee127 100644

--- a/docs/formatnetwork.html.in

+++ b/docs/formatnetwork.html.in

@@ -152,6 +152,23 @@

           Since 1.2.11, requires kernel 3.17 or

           newer

+

+        

+          The optional zone attribute of

+          the bridge element is used to specify

+          the firewalld

+          zone for the bridge of a network with forward

+          mode of "nat", "route", "open", or one with

+          no forward 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).

+          Since 5.1.0

+        

       
mtu

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng

index 1a18cd31b1..b45a7fcdc8 100644

--- a/docs/schemas/basictypes.rng

+++ b/docs/schemas/basictypes.rng

@@ -252,6 +252,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>

diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng

index f37c422bf3..2a6e3358fd 100644

--- a/docs/schemas/network.rng

+++ b/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"/>

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c

index 630a87fc07..1e3650b70f 100644

--- a/src/conf/network_conf.c

+++ b/src/conf/network_conf.c

@@ -206,6 +206,7 @@ virNetworkDefFree(virNetworkDefPtr def)

     VIR_FREE(def->name);

     VIR_FREE(def->bridge);

+    VIR_FREE(def->bridgeZone);

     VIR_FREE(def->domain);

     virNetworkForwardDefClear(&def->forward);

@@ -1689,6 +1690,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;

@@ -1925,6 +1927,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 "

@@ -1936,9 +1945,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),

@@ -2478,6 +2487,7 @@ virNetworkDefFormatBuf(virBufferPtr buf,

         virBufferAddLit(buf, "

         virBufferEscapeString(buf, " name='%s'", def->bridge);

+        virBufferEscapeString(buf, " zone='%s'", def->bridgeZone);

         if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||

             def->forward.type == VIR_NETWORK_FORWARD_NAT ||

             def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||

diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h

index 54c8ed1c4c..415792166f 100644

--- a/src/conf/network_conf.h

+++ b/src/conf/network_conf.h

@@ -237,6 +237,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 */

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c

index 823d5a9742..121d42b646 100644

--- a/src/network/bridge_driver_linux.c

+++ b/src/network/bridge_driver_linux.c

@@ -642,49 +642,68 @@ int networkAddFirewallRules(virNetworkDefPtr def)

     virFirewallPtr fw = NULL;

     int ret = -1;

-    /* if firewalld is active, try to set the "libvirt" zone. This is

-     * 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 (def->bridgeZone) {

-        /* 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 a firewalld zone has been specified, fail/log an error

+         * if we can't honor it

          */

-        if (virFirewallDZoneExists("libvirt")) {

-            if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)

-                goto cleanup;

-        } else {

-            unsigned long version;

-            int vresult = virFirewallDGetVersion(&version);

+        if (virFirewallDIsRegistered() < 0) {

+            virReportError(VIR_ERR_INTERNAL_ERROR,

+                           _("zone %s requested for network %s "

+                             "but firewalld is not active"),

+                           def->bridgeZone, def->name);

+            goto cleanup;

+        }

-            if (vresult < 0)

-                goto cleanup;

+        if (virFirewallDInterfaceSetZone(def->bridge, def->bridgeZone) < 0)

+            goto cleanup;

-            /* Support for nftables backend was added in firewalld

-             * 0.6.0. Support for rule priorities (required by the

-             * '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).

+    } else {

+

+        /* if firewalld is active, try to set the "libvirt" zone. This is

+         * 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 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 &&

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

+            if (virFirewallDZoneExists("libvirt")) {

+                if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)

+                    goto cleanup;

+            } else {

+                unsigned long version;

+                int vresult = virFirewallDGetVersion(&version);

+

+                if (vresult < 0)

+                    goto cleanup;

+

+                /* Support for nftables backend was added in firewalld

+                 * 0.6.0. Support for rule priorities (required by the

+                 * '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;

+                }

             }

         }

     }

diff --git a/tests/networkxml2xmlin/routed-network.xml b/tests/networkxml2xmlin/routed-network.xml

index ab5e15b1f6..fce01df132 100644

--- a/tests/networkxml2xmlin/routed-network.xml

+++ b/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">

diff --git a/tests/networkxml2xmlout/routed-network.xml b/tests/networkxml2xmlout/routed-network.xml

index 81abf06e9f..2e13cf4ffa 100644

--- a/tests/networkxml2xmlout/routed-network.xml

+++ b/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>

-- 

2.20.1