util: create private chains for virtual network firewall rules

Historically firewall rules for virtual networks were added straight
into the base chains. This works but has a number of bugs and design
limitations:

  - It is inflexible for admins wanting to add extra rules ahead
    of libvirt's rules, via hook scripts.

  - It is not clear to the admin that the rules were created by
    libvirt

  - Each rule must be deleted by libvirt individually since they
    are all directly in the builtin chains

  - The ordering of rules in the forward chain is incorrect
    when multiple networks are created, allowing traffic to
    mistakenly flow between networks in one direction.

To address all of these problems, libvirt needs to move to creating
rules in its own private chains. In the top level builtin chains,
libvirt will add links to its own private top level chains.

Addressing the traffic ordering bug requires some extra steps. With
everything going into the FORWARD chain there was interleaving of rules
for outbound traffic and inbound traffic for each network:

  -A FORWARD -d 192.168.3.0/24 -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -s 192.168.3.0/24 -i virbr1 -j ACCEPT
  -A FORWARD -i virbr1 -o virbr1 -j ACCEPT
  -A FORWARD -o virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -i virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -d 192.168.2.0/24 -o virbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -s 192.168.2.0/24 -i virbr0 -j ACCEPT
  -A FORWARD -i virbr0 -o virbr0 -j ACCEPT
  -A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable

The rule allowing outbound traffic from virbr1 would mistakenly
allow packets from virbr1 to virbr0, before the rule denying input
to virbr0 gets a chance to run.

What we really need todo is group the forwarding rules into three
distinct sets:

 * Cross rules - LIBVIRT_FWX

  -A FORWARD -i virbr1 -o virbr1 -j ACCEPT
  -A FORWARD -i virbr0 -o virbr0 -j ACCEPT

 * Incoming rules - LIBVIRT_FWI

  -A FORWARD -d 192.168.3.0/24 -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -o virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -d 192.168.2.0/24 -o virbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
  -A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable

 * Outgoing rules - LIBVIRT_FWO

  -A FORWARD -s 192.168.3.0/24 -i virbr1 -j ACCEPT
  -A FORWARD -i virbr1 -j REJECT --reject-with icmp-port-unreachable
  -A FORWARD -s 192.168.2.0/24 -i virbr0 -j ACCEPT
  -A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable

There is thus no risk of outgoing rules for one network mistakenly
allowing incoming traffic for another network, as all incoming rules
are evalated first.

With this in mind, we'll thus need three distinct chains linked from
the FORWARD chain, so we end up with:

        INPUT --> LIBVIRT_INP   (filter)

       OUTPUT --> LIBVIRT_OUT   (filter)

      FORWARD +-> LIBVIRT_FWX   (filter)
              +-> LIBVIRT_FWO
              \-> LIBVIRT_FWI

  POSTROUTING --> LIBVIRT_PRT   (nat & mangle)

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c1f619f514482258c38faf2b42728bec25fcfd4a..4d60766267d6f3d43e3e5165943592a7e2f07d8e 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2077,6 +2077,7 @@ iptablesRemoveOutputFixUdpChecksum;
 iptablesRemoveTcpInput;
 iptablesRemoveUdpInput;
 iptablesRemoveUdpOutput;
+iptablesSetupPrivateChains;
 
 
 # util/viriscsi.h

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 1e107ee42283808e81db13a91b21a96ce3987d7f..61f77f27358ed5ecfb84518d79d9adb9dd76e978 100644 (file)
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -36,6 +36,9 @@ VIR_LOG_INIT("network.bridge_driver_linux");
 
 int networkPreReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
 {
+    int ret = iptablesSetupPrivateChains();
+    if (ret < 0)
+        return -1;
     return 0;
 }
 

diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 7f0955b82d7bb471e8d8a5be9f63668a16fea388..770dcf04a67bd4e133abd80e1df8ca58928189c8 100644 (file)
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -37,6 +37,7 @@
 #include "virthread.h"
 #include "virstring.h"
 #include "virutil.h"
+#include "virhash.h"
 
 VIR_LOG_INIT("util.iptables");
 
@@ -48,6 +49,136 @@ enum {
 };
 
 
+typedef struct {
+    const char *parent;
+    const char *child;
+} iptablesGlobalChain;
+
+typedef struct {
+    virFirewallLayer layer;
+    const char *table;
+    iptablesGlobalChain *chains;
+    size_t nchains;
+    bool *changed;
+} iptablesGlobalChainData;
+
+
+static int
+iptablesPrivateChainCreate(virFirewallPtr fw,
+                           virFirewallLayer layer,
+                           const char *const *lines,
+                           void *opaque)
+{
+    iptablesGlobalChainData *data = opaque;
+    virHashTablePtr chains = NULL;
+    virHashTablePtr links = NULL;
+    const char *const *tmp;
+    int ret = -1;
+    size_t i;
+
+    if (!(chains = virHashCreate(50, NULL)))
+        goto cleanup;
+    if (!(links = virHashCreate(50, NULL)))
+        goto cleanup;
+
+    tmp = lines;
+    while (tmp && *tmp) {
+        if (STRPREFIX(*tmp, "-N ")) { /* eg "-N LIBVIRT_INP" */
+            if (virHashUpdateEntry(chains, *tmp + 3, (void *)0x1) < 0)
+                goto cleanup;
+        } else if (STRPREFIX(*tmp, "-A ")) { /* eg "-A INPUT -j LIBVIRT_INP" */
+            char *sep = strchr(*tmp + 3, ' ');
+            if (sep) {
+                *sep = '\0';
+                if (STRPREFIX(sep + 1, "-j ")) {
+                    if (virHashUpdateEntry(links, sep + 4,
+                                           (char *)*tmp + 3) < 0)
+                        goto cleanup;
+                }
+            }
+        }
+        tmp++;
+    }
+
+    for (i = 0; i < data->nchains; i++) {
+        const char *from;
+        if (!virHashLookup(chains, data->chains[i].child)) {
+            virFirewallAddRule(fw, layer,
+                               "--table", data->table,
+                               "--new-chain", data->chains[i].child, NULL);
+            *data->changed = true;
+        }
+
+        from = virHashLookup(links, data->chains[i].child);
+        if (!from || STRNEQ(from, data->chains[i].parent))
+            virFirewallAddRule(fw, layer,
+                               "--table", data->table,
+                               "--insert", data->chains[i].parent,
+                               "--jump", data->chains[i].child, NULL);
+    }
+
+    ret = 0;
+ cleanup:
+    virHashFree(chains);
+    virHashFree(links);
+    return ret;
+}
+
+
+int
+iptablesSetupPrivateChains(void)
+{
+    virFirewallPtr fw = NULL;
+    int ret = -1;
+    iptablesGlobalChain filter_chains[] = {
+        {"INPUT", "LIBVIRT_INP"},
+        {"OUTPUT", "LIBVIRT_OUT"},
+        {"FORWARD", "LIBVIRT_FWO"},
+        {"FORWARD", "LIBVIRT_FWI"},
+        {"FORWARD", "LIBVIRT_FWX"},
+    };
+    iptablesGlobalChain natmangle_chains[] = {
+        {"POSTROUTING",  "LIBVIRT_PRT"},
+    };
+    bool changed = false;
+    iptablesGlobalChainData data[] = {
+        { VIR_FIREWALL_LAYER_IPV4, "filter",
+          filter_chains, ARRAY_CARDINALITY(filter_chains), &changed },
+        { VIR_FIREWALL_LAYER_IPV4, "nat",
+          natmangle_chains, ARRAY_CARDINALITY(natmangle_chains), &changed },
+        { VIR_FIREWALL_LAYER_IPV4, "mangle",
+          natmangle_chains, ARRAY_CARDINALITY(natmangle_chains), &changed },
+        { VIR_FIREWALL_LAYER_IPV6, "filter",
+          filter_chains, ARRAY_CARDINALITY(filter_chains), &changed },
+        { VIR_FIREWALL_LAYER_IPV6, "nat",
+          natmangle_chains, ARRAY_CARDINALITY(natmangle_chains), &changed },
+        { VIR_FIREWALL_LAYER_IPV6, "mangle",
+          natmangle_chains, ARRAY_CARDINALITY(natmangle_chains), &changed },
+    };
+    size_t i;
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    for (i = 0; i < ARRAY_CARDINALITY(data); i++)
+        virFirewallAddRuleFull(fw, data[i].layer,
+                               false, iptablesPrivateChainCreate,
+                               &(data[i]), "--table", data[i].table,
+                               "--list-rules", NULL);
+
+    if (virFirewallApply(fw) < 0)
+        goto cleanup;
+
+    ret = changed ? 1 : 0;
+
+ cleanup:
+
+    virFirewallFree(fw);
+    return ret;
+}
+
+
 static void
 iptablesInput(virFirewallPtr fw,
               virFirewallLayer layer,

diff --git a/src/util/viriptables.h b/src/util/viriptables.h
index b978fe409bfefe3f09dff78234e38aa9b0f8e41b..94304401c5e7cb4f91167e547fcb8f3062a58c1e 100644 (file)
--- a/src/util/viriptables.h
+++ b/src/util/viriptables.h
@@ -24,6 +24,8 @@
 # include "virsocketaddr.h"
 # include "virfirewall.h"
 
+int              iptablesSetupPrivateChains      (void);
+
 void             iptablesAddTcpInput             (virFirewallPtr fw,
                                                   virFirewallLayer layer,
                                                   const char *iface,