Hello Anthony, Cool patch series.
> -----Original Message----- > From: > qemu-devel-bounces+chris.krumme=windriver....@nongnu.org > [mailto:qemu-devel-bounces+chris.krumme=windriver....@nongnu.o > rg] On Behalf Of Anthony Liguori > Sent: Tuesday, November 03, 2009 6:28 PM > To: qemu-devel@nongnu.org > Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin > Kirkland; Michael Tsirkin; Juan Quintela > Subject: [Qemu-devel] [PATCH 2/4] Add access control support > toqemu-bridge-helper > > We go to great lengths to restrict ourselves to just > cap_net_admin as an OS > enforced security mechanism. However, we further restrict > what we allow users > to do to simply adding a tap device to a bridge interface by > virtue of the fact > that this is the only functionality we expose. > > This is not good enough though. An administrator is likely > to want to restrict > the bridges that an unprivileged user can access, in > particular, to restrict > an unprivileged user from putting a guest on what should be > isolated networks. > > This patch implements a ACL mechanism that is enforced by > qemu-bridge-helper. > The ACLs are fairly simple whitelist/blacklist mechanisms > with a wildcard of > 'all'. > > An interesting feature of this ACL mechanism is that you can > include external > ACL files. The main reason to support this is so that you > can set different > file system permissions on those external ACL files. This allows an > administrator to implement rather sophisicated ACL policies > based on user/group > policies via the file system. > > If we fail to include an acl file, we are silent about it > making this mechanism > work pretty seamlessly. As an example: > > /etc/qemu/bridge.conf root:qemu 0640 > > deny all > allow br0 > include /etc/qemu/alice.conf > include /etc/qemu/bob.conf > > /etc/qemu/alice.conf root:alice 0640 > allow br1 > > /etc/qemu/bob.conf root:bob 0640 > allow br2 > > This ACL pattern allows any user in the qemu group to get a tap device > connected to br0 (which is bridged to the physical network). > > Users in the alice group can additionally get a tap device > connected to br1. > This allows br1 to act as a private bridge for the alice group. > > Users in the bob group can additionally get a tap device > connected to br2. > This allows br2 to act as a private bridge for the bob group. > > Under no circumstance can the bob group get access to br1 or > can the alice > group get access to br2. > > Signed-off-by: Anthony Liguori <aligu...@us.ibm.com> > --- > configure | 1 + > qemu-bridge-helper.c | 138 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 139 insertions(+), 0 deletions(-) > > diff --git a/configure b/configure > index a341e77..7c98257 100755 > --- a/configure > +++ b/configure > @@ -1864,6 +1864,7 @@ printf " '%s'" "$0" "$@" >> $config_host_mak > echo >> $config_host_mak > > echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> > $config_host_mak > +echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak > > case "$cpu" in > > i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p > pc|ppc64|s390|sparc|sparc64) > diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c > index f10d37c..0d059ed 100644 > --- a/qemu-bridge-helper.c > +++ b/qemu-bridge-helper.c > @@ -33,6 +33,106 @@ > > #include "net/tap-linux.h" > > +#define MAX_ACLS (128) > +#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf" > + > +enum { > + ACL_ALLOW = 0, > + ACL_ALLOW_ALL, > + ACL_DENY, > + ACL_DENY_ALL, > +}; > + > +typedef struct ACLRule > +{ > + int type; > + char iface[IFNAMSIZ]; > +} ACLRule; > + > +static int parse_acl_file(const char *filename, ACLRule > *acls, int *pacl_count) > +{ > + int acl_count = *pacl_count; > + FILE *f; > + char line[4096]; > + > + f = fopen(filename, "r"); > + if (f == NULL) { > + return -1; > + } > + > + while (acl_count != MAX_ACLS && > + fgets(line, sizeof(line), f) != NULL) { > + char *ptr = line; > + char *cmd, *arg, *argend; > + > + while (isspace(*ptr)) { > + ptr++; > + } > + > + /* skip comments and empty lines */ > + if (*ptr == '#' || *ptr == 0) { > + continue; > + } > + > + cmd = ptr; > + arg = strchr(cmd, ' '); > + if (arg == NULL) { > + arg = strchr(cmd, '\t'); > + } > + > + if (arg == NULL) { > + fprintf(stderr, "Invalid config line:\n %s\n", line); > + fclose(f); > + errno = EINVAL; > + return -1; > + } > + > + *arg = 0; No check is made for arg being in bounds. Thanks Chris > + arg++; > + while (isspace(*arg)) { > + arg++; > + } > + > + argend = arg + strlen(arg); > + while (arg != argend && isspace(*(argend - 1))) { > + argend--; > + } > + *argend = 0; > + > + if (strcmp(cmd, "deny") == 0) { > + if (strcmp(arg, "all") == 0) { > + acls[acl_count].type = ACL_DENY_ALL; > + } else { > + acls[acl_count].type = ACL_DENY; > + snprintf(acls[acl_count].iface, IFNAMSIZ, "%s", arg); > + } > + acl_count++; > + } else if (strcmp(cmd, "allow") == 0) { > + if (strcmp(arg, "all") == 0) { > + acls[acl_count].type = ACL_ALLOW_ALL; > + } else { > + acls[acl_count].type = ACL_ALLOW; > + snprintf(acls[acl_count].iface, IFNAMSIZ, "%s", arg); > + } > + acl_count++; > + } else if (strcmp(cmd, "include") == 0) { > + /* ignore errors */ > + parse_acl_file(arg, acls, &acl_count); > + } else { > + fprintf(stderr, "Unknown command `%s'\n", cmd); > + fclose(f); > + errno = EINVAL; > + return -1; > + } > + } > + > + *pacl_count = acl_count; > + > + fclose(f); > + > + return 0; > +} > + > static int has_vnet_hdr(int fd) > { > unsigned int features; > @@ -95,6 +195,9 @@ int main(int argc, char **argv) > const char *bridge; > char iface[IFNAMSIZ]; > int index; > + ACLRule acls[MAX_ACLS]; > + int acl_count = 0; > + int i, access_allowed; > > /* parse arguments */ > if (argc < 3 || argc > 4) { > @@ -115,6 +218,41 @@ int main(int argc, char **argv) > bridge = argv[index++]; > unixfd = atoi(argv[index++]); > > + /* parse default acl file */ > + if (parse_acl_file(DEFAULT_ACL_FILE, acls, &acl_count) == -1) { > + fprintf(stderr, "failed to parse default acl file `%s'\n", > + DEFAULT_ACL_FILE); > + return -errno; > + } > + > + /* validate bridge against acl -- default policy is to deny */ > + access_allowed = 0; > + for (i = 0; i < acl_count; i++) { > + switch (acls[i].type) { > + case ACL_ALLOW_ALL: > + access_allowed = 1; > + break; > + case ACL_ALLOW: > + if (strcmp(bridge, acls[i].iface) == 0) { > + access_allowed = 1; > + } > + break; > + case ACL_DENY_ALL: > + access_allowed = 0; > + break; > + case ACL_DENY: > + if (strcmp(bridge, acls[i].iface) == 0) { > + access_allowed = 0; > + } > + break; > + } > + } > + > + if (access_allowed == 0) { > + fprintf(stderr, "access denied by acl file\n"); > + return -EPERM; > + } > + > /* open a socket to use to control the network interfaces */ > ctlfd = socket(AF_INET, SOCK_STREAM, 0); > if (ctlfd == -1) { > -- > 1.6.2.5 > > > >
