Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

container=podman environment variable #11836

Closed
pboguslawski opened this issue Oct 1, 2021 · 18 comments · Fixed by #12100
Closed

container=podman environment variable #11836

pboguslawski opened this issue Oct 1, 2021 · 18 comments · Fixed by #12100
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@pboguslawski
Copy link

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

podman run adds container=podman to environment in container; this info should not be present there for security reasons (don't make bad guys life easier); if one really need to pass it to container, may use --env explicitly.

Steps to reproduce the issue:

  1. Run container podman run -it bash

  2. Execute set

Describe the results you received:

container=podman in set output

Describe the results you expected:

No podman traces.

Output of podman version:

Version:      3.0.1
API Version:  3.0.0
Go Version:   go1.15.9
Built:        Thu Jan  1 01:00:00 1970
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.19.6
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: 'conmon: /usr/bin/conmon'
    path: /usr/bin/conmon
    version: 'conmon version 2.0.25, commit: unknown'
  cpus: 2
  distribution:
    distribution: debian
    version: "11"
  eventLogger: journald
  hostname: [...]
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 5.10.0-8-amd64
  linkmode: dynamic
  memFree: 622333952
  memTotal: 4122234880
  ociRuntime:
    name: crun
    package: 'crun: /usr/bin/crun'
    path: /usr/bin/crun
    version: |-
      crun version 0.17
      commit: 0e9229ae34caaebcb86f1fde18de3acaf18c6d9a
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +YAJL
  os: linux
  remoteSocket:
    exists: true
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: true
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: false
    seccompEnabled: true
    selinuxEnabled: false
  slirp4netns:
    executable: ""
    package: ""
    version: ""
  swapFree: 2147479552
  swapTotal: 2147479552
  uptime: 1h 32m 6.75s (Approximately 0.04 days)
registries: {}
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 14
    paused: 0
    running: 14
    stopped: 0
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /var/lib/containers/storage
  graphStatus:
    Backing Filesystem: xfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  imageStore:
    number: 15
  runRoot: /run/containers/storage
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 3.0.0
  Built: 0
  BuiltTime: Thu Jan  1 01:00:00 1970
  GitCommit: ""
  GoVersion: go1.15.9
  OsArch: linux/amd64
  Version: 3.0.1

Package info (e.g. output of rpm -q podman or apt list podman):

# dpkg -s podman | grep Version
Version: 3.0.1+dfsg1-3+b2

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/master/troubleshooting.md)

No

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 1, 2021
@afbjorklund
Copy link
Contributor

afbjorklund commented Oct 2, 2021

It's a feature for systemd.

https://systemd.io/CONTAINER_INTERFACE/

If you write software that wants to detect whether it is run in a container, please check /proc/1/environ and look for the container= environment variable. Do not assume the environment variable is inherited down the process tree. It generally is not. Hence check the environment block of PID 1, not your own. Note though that this file is only accessible to root.

@pboguslawski
Copy link
Author

pboguslawski commented Oct 2, 2021

It's a feature for systemd.

There shouldn't be any redundant traces of host platform type for security reasons. Podman (1) should not assume that container executes any special configuration, i.e. systemd and (2) should not force things that are redundant (no flag to disable this env var in docs). This env var is set even with --systemd=false...

@afbjorklund
Copy link
Contributor

See #533 for discussion...

Docker uses /.dockerenv

@pboguslawski
Copy link
Author

pboguslawski commented Oct 2, 2021

See #533 for discussion...

Mentioned discussion does not say anything about requirement of putting /.dockerenv, /run/.containerenv, container=podman or other stuff like this in container. If any legitilate app must know if is running in container or not - admin may explicitly pass env container=podman or use quirks like --systemd=true.

Docker uses /.dockerenv

If docker were perfect, nobody would care about podman ;)

@afbjorklund
Copy link
Contributor

I think the original suggestion was:

@pboguslawski
Copy link
Author

think the original suggestion was:

Mentioned discussion does not say anything about requirement of putting /.dockerenv, /run/.containerenv, container=podman or other stuff like this in container. Just don't force such things in podman. If backward compatibility will be required add --paranoid-mode and disable putting all this stuff and you'll make some admins happy.

@mheon
Copy link
Member

mheon commented Oct 2, 2021

This is not going to change; it's an established part of our interface at this point. Allowing programs in the container (e.g. systemd) to identify not just that they are in a container, but what kind of containerization is in use, can be essential for proper functioning.

@mheon mheon closed this as completed Oct 2, 2021
@rhatdan
Copy link
Member

rhatdan commented Oct 3, 2021

We could consider a --noleak option, though to prevent this leak of information. I am not sure of the value of this though. I is pretty easy to know that you are inside of a container, especially if /proc/1 does not point to an init system. I guess there is some value in knowing you were launched by different container engines. Podman, Docker, Buildah, Containerd, CRI-O.

@pboguslawski
Copy link
Author

pboguslawski commented Oct 3, 2021

We could consider a --noleak option

--noleak or --paranoid (which exists in other apps) but this option should be present. Consider also disabling all other, not enabled explicitly, redundant (=not absolutely required for general containerization) stuff in container with this flag, including already spotted stuff (i.e. /run/.containerenv and #11835) and if similar things will be enabled by default if in the future (better avoid enabling by default if possible).

I is pretty easy to know that you are inside of a container, especially if /proc/1 does not point to an init system.

Don't make bad guys life easier.

@rhatdan
Copy link
Member

rhatdan commented Oct 3, 2021

Don't make bad guys life easier.
The inverse is Don't make everyone life suck, for very little security benefit.

@rhatdan
Copy link
Member

rhatdan commented Oct 3, 2021

Interested in opening a PR to add a --paranoid flag?

@rhatdan rhatdan reopened this Oct 3, 2021
@AkihiroSuda
Copy link
Collaborator

I don’t think “noleak”/“paranoid” mode is implementable.

Even when $container is unset, “bad guys” can easily detect whether they are in a container, just by inspecting /proc/mounts, /proc/PID/ns, /proc/PID/cgroup, /proc/PID/status, etc.

$container is not a security boundary, and unsetting it doesn’t make bad guys life harder at all.

@pboguslawski
Copy link
Author

unsetting it doesn’t make bad guys life harder at all.

It does: container=podman clearly states it's podman not other containerization technology (if it is also present in other places like /proc - should be removed IHMO from there also with same flag if not absolutely required for general containerization).

I don’t think “noleak”/“paranoid” mode is implementable.

Why do you think so?

@giuseppe
Copy link
Member

Why do you think so?

it is enough to look at the mount table to understand it is a tool based on containers/storage:

# grep containers /proc/self/mountinfo 
3094 3082 0:55 /containers/overlay-containers/5fa1b8f28d5b01bdd69b199b26d2affe9ab01d635f26d7ae5ab236ef92794ffb/userdata/hostname /etc/hostname rw,nosuid,nodev,relatime - tmpfs tmpfs rw,seclabel,size=3260920k,nr_inodes=815230,mode=700,uid=1000,gid=1000,inode64
3095 3082 0:55 /containers/overlay-containers/5fa1b8f28d5b01bdd69b199b26d2affe9ab01d635f26d7ae5ab236ef92794ffb/userdata/.containerenv /run/.containerenv rw,nosuid,nodev,relatime - tmpfs tmpfs rw,seclabel,size=3260920k,nr_inodes=815230,mode=700,uid=1000,gid=1000,inode64
3096 3082 0:55 /containers/overlay-containers/5fa1b8f28d5b01bdd69b199b26d2affe9ab01d635f26d7ae5ab236ef92794ffb/userdata/run/secrets /run/secrets rw,nosuid,nodev,relatime - tmpfs tmpfs rw,seclabel,size=3260920k,nr_inodes=815230,mode=700,uid=1000,gid=1000,inode64
3097 3082 0:55 /containers/overlay-containers/5fa1b8f28d5b01bdd69b199b26d2affe9ab01d635f26d7ae5ab236ef92794ffb/userdata/resolv.conf /etc/resolv.conf rw,nosuid,nodev,relatime - tmpfs tmpfs rw,seclabel,size=3260920k,nr_inodes=815230,mode=700,uid=1000,gid=1000,inode64
3098 3082 0:55 /containers/overlay-containers/5fa1b8f28d5b01bdd69b199b26d2affe9ab01d635f26d7ae5ab236ef92794ffb/userdata/hosts /etc/hosts rw,nosuid,nodev,relatime - tmpfs tmpfs rw,seclabel,size=3260920k,nr_inodes=815230,mode=700,uid=1000,gid=1000,inode64

I think it would be nice to have a way to unset an env variable, but container=podman doesn't leak more than what is already easily discoverable in a container.

@giuseppe
Copy link
Member

I think a --paranoid or similar option will give a false sense of security. I'd vote for --env-drop that can be used to remove the container env variable if you wish so

@pboguslawski
Copy link
Author

pboguslawski commented Oct 11, 2021

I'd vote for --env-drop

There is more optional stuff that should be removed so it's better to implement more general flag for all to avoid syntax mess IHMO.

@rhatdan
Copy link
Member

rhatdan commented Oct 12, 2021

I am fine with --noleak, and then explicitly explain what it does.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

rhatdan added a commit to rhatdan/podman that referenced this issue Nov 15, 2021
Podman adds a few environment variables by default, and
currently there is no way to get rid of them from your container.
This option will allow  you to specify which defaults you don't
want.

--unsetenv-all will remove all default environment variables.

Default environment variables can come from podman builtin,
containers.conf or from the container image.

Fixes: containers#11836

Signed-off-by: Daniel J Walsh <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants