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

Secret src not working if env variable matches id #5282

Closed
KenMacD opened this issue Jan 18, 2024 · 10 comments · Fixed by #5784
Closed

Secret src not working if env variable matches id #5282

KenMacD opened this issue Jan 18, 2024 · 10 comments · Fixed by #5784

Comments

@KenMacD
Copy link

KenMacD commented Jan 18, 2024

Description

When building with a secret, that secret is empty if an environment variable exists with a name matching the id of the secret.

Steps to reproduce the issue:

  1. Create a simple Dockerfile using a secret
FROM busybox

RUN --mount=type=secret,id=FOO \
    echo $FOO && \
    ls -al /run/secrets && \
    echo -n "/run/secrets/FOO is: " && \
    cat /run/secrets/FOO && \
    echo ""
  1. Create a file to store the secret:
echo -n "SECRET" >bar
  1. Build works correctly without environment variable:
$ env -u FOO buildah build --secret "id=FOO,src=bar" .
STEP 1/2: FROM busybox
STEP 2/2: RUN --mount=type=secret,id=FOO     echo $FOO &&     ls -al /run/secrets &&     echo -n"/run/secrets/FOO is: " &&     cat /run/secrets/FOO &&     echo ""

total 4
drwxr-xr-x    1 root     root             6 Jan 18 20:11 .
drwxr-xr-x    1 root     root            40 Jan 18 20:11 ..
-r--------    1 root     root             7 Jan 18 20:11 FOO
/run/secrets/FOO is: SECRET
COMMIT
  1. Run with an environment variable set, and secret fails:
FOO=ABCD buildah build --secret "id=FOO,src=bar" .
STEP 1/2: FROM busybox
STEP 2/2: RUN --mount=type=secret,id=FOO     echo $FOO &&     ls -al /run/secrets &&     echo -n "/run/secrets/FOO is: " &&     cat /run/secrets/FOO &&     echo ""

total 0
drwxr-xr-x    1 root     root             6 Jan 18 20:13 .
drwxr-xr-x    1 root     root            40 Jan 18 20:13 ..
-r--------    1 root     root             0 Jan 18 20:13 FOO
/run/secrets/FOO is:
COMMIT

Output of rpm -q buildah or apt list buildah:

$ readlink (which buildah)
/nix/store/b9x0ibs6p4ylqkng007z0wv36b7vd24s-buildah-wrapper-1.33.2/bin/buildah

Output of buildah version:

buildah version
Version:         1.33.2
Go Version:      go1.21.5
Image Spec:      1.1.0-rc.5
Runtime Spec:    1.1.0
CNI Spec:        1.0.0
libcni Version:  v1.1.2
image Version:   5.29.0
Git Commit:
Built:           Mon Dec 31 20:00:00 1979
OS/Arch:         linux/amd64
BuildPlatform:   linux/amd64

Output of podman version if reporting a podman build issue:

Client:       Podman Engine
Version:      4.8.3
API Version:  4.8.3
Go Version:   go1.21.5
Built:        Mon Dec 31 20:00:00 1979
OS/Arch:      linux/amd64

Output of cat /etc/*release:

DISTRIB_CODENAME=uakari
DISTRIB_DESCRIPTION="NixOS 24.05 (Uakari)"
DISTRIB_ID=nixos
DISTRIB_RELEASE="24.05"
LSB_VERSION="24.05 (Uakari)"
BUG_REPORT_URL="https://github.com/NixOS/nixpkgs/issues"
BUILD_ID="24.05.20240117.842d9d8"
DOCUMENTATION_URL="https://nixos.org/learn.html"
HOME_URL="https://nixos.org/"
ID=nixos
LOGO="nix-snowflake"
NAME=NixOS
PRETTY_NAME="NixOS 24.05 (Uakari)"
SUPPORT_URL="https://nixos.org/community.html"
VERSION="24.05 (Uakari)"
VERSION_CODENAME=uakari
VERSION_ID="24.05"

Output of uname -a:

Linux build 6.7.0 #1-NixOS SMP PREEMPT_DYNAMIC Sun Jan  7 20:18:38 UTC 2024 x86_64 GNU/Linux

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

cat /etc/containers/storage.conf
[storage]
driver = "overlay"
graphroot = "/var/lib/containers/storage"
runroot = "/run/containers/storage"
@TomSweeneyRedHat
Copy link
Member

@ashley-cui any thoughts?

@ashley-cui
Copy link
Member

Might be a bug? I'll take a look

@ashley-cui ashley-cui self-assigned this Jan 19, 2024
Copy link

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

@sebglon
Copy link

sebglon commented Oct 11, 2024

We have the same issue

@nalind
Copy link
Member

nalind commented Oct 16, 2024

The default is always to consult an environment variable if one that matches the "id" or "src" value exists. In the command line option that specifies the file, you can add the (currently undocumented, but we'll fix that) "type=file" option to force a file to be read:

buildah build --secret id=foo,src=filename,type=file ...

Or, as you would do in the "how do you remove a file named '-f' ?" case, you can alter the way the filename is specified by prepending a "./" if it's in the current directory, or using its full path, or any number of other things, so that it is no longer precisely the same as the name of an environment variable.

@KenMacD
Copy link
Author

KenMacD commented Oct 18, 2024

@nalind I think a src should always take priority over an id. I see that there's a workaround with appending ,type=file to the option, but that limits my builds to be able to be preformed with docker or podman but not both.

I don't understand why an id should ever override a src. Can you provide a example where someone would have src= set to a valid file and yet want the secret in the container to still be dependent on the environment variables of the user? This seems at best surprising and at worst a security risk.

@nalind
Copy link
Member

nalind commented Oct 21, 2024

@nalind I think a src should always take priority over an id. I see that there's a workaround with appending ,type=file to the option, but that limits my builds to be able to be preformed with docker or podman but not both.

I'm not sure I follow - when add "type=file" to the flag on a docker build command line (27.0.3-1, using a buildx that identifies itself as 0.17.1-1 257815a6fbaee88976808020bf04274388275ae8 fwiw), it understands it. I was tempted to add a "file=" while I was in there, along the lines of "env=", but that's not something docker build understands.

I don't understand why an id should ever override a src. Can you provide a example where someone would have src= set to a valid file and yet want the secret in the container to still be dependent on the environment variables of the user? This seems at best surprising and at worst a security risk.

It wouldn't have been my first choice, either, but that's the behavior that docker build (the version I have on the system where I'm writing this, at least) implements, and providing compatible behavior is the goal.

@KenMacD
Copy link
Author

KenMacD commented Oct 21, 2024

Oh, I didn't see it on the docker build --help output, so I didn't know it was supported on that side. So podman is keeping compatibility, not breaking it, so nevermind on that (and thanks for the info).

So I tried to create my error to see what I was running in to. Here's a Dockerfile for it:

FROM ubuntu:latest

RUN --mount=type=secret,id=FOO \
  echo $FOO && \
  ls -al /run/secrets && \
  echo -n"/run/secrets/FOO is: " && \
  cat /run/secrets/FOO && \
  echo ""

And a local secret stored in a text file: echo "ABCD" > foo.txt.

Now if I run it without an environment variable set I get:

STEP 1/2: FROM ubuntu:latest
STEP 2/2: RUN --mount=type=secret,id=FOO   echo $FOO &&   ls -al /run/secrets &&   echo -n"/run/secrets/FOO is: " &&   cat /run/secrets/FOO &&   echo ""

total 4
drwxr-xr-x 2 root root 46 Oct 21 19:06 .
drwxr-xr-x 6 root root 40 Oct 21 19:06 ..
-r-------- 1 root root  5 Oct 21 19:06 FOO
-n/run/secrets/FOO is:
ABCD

COMMIT
--> 3ce454ded2e3

So based on that I think this is still an open issue.

If I set $FOO though it's not that $FOO overrides the text file, it's that it breaks it entirely:

FOO=efgh podman build --secret id=FOO,src=foo.txt --no-cache -f Dockerfile
STEP 1/2: FROM ubuntu:latest
STEP 2/2: RUN --mount=type=secret,id=FOO   echo $FOO &&   ls -al /run/secrets &&   echo -n"/run/secrets/FOO is: " &&   cat /run/secrets/FOO &&   echo ""

total 0
drwxr-xr-x 2 root root 46 Oct 21 19:08 .
drwxr-xr-x 6 root root 40 Oct 21 19:08 ..
-r-------- 1 root root  0 Oct 21 19:08 FOO
-n/run/secrets/FOO is:

COMMIT
--> 4ba1cf20207f

So as you can see the environment variable is not actually used. The file is empty instead. Oddly if I remove the src= then it works with the environment variable:

FOO=efgh podman build --secret id=FOO --no-cache -f Dockerfile
STEP 1/2: FROM ubuntu:latest
STEP 2/2: RUN --mount=type=secret,id=FOO   echo $FOO &&   ls -al /run/secrets &&   echo -n"/run/secrets/FOO is: " &&   cat /run/secrets/FOO &&   echo ""

total 4
drwxr-xr-x 2 root root 46 Oct 21 19:09 .
drwxr-xr-x 6 root root 40 Oct 21 19:09 ..
-r-------- 1 root root  4 Oct 21 19:09 FOO
-n/run/secrets/FOO is:
efgh
COMMIT
--> e5bbdee5ba9a

@nalind
Copy link
Member

nalind commented Oct 21, 2024

Oh, I didn't see it on the docker build --help output, so I didn't know it was supported on that side. So podman is keeping compatibility, not breaking it, so nevermind on that (and thanks for the info).

So I tried to create my error to see what I was running in to. Here's a Dockerfile for it:

FROM ubuntu:latest

RUN --mount=type=secret,id=FOO
echo $FOO &&
ls -al /run/secrets &&
echo -n"/run/secrets/FOO is: " &&
cat /run/secrets/FOO &&
echo ""

And a local secret stored in a text file: echo "ABCD" > foo.txt.

Now if I run it without an environment variable set I get:

STEP 1/2: FROM ubuntu:latest
STEP 2/2: RUN --mount=type=secret,id=FOO   echo $FOO &&   ls -al /run/secrets &&   echo -n"/run/secrets/FOO is: " &&   cat /run/secrets/FOO &&   echo ""

total 4
drwxr-xr-x 2 root root 46 Oct 21 19:06 .
drwxr-xr-x 6 root root 40 Oct 21 19:06 ..
-r-------- 1 root root  5 Oct 21 19:06 FOO
-n/run/secrets/FOO is:
ABCD

COMMIT
--> 3ce454ded2e3

So based on that I think this is still an open issue.

I must be missing the issue here - a secret, whether or not it comes from the environment, isn't exposed through the environment, but only through the named file, and the contents of the file containing the secret appear to be correct. Being able to change that looks like a new feature in dockerfile 1.10 that we don't support yet.

If I set $FOO though it's not that $FOO overrides the text file, it's that it breaks it entirely:

FOO=efgh podman build --secret id=FOO,src=foo.txt --no-cache -f Dockerfile
STEP 1/2: FROM ubuntu:latest
STEP 2/2: RUN --mount=type=secret,id=FOO   echo $FOO &&   ls -al /run/secrets &&   echo -n"/run/secrets/FOO is: " &&   cat /run/secrets/FOO &&   echo ""

total 0
drwxr-xr-x 2 root root 46 Oct 21 19:08 .
drwxr-xr-x 6 root root 40 Oct 21 19:08 ..
-r-------- 1 root root  0 Oct 21 19:08 FOO
-n/run/secrets/FOO is:

COMMIT
--> 4ba1cf20207f

So as you can see the environment variable is not actually used. The file is empty instead. Oddly if I remove the src= then it works with the environment variable:

FOO=efgh podman build --secret id=FOO --no-cache -f Dockerfile
STEP 1/2: FROM ubuntu:latest
STEP 2/2: RUN --mount=type=secret,id=FOO   echo $FOO &&   ls -al /run/secrets &&   echo -n"/run/secrets/FOO is: " &&   cat /run/secrets/FOO &&   echo ""

total 4
drwxr-xr-x 2 root root 46 Oct 21 19:09 .
drwxr-xr-x 6 root root 40 Oct 21 19:09 ..
-r-------- 1 root root  4 Oct 21 19:09 FOO
-n/run/secrets/FOO is:
efgh
COMMIT
--> e5bbdee5ba9a

I am seeing the bug where it's still checking whether or not ${id} is set in the environment, instead of ${src}, when a "src" is specified, to determine whether to read a value from the environment or a file, and we're going to have to fix that.

@KenMacD
Copy link
Author

KenMacD commented Oct 22, 2024

So based on that I think this is still an open issue.

Sorry, I don't know how that ended up in the middle of my post. I'm sure I typed it at the end. I must have clicked accidentally.

The bug you said you're seeing is the one that that that line was supposed to be directed at. The first example worked as I would have expected it to.

@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Jan 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants