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</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
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 @@
<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>
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, "<bridge");
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