On 03/01/2013 03:25 PM, Serge Hallyn wrote:
> This patch enables lxc-attach to join the profile of the container it
> is attaching to.  Builds/runs fine with apparmor enabled and disabled.
> 
> Export new aa_get_profile(), and use it for attach_apparmor, but also
> handle profile names longer than 100 chars in lxc_start apparmor
> support.
> 
> Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>

Looks good, test built with/without apparmor to confirm that we don't
get weird failures and it all looks good.

Acked-by: Stéphane Graber <stgra...@ubuntu.com>

> ---
>  src/lxc/apparmor.c   | 105 
> ++++++++++++++++++++++++++++++++++++++++-----------
>  src/lxc/apparmor.h   |  22 ++++++++++-
>  src/lxc/attach.c     |   2 +
>  src/lxc/attach.h     |   1 +
>  src/lxc/lxc_attach.c |   6 +++
>  5 files changed, 111 insertions(+), 25 deletions(-)
> 
> diff --git a/src/lxc/apparmor.c b/src/lxc/apparmor.c
> index ae3d8f7..c2d1d2f 100644
> --- a/src/lxc/apparmor.c
> +++ b/src/lxc/apparmor.c
> @@ -1,4 +1,5 @@
>  #include <stdio.h>
> +#include <stdlib.h>
>  #include <unistd.h>
>  #include <errno.h>
>  #include <sys/types.h>
> @@ -16,21 +17,58 @@ lxc_log_define(lxc_apparmor, lxc);
>  #define AA_MOUNT_RESTR "/sys/kernel/security/apparmor/features/mount/mask"
>  #define AA_ENABLED_FILE "/sys/module/apparmor/parameters/enabled"
>  
> -static int aa_am_unconfined(void)
> +
> +/* caller must free the returned profile */
> +extern char *aa_get_profile(pid_t pid)
>  {
> +     char path[100], *space;
>       int ret;
> -     char path[100], p[100];
> -     sprintf(path, "/proc/%d/attr/current", getpid());
> -     FILE *f = fopen(path, "r");
> -     if (!f)
> -             return 0;
> -     ret = fscanf(f, "%99s", p);
> +     char *buf = NULL;
> +     int sz = 0;
> +     FILE *f;
> +
> +     ret = snprintf(path, 100, "/proc/%d/attr/current", pid);
> +     if (ret < 0 || ret >= 100) {
> +             ERROR("path name too long");
> +             return NULL;
> +     }
> +again:
> +     f = fopen(path, "r");
> +     if (!f) {
> +             SYSERROR("opening %s\n", path);
> +             return NULL;
> +     }
> +     sz += 1024;
> +     buf = realloc(buf, sz);
> +     if (!buf) {
> +             ERROR("out of memory");
> +             fclose(f);
> +             return NULL;
> +     }
> +     ret = fread(buf, 1, sz, f);
>       fclose(f);
> -     if (ret < 1)
> -             return 0;
> -     if (strcmp(p, "unconfined") == 0)
> -             return 1;
> -     return 0;
> +     if (ret >= sz)
> +             goto again;
> +     if (ret < 0) {
> +             ERROR("reading %s\n", path);
> +             free(buf);
> +             return NULL;
> +     }
> +     space = index(buf, ' ');
> +     if (space)
> +             *space = '\0';
> +     return buf;
> +}
> +
> +static int aa_am_unconfined(void)
> +{
> +     char *p = aa_get_profile(getpid());
> +     int ret = 0;
> +     if (!p || strcmp(p, "unconfined") == 0)
> +             ret = 1;
> +     if (p)
> +             free(p);
> +     return ret;
>  }
>  
>  /* aa_getcon is not working right now.  Use our hand-rolled version below */
> @@ -61,36 +99,57 @@ extern void apparmor_handler_init(struct lxc_handler 
> *handler)
>  }
>  
>  #define AA_DEF_PROFILE "lxc-container-default"
> -extern int apparmor_load(struct lxc_handler *handler)
> +
> +extern int do_apparmor_load(int aa_enabled, char *aa_profile,
> +                                int umount_proc, int dropprivs)
>  {
> -     if (!handler->aa_enabled) {
> +     if (!aa_enabled) {
>               INFO("apparmor not enabled");
>               return 0;
>       }
>       INFO("setting up apparmor");
>  
> -     if (!handler->conf->aa_profile)
> -             handler->conf->aa_profile = AA_DEF_PROFILE;
> +     if (!aa_profile)
> +             aa_profile = AA_DEF_PROFILE;
>  
> -     if (strcmp(handler->conf->aa_profile, "unconfined") == 0 &&
> -         aa_am_unconfined()) {
> +     if (strcmp(aa_profile, "unconfined") == 0 && !dropprivs && 
> aa_am_unconfined()) {
>               INFO("apparmor profile unchanged");
>               return 0;
>       }
>  
> -     //if (aa_change_onexec(handler->conf->aa_profile) < 0) {
> -     if (aa_change_profile(handler->conf->aa_profile) < 0) {
> -             SYSERROR("failed to change apparmor profile to %s", 
> handler->conf->aa_profile);
> +     //if (aa_change_onexec(aa_profile) < 0) {
> +     if (aa_change_profile(aa_profile) < 0) {
> +             SYSERROR("failed to change apparmor profile to %s", aa_profile);
>               return -1;
>       }
> -     if (handler->conf->lsm_umount_proc == 1)
> +     if (umount_proc == 1)
>               umount("/proc");
>  
> -     INFO("changed apparmor profile to %s", handler->conf->aa_profile);
> +     INFO("changed apparmor profile to %s", aa_profile);
>  
>       return 0;
>  }
>  
> +extern int apparmor_load(struct lxc_handler *handler)
> +{
> +     if (!handler->conf->aa_profile)
> +             handler->conf->aa_profile = AA_DEF_PROFILE;
> +     return do_apparmor_load(handler->aa_enabled, 
> +                             handler->conf->aa_profile,
> +                             handler->conf->lsm_umount_proc, 0);
> +}
> +
> +extern int attach_apparmor(char *profile)
> +{
> +     if (!profile)
> +             return 0;
> +     if (!check_apparmor_enabled())
> +             return 0;
> +     if (strcmp(profile, "unconfined") == 0)
> +             return 0;
> +     return do_apparmor_load(1, profile, 0, 1);
> +}
> +
>  /*
>   * this will likely move to a generic lsm.c, as selinux and smack will both
>   * also want proc mounted in the container so as to transition
> diff --git a/src/lxc/apparmor.h b/src/lxc/apparmor.h
> index af879c0..d714478 100644
> --- a/src/lxc/apparmor.h
> +++ b/src/lxc/apparmor.h
> @@ -3,16 +3,34 @@
>  
>  struct lxc_handler;
>  
> +/*
> + * apparmor_handler_init is really just a wrapper around 
> check_apparmor_enabled
> + * to allow us to keep from having #ifdef APPARMOR in start.c
> + */
> +extern void apparmor_handler_init(struct lxc_handler *handler);
> +
>  #if HAVE_APPARMOR
> +extern char *aa_get_profile(pid_t pid);
> +extern int do_apparmor_load(int aa_enabled, char *aa_profile,
> +                                int umount_proc, int dropprivs);
>  extern int apparmor_load(struct lxc_handler *handler);
> +extern int attach_apparmor(char *profile);
>  extern int lsm_mount_proc_if_needed(char *root_src, char *rootfs_tgt);
> -extern void apparmor_handler_init(struct lxc_handler *handler);
>  #else
> +static inline char *aa_get_profile(pid_t pid) {
> +     return NULL;
> +}
> +static inline int do_apparmor_load(int aa_enabled, char *aa_profile,
> +                                int umount_proc, int dropprivs) {
> +     return 0;
> +}
> +static inline int attach_apparmor(char *profile) {
> +     return 0;
> +}
>  static inline int apparmor_load(struct lxc_handler *handler) {
>       return 0;
>  }
>  static inline int lsm_mount_proc_if_needed(char *root_src, char *rootfs_tgt) 
> {
>       return 0;
>  }
> -extern void apparmor_handler_init(struct lxc_handler *handler);
>  #endif
> diff --git a/src/lxc/attach.c b/src/lxc/attach.c
> index b86d2f4..af3d7a0 100644
> --- a/src/lxc/attach.c
> +++ b/src/lxc/attach.c
> @@ -44,6 +44,7 @@
>  #include "caps.h"
>  #include "cgroup.h"
>  #include "config.h"
> +#include "apparmor.h"
>  
>  lxc_log_define(lxc_attach, lxc);
>  
> @@ -137,6 +138,7 @@ struct lxc_proc_context_info 
> *lxc_proc_get_context_info(pid_t pid)
>               errno = ENOENT;
>               goto out_error;
>       }
> +     info->aa_profile = aa_get_profile(pid);
>  
>       return info;
>  
> diff --git a/src/lxc/attach.h b/src/lxc/attach.h
> index aab47e3..4d4f719 100644
> --- a/src/lxc/attach.h
> +++ b/src/lxc/attach.h
> @@ -27,6 +27,7 @@
>  #include <sys/types.h>
>  
>  struct lxc_proc_context_info {
> +     char *aa_profile;
>       unsigned long personality;
>       unsigned long long capability_mask;
>  };
> diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
> index 370bfbc..e1511ef 100644
> --- a/src/lxc/lxc_attach.c
> +++ b/src/lxc/lxc_attach.c
> @@ -41,6 +41,7 @@
>  #include "sync.h"
>  #include "log.h"
>  #include "namespace.h"
> +#include "apparmor.h"
>  
>  #if HAVE_SYS_PERSONALITY_H
>  #include <sys/personality.h>
> @@ -265,6 +266,11 @@ int main(int argc, char *argv[])
>               lxc_sync_fini_parent(handler);
>               lxc_cgroup_dispose_attach(cgroup_data);
>  
> +             if (attach_apparmor(init_ctx->aa_profile) < 0) {
> +                     ERROR("failed switching apparmor profiles");
> +                     return -1;
> +             }
> +
>               /* A description of the purpose of this functionality is
>                * provided in the lxc-attach(1) manual page. We have to
>                * remount here and not in the parent process, otherwise
> 


-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_feb
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to