[libvirt] [PATCH 11/26] Introduce an object for managing firewall rulesets
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Apr 15 21:15:02 UTC 2014
On 04/08/2014 11:38 AM, Daniel P. Berrange wrote:
> The network and nwfilter drivers both have a need to update
> firewall rules. The currently share no code for interacting
> with iptables / firewalld. The nwfilter driver is fairly
> tied to the concept of creating shell scripts to execute
> which makes it very hard to port to talk to firewalld via
> DBus APIs.
>
> This patch introduces a virFirewallPtr object which is able
> to represent a complete sequence of rule changes, with the
> ability to have multiple transactional checkpoints with
> rollbacks. By formally separating the definition of the rules
> to be applied from the mechanism used to apply them, it is
> also possible to write a firewall engine that uses firewalld
> DBus APIs natively instead of via the slow firewalld-cmd.
>
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
[...]
>
> +
> # util/virhash.h
> virHashAddEntry;
> virHashCreate;
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index cbbaa83..e0bc970 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -129,6 +129,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
> "Systemd",
> "Bhyve",
> "Crypto",
> + "Firewall",
> )
>
> +static int
> +virFirewallValidateBackend(virFirewallBackend backend)
> +{
> + VIR_DEBUG("Validating backend %d", backend);
> +#if WITH_DBUS
> + if (backend == VIR_FIREWALL_BACKEND_AUTOMATIC ||
> + backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> + int rv = virDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE);
Nit: empty line after var. decl. ?
> + VIR_DEBUG("Firewalled is registered ? %d", rv);
> + if (rv < 0) {
> + if (rv == -2) {
> + if (backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("firewalld firewall backend requested, but service is not running"));
> + return -1;
> + } else {
> + VIR_DEBUG("firewalld service not running, trying direct backend");
> + backend = VIR_FIREWALL_BACKEND_DIRECT;
> + }
> + } else {
> + return -1;
> + }
> + } else {
> + VIR_DEBUG("firewalld service running, using firewalld backend");
> + backend = VIR_FIREWALL_BACKEND_FIREWALLD;
> + }
> + }
> +#else
> + if (backend == VIR_FIREWALL_BACKEND_AUTOMATIC) {
> + VIR_DEBUG("DBus support disabled, trying direct backend");
> + backend = VIR_FIREWALL_BACKEND_DIRECT;
> + } else if (backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("firewalld firewall backend requested, but DBus support disabled"));
> + return -1;
> + }
> +#endif
> +
> + if (backend == VIR_FIREWALL_BACKEND_DIRECT) {
> + const char *commands[] = {
> + IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH
> + };
> + size_t i;
Also here?
+
+#define VIR_FIREWALL_RETURN_IF_ERROR(firewall) \
+ if (!firewall || firewall->err) \
+ return;
+
+#define VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, ruel)\
ruel -> rule -- seems unused since it compiles
+
+int
+virFirewallApply(virFirewallPtr firewall)
+{
+ size_t i, j;
+ int ret = -1;
+
+ virMutexLock(&ruleLock);
+ if (virFirewallInitialize() < 0)
+ goto cleanup;
+
+ if (!firewall || firewall->err == ENOMEM) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (firewall->err) {
+ virReportSystemError(firewall->err, "%s",
+ _("Unable to create rule"));
+ goto cleanup;
+ }
+
+ VIR_DEBUG("Applying groups for %p", firewall);
+ for (i = 0; i < firewall->ngroups; i++) {
+ if (virFirewallApplyGroup(firewall, i) < 0) {
+ VIR_DEBUG("Rolling back groups upto %zu for %p", i, firewall);
+ size_t first = i;
+ virErrorPtr saved_error = virSaveLastError();
+
+ /*
+ * Look at any inheritance markers to figure out
+ * what the first rollback group we need to apply is
+ */
+ for (j = 0; j <= i; j++) {
+ VIR_DEBUG("Checking inheritance of group %zu", i - j);
+ if (firewall->groups[i - j]->rollbackFlags &
+ VIR_FIREWALL_ROLLBACK_INHERIT_PREVIOUS)
+ first = (i - j) - 1;
if j = i then first = -1. This doesn't seem right. Limit the loop test
to j < i ?
> + }
> + /*
> + * Now apply all rollback groups in order
> + */
> + for (j = first; j <= i; j++) {
> + VIR_DEBUG("Rolling back group %zu", j);
> + virFirewallRollbackGroup(firewall, j);
> + }
> +
> + virSetError(saved_error);
> + virFreeError(saved_error);
> + VIR_DEBUG("Done rolling back groups for %p", firewall);
> + goto cleanup;
> + }
> + }
> + VIR_DEBUG("Done applying groups for %p", firewall);
> +
> + ret = 0;
> + cleanup:
> + virMutexUnlock(&ruleLock);
> + return ret;
> +}
> +
> +static int
> +testFirewallChainedRollback(const void *opaque ATTRIBUTE_UNUSED)
> +{
> + virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
> + virFirewallPtr fw = NULL;
> + int ret = -1;
> + const char *actual = NULL;
> + const char *expected =
> + IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
> + IPTABLES_PATH " -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
> + IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
> + IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
> + IPTABLES_PATH " -D INPUT --source-host 192.168.122.127 --jump REJECT\n"
> + IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"
> + IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
> + IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
> + const struct testFirewallData *data = opaque;
> +
> + fwDisabled = data->fwDisabled;
> + if (virFirewallSetBackend(data->tryBackend) < 0)
> + goto cleanup;
> +
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
> + } else {
> + fwBuf = &cmdbuf;
> + fwError = true;
> + }
> +
> + fw = virFirewallNew();
> +
> + virFirewallStartTransaction(fw, 0);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-A", "INPUT",
> + "--source-host", "192.168.122.1",
> + "--jump", "ACCEPT", NULL);
> +
> + virFirewallStartRollback(fw, 0);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-D", "INPUT",
> + "--source-host", "192.168.122.1",
> + "--jump", "ACCEPT", NULL);
> +
Hm, wondering why this rule doesn't make it into the expected output...
> +
> + virFirewallStartTransaction(fw, 0);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-A", "INPUT",
> + "--source-host", "192.168.122.127",
> + "--jump", "REJECT", NULL);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-A", "INPUT",
> + "--source-host", "!192.168.122.1",
> + "--jump", "REJECT", NULL);
> +
> + virFirewallStartRollback(fw, 0);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-D", "INPUT",
> + "--source-host", "192.168.122.127",
> + "--jump", "REJECT", NULL);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-D", "INPUT",
> + "--source-host", "!192.168.122.1",
> + "--jump", "REJECT", NULL);
> +
> +
> + virFirewallStartTransaction(fw, 0);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-A", "INPUT",
> + "--source-host", "192.168.122.255",
> + "--jump", "REJECT", NULL);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-A", "INPUT",
> + "--source-host", "!192.168.122.1",
> + "--jump", "REJECT", NULL);
> +
> + virFirewallStartRollback(fw, VIR_FIREWALL_ROLLBACK_INHERIT_PREVIOUS);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-D", "INPUT",
> + "--source-host", "192.168.122.255",
> + "--jump", "REJECT", NULL);
> +
> + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> + "-D", "INPUT",
> + "--source-host", "!192.168.122.1",
> + "--jump", "REJECT", NULL);
> +
> + if (virFirewallApply(fw) == 0) {
> + fprintf(stderr, "Firewall apply unexpectedly worked\n");
> + goto cleanup;
> + }
> +
> + if (virtTestOOMActive())
> + goto cleanup;
> +
> + if (virBufferError(&cmdbuf))
> + goto cleanup;
> +
> + actual = virBufferCurrentContent(&cmdbuf);
> +
> + if (STRNEQ_NULLABLE(expected, actual)) {
> + fprintf(stderr, "Unexected command execution\n");
> + virtTestDifference(stderr, expected, actual);
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + virBufferFreeAndReset(&cmdbuf);
> + fwBuf = NULL;
> + virCommandSetDryRun(NULL, NULL, NULL);
> + virFirewallFree(fw);
> + return ret;
> +}
> +
ACK with nits addressed.
More information about the libvir-list
mailing list