qemu-devel
[Top][All Lists]
Advanced

[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
> 
> 
> 
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]