11:13 p.m.
This series fixes routed networks when a newer firewalld (>= 1.0.0) is
present [1]. Firewalld 1.0.0 included a change that disallows implicit
forwarding between zones [2]. libvirt was relying on this behavior to
allow routed networks to function.
Firewalld policies are added. Policies have been supported since
firewalld 0.9.0. If the running firewall does not support policies, then
it will fallback to the current zone only behavior.
My goal is to get libvirt to a fully native firewalld backend; no
iptables rules. This series is phase 1 of that effort. The next steps
are:
1. introduce a "libvirt-nat" zone and policies
- the current "libvirt" zone will become obsolete
2. go full native firewalld, do not use iptables directly
- currently a hybrid of iptables + firewalld is used
v3:
- rebase, retest, resend
v2:
- keep existing libvirt zone as is
- remove "<forward />" in libvirt-routed zone because this feature
requires firewalld >= 0.9.0. Has no impact since the added policies
allow forwarding libvirt-routed <--> ANY zone (including itself).
- add probe for policies: virFirewallDGetPolicies(),
virFirewallDPolicyExists()
[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2055706
[2]: https://github.com/firewalld/firewalld/issues/177
Eric Garver (5):
util: add virFirewallDGetPolicies()
util: add virFirewallDPolicyExists()
network: firewalld: add zone for routed networks
network: firewalld: add policies for routed networks
network: firewalld: add support for routed networks
src/libvirt_private.syms | 2 +
src/network/bridge_driver_linux.c | 11 +++-
src/network/libvirt-routed-in.policy | 11 ++++
src/network/libvirt-routed-out.policy | 12 +++++
src/network/libvirt-routed.zone | 10 ++++
src/network/libvirt-to-host.policy | 20 ++++++++
src/network/meson.build | 20 ++++++++
src/util/virfirewalld.c | 72 +++++++++++++++++++++++++++
src/util/virfirewalld.h | 2 +
9 files changed, 159 insertions(+), 1 deletion(-)
create mode 100644 src/network/libvirt-routed-in.policy
create mode 100644 src/network/libvirt-routed-out.policy
create mode 100644 src/network/libvirt-routed.zone
create mode 100644 src/network/libvirt-to-host.policy
--
2.35.3
11:13 p.m.
New subject: [PATCH v3 1/5] util: add virFirewallDGetPolicies()
Signed-off-by: Eric Garver <eric(a)garver.life>
---
src/libvirt_private.syms | 1 +
src/util/virfirewalld.c | 41 ++++++++++++++++++++++++++++++++++++++++
src/util/virfirewalld.h | 1 +
3 files changed, 43 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 25794bc2f417..32c8bdeb23ee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2370,6 +2370,7 @@ virFirewallStartTransaction;
# util/virfirewalld.h
virFirewallDApplyRule;
virFirewallDGetBackend;
+virFirewallDGetPolicies;
virFirewallDGetVersion;
virFirewallDGetZones;
virFirewallDInterfaceSetZone;
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index c909901833f7..0912508dbc45 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -223,6 +223,47 @@ virFirewallDGetZones(char ***zones, size_t *nzones)
return 0;
}
+/**
+ * virFirewallDGetPolicies:
+ * @policies: array of char *, each entry is a null-terminated policy name
+ * @npolicies: number of entries in @policies
+ *
+ * Get the number of currently active firewalld policies, and their names
+ * in an array of null-terminated strings. The memory pointed to by
+ * @policies will belong to the caller, and must be freed.
+ *
+ * Returns 0 on success, -1 (and failure logged) on error
+ */
+int
+virFirewallDGetPolicies(char ***policies, size_t *npolicies)
+{
+ GDBusConnection *sysbus = virGDBusGetSystemBus();
+ g_autoptr(GVariant) reply = NULL;
+ g_autoptr(GVariant) array = NULL;
+
+ *npolicies = 0;
+ *policies = NULL;
+
+ if (!sysbus)
+ return -1;
+
+ if (virGDBusCallMethod(sysbus,
+ &reply,
+ G_VARIANT_TYPE("(as)"),
+ NULL,
+ VIR_FIREWALL_FIREWALLD_SERVICE,
+ "/org/fedoraproject/FirewallD1",
+ "org.fedoraproject.FirewallD1.policy",
+ "getPolicies",
+ NULL) < 0)
+ return -1;
+
+ g_variant_get(reply, "(@as)", &array);
+ *policies = g_variant_dup_strv(array, npolicies);
+
+ return 0;
+}
+
/**
* virFirewallDZoneExists:
diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h
index c396802a2f56..ef05896e2b8b 100644
--- a/src/util/virfirewalld.h
+++ b/src/util/virfirewalld.h
@@ -33,6 +33,7 @@ int virFirewallDGetVersion(unsigned long *version);
int virFirewallDGetBackend(void);
int virFirewallDIsRegistered(void);
int virFirewallDGetZones(char ***zones, size_t *nzones);
+int virFirewallDGetPolicies(char ***policies, size_t *npolicies);
bool virFirewallDZoneExists(const char *match);
int virFirewallDApplyRule(virFirewallLayer layer,
char **args, size_t argsLen,
--
2.35.3
11:13 p.m.
New subject: [PATCH v3 2/5] util: add virFirewallDPolicyExists()
Signed-off-by: Eric Garver <eric(a)garver.life>
---
src/libvirt_private.syms | 1 +
src/util/virfirewalld.c | 31 +++++++++++++++++++++++++++++++
src/util/virfirewalld.h | 1 +
3 files changed, 33 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 32c8bdeb23ee..92b6062fabda 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2375,6 +2375,7 @@ virFirewallDGetVersion;
virFirewallDGetZones;
virFirewallDInterfaceSetZone;
virFirewallDIsRegistered;
+virFirewallDPolicyExists;
virFirewallDSynchronize;
virFirewallDZoneExists;
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index 0912508dbc45..ad879164c3a8 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -296,6 +296,37 @@ virFirewallDZoneExists(const char *match)
}
+/**
+ * virFirewallDPolicyExists:
+ * @match: name of policy to look for
+ *
+ * Returns true if the requested policy exists, or false if it doesn't exist
+ */
+bool
+virFirewallDPolicyExists(const char *match)
+{
+ size_t npolicies = 0, i;
+ char **policies = NULL;
+ bool result = false;
+
+ if (virFirewallDGetPolicies(&policies, &npolicies) < 0)
+ goto cleanup;
+
+ for (i = 0; i < npolicies; i++) {
+ if (STREQ_NULLABLE(policies[i], match))
+ result = true;
+ }
+
+ cleanup:
+ VIR_DEBUG("Requested policy '%s' %s exist",
+ match, result ? "does" : "doesn't");
+ for (i = 0; i < npolicies; i++)
+ VIR_FREE(policies[i]);
+ VIR_FREE(policies);
+ return result;
+}
+
+
/**
* virFirewallDApplyRule:
* @layer: which layer to apply the rule to
diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h
index ef05896e2b8b..fa4c9e702ccb 100644
--- a/src/util/virfirewalld.h
+++ b/src/util/virfirewalld.h
@@ -35,6 +35,7 @@ int virFirewallDIsRegistered(void);
int virFirewallDGetZones(char ***zones, size_t *nzones);
int virFirewallDGetPolicies(char ***policies, size_t *npolicies);
bool virFirewallDZoneExists(const char *match);
+bool virFirewallDPolicyExists(const char *match);
int virFirewallDApplyRule(virFirewallLayer layer,
char **args, size_t argsLen,
bool ignoreErrors,
--
2.35.3
11:13 p.m.
New subject: [PATCH v3 3/5] network: firewalld: add zone for routed networks
This zone will be used for the routed network by default.
Note that this zone definition omits "forward" aka intra-zone
forwarding, because it requires firewalld >= 0.9.0.
Signed-off-by: Eric Garver <eric(a)garver.life>
---
src/network/libvirt-routed.zone | 10 ++++++++++
src/network/meson.build | 5 +++++
2 files changed, 15 insertions(+)
create mode 100644 src/network/libvirt-routed.zone
diff --git a/src/network/libvirt-routed.zone b/src/network/libvirt-routed.zone
new file mode 100644
index 000000000000..ed7dd936a242
--- /dev/null
+++ b/src/network/libvirt-routed.zone
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8"?>
+<zone>
+ <short>libvirt-routed</short>
+
+ <description>
+ This zone is intended to be used only by routed libvirt virtual networks -
+ libvirt will add the bridge devices for all new virtual networks to this
+ zone by default.
+ </description>
+</zone>
diff --git a/src/network/meson.build b/src/network/meson.build
index 395074a0a0a6..dcb31af6448b 100644
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -101,5 +101,10 @@ if conf.has('WITH_NETWORK')
install_dir: prefix / 'lib' / 'firewalld' / 'zones',
rename: [ 'libvirt.xml' ],
)
+ install_data(
+ 'libvirt-routed.zone',
+ install_dir: prefix / 'lib' / 'firewalld' / 'zones',
+ rename: [ 'libvirt-routed.xml' ],
+ )
endif
endif
--
2.35.3
11:13 p.m.
New subject: [PATCH v3 4/5] network: firewalld: add policies for routed networks
Signed-off-by: Eric Garver <eric(a)garver.life>
---
src/network/libvirt-routed-in.policy | 11 +++++++++++
src/network/libvirt-routed-out.policy | 12 ++++++++++++
src/network/libvirt-to-host.policy | 20 ++++++++++++++++++++
src/network/meson.build | 15 +++++++++++++++
4 files changed, 58 insertions(+)
create mode 100644 src/network/libvirt-routed-in.policy
create mode 100644 src/network/libvirt-routed-out.policy
create mode 100644 src/network/libvirt-to-host.policy
diff --git a/src/network/libvirt-routed-in.policy b/src/network/libvirt-routed-in.policy
new file mode 100644
index 000000000000..dd691efbb64c
--- /dev/null
+++ b/src/network/libvirt-routed-in.policy
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<policy target="ACCEPT">
+ <short>libvirt-routed-in</short>
+
+ <description>
+ This policy is used to allow routed traffic to the virtual machines.
+ </description>
+
+ <ingress-zone name="ANY" />
+ <egress-zone name="libvirt-routed" />
+</policy>
diff --git a/src/network/libvirt-routed-out.policy
b/src/network/libvirt-routed-out.policy
new file mode 100644
index 000000000000..efa0030569d6
--- /dev/null
+++ b/src/network/libvirt-routed-out.policy
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="utf-8"?>
+<policy target="ACCEPT">
+ <short>libvirt-routed-out</short>
+
+ <description>
+ This policy is used to allow routed virtual machine traffic to the rest of
+ the network.
+ </description>
+
+ <ingress-zone name="libvirt-routed" />
+ <egress-zone name="ANY" />
+</policy>
diff --git a/src/network/libvirt-to-host.policy b/src/network/libvirt-to-host.policy
new file mode 100644
index 000000000000..b20aecaf4249
--- /dev/null
+++ b/src/network/libvirt-to-host.policy
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8"?>
+<policy target="REJECT">
+ <short>libvirt-to-host</short>
+
+ <description>
+ This policy is used to filter traffic from virtual machines to the
+ host.
+ </description>
+
+ <ingress-zone name="libvirt-routed" />
+ <egress-zone name="HOST" />
+
+ <protocol value='icmp'/>
+ <protocol value='ipv6-icmp'/>
+ <service name='dhcp'/>
+ <service name='dhcpv6'/>
+ <service name='dns'/>
+ <service name='ssh'/>
+ <service name='tftp'/>
+</policy>
diff --git a/src/network/meson.build b/src/network/meson.build
index dcb31af6448b..26b95367748f 100644
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -106,5 +106,20 @@ if conf.has('WITH_NETWORK')
install_dir: prefix / 'lib' / 'firewalld' / 'zones',
rename: [ 'libvirt-routed.xml' ],
)
+ install_data(
+ 'libvirt-to-host.policy',
+ install_dir: prefix / 'lib' / 'firewalld' / 'policies',
+ rename: [ 'libvirt-to-host.xml' ],
+ )
+ install_data(
+ 'libvirt-routed-out.policy',
+ install_dir: prefix / 'lib' / 'firewalld' / 'policies',
+ rename: [ 'libvirt-routed-out.xml' ],
+ )
+ install_data(
+ 'libvirt-routed-in.policy',
+ install_dir: prefix / 'lib' / 'firewalld' / 'policies',
+ rename: [ 'libvirt-routed-in.xml' ],
+ )
endif
endif
--
2.35.3
11:13 p.m.
New subject: [PATCH v3 5/5] network: firewalld: add support for routed networks
Signed-off-by: Eric Garver <eric(a)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
9:38 p.m.
New subject: [PATCH v3 5/5] network: firewalld: add support for routed networks
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(a)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:44 a.m.
New subject: [PATCH v3 5/5] network: firewalld: add support for routed networks
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(a)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 {
684
days inactive
716
days old
7 comments
3 participants
participants (3)
-
Eric Garver -
Eric Garver -
Laine Stump