[lxc-devel] [PATCH v2 2/2] support setting lsm label at exec or immediately
Dwight Engen
dwight.engen at oracle.com
Fri Oct 18 18:32:12 UTC 2013
On Fri, 18 Oct 2013 11:18:17 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> Quoting Dwight Engen (dwight.engen at oracle.com):
> > - Add attach test cases
> >
> > - Moved setting of LSM label later to avoid failure of IPC between
> > parent and child during attach
> >
> > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> > ---
> > v2: detect which lsm to test at runtime vs. compile time
> >
> > .gitignore | 1 +
> > src/lxc/attach.c | 20 ++-
> > src/lxc/attach_options.h | 5 +-
> > src/lxc/lsm/apparmor.c | 25 +--
> > src/lxc/lsm/lsm.c | 4 +-
> > src/lxc/lsm/lsm.h | 7 +-
> > src/lxc/lsm/nop.c | 3 +-
> > src/lxc/lsm/selinux.c | 22 ++-
> > src/lxc/lxc_attach.c | 2 +-
> > src/lxc/start.c | 8 +-
> > src/python-lxc/lxc.c | 3 +-
> > src/python-lxc/lxc/__init__.py | 3 +-
> > src/tests/Makefile.am | 11 +-
> > src/tests/attach.c | 392
> > +++++++++++++++++++++++++++++++++++++++++ 14 files changed, 463
> > insertions(+), 43 deletions(-) create mode 100644 src/tests/attach.c
> >
> > diff --git a/.gitignore b/.gitignore
> > index df8d5e1..b1223cd 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -77,6 +77,7 @@ src/lxc/lxc-user-nic
> > src/python-lxc/build/
> > src/python-lxc/lxc/__pycache__/
> >
> > +src/tests/lxc-test-attach
> > src/tests/lxc-test-cgpath
> > src/tests/lxc-test-clonetest
> > src/tests/lxc-test-concurrent
> > diff --git a/src/lxc/attach.c b/src/lxc/attach.c
> > index 37cefb0..aea0c33 100644
> > --- a/src/lxc/attach.c
> > +++ b/src/lxc/attach.c
> > @@ -918,15 +918,6 @@ int attach_child_main(void* data)
> > rexit(-1);
> > }
> >
> > - /* load apparmor profile */
> > - if ((options->namespaces & CLONE_NEWNS) &&
> > (options->attach_flags & LXC_ATTACH_APPARMOR)) {
> > - ret = lsm_process_label_set(init_ctx->lsm_label,
> > 0);
> > - if (ret < 0) {
> > - shutdown(ipc_socket, SHUT_RDWR);
> > - rexit(-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
> > @@ -1023,6 +1014,17 @@ int attach_child_main(void* data)
> >
> > shutdown(ipc_socket, SHUT_RDWR);
> > close(ipc_socket);
> > +
> > + /* set new apparmor profile/selinux context */
> > + if ((options->namespaces & CLONE_NEWNS) &&
> > (options->attach_flags & LXC_ATTACH_LSM)) {
> > + int on_exec;
> > +
> > + on_exec = options->attach_flags &
> > LXC_ATTACH_LSM_EXEC ? 1 : 0;
> > + ret = lsm_process_label_set(init_ctx->lsm_label,
> > 0, on_exec);
> > + if (ret < 0) {
> > + rexit(-1);
> > + }
> > + }
> > lxc_proc_put_context_info(init_ctx);
> >
> > /* The following is done after the communication socket is
> > diff --git a/src/lxc/attach_options.h b/src/lxc/attach_options.h
> > index 5291e4f..c8c4d0a 100644
> > --- a/src/lxc/attach_options.h
> > +++ b/src/lxc/attach_options.h
> > @@ -36,10 +36,11 @@ enum {
> > LXC_ATTACH_MOVE_TO_CGROUP = 0x00000001,
> > LXC_ATTACH_DROP_CAPABILITIES = 0x00000002,
> > LXC_ATTACH_SET_PERSONALITY = 0x00000004,
> > - LXC_ATTACH_APPARMOR = 0x00000008,
> > + LXC_ATTACH_LSM_EXEC = 0x00000008,
> >
> > /* the following are off by default */
> > LXC_ATTACH_REMOUNT_PROC_SYS = 0x00010000,
> > + LXC_ATTACH_LSM_NOW = 0x00020000,
> >
> > /* we have 16 bits for things that are on by default
> > * and 16 bits that are off by default, that should
> > @@ -49,6 +50,8 @@ enum {
> > LXC_ATTACH_DEFAULT = 0x0000FFFF
> > };
> >
> > +#define LXC_ATTACH_LSM (LXC_ATTACH_LSM_EXEC | LXC_ATTACH_LSM_NOW)
> > +
> > typedef struct lxc_attach_options_t lxc_attach_options_t;
> > typedef int (*lxc_attach_exec_t)(void* payload);
> >
> > diff --git a/src/lxc/lsm/apparmor.c b/src/lxc/lsm/apparmor.c
> > index 146564f..cf8020d 100644
> > --- a/src/lxc/lsm/apparmor.c
> > +++ b/src/lxc/lsm/apparmor.c
> > @@ -130,13 +130,14 @@ static int apparmor_am_unconfined(void)
> > *
> > * @label : the profile to set
> > * @default : use the default profile if label is NULL
> > + * @on_exec : the new profile will take effect on exec(2) not
> > immediately *
> > * Returns 0 on success, < 0 on failure
> > *
> > - * Notes: This relies on /proc being available. The new context
> > - * will take effect immediately.
> > + * Notes: This relies on /proc being available.
> > */
> > -static int apparmor_process_label_set(const char *label, int
> > use_default) +static int apparmor_process_label_set(const char
> > *label, int use_default,
> > + int on_exec)
> > {
> > if (!apparmor_enabled())
> > return 0;
> > @@ -153,15 +154,19 @@ static int apparmor_process_label_set(const
> > char *label, int use_default) return 0;
> > }
> >
> > - /* XXX: instant instead of aa_change_onexec(), may be used
> > by attach
> > - * when using a function that doesn't exec
> > - */
> > - if (aa_change_profile(label) < 0) {
> > - SYSERROR("failed to change apparmor profile to
> > %s", label);
> > - return -1;
> > + if (on_exec) {
> > + if (aa_change_onexec(label) < 0) {
> > + SYSERROR("failed to change exec apparmor
> > profile to %s", label);
> > + return -1;
> > + }
> > + } else {
> > + if (aa_change_profile(label) < 0) {
> > + SYSERROR("failed to change apparmor
> > profile to %s", label);
> > + return -1;
> > + }
> > }
> >
> > - INFO("changed apparmor profile to %s", label);
> > + INFO("changed apparmor%s profile to %s", on_exec ? "
> > exec" : "", label); return 0;
> > }
> >
> > diff --git a/src/lxc/lsm/lsm.c b/src/lxc/lsm/lsm.c
> > index 508d640..9273101 100644
> > --- a/src/lxc/lsm/lsm.c
> > +++ b/src/lxc/lsm/lsm.c
> > @@ -85,13 +85,13 @@ char *lsm_process_label_get(pid_t pid)
> > return drv->process_label_get(pid);
> > }
> >
> > -int lsm_process_label_set(const char *label, int use_default)
> > +int lsm_process_label_set(const char *label, int use_default, int
> > on_exec) {
> > if (!drv) {
> > ERROR("LSM driver not inited");
> > return -1;
> > }
> > - return drv->process_label_set(label, use_default);
> > + return drv->process_label_set(label, use_default, on_exec);
> > }
> >
> > /*
> > diff --git a/src/lxc/lsm/lsm.h b/src/lxc/lsm/lsm.h
> > index 5a6cf15..4128575 100644
> > --- a/src/lxc/lsm/lsm.h
> > +++ b/src/lxc/lsm/lsm.h
> > @@ -33,7 +33,8 @@ struct lsm_drv {
> >
> > int (*enabled)(void);
> > char *(*process_label_get)(pid_t pid);
> > - int (*process_label_set)(const char *label, int
> > use_default);
> > + int (*process_label_set)(const char *label, int
> > use_default,
> > + int on_exec);
> > };
> >
> > #if HAVE_APPARMOR || HAVE_SELINUX
> > @@ -41,7 +42,7 @@ void lsm_init(void);
> > int lsm_enabled(void);
> > const char *lsm_name(void);
> > char *lsm_process_label_get(pid_t pid);
> > -int lsm_process_label_set(const char *label, int
> > use_default); +int lsm_process_label_set(const char *label,
> > int use_default, int on_exec); int lsm_proc_mount(struct
> > lxc_conf *lxc_conf); void lsm_proc_unmount(struct lxc_conf
> > *lxc_conf); #else
> > @@ -49,7 +50,7 @@ static inline void lsm_init(void) { }
> > static inline int lsm_enabled(void) { return 0; }
> > static inline const char *lsm_name(void) { return NULL; }
> > static inline char *lsm_process_label_get(pid_t pid)
> > { return NULL; } -static inline int
> > lsm_process_label_set(char *label, int use_default) { return 0; }
> > +static inline int lsm_process_label_set(char *label, int
> > use_default, int on_exec) { return 0; } static inline int
> > lsm_proc_mount(struct lxc_conf *lxc_conf) { return 0; } static
> > inline void lsm_proc_unmount(struct lxc_conf *lxc_conf) { }
> > #endif diff --git a/src/lxc/lsm/nop.c b/src/lxc/lsm/nop.c index
> > e39b0f5..e5db124 100644 --- a/src/lxc/lsm/nop.c
> > +++ b/src/lxc/lsm/nop.c
> > @@ -29,7 +29,8 @@ static char *nop_process_label_get(pid_t pid)
> > return NULL;
> > }
> >
> > -static int nop_process_label_set(const char *label, int
> > use_default) +static int nop_process_label_set(const char *label,
> > int use_default,
> > + int on_exec)
> > {
> > return 0;
> > }
> > diff --git a/src/lxc/lsm/selinux.c b/src/lxc/lsm/selinux.c
> > index ef5beb0..b1b0253 100644
> > --- a/src/lxc/lsm/selinux.c
> > +++ b/src/lxc/lsm/selinux.c
> > @@ -61,13 +61,14 @@ static char *selinux_process_label_get(pid_t
> > pid) *
> > * @label : the context to set
> > * @default : use the default context if label is NULL
> > + * @on_exec : the new context will take effect on exec(2) not
> > immediately *
> > * Returns 0 on success, < 0 on failure
> > *
> > - * Notes: This relies on /proc being available. The new context
> > - * will take effect on the next exec(2).
> > + * Notes: This relies on /proc being available.
> > */
> > -static int selinux_process_label_set(const char *label, int
> > use_default) +static int selinux_process_label_set(const char
> > *label, int use_default,
> > + int on_exec)
> > {
> > if (!label) {
> > if (use_default)
> > @@ -78,12 +79,19 @@ static int selinux_process_label_set(const char
> > *label, int use_default) if (!strcmp(label, "unconfined_t"))
> > return 0;
> >
> > - if (setexeccon_raw((char *)label) < 0) {
> > - SYSERROR("failed to set new SELinux context %s",
> > label);
> > - return -1;
> > + if (on_exec) {
> > + if (setexeccon_raw((char *)label) < 0) {
> > + SYSERROR("failed to set new SELinux exec
> > context %s", label);
> > + return -1;
> > + }
> > + } else {
> > + if (setcon_raw((char *)label) < 0) {
> > + SYSERROR("failed to set new SELinux
> > context %s", label);
> > + return -1;
> > + }
> > }
> >
> > - INFO("changed SELinux context to %s", label);
> > + INFO("changed SELinux%s context to %s", on_exec ? "
> > exec" : "", label); return 0;
> > }
> >
> > diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
> > index bd4e674..142d605 100644
> > --- a/src/lxc/lxc_attach.c
> > +++ b/src/lxc/lxc_attach.c
> > @@ -199,7 +199,7 @@ int main(int argc, char *argv[])
> > if (remount_sys_proc)
> > attach_options.attach_flags |=
> > LXC_ATTACH_REMOUNT_PROC_SYS; if (elevated_privileges)
> > - attach_options.attach_flags &=
> > ~(LXC_ATTACH_MOVE_TO_CGROUP | LXC_ATTACH_DROP_CAPABILITIES |
> > LXC_ATTACH_APPARMOR);
> > + attach_options.attach_flags &=
> > ~(LXC_ATTACH_MOVE_TO_CGROUP | LXC_ATTACH_DROP_CAPABILITIES |
> > LXC_ATTACH_LSM_EXEC);
>
> Should LXC_ATTACH_LSM_NOW also be added here? (Or just
> LXC_ATTACH_LSM?)
No I don't think so, this preserves the previous behavior (changing
label at exec(2) time), which is all the lxc-attach command can do
anyway (ie. it can't run a function).
More information about the lxc-devel
mailing list