Hey, Lennart Poettering [2014-11-20 12:29 +0100]: > > d /var/lib/containers 0700 - - - > > > > to tmpfiles.d/var.conf? I can also add this to the Debian tmpfiles.d > > file, but it's not really Debian specific. > > Sounds resonable. But first, can you elaborate on the reason for 0700 > rather than 0755?
Mostly so that users on the host can't call suid root binaries in the container. If containers are restricted with selinux/apparmor or started as user (both happens in Ubuntu for LXC, for example) this would open a way to escalate root privs in a container into root privs on the host. https://launchpad.net/bugs/1244635 has the details/history of this. I know that upstream systemd doesn't ship AppArmor/SELinux profiles, and thus your stanza is that containers are inherently insecure. So if you aren't convinced by the calling of suid root binaries, 0755 is also ok for upstream, or we just skip this part entirely (it's really just a small detail, after all). Patch attached. > > Second, [email protected] uses --link-journal=guest. If you > > don't have a persistant journal, and /var/log/journal/ does not exist, > > then containers fail to start in a rather unfriendly way: > > Hmm, another option would be to introduce --link-journal=try-guest > which is identical to --link-journal=guest except that it becomes a > NOP if /var/log/journal doesn't exist and doesn't even generate an > error or warning. Then, we could change "-j" to mean > --link-journal=try-guest and make that the default to use in the unit > file. I think that would be the best option really. Excellent, sounds good! Patch attached. I went with a new link_journal_try flag instead of introducing LINK_TRY_HOST/LINK_TRY_GUEST, as that would make a lot of if statements more unwieldy, and it's easy to forget to always check for both cases. But if you prefer that, I'm happy to change the patch accordingly. Thanks, Martin -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 62a73c5c87b10ba3a40d012822aa000b176667c3 Mon Sep 17 00:00:00 2001 From: Martin Pitt <[email protected]> Date: Thu, 20 Nov 2014 14:30:52 +0100 Subject: [PATCH 1/2] nspawn: Add try-{host,guest} journal link modes --link-journal={host,guest} fail if the host does not have persistent journalling enabled and /var/log/journal/ does not exist. Even worse, as there is no stdout/err any more, there is no error message to point that out. Introduce two new modes "try-host" and "try-guest" which don't fail in this case, and instead just silently skip the guest journal setup. Change -j to mean "try-guest" instead of "guest", and fix the wrong --help output for it (it said "host" before). Change [email protected] to use "try-guest" so that this unit works with both persistent and non-persistent journals on the host without failing. https://bugs.debian.org/770275 --- man/systemd-nspawn.xml | 11 ++++++++--- src/nspawn/nspawn.c | 37 +++++++++++++++++++++++++++++-------- units/[email protected] | 2 +- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml index b3a2d32..75db65e 100644 --- a/man/systemd-nspawn.xml +++ b/man/systemd-nspawn.xml @@ -439,7 +439,9 @@ versa). Takes one of <literal>no</literal>, <literal>host</literal>, + <literal>try-host</literal>, <literal>guest</literal>, + <literal>try-guest</literal>, <literal>auto</literal>. If <literal>no</literal>, the journal is not linked. If <literal>host</literal>, @@ -453,8 +455,11 @@ guest file system (beneath <filename>/var/log/journal/<replaceable>machine-id</replaceable></filename>) and the subdirectory is symlinked into the host - at the same location. If - <literal>auto</literal> (the default), + at the same location. <literal>try-host</literal> + and <literal>try-guest</literal> do the same + but do not fail if the host does not have + persistant journalling enabled. + If <literal>auto</literal> (the default), and the right subdirectory of <filename>/var/log/journal</filename> exists, it will be bind mounted @@ -473,7 +478,7 @@ <term><option>-j</option></term> <listitem><para>Equivalent to - <option>--link-journal=guest</option>.</para></listitem> + <option>--link-journal=try-guest</option>.</para></listitem> </varlistentry> <varlistentry> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index c2311b3..5a80cf2 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -124,6 +124,7 @@ static bool arg_private_network = false; static bool arg_read_only = false; static bool arg_boot = false; static LinkJournal arg_link_journal = LINK_AUTO; +static bool link_journal_try = false; static uint64_t arg_retain = (1ULL << CAP_CHOWN) | (1ULL << CAP_DAC_OVERRIDE) | @@ -202,8 +203,9 @@ static void help(void) { " --capability=CAP In addition to the default, retain specified\n" " capability\n" " --drop-capability=CAP Drop the specified capability from the default set\n" - " --link-journal=MODE Link up guest journal, one of no, auto, guest, host\n" - " -j Equivalent to --link-journal=host\n" + " --link-journal=MODE Link up guest journal, one of no, auto, guest, host,\n" + " try-guest, try-host\n" + " -j Equivalent to --link-journal=try-guest\n" " --read-only Mount the root directory read-only\n" " --bind=PATH[:PATH] Bind mount a file or directory from the host into\n" " the container\n" @@ -428,6 +430,7 @@ static int parse_argv(int argc, char *argv[]) { case 'j': arg_link_journal = LINK_GUEST; + link_journal_try = true; break; case ARG_LINK_JOURNAL: @@ -439,7 +442,13 @@ static int parse_argv(int argc, char *argv[]) { arg_link_journal = LINK_GUEST; else if (streq(optarg, "host")) arg_link_journal = LINK_HOST; - else { + else if (streq(optarg, "try-guest")) { + arg_link_journal = LINK_GUEST; + link_journal_try = true; + } else if (streq(optarg, "try-host")) { + arg_link_journal = LINK_HOST; + link_journal_try = true; + } else { log_error("Failed to parse link journal mode %s", optarg); return -EINVAL; } @@ -1404,8 +1413,13 @@ static int setup_journal(const char *directory) { if (arg_link_journal == LINK_GUEST) { if (symlink(q, p) < 0) { - log_error("Failed to symlink %s to %s: %m", q, p); - return -errno; + if (link_journal_try) { + log_debug("Failed to symlink %s to %s, skipping journal setup: %m", q, p); + return 0; + } else { + log_error("Failed to symlink %s to %s: %m", q, p); + return -errno; + } } r = mkdir_p(q, 0755); @@ -1415,10 +1429,17 @@ static int setup_journal(const char *directory) { } if (arg_link_journal == LINK_HOST) { - r = mkdir_p(p, 0755); + /* don't create parents here -- if the host doesn't have + * permanent journal set up, don't force it here */ + r = mkdir(p, 0755); if (r < 0) { - log_error("Failed to create %s: %m", p); - return r; + if (link_journal_try) { + log_debug("Failed to create %s, skipping journal setup: %m", p); + return 0; + } else { + log_error("Failed to create %s: %m", p); + return r; + } } } else if (access(p, F_OK) < 0) diff --git a/units/[email protected] b/units/[email protected] index dec2ce7..e3eaa53 100644 --- a/units/[email protected] +++ b/units/[email protected] @@ -10,7 +10,7 @@ Description=Container %i Documentation=man:systemd-nspawn(1) [Service] -ExecStart=@bindir@/systemd-nspawn --quiet --keep-unit --boot --link-journal=guest --directory=/var/lib/container/%i +ExecStart=@bindir@/systemd-nspawn --quiet --keep-unit --boot --link-journal=try-guest --directory=/var/lib/container/%i KillMode=mixed Type=notify RestartForceExitStatus=133 -- 2.1.3
From 2475d7c3bebe930a223f8464fc622d820bf65d83 Mon Sep 17 00:00:00 2001 From: Martin Pitt <[email protected]> Date: Thu, 20 Nov 2014 14:37:08 +0100 Subject: [PATCH 2/2] tmpfiles.d: Create /var/lib/containers Create /var/lib/containers so that it exists with an appropriate mode. We want 0700 by default so that users on the host aren't able to call suid root binaries in the container. This becomes a security issue if a user can enter a container as root, create a suid root binary, and call that from the host. (This assumes that containers are caged by mandatory access control or are started as user). --- tmpfiles.d/var.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tmpfiles.d/var.conf b/tmpfiles.d/var.conf index 4b63e41..b4bf3bc 100644 --- a/tmpfiles.d/var.conf +++ b/tmpfiles.d/var.conf @@ -18,4 +18,6 @@ f /var/log/btmp 0600 root utmp - d /var/cache 0755 - - - d /var/lib 0755 - - - +d /var/lib/containers 0700 - - - + d /var/spool 0755 - - - -- 2.1.3
signature.asc
Description: Digital signature
_______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
