Re: [systemd-devel] [PATCH] virt: fix container detection when we're not PID 1
On Wed, 10.12.14 13:52, Jan Synacek (jsyna...@redhat.com) wrote: > Lennart Poettering 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
Re: [systemd-devel] [PATCH] virt: fix container detection when we're not PID 1
On Wed, Dec 10, 2014 at 01:52:08PM +0100, Jan Synacek wrote: > Lennart Poettering 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
Lennart Poettering 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
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
[systemd-devel] [PATCH] virt: fix container detection when we're not PID 1
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
