Re: [libvirt] [PATCH 5/5] 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. If no zone is specified and the default zone ("libvirt") can't be set, it will be ignored. (BTW, Federico Simoncelli proposed a similar patch 5 1.2 years (!!) ago, but I misunderstood its usefulness at first, and by the time I did we were both too busy to revisit it. libvirt's code has changed so much in the intervening time that it was simpler to just rewrite from scratch) Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- docs/formatnetwork.html.in | 17 +++++++++ docs/news.xml | 40 ++++++++++++++++++++++ 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 | 30 ++++++++++++---- tests/networkxml2xmlin/routed-network.xml | 2 +- tests/networkxml2xmlout/routed-network.xml | 2 +- 9 files changed, 107 insertions(+), 11 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 156cfae4ec..4aecdfe8e0 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.0.0</span>

+ </p> </dd> <dt><code>mtu</code></dt> diff --git a/docs/news.xml b/docs/news.xml index 8c608cdc36..d894821ed5 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -58,6 +58,19 @@ trunking configuration. </description> </change> + <change> + <summary> + network: support setting a firewalld "zone" for all virtual network bridges + </summary> + <description> + All libvirt virtual networks with bridges managed by libvirt + (i.e. those with <code>forward</code> mode of "nat", + "route", "open", or no forward mode) will now be placed in a + special firewalld zone called "libvirt" by default, and the

+ zone of any network bridge can be changed using the <code>zone</code> + attribute of the network's <code>bridge</code> element. + </description> + </change> </section> <section title="Removed features"> <change> @@ -123,6 +136,33 @@ </change> </section> <section title="Bug fixes">

+ <change> + <summary> + network: fix virtual networks on systems using firewalld+nftables + </summary> + <description> + Because of the transitional state of firewalld's new support + for nftables, not all iptables features required by libvirt + are yet available, so libvirt must continue to use iptables

+ for its own packet filtering rules even when the firewalld + backend is set to use nftables, but due to the way iptables

+ support is implemented in kernels using nftables (iptables

+ rules are converted to nftables rules and processed in a + separate hook from the native nftables rules), guest + networking was broken on hosts with firewalld configured to + use nftables as the backend. This has been fixed by putting + libvirt-managed bridges in their own firewalld zone, so that + guest traffic can be forwarded beyond the host, and so that

+ host services can be exposed to guests on the virtual + network without opening up those same services to the rest + of the physical network. This means that host access from + virtual machines is no longer controlled by the firewalld + default zone (usually "public"), but rather by the new

+ firewalld zone called "libvirt" (unless configured otherwise + using the new <code>zone</code> attribute of the network + <code>bridge</code> element). + </description> + </change>

</section> </release> <release version="v4.10.0" date="2018-12-03"> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 9a63720ff7..9b3dcad4a5 100644 --- a/docs/schemas/basictypes.rng +++ b/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> 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 e035d8aba7..b09cb1dae2 100644 --- a/src/conf/network_conf.c +++ b/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); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index c630674300..69ee8d7f2a 100644 --- a/src/conf/network_conf.h +++ b/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 (default "libvirt" */

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 a32f19bcf0..8c585594d1 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -639,13 +639,29 @@ int networkAddFirewallRules(virNetworkDefPtr def) virFirewallPtr fw = NULL; int ret = -1; - - /* if firewalld is active, try to set the default "libvirt" zone, - * but ignore failure, since the version of firewalld on the host - * may have failed to load the libvirt zone - */ - if (virFirewallDStatus() >= 0) - ignore_value(virFirewallDInterfaceSetZone(def->bridge, "libvirt")); + if (!def->bridgeZone) { + /* if firewalld is active and no zone has been explicitly set + * in the config, try to set the default "libvirt" zone, but + * ignore failure, since the version of firewalld on the host + * may have failed to load the libvirt zone + */ + if (virFirewallDStatus() >= 0) + ignore_value(virFirewallDInterfaceSetZone(def->bridge, "libvirt")); + + } else { + /* if a zone has been specified, fail/log an error if we can't + * honor it + */ + if (virFirewallDStatus() < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("zone %s requested for network %s "

+ "but firewalld is not active"), + def->bridgeZone, def->name);

+ goto cleanup; + } + if (virFirewallDInterfaceSetZone(def->bridge, def->bridgeZone) < 0) + goto cleanup; + } fw = virFirewallNew(); 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>