[systemd-devel] [PATCH] virt: fix container detection when we're not PID 1

2014-12-10 Thread Jan Synacek
systemd-detect-virt would print none when using nspawn to run a shell
inside a container and then running systemd-detect-virt in it, because
the shell would be PID 1, not the actuall systemd-detect-virt process.
---
 src/shared/virt.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/shared/virt.c b/src/shared/virt.c
index f9c4e67..298e005 100644
--- a/src/shared/virt.c
+++ b/src/shared/virt.c
@@ -275,18 +275,10 @@ int detect_container(const char **id) {
 goto finish;
 }
 
-if (getpid() == 1) {
-/* If we are PID 1 we can just check our own
- * environment variable */
-
-e = getenv(container);
-if (isempty(e)) {
-r = 0;
-goto finish;
-}
-} else {
-
-/* Otherwise, PID 1 dropped this information into a
+/* Check our own environment variable */
+e = getenv(container);
+if (isempty(e)) {
+/* PID 1 dropped this information into a
  * file in /run. This is better than accessing
  * /proc/1/environ, since we don't need CAP_SYS_PTRACE
  * for that. */
@@ -300,7 +292,8 @@ int detect_container(const char **id) {
 return r;
 
 e = m;
-}
+} else
+r = 0;
 
 /* We only recognize a selected few here, since we want to
  * enforce a redacted namespace */
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] virt: fix container detection when we're not PID 1

2014-12-10 Thread Lennart Poettering
On Wed, 10.12.14 09:21, Jan Synacek (jsyna...@redhat.com) wrote:

 systemd-detect-virt would print none when using nspawn to run a shell
 inside a container and then running systemd-detect-virt in it, because
 the shell would be PID 1, not the actuall systemd-detect-virt
 process.

So, previously the code read the env var directly from
/proc/1/environ, but that file is only readable with privs, hence I
added code to PID 1 to write the value of that env var to
/run/systemd/container which is readable without privs. Now, if you
run a shell as PID 1 that will obviously not happen and the detection
won't work after all. 

Simply relying that $container is inherited from PID 1 down is
something I'd really like to avoid, though.

I have now made a change to the code that falls back to
getenv_for_pid() if /rub/systemd/container does not exist. THis will
only be ffective with perms however. The new code hence still isn't
perfect: if you boot up with only a shell as PID 1 and drop privileges
the code will still not be able to detect the container manager. Not
sure what other option we have, though.

 ---
  src/shared/virt.c | 19 ++-
  1 file changed, 6 insertions(+), 13 deletions(-)
 
 diff --git a/src/shared/virt.c b/src/shared/virt.c
 index f9c4e67..298e005 100644
 --- a/src/shared/virt.c
 +++ b/src/shared/virt.c
 @@ -275,18 +275,10 @@ int detect_container(const char **id) {
  goto finish;
  }
  
 -if (getpid() == 1) {
 -/* If we are PID 1 we can just check our own
 - * environment variable */
 -
 -e = getenv(container);
 -if (isempty(e)) {
 -r = 0;
 -goto finish;
 -}
 -} else {
 -
 -/* Otherwise, PID 1 dropped this information into a
 +/* Check our own environment variable */
 +e = getenv(container);
 +if (isempty(e)) {
 +/* PID 1 dropped this information into a
   * file in /run. This is better than accessing
   * /proc/1/environ, since we don't need CAP_SYS_PTRACE
   * for that. */
 @@ -300,7 +292,8 @@ int detect_container(const char **id) {
  return r;
  
  e = m;
 -}
 +} else
 +r = 0;
  
  /* We only recognize a selected few here, since we want to
   * enforce a redacted namespace */
 -- 
 1.9.3
 
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] virt: fix container detection when we're not PID 1

2014-12-10 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Wed, 10.12.14 09:21, Jan Synacek (jsyna...@redhat.com) wrote:

 systemd-detect-virt would print none when using nspawn to run a shell
 inside a container and then running systemd-detect-virt in it, because
 the shell would be PID 1, not the actuall systemd-detect-virt
 process.

 So, previously the code read the env var directly from
 /proc/1/environ, but that file is only readable with privs, hence I
 added code to PID 1 to write the value of that env var to
 /run/systemd/container which is readable without privs. Now, if you
 run a shell as PID 1 that will obviously not happen and the detection
 won't work after all. 

 Simply relying that $container is inherited from PID 1 down is
 something I'd really like to avoid, though.

Could you please explain why? I don't see why that might be a problem.

 I have now made a change to the code that falls back to
 getenv_for_pid() if /rub/systemd/container does not exist. THis will
 only be ffective with perms however. The new code hence still isn't
 perfect: if you boot up with only a shell as PID 1 and drop privileges
 the code will still not be able to detect the container manager. Not
 sure what other option we have, though.

Thanks!

-- 
Jan Synacek
Software Engineer, Red Hat


signature.asc
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] virt: fix container detection when we're not PID 1

2014-12-10 Thread Zbigniew Jędrzejewski-Szmek
On Wed, Dec 10, 2014 at 01:52:08PM +0100, Jan Synacek wrote:
 Lennart Poettering lenn...@poettering.net writes:
  On Wed, 10.12.14 09:21, Jan Synacek (jsyna...@redhat.com) wrote:
 
  systemd-detect-virt would print none when using nspawn to run a shell
  inside a container and then running systemd-detect-virt in it, because
  the shell would be PID 1, not the actuall systemd-detect-virt
  process.
 
  So, previously the code read the env var directly from
  /proc/1/environ, but that file is only readable with privs, hence I
  added code to PID 1 to write the value of that env var to
  /run/systemd/container which is readable without privs. Now, if you
  run a shell as PID 1 that will obviously not happen and the detection
  won't work after all. 
 
  Simply relying that $container is inherited from PID 1 down is
  something I'd really like to avoid, though.
 
 Could you please explain why? I don't see why that might be a problem.
Because container is a completely generic name, and it is not exported
to children by systemd.
 
  I have now made a change to the code that falls back to
  getenv_for_pid() if /rub/systemd/container does not exist. THis will
  only be ffective with perms however. The new code hence still isn't
  perfect: if you boot up with only a shell as PID 1 and drop privileges
  the code will still not be able to detect the container manager. Not
  sure what other option we have, though.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] virt: fix container detection when we're not PID 1

2014-12-10 Thread Lennart Poettering
On Wed, 10.12.14 13:52, Jan Synacek (jsyna...@redhat.com) wrote:

 Lennart Poettering lenn...@poettering.net writes:
  On Wed, 10.12.14 09:21, Jan Synacek (jsyna...@redhat.com) wrote:
 
  systemd-detect-virt would print none when using nspawn to run a shell
  inside a container and then running systemd-detect-virt in it, because
  the shell would be PID 1, not the actuall systemd-detect-virt
  process.
 
  So, previously the code read the env var directly from
  /proc/1/environ, but that file is only readable with privs, hence I
  added code to PID 1 to write the value of that env var to
  /run/systemd/container which is readable without privs. Now, if you
  run a shell as PID 1 that will obviously not happen and the detection
  won't work after all. 
 
  Simply relying that $container is inherited from PID 1 down is
  something I'd really like to avoid, though.
 
 Could you please explain why? I don't see why that might be a problem.

Well, systemd when running as PID 1 tries hard to pass a fully cleaned
up environment to system services, so that they always run in the same
clean execution environment. Checking $container of your own process
will hence generally not work unless you do it from PID 1 -- if you
booted with systemd as PID 1. Moreover we really should have a way to
detect containers even if some process way down the tree cleans up the
environment, which many processes actually do.

Then, something not to ignore: $container can be set by an
unprivileged user for any process it launches. Thus, you can
relatively easily programs about the execution context they are
running in, which is particularly dangerous for SUID programs (see the
whole discussion around secure_getenv() regarding this). Hence I am a
lot more comfortable with checking /run/systemd/container and
/proc/1/environ, since neither is fakeable without privileges.

Hope that makese sense,

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel