Add containerenv information to /run/.containerenv

[rhatdan]

We have been asked to leak some information into the container
to indicate:

• The name and id of the container
• The version of podman used to launch the container
• The image name and ID the container is based on.
• Whether the container engine is running in rootless mode.

Fixes: #6192

Signed-off-by: Daniel J Walsh [email protected]

[AkihiroSuda]

Exposing ID by default might be potentially dangerous. Could this be optional?

[rhatdan]

Please explain how container ID is risky?

We can add something to containers.conf to disable this, but I am not sure it is worth the effort.

[AkihiroSuda]

An attacker might want to know the container ID to determine the path of the attacker-crafted malicious file in the container rootfs.

Such path would be useful for the attacker when the attacker somehow managed to gain write access to e.g. /sys/kernel/uevent_helper.

[mheon]

I'm a little iffy about exposing full version numbers. Usual practice for hardening is to disable that wherever possible to make it harder to identify if your instance is affected by vulnerabilities. Full image ID also has the same issue. I would not do those by default. Also, I think your leading \ in the string you write to the file is unnecessary.

…

On Wed, Nov 25, 2020, 09:05 Akihiro Suda ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In libpod/container_internal_linux.go <#8476 (comment)>: > // Empty string for now, but we may consider populating this later - containerenvPath, err := c.writeStringToRundir(".containerenv", "") + // Populate the .containerenv with container information + containerenv := fmt.Sprintf(`\ +engine="podman-%s" +name=%q +id=%q Exposing ID by default might be potentially dangerous. Could this be optional? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8476 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3AOCHEVE2EQA2GPH4N4X3SRUFK3ANCNFSM4UCI5TNA> .

[rhatdan]

I am not crazy about security by obscurity. It is likely pretty easier for the processes within a container to figure out if the system is vulnerable. Doing a checksum of a couple of executables makes it easy to figure out if you are vulnerable.

[rhatdan]

@cgwalters @debarshiray @juanje @alexlarsson PTAL

[debarshiray]

I'm a little iffy about exposing full version numbers. Usual practice for
hardening is to disable that wherever possible to make it harder to
identify if your instance is affected by vulnerabilities.

It is likely pretty easier for the processes within a container to figure out
if the system is vulnerable. Doing a checksum of a couple of executables
makes it easy to figure out if you are vulnerable.

As long as images have the package (eg., RPM) database, then one can just do rpm -q .... However, I can imagine images that don't have the database present.

[rhatdan]

@debarshiray You want these fields for logging, and better reporting to the user, what is going on, correct?

[TomSweeneyRedHat]

Couple nits in docs. Once that and the security question is solved, LGTM.

[debarshiray]

You want these fields for logging, and better reporting to the user, what
is going on, correct?

Yes, correct. eg., there is an interest to customize the shell prompt and such things to indicate which container and/or image is in play.

[rhatdan]

/hold

[rhatdan]

/hold cancel

[rhatdan]

@containers/podman-maintainers PTAL
@AkihiroSuda We are now only leaking the information in --privileged mode.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AkihiroSuda, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[TomSweeneyRedHat]

LGTM
one of the tests isn't hip and I'm not sure why.
Do you need to touch up containers/buildah#2821 for the security issue @AkihiroSuda raised here?

[mheon]

/lgtm