[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-he
From: |
Krumme, Chris |
Subject: |
RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper |
Date: |
Wed, 4 Nov 2009 05:38:05 -0800 |
Hello Anthony,
Cool patch series.
> -----Original Message-----
> From:
> address@hidden
> [mailto:address@hidden
> rg] On Behalf Of Anthony Liguori
> Sent: Tuesday, November 03, 2009 6:28 PM
> To: address@hidden
> 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 <address@hidden>
> ---
> 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
>
>
>
>