Signed-off-by: Eric Garver <eric@garver.life>
---
 src/network/bridge_driver_linux.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index a0f593b06636..d9597d91beed 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -857,8 +857,17 @@ int networkAddFirewallRules(virNetworkDef *def)
              * 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
+             *
+             * Routed networks use a different zone and policy which we also
+             * need to verify exist. Probing for the policy guarantees the
+             * running firewalld has support for policies (firewalld >= 0.9.0).
              */
-            if (virFirewallDZoneExists("libvirt")) {
+            if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE &&
+                virFirewallDPolicyExists("libvirt-routed-out") &&
+                virFirewallDZoneExists("libvirt-routed")) {
+                if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0)
+                    return -1;
+            } else if (virFirewallDZoneExists("libvirt")) {
                 if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)
                     return -1;
             } else {
-- 
2.35.3

Hey Eric!

I *finally* set things up to test this adequately, and it all looks good 
and is operating properly. The one nit I found with the content of the 
patches was that the new zone/policy files weren't added to the 
specfile, so I've done that.

I'm now ready to push all the patches, but wanted to put more 
explanation into this final patch that turns it all on. Does the 
following sound okay to you?:

network: firewalld: allow incoming connections to guests on routed networks

Prior to firewalld version 0.9.0, the default action of ACCEPT in the
"libvirt" zone (subsequently overridden with a lower priority "REJECT"
action) would result in an implicit rule that allowed incoming sessions
through the zone; libvirt relied on this implicit rule to permit
incoming connections to guests that were connected via a libvirt
"routed" network.

Starting in firewalld 0.9.0, the rules generated for this same
zonefile changed such that incoming sessions through the libvirt zone
were no longer allowed, breaking the longstanding convention that they
should be allowed (only for routed networks).

This patch changes the zone for routed networks from "libvirt" to
the newly-added "libvirt-routed" zone so that incoming sessions to
guests on routed networks are once again allowed.

Resolves: https://bugzilla.redhat.com/2055706

"

On 9/22/22 11:13 AM, Eric Garver wrote:
> Signed-off-by: Eric Garver <eric@garver.life>
> ---
>   src/network/bridge_driver_linux.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index a0f593b06636..d9597d91beed 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -857,8 +857,17 @@ int networkAddFirewallRules(virNetworkDef *def)
>                * 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
> +             *
> +             * Routed networks use a different zone and policy which we also
> +             * need to verify exist. Probing for the policy guarantees the
> +             * running firewalld has support for policies (firewalld >= 0.9.0).
>                */
> -            if (virFirewallDZoneExists("libvirt")) {
> +            if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE &&
> +                virFirewallDPolicyExists("libvirt-routed-out") &&
> +                virFirewallDZoneExists("libvirt-routed")) {
> +                if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0)
> +                    return -1;
> +            } else if (virFirewallDZoneExists("libvirt")) {
>                   if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)
>                       return -1;
>               } else {

On Mon, Oct 24, 2022 at 09:38:17AM -0400, Laine Stump wrote:
> Hey Eric!
> 
> I *finally* set things up to test this adequately, and it all looks good and
> is operating properly. The one nit I found with the content of the patches
> was that the new zone/policy files weren't added to the specfile, so I've
> done that.

Thanks! Sorry I missed it.

> I'm now ready to push all the patches, but wanted to put more explanation
> into this final patch that turns it all on. Does the following sound okay to
> you?:
> 
> network: firewalld: allow incoming connections to guests on routed networks
> 
> Prior to firewalld version 0.9.0, the default action of ACCEPT in the

s/0.9.0/1.0.0/

> "libvirt" zone (subsequently overridden with a lower priority "REJECT"
> action) would result in an implicit rule that allowed incoming sessions
> through the zone; libvirt relied on this implicit rule to permit
> incoming connections to guests that were connected via a libvirt
> "routed" network.
> 
> Starting in firewalld 0.9.0, the rules generated for this same

s/0.9.0/1.0.0/

> zonefile changed such that incoming sessions through the libvirt zone
> were no longer allowed, breaking the longstanding convention that they
> should be allowed (only for routed networks).

The behavior changed in firewalld 1.0.0. However, the new zone,
"libvirt-routed", will be active when firewalld >= 0.9.0 is present.

> This patch changes the zone for routed networks from "libvirt" to
> the newly-added "libvirt-routed" zone so that incoming sessions to
> guests on routed networks are once again allowed.
> 
> Resolves: https://bugzilla.redhat.com/2055706

Otherwise, lgtm.

Thanks again.
Eric.

> 
> "
> 
> On 9/22/22 11:13 AM, Eric Garver wrote:
> > Signed-off-by: Eric Garver <eric@garver.life>
> > ---
> >   src/network/bridge_driver_linux.c | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> > index a0f593b06636..d9597d91beed 100644
> > --- a/src/network/bridge_driver_linux.c
> > +++ b/src/network/bridge_driver_linux.c
> > @@ -857,8 +857,17 @@ int networkAddFirewallRules(virNetworkDef *def)
> >                * 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
> > +             *
> > +             * Routed networks use a different zone and policy which we also
> > +             * need to verify exist. Probing for the policy guarantees the
> > +             * running firewalld has support for policies (firewalld >= 0.9.0).
> >                */
> > -            if (virFirewallDZoneExists("libvirt")) {
> > +            if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE &&
> > +                virFirewallDPolicyExists("libvirt-routed-out") &&
> > +                virFirewallDZoneExists("libvirt-routed")) {
> > +                if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0)
> > +                    return -1;
> > +            } else if (virFirewallDZoneExists("libvirt")) {
> >                   if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)
> >                       return -1;
> >               } else {
>