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

podman history exposes proxy related build args #2937

Closed
rolweber opened this issue Jan 26, 2021 · 22 comments · Fixed by #3782
Closed

podman history exposes proxy related build args #2937

rolweber opened this issue Jan 26, 2021 · 22 comments · Fixed by #3782

Comments

@rolweber
Copy link

Description

I'm trying to use podman build as a drop-in replacement for docker build.
I'm (ab)using the no_proxy build arg to pass an API key at build time, which should not be exposed in the image.

With docker, no_proxy and other pre-defined build args do not appear in the image history:
https://docs.docker.com/engine/reference/builder/#predefined-args
With podman, the value of no_proxy appears in the image history. I didn't try the other ones.

Steps to reproduce the issue:

  1. podman build -t whatever --build-arg "no-proxy=to-be-kept-secret" .
  2. podman history --no-trunc --format json whatever

I'll add a Dockerfile, shell script and Makefile to reproduce the problem conveniently.

Describe the results you received:

The history shows the value I passed as the no_proxy build argument.

Describe the results you expected:

The history should not show the value

I passed as the no_proxy build argument.

Output of podman version if reporting a podman build issue:

Version:      2.2.1
API Version:  2.1.0
Go Version:   go1.15.5
Built:        Tue Dec  8 15:37:50 2020
OS/Arch:      linux/amd64

Output of cat /etc/*release:

Linux Client for e-business (Fedora) 33 (Daily)
Fedora release 33 (Thirty Three)
Open Client Fedora 33 (Daily)
NAME=Fedora
VERSION="33 (Workstation Edition)"
ID=fedora
VERSION_ID=33
VERSION_CODENAME=""
PLATFORM_ID="platform:f33"
PRETTY_NAME="Fedora 33 (Workstation Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:33"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f33/system-administrators-guide/"
SUPPORT_URL="https://fedoraproject.org/wiki/Communicating_and_getting_help"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=33
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=33
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Workstation Edition"
VARIANT_ID=workstation
Fedora release 33 (Thirty Three)
Fedora release 33 (Thirty Three)

Output of uname -a:

Linux li-d94ead45-8c12-2249-903a-e2804902b72a.ibm.com 5.10.8-200.fc33.x86_64 #1 SMP Sun Jan 17 19:53:01 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Output of cat /etc/containers/storage.conf:

# This file is is the configuration file for all tools
# that use the containers/storage library.
# See man 5 containers-storage.conf for more information
# The "container storage" table contains all of the server options.
[storage]

# Default Storage Driver
driver = "overlay"

# Temporary storage location
runroot = "/var/run/containers/storage"

# Primary Read/Write location of container storage
graphroot = "/var/lib/containers/storage"

# Storage path for rootless users
#
# rootless_storage_path = "$HOME/.local/share/containers/storage"

[storage.options]
# Storage options to be passed to underlying storage drivers

# AdditionalImageStores is used to pass paths to additional Read/Only image stores
# Must be comma separated list.
additionalimagestores = [
]

# Remap-UIDs/GIDs is the mapping from UIDs/GIDs as they should appear inside of
# a container, to the UIDs/GIDs as they should appear outside of the container,
# and the length of the range of UIDs/GIDs.  Additional mapped sets can be
# listed and will be heeded by libraries, but there are limits to the number of
# mappings which the kernel will allow when you later attempt to run a
# container.
#
# remap-uids = 0:1668442479:65536
# remap-gids = 0:1668442479:65536

# Remap-User/Group is a user name which can be used to look up one or more UID/GID
# ranges in the /etc/subuid or /etc/subgid file.  Mappings are set up starting
# with an in-container ID of 0 and then a host-level ID taken from the lowest
# range that matches the specified name, and using the length of that range.
# Additional ranges are then assigned, using the ranges which specify the
# lowest host-level IDs first, to the lowest not-yet-mapped in-container ID,
# until all of the entries have been used for maps.
#
# remap-user = "containers"
# remap-group = "containers"

# Root-auto-userns-user is a user name which can be used to look up one or more UID/GID
# ranges in the /etc/subuid and /etc/subgid file.  These ranges will be partitioned
# to containers configured to create automatically a user namespace.  Containers
# configured to automatically create a user namespace can still overlap with containers
# having an explicit mapping set.
# This setting is ignored when running as rootless.
# root-auto-userns-user = "storage"
#
# Auto-userns-min-size is the minimum size for a user namespace created automatically.
# auto-userns-min-size=1024
#
# Auto-userns-max-size is the minimum size for a user namespace created automatically.
# auto-userns-max-size=65536

[storage.options.overlay]
# ignore_chown_errors can be set to allow a non privileged user running with
# a single UID within a user namespace to run containers. The user can pull
# and use any image even those with multiple uids.  Note multiple UIDs will be
# squashed down to the default uid in the container.  These images will have no
# separation between the users in the container. Only supported for the overlay
# and vfs drivers.
#ignore_chown_errors = "false"

# Path to an helper program to use for mounting the file system instead of mounting it
# directly.
#mount_program = "/usr/bin/fuse-overlayfs"

# mountopt specifies comma separated list of extra mount options
mountopt = "nodev,metacopy=on"

# Set to skip a PRIVATE bind mount on the storage home directory.
# skip_mount_home = "false"

# Size is used to set a maximum size of the container image.
# size = ""

# ForceMask specifies the permissions mask that is used for new files and
# directories.
#
# The values "shared" and "private" are accepted.
# Octal permission masks are also accepted.
#
#  "": No value specified.
#     All files/directories, get set with the permissions identified within the
#     image.
#  "private": it is equivalent to 0700.
#     All files/directories get set with 0700 permissions.  The owner has rwx
#     access to the files. No other users on the system can access the files.
#     This setting could be used with networked based homedirs.
#  "shared": it is equivalent to 0755.
#     The owner has rwx access to the files and everyone else can read, access
#     and execute them. This setting is useful for sharing containers storage
#     with other users.  For instance have a storage owned by root but shared
#     to rootless users as an additional store.
#     NOTE:  All files within the image are made readable and executable by any
#     user on the system. Even /etc/shadow within your image is now readable by
#     any user.
#
#   OCTAL: Users can experiment with other OCTAL Permissions.
#
#  Note: The force_mask Flag is an experimental feature, it could change in the
#  future.  When "force_mask" is set the original permission mask is stored in
#  the "user.containers.override_stat" xattr and the "mount_program" option must
#  be specified. Mount programs like "/usr/bin/fuse-overlayfs" present the
#  extended attribute permissions to processes within containers rather then the
#  "force_mask"  permissions.
#
# force_mask = ""

[storage.options.thinpool]
# Storage Options for thinpool

# autoextend_percent determines the amount by which pool needs to be
# grown. This is specified in terms of % of pool size. So a value of 20 means
# that when threshold is hit, pool will be grown by 20% of existing
# pool size.
# autoextend_percent = "20"

# autoextend_threshold determines the pool extension threshold in terms
# of percentage of pool size. For example, if threshold is 60, that means when
# pool is 60% full, threshold has been hit.
# autoextend_threshold = "80"

# basesize specifies the size to use when creating the base device, which
# limits the size of images and containers.
# basesize = "10G"

# blocksize specifies a custom blocksize to use for the thin pool.
# blocksize="64k"

# directlvm_device specifies a custom block storage device to use for the
# thin pool. Required if you setup devicemapper.
# directlvm_device = ""

# directlvm_device_force wipes device even if device already has a filesystem.
# directlvm_device_force = "True"

# fs specifies the filesystem type to use for the base device.
# fs="xfs"

# log_level sets the log level of devicemapper.
# 0: LogLevelSuppress 0 (Default)
# 2: LogLevelFatal
# 3: LogLevelErr
# 4: LogLevelWarn
# 5: LogLevelNotice
# 6: LogLevelInfo
# 7: LogLevelDebug
# log_level = "7"

# min_free_space specifies the min free space percent in a thin pool require for
# new device creation to succeed. Valid values are from 0% - 99%.
# Value 0% disables
# min_free_space = "10%"

# mkfsarg specifies extra mkfs arguments to be used when creating the base
# device.
# mkfsarg = ""

# metadata_size is used to set the `pvcreate --metadatasize` options when
# creating thin devices. Default is 128k
# metadata_size = ""

# Size is used to set a maximum size of the container image.
# size = ""

# use_deferred_removal marks devicemapper block device for deferred removal.
# If the thinpool is in use when the driver attempts to remove it, the driver
# tells the kernel to remove it as soon as possible. Note this does not free
# up the disk space, use deferred deletion to fully remove the thinpool.
# use_deferred_removal = "True"

# use_deferred_deletion marks thinpool device for deferred deletion.
# If the device is busy when the driver attempts to delete it, the driver
# will attempt to delete device every 30 seconds until successful.
# If the program using the driver exits, the driver will continue attempting
# to cleanup the next time the driver is used. Deferred deletion permanently
# deletes the device and all data stored in device will be lost.
# use_deferred_deletion = "True"

# xfs_nospace_max_retries specifies the maximum number of retries XFS should
# attempt to complete IO when ENOSPC (no space) error is returned by
# underlying storage device.
# xfs_nospace_max_retries = "0"
@rolweber
Copy link
Author

To reproduce, extract this archive on a Linux x86 system with GNU make and podman installed:
no_proxy_exposed.zip

Run make podman to reproduce the problem. The output shows the history with the value passed to no_proxy.

Run make docker for comparison.

@rolweber
Copy link
Author

If there's a better way to pass secret values to the build while keeping them out of the image, I'd love to learn it.
It'd be OK if that works only with podman but not docker.

@rhatdan
Copy link
Member

rhatdan commented Jan 26, 2021

This is work going on with podman secrets, but it is not complete yet.
We do support --volume which allows you to mount a volume into the container that could hold the secret.
We should fix the issue you talked about though.

@nalind WDYT? Could you point me at where this information is recorded?

@nalind
Copy link
Member

nalind commented Jan 26, 2021

That's encoded at

return "|" + strconv.Itoa(len(strings.Split(buildArgs, " "))) + " " + buildArgs + " /bin/sh -c " + node.Original[4:]
so that we can use all argument values to better avoid (or ensure) a cache hit when the value for the specified argument value changes (or doesn't). It looks like we're encoding them differently than docker build does for this case.

@rhatdan
Copy link
Member

rhatdan commented Jan 26, 2021

Should we just not encode the PROXY environment and encode everything else?

@rhatdan
Copy link
Member

rhatdan commented Feb 12, 2021

Since you could use volumes or secrets now, you have alternatives that are better then just hacking on environment variables. I am going to close this issue.

@rhatdan rhatdan closed this as completed Feb 12, 2021
@rolweber
Copy link
Author

It's correct that my problem is resolved. But the reason why docker hides the proxy-related implicit build args is that they may contain basic auth credentials for an internal proxy. Folks that intend to use podman as a drop-in replacement for docker may unknowingly expose their proxy credentials in the image history.

@rhatdan rhatdan reopened this Feb 12, 2021
@github-actions
Copy link

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

@rhatdan
Copy link
Member

rhatdan commented May 25, 2021

@ashley-cui what is the state of buildah secrets?

@ashley-cui
Copy link
Member

ashley-cui commented Jun 3, 2021

@rhatdan
Copy link
Member

rhatdan commented Jun 7, 2021

@rolweber Would the use of secrets satisfy this issue?

@rolweber
Copy link
Author

rolweber commented Jun 7, 2021

Not really, see #2937 (comment).

The issue exists for people who specify a build-time proxy with basic auth credentials as an implicit build argument. With docker, their credentials are safe. With podman, their credentials appear in the image history. Why should they have to rewrite their Dockerfile to use secrets, if they only want to specify a default proxy for the build?

But I'll remember these RUN secrets for my own purposes :-)

@github-actions
Copy link

github-actions bot commented Jul 8, 2021

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

@rhatdan
Copy link
Member

rhatdan commented Jul 8, 2021

@ashley-cui Any update on this?

@ashley-cui
Copy link
Member

Have not gotten to it yet.

@github-actions
Copy link

github-actions bot commented Aug 8, 2021

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

@rhatdan
Copy link
Member

rhatdan commented Aug 9, 2021

@ashley-cui Any time to work on this?

@ashley-cui
Copy link
Member

Probably can get to it this week or the next :)

@github-actions
Copy link

github-actions bot commented Sep 9, 2021

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

@github-actions
Copy link

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

@rhatdan
Copy link
Member

rhatdan commented Oct 12, 2021

@flouthoc PTAL

@flouthoc
Copy link
Collaborator

Hi @rolweber , Apologies for late response.

Above PR should close this issue and matches buildah behavior to docker/buildkit

flouthoc added a commit to flouthoc/buildah that referenced this issue Feb 15, 2022
`Buildkit/Docker` adds variables in pre-allowlist for e.g `proxy
variables` to OCI/Docker history only if user explicitly specifies them in
Dockerfile using `ARG`.

By default variables in pre-allowlist e.g `proxy variables` will be used
normally but will not be leaked into `docker/OCI` history of images.

A test for following behviour is added with this commit and similar test
can be verified against `Docker/Buildkit`

Closes: containers#2937

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/buildah that referenced this issue Feb 15, 2022
`Buildkit/Docker` adds variables in pre-allowlist for e.g `proxy
variables` to OCI/Docker history only if user explicitly specifies them in
Dockerfile using `ARG`.

By default variables in pre-allowlist e.g `proxy variables` will be used
normally but will not be leaked into `docker/OCI` history of images.

A test for following behviour is added with this commit and similar test
can be verified against `Docker/Buildkit`

Closes: containers#2937

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/buildah that referenced this issue Feb 15, 2022
`Buildkit/Docker` adds variables in pre-allowlist for e.g `proxy
variables` to OCI/Docker history only if user explicitly specifies them in
Dockerfile using `ARG`.

By default variables in pre-allowlist e.g `proxy variables` will be used
normally but will not be leaked into `docker/OCI` history of images.

A test for following behviour is added with this commit and similar test
can be verified against `Docker/Buildkit`

Closes: containers#2937

Signed-off-by: Aditya R <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants