[lxc-devel] [PATCH RFC] apparmor: auto-generate the blacklist rules
Stéphane Graber
stgraber at ubuntu.com
Fri Mar 7 23:01:54 UTC 2014
On Wed, Mar 05, 2014 at 12:54:23PM -0600, Serge Hallyn wrote:
> This uses the generate-apparmor-rules.py script I sent out some time
> ago to auto-generate apparmor rules based on a higher level set of
> block/allow rules.
>
> Add apparmor policy testcase to make sure that some of the paths we
> expect to be denied (and allowed) write access to are in fact in
> effect in the final policy.
>
> With this policy, libvirt in a container is able to start its
> default network, which previously it could not.
Thanks for this change, it'll make our lifes much easier. Quite a few
comments below though :)
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
> config/apparmor/Makefile.am | 6 +-
> config/apparmor/abstractions/container-base | 51 -------
> config/apparmor/abstractions/container-base.in | 44 ++++++
> config/apparmor/container-rules.base | 6 +
> config/apparmor/lxc-generate-aa-rules.py | 124 ++++++++++++++++
> src/tests/Makefile.am | 5 +-
> src/tests/aa.c | 190 +++++++++++++++++++++++++
> 7 files changed, 373 insertions(+), 53 deletions(-)
> delete mode 100644 config/apparmor/abstractions/container-base
> create mode 100644 config/apparmor/abstractions/container-base.in
> create mode 100644 config/apparmor/container-rules.base
> create mode 100755 config/apparmor/lxc-generate-aa-rules.py
> create mode 100644 src/tests/aa.c
>
> diff --git a/config/apparmor/Makefile.am b/config/apparmor/Makefile.am
> index 801866d..a917fd7 100644
> --- a/config/apparmor/Makefile.am
> +++ b/config/apparmor/Makefile.am
> @@ -7,8 +7,12 @@ EXTRA_DIST = \
> lxc-containers \
> usr.bin.lxc-start
>
> +apparmor: container-rules.base
> + ./lxc-generate-aa-rules.py container-rules.base > container-rules
The script should be added to extra_DIST I think.
> +
> if ENABLE_APPARMOR
> -install-apparmor:
> +install-apparmor: apparmor
> + cat abstractions/container-base.in container-rules >> abstractions/container-base
> $(MKDIR_P) $(DESTDIR)$(sysconfdir)/apparmor.d/
> $(MKDIR_P) $(DESTDIR)$(sysconfdir)/apparmor.d/abstractions/lxc/
> $(MKDIR_P) $(DESTDIR)$(sysconfdir)/apparmor.d/lxc/
This change should be conditional on python3 being present on the system.
container-base and container-rules also should be added to gitignore.
> diff --git a/config/apparmor/abstractions/container-base b/config/apparmor/abstractions/container-base
> deleted file mode 100644
> index d1cd84a..0000000
> --- a/config/apparmor/abstractions/container-base
> +++ /dev/null
> @@ -1,51 +0,0 @@
> - network,
> - capability,
> - file,
> - umount,
> - dbus,
> -
> - # ignore DENIED message on / remount
> - deny mount options=(ro, remount) -> /,
> -
> - # allow tmpfs mounts everywhere
> - mount fstype=tmpfs,
> -
> - # allow mqueue mounts everywhere
> - mount fstype=mqueue,
> -
> - # allow fuse mounts everywhere
> - mount fstype=fuse.*,
> -
> - # allow bind mount of /lib/init/fstab for lxcguest
> - mount options=(rw, bind) /lib/init/fstab.lxc/ -> /lib/init/fstab/,
> -
> - # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted
> - mount fstype=binfmt_misc -> /proc/sys/fs/binfmt_misc/,
> - deny @{PROC}/sys/fs/** wklx,
> -
> - # allow efivars to be mounted, writing to it will be blocked though
> - mount fstype=efivarfs -> /sys/firmware/efi/efivars/,
> -
> - # block some other dangerous paths
> - deny @{PROC}/sysrq-trigger rwklx,
> - deny @{PROC}/mem rwklx,
> - deny @{PROC}/kmem rwklx,
> - deny @{PROC}/sys/kernel/[^s][^h][^m]* wklx,
> - deny @{PROC}/sys/kernel/*/** wklx,
> -
> - # deny writes in /sys except for /sys/fs/cgroup, also allow
> - # fusectl, securityfs and debugfs to be mounted there (read-only)
> - mount fstype=fusectl -> /sys/fs/fuse/connections/,
> - mount fstype=securityfs -> /sys/kernel/security/,
> - mount fstype=debugfs -> /sys/kernel/debug/,
> - deny mount fstype=debugfs -> /var/lib/ureadahead/debugfs/,
> - mount fstype=proc -> /proc/,
> - mount fstype=sysfs -> /sys/,
> - deny /sys/[^f]*/** wklx,
> - deny /sys/f[^s]*/** wklx,
> - deny /sys/fs/[^c]*/** wklx,
> - deny /sys/fs/c[^g]*/** wklx,
> - deny /sys/fs/cg[^r]*/** wklx,
> - deny /sys/firmware/efi/efivars/** rwklx,
> - deny /sys/kernel/security/** rwklx,
> - mount options=(move) /sys/fs/cgroup/cgmanager/ -> /sys/fs/cgroup/cgmanager.lower/,
> diff --git a/config/apparmor/abstractions/container-base.in b/config/apparmor/abstractions/container-base.in
> new file mode 100644
> index 0000000..437f2a8
> --- /dev/null
> +++ b/config/apparmor/abstractions/container-base.in
> @@ -0,0 +1,44 @@
> + network,
> + capability,
> + file,
> + umount,
> + dbus,
> +
> + # ignore DENIED message on / remount
> + deny mount options=(ro, remount) -> /,
> +
> + # allow tmpfs mounts everywhere
> + mount fstype=tmpfs,
> +
> + # allow mqueue mounts everywhere
> + mount fstype=mqueue,
> +
> + # allow fuse mounts everywhere
> + mount fstype=fuse.*,
> +
> + # allow bind mount of /lib/init/fstab for lxcguest
> + mount options=(rw, bind) /lib/init/fstab.lxc/ -> /lib/init/fstab/,
> +
> + # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted
> + mount fstype=binfmt_misc -> /proc/sys/fs/binfmt_misc/,
> + deny @{PROC}/sys/fs/** wklx,
> +
> + # allow efivars to be mounted, writing to it will be blocked though
> + mount fstype=efivarfs -> /sys/firmware/efi/efivars/,
> +
> + # block some other dangerous paths
> + deny @{PROC}/sysrq-trigger rwklx,
> + deny @{PROC}/mem rwklx,
> + deny @{PROC}/kmem rwklx,
> +
> + # deny writes in /sys except for /sys/fs/cgroup, also allow
> + # fusectl, securityfs and debugfs to be mounted there (read-only)
> + mount fstype=fusectl -> /sys/fs/fuse/connections/,
> + mount fstype=securityfs -> /sys/kernel/security/,
> + mount fstype=debugfs -> /sys/kernel/debug/,
> + deny mount fstype=debugfs -> /var/lib/ureadahead/debugfs/,
> + mount fstype=proc -> /proc/,
> + mount fstype=sysfs -> /sys/,
> + deny /sys/firmware/efi/efivars/** rwklx,
> + deny /sys/kernel/security/** rwklx,
> + mount options=(move) /sys/fs/cgroup/cgmanager/ -> /sys/fs/cgroup/cgmanager.lower/,
> diff --git a/config/apparmor/container-rules.base b/config/apparmor/container-rules.base
> new file mode 100644
> index 0000000..fc98f65
> --- /dev/null
> +++ b/config/apparmor/container-rules.base
> @@ -0,0 +1,6 @@
> +block /sys
> +allow /sys/fs/cgroup/**
> +allow /sys/devices/virtual/net/**
> +allow /sys/class/net/**
> +block /proc/sys/kernel
> +allow /proc/sys/kernel/shm*
> diff --git a/config/apparmor/lxc-generate-aa-rules.py b/config/apparmor/lxc-generate-aa-rules.py
> new file mode 100755
> index 0000000..1972b4c
> --- /dev/null
> +++ b/config/apparmor/lxc-generate-aa-rules.py
> @@ -0,0 +1,124 @@
> +#!/usr/bin/python3
> +
> +import sys
> +
> +blocks=[]
> +
> +#
> +# blocks is an array of paths under which we want to block by
> +# default.
> +#
> +# blocks[0] = ['path' = '/sys', 'children' = [A,B] ]
> +# blocks[1] = ['path' = '/proc/sys', 'children' = [ E ] ]
> +# A = [ 'path' = 'fs', children = [C] ]
> +# C = [ 'path' = 'cgroup', children = [F] ]
> +# B = [ 'path' = 'class', children = [D] ]
> +# D = [ 'path' = 'net', children = [F] ]
> +# E = [ 'path' = 'shm*' ]
> +# F = [ 'path' = '**' ]
> +
> +def add_block(path):
> + for b in blocks:
> + if b['path'] == path:
> + # duplicate
> + return
> + blocks.append({'path': path.strip(), 'children': []})
> +
> +def child_get(prev, path):
> + for p in prev:
> + if p['path'] == path:
> + return p
> + return None
> +
> +def add_allow(path):
> + # find which block we belong to
> + found=None
> + for b in blocks:
> + l=len(b['path'])
> + if len(path) <= l:
> + continue
> + if path[0:l] == b['path']:
> + found=b
> + break
> + if found is None:
> + print("allow with no previous block at %s" % path)
> + sys.exit(1)
> + p = path[l:].strip()
> + while p[:1] == "/":
> + p = p[1:]
> + prev = b['children']
> + for s in p.split('/'):
> + n = {'path': s.strip(), 'children' : [] }
> + tmp = child_get(prev, n['path'])
> + if tmp is not None:
> + prev = tmp
> + else:
> + prev.append(n)
> + prev = n['children']
> +
> +config="config"
> +if len(sys.argv) > 1:
> + config=sys.argv[1]
> +with open(config) as f:
> + for x in f.readlines():
> + x.strip()
> + if x[:1] == '#':
> + continue
> + try:
> + (cmd,path) = x.split(' ')
> + except: # blank line
> + continue
> + if cmd == "block":
> + add_block(path)
> + elif cmd == "allow":
> + add_allow(path)
> + else:
> + print("Unknown command: %s"% cmd)
> + sys.exit(1)
> +
> +# print("%s" % blocks)
> +
> +denies=[]
> +
> +def collect_chars(children, ref, index):
> + r = ""
> + for c in children:
> + if index >= len(c['path']):
> + continue
> + if ref[0:index] != c['path'][0:index]:
> + continue
> + if c['path'][index] not in r:
> + r = r + c['path'][index]
> + return r
> +
> +def append_deny(s):
> + s = "%s wklx," % s
> + if s not in denies:
> + denies.append(s)
> +
> +def gen_denies(pathsofar, children):
> + for c in children:
> + for char in range(len(c['path'])):
> + if char == len(c['path'])-1 and c['path'][char] == '*':
> + continue
> + if char == len(c['path'])-2 and c['path'][char:char+2] == '**':
> + continue
> + x = collect_chars(children, c['path'], char)
> + newdeny = "deny %s/%s[^%s]*{,/**}" % (pathsofar, c['path'][0:char], x)
> + append_deny(newdeny)
> + if c['path'] != '**' and c['path'][len(c['path'])-1] != '*':
> + newdeny = "deny %s/%s?*{,/**}" % (pathsofar, c['path'])
> + append_deny(newdeny)
> + elif c['path'] != '**':
> + newdeny = "deny %s/%s/**"% (pathsofar, c['path'])
> + append_deny(newdeny)
> + if len(c['children']) != 0:
> + newpath = "%s/%s" % (pathsofar, c['path'])
> + gen_denies(newpath, c['children'])
> +
> +for b in blocks:
> + gen_denies(b['path'], b['children'])
> +
> +denies.sort()
> +for d in denies:
> + print(d)
I haven't looked at the code too closely but the syntax doesn't look
pep8-compliant. As a general rule, all python3 code in LXC should be
pyflakes3 and pep8 clean.
I'm happy to help if you get into any trouble getting pep8 to be happy,
I'm getting pretty good at this :)
> diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
> index 9ca68ba..e107123 100644
> --- a/src/tests/Makefile.am
> +++ b/src/tests/Makefile.am
> @@ -22,6 +22,7 @@ lxc_test_reboot_SOURCES = reboot.c
> lxc_test_list_SOURCES = list.c
> lxc_test_attach_SOURCES = attach.c
> lxc_test_device_add_remove_SOURCES = device_add_remove.c
> +lxc_test_apparmor_SOURCES = aa.c
>
> AM_CFLAGS=-I$(top_srcdir)/src \
> -DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \
> @@ -44,7 +45,8 @@ bin_PROGRAMS = lxc-test-containertests lxc-test-locktests lxc-test-startone \
> lxc-test-shutdowntest lxc-test-get_item lxc-test-getkeys lxc-test-lxcpath \
> lxc-test-cgpath lxc-test-clonetest lxc-test-console \
> lxc-test-snapshot lxc-test-concurrent lxc-test-may-control \
> - lxc-test-reboot lxc-test-list lxc-test-attach lxc-test-device-add-remove
> + lxc-test-reboot lxc-test-list lxc-test-attach lxc-test-device-add-remove \
> + lxc-test-apparmor
>
> bin_SCRIPTS = lxc-test-autostart
>
> @@ -68,6 +70,7 @@ EXTRA_DIST = \
> list.c \
> locktests.c \
> lxcpath.c \
> + lxc-test-apparmor \
> lxc-test-autostart \
> lxc-test-ubuntu \
> lxc-test-unpriv \
Binaries build by the build system shouldn't be added to EXTRA_DIST,
however they should be added to .gitignore.
> diff --git a/src/tests/aa.c b/src/tests/aa.c
> new file mode 100644
> index 0000000..946c6d9
> --- /dev/null
> +++ b/src/tests/aa.c
> @@ -0,0 +1,190 @@
> +/* liblxcapi
> + *
> + * Copyright © 2014 Serge Hallyn <serge.hallyn at ubuntu.com>.
> + * Copyright © 2014 Canonical Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +/* Test apparmor rules */
> +#include <lxc/lxccontainer.h>
> +#include "lxc/utils.h"
> +
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#define MYNAME "test-aa"
> +
> +static void try_to_remove(void)
> +{
> + struct lxc_container *c;
> + c = lxc_container_new(MYNAME, NULL);
> + if (c) {
> + if (c->is_defined(c))
> + c->destroy(c);
> + lxc_container_put(c);
> + }
> +}
> +
> +static int test_attach_write_file(void* payload)
> +{
> + char *fnam = payload;
> + FILE *f;
> +
> + f = fopen(fnam, "w");
> + if (f) {
> + printf("yes\n");
> + fclose(f);
> + return 1;
> + }
> + printf("no\n");
> + return 0;
> +}
> +
> +/*
> + * try opening a file attached to a container. Return 0 on open fail. Return
> + * 1 if the file open succeeded. Return -1 if attach itself failed - perhas an
> + * older kernel.
> + */
> +static int do_test_file_open(struct lxc_container *c, char *fnam)
> +{
> + int fret = -1;
> + int ret;
> + pid_t pid;
> + int pipefd[2];
> + char result[1024];
> + lxc_attach_options_t attach_options = LXC_ATTACH_OPTIONS_DEFAULT;
> +
> + ret = pipe(pipefd);
> + if (ret < 0) {
> + fprintf(stderr, "pipe failed %d", ret);
> + return fret;
> + }
> + attach_options.stdout_fd = pipefd[1];
> + attach_options.attach_flags &= ~(LXC_ATTACH_LSM_EXEC|LXC_ATTACH_DROP_CAPABILITIES);
> + attach_options.attach_flags |= LXC_ATTACH_LSM_NOW;
> + ret = c->attach(c, test_attach_write_file, fnam, &attach_options, &pid);
> + if (ret < 0) {
> + fprintf(stderr, "attach failed");
> + goto err1;
> + }
> +
> + ret = read(pipefd[0], result, sizeof(result)-1);
> + if (ret < 0) {
> + fprintf(stderr, "read failed %d", ret);
> + goto err2;
> + }
> +
> + fret = 1;
> + if (strncmp(result, "no", 2) == 0)
> + fret = 0;
> +
> +err2:
> + wait_for_pid(pid);
> +err1:
> + close(pipefd[0]);
> + close(pipefd[1]);
> + return fret;
> +}
> +
> +char *files_to_allow[] = { "/sys/class/net/lo/ifalias",
> + "/proc/sys/kernel/shmmax",
> + NULL };
> +
> +char *files_to_deny[] = { "/proc/mem", "/proc/kmem",
> + "/sys/kernel/uevent_helper",
> + "/proc/sys/fs/file-nr",
> + "/sys/kernel/mm/ksm/pages_to_scan",
> + "/proc/sys/kernel/sysrq",
> + NULL };
> +
> +static bool test_aa_policy(struct lxc_container *c)
> +{
> + int i, ret;
> +
> + for (i = 0; files_to_deny[i]; i++) {
> + ret = do_test_file_open(c, files_to_deny[i]);
> + if (ret < 0) {
> + fprintf(stderr, "attach failed; skipping test");
> + return true;
> + }
> + if (ret > 0) {
> + fprintf(stderr, "failed - opened %s\n",
> + files_to_deny[i]);
> + return false;
> + }
> + fprintf(stderr, "passed with %s\n", files_to_deny[i]);
> + }
> +
> + for (i = 0; files_to_allow[i]; i++) {
> + ret = do_test_file_open(c, files_to_allow[i]);
> + if (ret < 0) {
> + fprintf(stderr, "attach failed; skipping test");
> + return true;
> + }
> + if (ret == 0) {
> + fprintf(stderr, "failed - could not open %s\n",
> + files_to_allow[i]);
> + return false;
> + }
> + fprintf(stderr, "passed with %s\n", files_to_allow[i]);
> + }
> +
> + return true;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + struct lxc_container *c;
> + try_to_remove();
> + c = lxc_container_new(MYNAME, NULL);
> + if (!c) {
> + fprintf(stderr, "%s: %d: failed to load first container\n", __FILE__, __LINE__);
> + exit(1);
> + }
> +
> + if (c->is_defined(c)) {
> + fprintf(stderr, "%d: %s thought it was defined\n", __LINE__, MYNAME);
> + goto err;
> + }
> + if (!c->set_config_item(c, "lxc.network.type", "empty")) {
> + fprintf(stderr, "%s: %d: failed to set network type\n", __FILE__, __LINE__);
> + goto err;
> + }
> + c->save_config(c, NULL);
> + if (!c->createl(c, "download", NULL, NULL, 0, "-d", "ubuntu", "-r", "trusty", "-a", "amd64", NULL)) {
> + fprintf(stderr, "%s: %d: failed to create container\n", __FILE__, __LINE__);
> + goto err;
> + }
> + c->want_daemonize(c, true);
> + if (!c->startl(c, 0, NULL)) {
> + fprintf(stderr, "Error starting container");
> + goto err;
> + }
> +
> + if (!test_aa_policy(c)) {
> + c->stop(c);
> + goto err;
> + }
> +
> + c->stop(c);
> +
> + try_to_remove();
> + exit(0);
> +
> +err:
> + try_to_remove();
> + exit(1);
> +}
> --
> 1.9.0
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140307/6050a238/attachment.pgp>
More information about the lxc-devel
mailing list