On Tue, Oct 13, 2009 at 12:57:13PM +0200, Gerhard Stenzel wrote:
> +#ifdef ENABLE_EBTABLES_LOKKIT
> +static void
> +notifyRulesUpdated(const char *table,
> + const char *path)
> +{
> + char arg[PATH_MAX];
> + const char *argv[4];
> +
> + snprintf(arg, sizeof(arg), "--custom-rules=ipv4:%s:%s", table, path);
> +
> + argv[0] = (char *) LOKKIT_PATH;
> + argv[1] = (char *) "--nostart";
> + argv[2] = arg;
> + argv[3] = NULL;
This is better declared upfront as
const char *const argv[] = {
LOKKIT_PATH, "--nostart", arg, NULL,
};
> +
> + char ebuf[1024];
> + if (virRun(NULL, argv, NULL) < 0)
> + VIR_WARN(_("Failed to run '%s %s': %s"),
> + LOKKIT_PATH, arg, virStrerror(errno, ebuf, sizeof ebuf));
> +}
> +
> +static void
> +notifyRulesRemoved(const char *table,
> + const char *path)
> +{
> +/* 10 MB limit on config file size as a sanity check */
> +#define MAX_FILE_LEN (1024*1024*10)
> +
> + char arg[PATH_MAX];
> + char *content;
> + int len;
> + FILE *f = NULL;
> +
> + len = virFileReadAll(SYSCONF_DIR "/sysconfig/system-config-firewall",
> + MAX_FILE_LEN, &content);
> + if (len < 0) {
> + VIR_WARN("%s", _("Failed to read " SYSCONF_DIR
> + "/sysconfig/system-config-firewall"));
> + return;
> + }
> +
> + snprintf(arg, sizeof(arg), "--custom-rules=ipv4:%s:%s", table, path);
> +
> + if (!stripLine(content, len, arg)) {
> + VIR_FREE(content);
> + return;
> + }
> +
> + if (!(f = fopen(SYSCONF_DIR "/sysconfig/system-config-firewall", "w")))
> + goto write_error;
> +
> + if (fputs(content, f) == EOF)
> + goto write_error;
> +
> + if (fclose(f) == EOF) {
> + f = NULL;
> + goto write_error;
> + }
This fopen/fputs/fclose triple could be nicely replaced with a
single call to
if (virFileWriteStr(SYSCONF_DIR "/sysconfig/system-config-firewall",
content) < 0)
goto write_error;
> +
> + VIR_FREE(content);
> +
> + return;
> +
> + write_error:;
> + char ebuf[1024];
> + VIR_WARN(_("Failed to write to " SYSCONF_DIR
> + "/sysconfig/system-config-firewall : %s"),
> + virStrerror(errno, ebuf, sizeof ebuf));
> + if (f)
> + fclose(f);
> + VIR_FREE(content);
> +
> +#undef MAX_FILE_LEN
> +}
> +
> +static ebtRules *
> +ebtRulesNew(const char *table,
> + const char *chain)
> +{
> + ebtRules *rules;
> +
> + if (VIR_ALLOC(rules) < 0)
> + return NULL;
> +
> + if (!(rules->table = strdup(table)))
> + goto error;
> +
> + if (!(rules->chain = strdup(chain)))
> + goto error;
> +
> + rules->rules = NULL;
> + rules->nrules = 0;
> +
> +#ifdef ENABLE_EBTABLES_LOKKIT
> + if (virFileBuildPath(LOCAL_STATE_DIR "/lib/libvirt/ebtables", table, NULL,
> + rules->dir, sizeof(rules->dir)) < 0)
> + goto error;
> +
> + if (virFileBuildPath(rules->dir, chain, ".chain", rules->path, sizeof(rules->path)) < 0)
> + goto error;
> +#endif /* ENABLE_EBTABLES_LOKKIT */
Don't forget to add a rule to src/Makefile.am to ensure that the
new ebtables directory is created by the 'install-data-local' rule
> index 0000000..a3f403a
> --- /dev/null
> +++ b/src/util/ebtables.h
> @@ -0,0 +1,134 @@
> +/*
> + * Copyright (C) 2009 IBM Corp.
> + * Copyright (C) 2007, 2008 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + * based on iptables.h
> + * Authors:
> + * Gerhard Stenzel <gerhard.stenzel@xxxxxxxxxx>
> + */
> +
> +#ifndef __QEMUD_EBTABLES_H__
> +#define __QEMUD_EBTABLES_H__
> +
> +typedef struct
> +{
> + char *rule;
> + const char **argv;
> + int command_idx;
> +} ebtRule;
> +
> +typedef struct
> +{
> + char *table;
> + char *chain;
> +
> + int nrules;
> + ebtRule *rules;
> +
> +#ifdef ENABLE_EBTABLES_LOKKIT
> +
> + char dir[PATH_MAX];
> + char path[PATH_MAX];
> +
> +#endif /* ENABLE_EBTABLES_LOKKIT */
> +
> +} ebtRules;
> +
> +struct _ebtablesContext
> +{
> + ebtRules *input_filter;
> + ebtRules *forward_filter;
> + ebtRules *nat_postrouting;
> +};
Since the caller of the API does not need to know the internals
of the struct, it is better to move these 3 struct definitions
into the ebtables.c file.
> +typedef struct _ebtablesContext ebtablesContext;
> +
> +ebtablesContext *ebtablesContextNew (const char *driver);
> +void ebtablesContextFree (ebtablesContext *ctx);
> +
> +void ebtablesSaveRules (ebtablesContext *ctx);
> +void ebtablesReloadRules (ebtablesContext *ctx);
> +
> +int ebtablesAddTcpInput (ebtablesContext *ctx,
> + const char *iface,
> + int port);
> +int ebtablesRemoveTcpInput (ebtablesContext *ctx,
> + const char *iface,
> + int port);
> +
> +int ebtablesAddUdpInput (ebtablesContext *ctx,
> + const char *iface,
> + int port);
> +int ebtablesRemoveUdpInput (ebtablesContext *ctx,
> + const char *iface,
> + int port);
> +
> +int ebtablesAddForwardAllowOut (ebtablesContext *ctx,
> + const char *network,
> + const char *iface,
> + const char *physdev);
> +int ebtablesRemoveForwardAllowOut (ebtablesContext *ctx,
> + const char *network,
> + const char *iface,
> + const char *physdev);
> +
> +int ebtablesAddForwardAllowRelatedIn(ebtablesContext *ctx,
> + const char *network,
> + const char *iface,
> + const char *physdev);
> +int ebtablesRemoveForwardAllowRelatedIn(ebtablesContext *ctx,
> + const char *network,
> + const char *iface,
> + const char *physdev);
> +
> +int ebtablesAddForwardAllowIn (ebtablesContext *ctx,
> + const char *iface,
> + const char *physdev);
> +int ebtablesRemoveForwardAllowIn (ebtablesContext *ctx,
> + const char *iface,
> + const char *physdev);
> +
> +int ebtablesAddForwardAllowCross (ebtablesContext *ctx,
> + const char *iface);
> +int ebtablesRemoveForwardAllowCross (ebtablesContext *ctx,
> + const char *iface);
> +
> +int ebtablesAddForwardRejectOut (ebtablesContext *ctx,
> + const char *iface);
> +int ebtablesRemoveForwardRejectOut (ebtablesContext *ctx,
> + const char *iface);
> +
> +int ebtablesAddForwardRejectIn (ebtablesContext *ctx,
> + const char *iface);
> +int ebtablesRemoveForwardRejectIn (ebtablesContext *ctx,
> + const char *iface);
> +
> +int ebtablesAddForwardMasquerade (ebtablesContext *ctx,
> + const char *network,
> + const char *physdev);
> +int ebtablesRemoveForwardMasquerade (ebtablesContext *ctx,
> + const char *network,
> + const char *physdev);
> +
> +int ebtablesAddForwardPolicyReject(ebtablesContext *ctx);
> +
> +int ebtablesRemoveForwardPolicyReject(ebtablesContext *ctx);
> +
> +int ebtablesForwardPolicyReject(ebtablesContext *ctx,
> + int action);
There seem like there's quite a lot of methods here that we don't
actually need for MAC address filtering. It'd be a little clearer
if you removed the ones that aren't relevant. Alot of the complexity
of the iptables.h/c file on which this is based is relating to the
masquerading, and punching holes for DHCP/DNS. ebtables ought to be
alot simpler, since we're pretty much just doing a 'ALLOW <mac>'
followed by 'DENY ALL' on the tap device in question.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list