[lxc-devel] [PATCH] apparmor: improve behavior when kernel lacks mount restrictions (v2)
Serge Hallyn
serge.hallyn at ubuntu.com
Sat Sep 20 03:47:38 UTC 2014
(Dwight, I took the liberty of adding your Ack but the code did
change a bit to continue passing the char *label from attach.
Tested that "lxc-start -n u1 -s lxc.aa_profile=p2; lxc-attach -n u1"
does attach you to the p2 profile)
Apparmor policies require mount restrictions to fullfill many of
their promises - for instance if proc can be mounted anywhere,
then 'deny /proc/sysrq-trigger w' prevents only accidents, not
malice.
The mount restrictions are not available in the upstream kernel.
We can detect their presence through /sys. In the past, when
we detected it missing, we would not enable apparmor. But that
prevents apparmor from helping to prevent accidents.
At the same time, if the user accidentaly boots a kernel which
has regressed, we do not want them starting the container thinking
they are more protected than they are.
This patch:
1. adds a lxc.aa_allow_incomplete = 1 container config flag. If
not set, then any container which is not set to run unconfined
will refuse to run. If set, then the container will run with
apparmor protection.
2. to pass this flag to the apparmor driver, we pass the container
configuration (lxc_conf) to the lsm_label_set hook.
3. add a testcase. To test the case were a kernel does not
provide mount restrictions, we mount an empty directory over
the /sys/kernel/security/apparmor/features/mount directory. In
order to have that not be unmounted in a new namespace, we must
test using unprivileged containers (who cannot remove bind mounts
which hide existing mount contents).
Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
Acked-by: Dwight Engen <dwight.engen at oracle.com>
---
src/lxc/attach.c | 3 +-
src/lxc/conf.h | 1 +
src/lxc/confile.c | 14 +++
src/lxc/lsm/apparmor.c | 35 +++++--
src/lxc/lsm/lsm.c | 5 +-
src/lxc/lsm/lsm.h | 10 +-
src/lxc/lsm/nop.c | 4 +-
src/lxc/lsm/selinux.c | 9 +-
src/lxc/start.c | 7 +-
src/tests/Makefile.am | 1 +
src/tests/lxc-test-apparmor-mount | 189 ++++++++++++++++++++++++++++++++++++++
11 files changed, 253 insertions(+), 25 deletions(-)
create mode 100755 src/tests/lxc-test-apparmor-mount
diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 923c497..7cadcbd 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -1043,7 +1043,8 @@ static int attach_child_main(void* data)
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);
+ ret = lsm_process_label_set(init_ctx->lsm_label,
+ init_ctx->container->lxc_conf, 0, on_exec);
if (ret < 0) {
rexit(-1);
}
diff --git a/src/lxc/conf.h b/src/lxc/conf.h
index df66f99..97d9f91 100644
--- a/src/lxc/conf.h
+++ b/src/lxc/conf.h
@@ -311,6 +311,7 @@ struct lxc_conf {
struct lxc_list hooks[NUM_LXC_HOOKS];
char *lsm_aa_profile;
+ int lsm_aa_allow_incomplete;
char *lsm_se_context;
int tmp_umount_proc;
char *seccomp; // filename with the seccomp rules
diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 5de1241..af55feb 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -58,6 +58,7 @@ static int config_tty(const char *, const char *, struct lxc_conf *);
static int config_ttydir(const char *, const char *, struct lxc_conf *);
static int config_kmsg(const char *, const char *, struct lxc_conf *);
static int config_lsm_aa_profile(const char *, const char *, struct lxc_conf *);
+static int config_lsm_aa_incomplete(const char *, const char *, struct lxc_conf *);
static int config_lsm_se_context(const char *, const char *, struct lxc_conf *);
static int config_cgroup(const char *, const char *, struct lxc_conf *);
static int config_idmap(const char *, const char *, struct lxc_conf *);
@@ -108,6 +109,7 @@ static struct lxc_config_t config[] = {
{ "lxc.devttydir", config_ttydir },
{ "lxc.kmsg", config_kmsg },
{ "lxc.aa_profile", config_lsm_aa_profile },
+ { "lxc.aa_allow_incomplete", config_lsm_aa_incomplete },
{ "lxc.se_context", config_lsm_se_context },
{ "lxc.cgroup", config_cgroup },
{ "lxc.id_map", config_idmap },
@@ -1145,6 +1147,16 @@ static int config_lsm_aa_profile(const char *key, const char *value,
return config_string_item(&lxc_conf->lsm_aa_profile, value);
}
+static int config_lsm_aa_incomplete(const char *key, const char *value,
+ struct lxc_conf *lxc_conf)
+{
+ int v = atoi(value);
+
+ lxc_conf->lsm_aa_allow_incomplete = v == 1 ? 1 : 0;
+
+ return 0;
+}
+
static int config_lsm_se_context(const char *key, const char *value,
struct lxc_conf *lxc_conf)
{
@@ -2257,6 +2269,8 @@ int lxc_get_config_item(struct lxc_conf *c, const char *key, char *retv,
return lxc_get_arch_entry(c, retv, inlen);
else if (strcmp(key, "lxc.aa_profile") == 0)
v = c->lsm_aa_profile;
+ else if (strcmp(key, "lxc.aa_allow_incomplete") == 0)
+ return lxc_get_conf_int(c, retv, inlen, c->lsm_aa_allow_incomplete);
else if (strcmp(key, "lxc.se_context") == 0)
v = c->lsm_se_context;
else if (strcmp(key, "lxc.logfile") == 0)
diff --git a/src/lxc/lsm/apparmor.c b/src/lxc/lsm/apparmor.c
index f4c8d26..8de0115 100644
--- a/src/lxc/lsm/apparmor.c
+++ b/src/lxc/lsm/apparmor.c
@@ -29,6 +29,7 @@
#include "log.h"
#include "lsm/lsm.h"
+#include "conf.h"
lxc_log_define(lxc_apparmor, lxc);
@@ -39,17 +40,23 @@ static int aa_enabled = 0;
#define AA_MOUNT_RESTR "/sys/kernel/security/apparmor/features/mount/mask"
#define AA_ENABLED_FILE "/sys/module/apparmor/parameters/enabled"
+static int mount_feature_enabled(void)
+{
+ struct stat statbuf;
+ int ret;
+ ret = stat(AA_MOUNT_RESTR, &statbuf);
+ if (ret != 0)
+ return 0;
+ return 1;
+}
+
/* aa_getcon is not working right now. Use our hand-rolled version below */
static int apparmor_enabled(void)
{
- struct stat statbuf;
FILE *fin;
char e;
int ret;
- ret = stat(AA_MOUNT_RESTR, &statbuf);
- if (ret != 0)
- return 0;
fin = fopen(AA_ENABLED_FILE, "r");
if (!fin)
return 0;
@@ -124,6 +131,7 @@ static int apparmor_am_unconfined(void)
* apparmor_process_label_set: Set AppArmor process profile
*
* @label : the profile to set
+ * @conf : the container configuration to use @label is NULL
* @default : use the default profile if label is NULL
* @on_exec : this is ignored. Apparmor profile will be changed immediately
*
@@ -131,9 +139,11 @@ static int apparmor_am_unconfined(void)
*
* Notes: This relies on /proc being available.
*/
-static int apparmor_process_label_set(const char *label, int use_default,
- int on_exec)
+static int apparmor_process_label_set(const char *inlabel, struct lxc_conf *conf,
+ int use_default, int on_exec)
{
+ const char *label = inlabel ? inlabel : conf->lsm_aa_profile;
+
if (!aa_enabled)
return 0;
@@ -141,9 +151,20 @@ static int apparmor_process_label_set(const char *label, int use_default,
if (use_default)
label = AA_DEF_PROFILE;
else
- return 0;
+ label = "unconfined";
+ }
+
+ if (!mount_feature_enabled() && strcmp(label, "unconfined") != 0) {
+ WARN("Incomplete AppArmor support in your kernel");
+ if (!conf->lsm_aa_allow_incomplete) {
+ ERROR("If you really want to start this container, set");
+ ERROR("lxc.aa_allow_incomplete = 1");
+ ERROR("in your container configuration file");
+ return -1;
+ }
}
+
if (strcmp(label, "unconfined") == 0 && apparmor_am_unconfined()) {
INFO("apparmor profile unchanged");
return 0;
diff --git a/src/lxc/lsm/lsm.c b/src/lxc/lsm/lsm.c
index c994046..79f837f 100644
--- a/src/lxc/lsm/lsm.c
+++ b/src/lxc/lsm/lsm.c
@@ -85,13 +85,14 @@ 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 on_exec)
+int lsm_process_label_set(const char *label, struct lxc_conf *conf,
+ int use_default, int on_exec)
{
if (!drv) {
ERROR("LSM driver not inited");
return -1;
}
- return drv->process_label_set(label, use_default, on_exec);
+ return drv->process_label_set(label, conf, use_default, on_exec);
}
#endif
diff --git a/src/lxc/lsm/lsm.h b/src/lxc/lsm/lsm.h
index 93f5b87..84e992d 100644
--- a/src/lxc/lsm/lsm.h
+++ b/src/lxc/lsm/lsm.h
@@ -33,8 +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 on_exec);
+ int (*process_label_set)(const char *label, struct lxc_conf *conf,
+ int use_default, int on_exec);
};
#if HAVE_APPARMOR || HAVE_SELINUX
@@ -42,13 +42,15 @@ 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 on_exec);
+int lsm_process_label_set(const char *label, struct lxc_conf *conf,
+ int use_default, int on_exec);
#else
static inline void lsm_init(void) { }
static inline int lsm_enabled(void) { return 0; }
static inline const char *lsm_name(void) { return "none"; }
static inline char *lsm_process_label_get(pid_t pid) { return NULL; }
-static inline int lsm_process_label_set(const char *label, int use_default, int on_exec) { return 0; }
+static inline int lsm_process_label_set(const char *label,
+ struct lxc-conf *conf, int use_default, int on_exec) { return 0; }
#endif
#endif
diff --git a/src/lxc/lsm/nop.c b/src/lxc/lsm/nop.c
index e5db124..c13d8f5 100644
--- a/src/lxc/lsm/nop.c
+++ b/src/lxc/lsm/nop.c
@@ -29,8 +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,
- int on_exec)
+static int nop_process_label_set(const char *label, struct lxc_conf *conf,
+ int use_default, int on_exec)
{
return 0;
}
diff --git a/src/lxc/lsm/selinux.c b/src/lxc/lsm/selinux.c
index f8dad95..46554d8 100644
--- a/src/lxc/lsm/selinux.c
+++ b/src/lxc/lsm/selinux.c
@@ -29,6 +29,7 @@
#include "log.h"
#include "lsm/lsm.h"
+#include "conf.h"
#define DEFAULT_LABEL "unconfined_t"
@@ -61,7 +62,8 @@ static char *selinux_process_label_get(pid_t pid)
/*
* selinux_process_label_set: Set SELinux context of a process
*
- * @label : the context to set
+ * @label : label string
+ * @conf : the container configuration to use @label is NULL
* @default : use the default context if label is NULL
* @on_exec : the new context will take effect on exec(2) not immediately
*
@@ -69,9 +71,10 @@ static char *selinux_process_label_get(pid_t pid)
*
* Notes: This relies on /proc being available.
*/
-static int selinux_process_label_set(const char *label, int use_default,
- int on_exec)
+static int selinux_process_label_set(const char *inlabel, struct lxc_conf *conf,
+ int use_default, int on_exec)
{
+ const char *label = inlabel ? inlabel : conf->lsm_se_context;
if (!label) {
if (use_default)
label = DEFAULT_LABEL;
diff --git a/src/lxc/start.c b/src/lxc/start.c
index d9fbc8c..d7d6708 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -615,7 +615,6 @@ static int do_start(void *data)
{
struct lxc_list *iterator;
struct lxc_handler *handler = data;
- const char *lsm_label = NULL;
if (sigprocmask(SIG_SETMASK, &handler->oldmask, NULL)) {
SYSERROR("failed to set sigprocmask");
@@ -695,11 +694,7 @@ static int do_start(void *data)
return -1;
/* Set the label to change to when we exec(2) the container's init */
- if (!strcmp(lsm_name(), "AppArmor"))
- lsm_label = handler->conf->lsm_aa_profile;
- else if (!strcmp(lsm_name(), "SELinux"))
- lsm_label = handler->conf->lsm_se_context;
- if (lsm_process_label_set(lsm_label, 1, 1) < 0)
+ if (lsm_process_label_set(NULL, handler->conf, 1, 1) < 0)
goto out_warn_father;
/* Some init's such as busybox will set sane tty settings on stdin,
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index 35bc1b3..e0969eb 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -52,6 +52,7 @@ bin_SCRIPTS = lxc-test-autostart
if DISTRO_UBUNTU
bin_SCRIPTS += \
+ lxc-test-apparmor-mount \
lxc-test-checkpoint-restore \
lxc-test-ubuntu \
lxc-test-unpriv \
diff --git a/src/tests/lxc-test-apparmor-mount b/src/tests/lxc-test-apparmor-mount
new file mode 100755
index 0000000..503381e
--- /dev/null
+++ b/src/tests/lxc-test-apparmor-mount
@@ -0,0 +1,189 @@
+#!/bin/sh
+
+# apparmor_mount: test proper handling of apparmor in kernels
+# without mount features
+
+# These require the ubuntu lxc package to be installed.
+
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# This test assumes an Ubuntu host
+
+set -e
+
+FAIL() {
+ echo -n "Failed " >&2
+ echo "$*" >&2
+ exit 1
+}
+
+run_cmd() {
+ sudo -i -u $TUSER \
+ env http_proxy=${http_proxy:-} https_proxy=${https_proxy:-} \
+ XDG_RUNTIME_DIR=/run/user/$(id -u $TUSER) $*
+}
+
+DONE=0
+MOUNTSR=/sys/kernel/security/apparmor/features/mount
+dnam=`mktemp -d`
+cname=`basename $dnam`
+cleanup() {
+ run_cmd lxc-destroy -f -n $cname || true
+ umount -l $MOUNTSR || true
+ rmdir $dnam || true
+ pkill -u $(id -u $TUSER) -9
+ sed -i '/lxcunpriv/d' /run/lxc/nics /etc/lxc/lxc-usernet
+ sed -i '/^lxcunpriv:/d' /etc/subuid /etc/subgid
+ rm -Rf $HDIR /run/user/$(id -u $TUSER)
+ deluser $TUSER
+ if [ $DONE -eq 0 ]; then
+ echo "FAIL"
+ exit 1
+ fi
+ echo "PASS"
+}
+
+trap cleanup exit
+
+# Only run on a normally configured ubuntu lxc system
+if [ ! -d /sys/class/net/lxcbr0 ]; then
+ echo "lxcbr0 is not configured."
+ exit 1
+fi
+if [ "$(id -u)" != "0" ]; then
+ echo "ERROR: Must run as root."
+ exit 1
+fi
+
+# This would be much simpler if we could run it as
+# root. However, in order to not have the bind mount
+# of an empty directory over the securitfs 'mount' directory
+# be removed, we need to do this as non-root.
+
+which newuidmap >/dev/null 2>&1 || { echo "'newuidmap' command is missing" >&2; exit 1; }
+# create a test user
+TUSER=lxcunpriv
+HDIR=/home/$TUSER
+
+ARCH=i386
+if type dpkg >/dev/null 2>&1; then
+ ARCH=$(dpkg --print-architecture)
+fi
+
+deluser $TUSER && rm -Rf $HDIR || true
+useradd $TUSER
+
+mkdir -p $HDIR
+echo "$TUSER veth lxcbr0 2" > /etc/lxc/lxc-usernet
+sed -i '/^lxcunpriv:/d' /etc/subuid /etc/subgid
+
+usermod -v 910000-919999 -w 910000-919999 $TUSER
+
+mkdir -p $HDIR/.config/lxc/
+cat > $HDIR/.config/lxc/default.conf << EOF
+lxc.network.type = veth
+lxc.network.link = lxcbr0
+lxc.id_map = u 0 910000 9999
+lxc.id_map = g 0 910000 9999
+EOF
+chown -R $TUSER: $HDIR
+
+mkdir -p /run/user/$(id -u $TUSER)
+chown -R $TUSER: /run/user/$(id -u $TUSER)
+
+cd $HDIR
+
+cgm create all $TUSER
+cgm chown all $TUSER $(id -u $TUSER) $(id -g $TUSER)
+cgm movepid all $TUSER $$
+
+run_cmd mkdir -p $HDIR/.cache/lxc
+[ -d /var/cache/lxc/download ] && \
+ cp -R /var/cache/lxc/download $HDIR/.cache/lxc && \
+ chown -R $TUSER: $HDIR/.cache/lxc
+
+run_cmd lxc-create -t download -n $cname -- -d ubuntu -r trusty -a $ARCH
+
+echo "test default confined container"
+run_cmd lxc-start -n $cname -d
+run_cmd lxc-wait -n $cname -s RUNNING
+pid=`run_cmd lxc-info -p -H -n $cname`
+profile=`cat /proc/$pid/attr/current`
+if [ "x$profile" != "xlxc-container-default (enforce)" ]; then
+ echo "FAIL: confined container was in profile $profile"
+ exit 1
+fi
+run_cmd lxc-stop -n $cname
+
+echo "test regular unconfined container"
+echo "lxc.aa_profile = unconfined" >> $HDIR/.local/share/lxc/$cname/config
+run_cmd lxc-start -n $cname -d
+run_cmd lxc-wait -n $cname -s RUNNING
+pid=`run_cmd lxc-info -p -H -n $cname`
+profile=`cat /proc/$pid/attr/current`
+if [ "x$profile" != "xunconfined" ]; then
+ echo "FAIL: unconfined container was in profile $profile"
+ exit 1
+fi
+run_cmd lxc-stop -n $cname
+
+echo "masking $MOUNTSR"
+mount --bind $dnam $MOUNTSR
+
+echo "test default confined container"
+sed -i '/aa_profile/d' $HDIR/.local/share/lxc/$cname/config
+run_cmd lxc-start -n $cname -d || true
+sleep 3
+pid=`run_cmd lxc-info -p -H -n $cname` || true
+if [ -n "$pid" -a "$pid" != "-1" ]; then
+ echo "FAIL: confined container started without mount restrictions"
+ echo "pid was $pid"
+ exit 1
+fi
+
+echo "test regular unconfined container"
+echo "lxc.aa_profile = unconfined" >> $HDIR/.local/share/lxc/$cname/config
+run_cmd lxc-start -n $cname -d
+run_cmd lxc-wait -n $cname -s RUNNING
+pid=`run_cmd lxc-info -p -H -n $cname`
+if [ "$pid" = "-1" ]; then
+ echo "FAIL: unconfined container failed to start without mount restrictions"
+ exit 1
+fi
+profile=`cat /proc/$pid/attr/current`
+if [ "x$profile" != "xunconfined" ]; then
+ echo "FAIL: confined container was in profile $profile"
+ exit 1
+fi
+run_cmd lxc-stop -n $cname
+
+echo "testing override"
+sed -i '/aa_profile/d' $HDIR/.local/share/lxc/$cname/config
+echo "lxc.aa_allow_incomplete = 1" >> $HDIR/.local/share/lxc/$cname/config
+run_cmd lxc-start -n $cname -d
+run_cmd lxc-wait -n $cname -s RUNNING
+pid=`run_cmd lxc-info -p -H -n $cname`
+if [ "$pid" = "-1" ]; then
+ echo "FAIL: excepted container failed to start without mount restrictions"
+ exit 1
+fi
+profile=`cat /proc/$pid/attr/current`
+if [ "x$profile" != "xlxc-container-default (enforce)" ]; then
+ echo "FAIL: confined container was in profile $profile"
+ exit 1
+fi
+run_cmd lxc-stop -n $cname
+
+DONE=1
--
2.1.0
More information about the lxc-devel
mailing list