apparmor: improve behavior when kernel lacks mount restrictions (v2)

(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@ubuntu.com>
Acked-by: Dwight Engen <dwight.engen@oracle.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

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);
 		}

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

src/lxc/confile.c

@@ -59,6 +59,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 *);
@@ -109,6 +110,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                },
@@ -1146,6 +1148,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)
 {
@@ -2259,6 +2271,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)

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;

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

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

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;
 }

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;

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,

src/tests/Makefile.am

@@ -52,6 +52,7 @@ bin_SCRIPTS = lxc-test-autostart lxc-test-cloneconfig
 
 if DISTRO_UBUNTU
 bin_SCRIPTS += \
+	lxc-test-apparmor-mount \
 	lxc-test-checkpoint-restore \
 	lxc-test-ubuntu \
 	lxc-test-unpriv \
@@ -75,6 +76,7 @@ EXTRA_DIST = \
 	locktests.c \
 	lxcpath.c \
 	lxc-test-autostart \
+	lxc-test-apparmor-mount \
 	lxc-test-checkpoint-restore \
 	lxc-test-cloneconfig \
 	lxc-test-ubuntu \

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