On Wed, Jan 09, 2019 at 09:57:37PM -0500, Laine Stump wrote:
> 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)
Should this paragraph go below the --- ?
>
> Signed-off-by: Laine Stump <laine@xxxxxxxxx>
> ---
> 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>
Either this change belongs in the previous patch, or we should
put both changes in a patch #6 separate from the code.
> </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>
> --
> 2.20.1
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
