[PATCH v2 0/5] network: firewalld: fix routed network
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. 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
Signed-off-by: Eric Garver <eric@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 bfedd853268d..64d932e929eb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2340,6 +2340,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
Signed-off-by: Eric Garver <eric@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 64d932e929eb..9ea3062f75e9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2345,6 +2345,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
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@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 b5eff0c3ab6b..d12e36ce1ca3 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -100,5 +100,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
Signed-off-by: Eric Garver <eric@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 d12e36ce1ca3..49ffad24f405 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -105,5 +105,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
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 98d2a33a1da0..c96d8f624b4d 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -858,8 +858,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
participants (1)
-
Eric Garver