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

Bad image used by podman run with an image with a tag then without an explicit tag #11964

Closed
guillaumerose opened this issue Oct 14, 2021 · 26 comments · Fixed by #12010
Closed
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

@guillaumerose
Copy link
Contributor

guillaumerose commented Oct 14, 2021

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

/kind bug

Description

With podman machine on macOS:

When running podman run with an explicit tag then the same image without a tag, podman uses the first image it found in his storage.
It should pick latest instead (if we want to match what docker is doing).

Steps to reproduce the issue:

  1. podman run -it ubuntu:18.04 cat /etc/os-release

  2. podman run -it ubuntu cat /etc/os-release

Describe the results you received:

$ podman run -it ubuntu:18.04 cat /etc/os-release
Resolved "ubuntu" as an alias (/etc/containers/registries.conf.d/000-shortnames.conf)
Trying to pull docker.io/library/ubuntu:18.04...
Getting image source signatures
Copying blob sha256:284055322776031bac33723839acb0db2d063a525ba4fa1fd268a831c7553b26
Copying blob sha256:284055322776031bac33723839acb0db2d063a525ba4fa1fd268a831c7553b26
Copying config sha256:5a214d77f5d747e6ed81632310baa6190301feeb875cf6bf9da560108fa09972
Writing manifest to image destination
Storing signatures
PRETTY_NAME="Ubuntu 18.04.6 LTS"
...

$ podman run -it ubuntu cat /etc/os-release 
PRETTY_NAME="Ubuntu 18.04.6 LTS"
...

Describe the results you expected:

Second call without the tag should use ubuntu:latest image like Docker.

$ docker run -it ubuntu:18.04 cat /etc/os-release
Unable to find image 'ubuntu:18.04' locally
18.04: Pulling from library/ubuntu
284055322776: Pull complete 
Digest: sha256:bfb4cabd667790ead5c95d9fe341937f0c21118fa79bc768d51c5da9d1dbe917
Status: Downloaded newer image for ubuntu:18.04
PRETTY_NAME="Ubuntu 18.04.6 LTS"
...

$ docker run -it ubuntu cat /etc/os-release 
Unable to find image 'ubuntu:latest' locally
latest: Pulling from library/ubuntu
f3ef4ff62e0d: Pull complete 
Digest: sha256:a0d9e826ab87bd665cfc640598a871b748b4b70a01a4f3d174d4fb02adad07a9
Status: Downloaded newer image for ubuntu:latest
PRETTY_NAME="Ubuntu 20.04.3 LTS"
...

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

Client:
Version:      3.4.0
API Version:  3.4.0
Go Version:   go1.17.2
Built:        Thu Sep 30 20:44:31 2021
OS/Arch:      darwin/amd64

Server:
Version:      3.4.0
API Version:  3.4.0
Go Version:   go1.16.8
Built:        Thu Sep 30 21:32:16 2021
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.23.1
  cgroupControllers: []
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: conmon-2.0.30-2.fc35.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.0.30, commit: '
  cpus: 1
  distribution:
    distribution: fedora
    variant: coreos
    version: "35"
  eventLogger: journald
  hostname: linux
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
  kernel: 5.14.10-300.fc35.x86_64
  linkmode: dynamic
  logDriver: journald
  memFree: 1273630720
  memTotal: 2061524992
  ociRuntime:
    name: crun
    package: crun-1.2-1.fc35.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.2
      commit: 4f6c8e0583c679bfee6a899c05ac6b916022561b
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
  os: linux
  remoteSocket:
    exists: true
    path: /run/user/1000/podman/podman.sock
  security:
    apparmorEnabled: false
    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: true
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: true
  serviceIsRemote: true
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.1.12-2.fc35.x86_64
    version: |-
      slirp4netns version 1.1.12
      commit: 7a104a101aa3278a2152351a082a6df71f57c9a3
      libslirp: 4.6.1
      SLIRP_CONFIG_VERSION_MAX: 3
      libseccomp: 2.5.0
  swapFree: 0
  swapTotal: 0
  uptime: 16m 34.83s
plugins:
  log:
  - k8s-file
  - none
  - journald
  network:
  - bridge
  - macvlan
  volume:
  - local
registries:
  search:
  - docker.io
store:
  configFile: /var/home/core/.config/containers/storage.conf
  containerStore:
    number: 2
    paused: 0
    running: 0
    stopped: 2
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /var/home/core/.local/share/containers/storage
  graphStatus:
    Backing Filesystem: xfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  imageStore:
    number: 3
  runRoot: /run/user/1000/containers
  volumePath: /var/home/core/.local/share/containers/storage/volumes
version:
  APIVersion: 3.4.0
  Built: 1633030336
  BuiltTime: Thu Sep 30 19:32:16 2021
  GitCommit: ""
  GoVersion: go1.16.8
  OsArch: linux/amd64
  Version: 3.4.0

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)

Yes

Additional environment details (AWS, VirtualBox, physical, etc.):

Installed with brew on macOS.

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

rhatdan commented Oct 14, 2021

@mtrmac @vrothberg @mheon WDYT?

@vrothberg
Copy link
Member

That is interesting, thanks @guillaumerose!

To explain the behavior. If we're looking for a short name such as ubuntu, we will look into the local storage and try to find an image that matches this name regardless of the tag. Docker seems to first normalize the tag to ubuntu:latest and since no local image matches, it will pull down ubuntu:latest.

@vrothberg vrothberg self-assigned this Oct 14, 2021
@vrothberg
Copy link
Member

we will look into the local storage and try to find an image that matches this name regardless of the tag

Note that latest has precedence though. I am torn. It may very well break existing Podman users; the behavior is pretty old.

@vrothberg
Copy link
Member

@rhatdan WDYT?

@rhatdan
Copy link
Member

rhatdan commented Oct 14, 2021

Lets fix it for podman 4.0. I think we state that when you don't specify a tag, it defaults to latest. But I agree this is a breaking change.

@vrothberg
Copy link
Member

If we change that in libimage, it would also impact Buildah (unless we make it optional). I just want to make sure everybody is on board.

In short: Docker always uses the latest tag, Podman/Buildah would use latest only if there is a matching local image with local or no matching image. If there's a non-latest local matching image the tag would be used (in this example 18.04).

@rhatdan
Copy link
Member

rhatdan commented Oct 14, 2021

Lets talk at standup.

@mtrmac
Copy link
Collaborator

mtrmac commented Oct 14, 2021

Personally, the (what is now) inRepoTags matching always looked unexpected to me, but I could never find a rationale for why it was that way, so I preserved it as best as I could; if we could just drop that, I’d like it very much.

@vrothberg vrothberg added the 4.0 label Oct 15, 2021
vrothberg added a commit to vrothberg/common that referenced this issue Oct 18, 2021
For reasons buried in the history of Podman, looking up an untagged
image would match *any* tag of matching image.  For instance,
looking up `centos` would match a local image `centos:foobar`.
Docker only looks for `centos:latest`.

Context: github.com/containers/podman/issues/11964
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
vrothberg added a commit to vrothberg/buildah that referenced this issue Oct 19, 2021
Testing changes for github.com/containers/podman/issues/11964 in Buildah

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
vrothberg added a commit to vrothberg/buildah that referenced this issue Oct 19, 2021
Testing changes for github.com/containers/podman/issues/11964 in Buildah

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
@vrothberg
Copy link
Member

Since I am on it: How do you want the remainder of image look up to behave? We already decided that foo must always resolve to foo:latest. But what about the rest. Let's consider the following example:

  • We have the following search registries: reg.a, reg.b
  • We have the following image in the local storage reg.c/test:latest

Now we want to look up test. How should it behave?

At the time of writing, Podman will check for matching images in localhost/test, reg.a/test, reg.b/test and then try really hard to find matching images in the local storage and eventually find reg.c/test:latest.

If we got rid of the "trying really hard" part, Podman would not be able to find reg.c/test:latest. Note that Docker behaves exactly like that but a lot of our tests break (see #12010).

I just want confirmation before tackling more tests.

@vrothberg
Copy link
Member

@rhatdan @mtrmac PTAL

@mtrmac
Copy link
Collaborator

mtrmac commented Oct 20, 2021

Only using the search registries, and in that order (not even preferring already-local reg.b/test over a non-yet-pulled reg.a/test) makes most conceptual sense to me; that way, podman {pull,run} test refers to the same images with empty storage and after years of history, and it is possible to write reliable instructions or scripts.

I have absolutely no idea whether that’s practical to change now. Looking at the tests in #12010, I think it’s a good guess that quite a few users (and scripts) everywhere now depend on the liberal lookup behavior — but I have no way to quantify that.

@vrothberg
Copy link
Member

@baude PTAL

@vrothberg
Copy link
Member

@mtrmac, I share your thoughts. I think it's cleaner but I am terrified of how impactful this change would be.

A middle ground would be to stop matching foo to non-latest tags but keep the logic for matching any local image with the same name.

@mtrmac
Copy link
Collaborator

mtrmac commented Oct 20, 2021

Perhaps — I have absolutely no idea whether users rely more on lax tag matching or on lax repo matching. It could be either one.

@vrothberg
Copy link
Member

Fair point. We could also just do nothing and keep things are they are 😈 So far, I've only seen one complaint/issue on the current behavior.

@rhatdan
Copy link
Member

rhatdan commented Oct 21, 2021

Why can't we have both. Always append on the :latest and then return as we do now, if there are multiple foobar:latest?

@vrothberg
Copy link
Member

@rhatdan, we can do that but it is not what Docker does. I just want to make sure that everybody is on board with this "middle ground". Some users may file an issue down the road if they rely on this specific behavior of Docker.

@mtrmac
Copy link
Collaborator

mtrmac commented Oct 21, 2021

Always append on the :latest and then return as we do now, if there are multiple foobar:latest?

I’m not sure what you are proposing. Are you saying we should just keep the current behavior (where running ubuntu might not run ubuntu:latest)? That we should only strictly look for foobar:latest as specified (what Valentin originally implemented)? That we should allow the kind of loose matches the code currently looks for, but only if there is exactly one loose match? Something else?

@rhatdan
Copy link
Member

rhatdan commented Oct 21, 2021

No I am saying if user specifies ubuntu we look for ubuntu:latest, If it exists as

localhost/ubuntu:latest
docker.io/ubuntu:latest

Then we return to the old system and return localhost/ubuntu (Which I think is what we do now).
If we had
localhost/ubuntu:2.5
docker.io/ubuntu:1.0
And the user specified ubuntu we would either pull ubuntu:latest off of container registries, or return an error.

What does Docker do in scenario one now?

@thoraxe
Copy link

thoraxe commented Oct 21, 2021

There is a theoretical security issue here.

If someone can tag an image into the system that has a higher search priority, an ambiguous run request could result in the unintended image being run, no? It's a bit of a long shot security problem, because my assumption is that someone with the access to inject a malicious tag has sufficient access already to run a malicious image.

@mtrmac
Copy link
Collaborator

mtrmac commented Oct 21, 2021

For an attacker with access to the full Podman surface (whether through CLI or API), I can’t see that this could be a privilege escalation.

For a hypothetical platform that shares a single Podman instance among unrelated parties, and which allows clients to cause arbitrary images to be pulled and run, yes, this could result in one client affecting another, in principle. I’m not sure such a thing could be built in a way that isolates the parties enough, but I suppose it could be possible.

@rhatdan
Copy link
Member

rhatdan commented Oct 25, 2021

A hacker like this could also remove the image, and create and tag a new image to attack future users. I don't see this as something we should care about.

@mtrmac
Copy link
Collaborator

mtrmac commented Oct 25, 2021

That really depends on what API operations the platform actually passes through to Podman (it can’t be a full access to the API like tagging images) — but I agree it can only matter in a quite unlikely scenario.

@vrothberg
Copy link
Member

vrothberg commented Nov 12, 2021

I want to find consensus on how to proceed. To summarize the issue:

  • We have consensus that Podman should only resolve to local images with the latest tag. Hence, even if there's a local image foo:123 Podman would (conceptually) resolve foo to foo:latest and ignore foo:123.

We do not have consensus on the already mentioned scenario:

* We have the following search registries: `reg.a`, `reg.b`

* We have the following image in the local storage `reg.c/test:latest`

Now we want to look up test. How should it behave?

At the time of writing, Podman will check for matching images in localhost/test, reg.a/test, reg.b/test and then try really hard to find matching images in the local storage and eventually find reg.c/test:latest.

If we got rid of the "trying really hard" part, Podman would not be able to find reg.c/test:latest. Note that Docker behaves exactly like that but a lot of our tests break (see #12010).

I just want confirmation before tackling more tests.

I think we run at risk of breaking existing Podman users. In theory, we also run at risk with at breaking users with the latest-only normalization but I think that's more justifiable.

Shall we perform the 2nd change, yay or nay?

@rhatdan @mtrmac @baude

@mtrmac
Copy link
Collaborator

mtrmac commented Nov 13, 2021

My fairly weak vote is in favor of strictly matching only exactly the requested repo (or a repo that results from the application of localhost/ or the search list); and a very weak vote in favor of prioritizing the search order over local image availability (per #11964 (comment) ).

Due to the risk fo breakage, either should probably happen only during a major version bump.

(Another thing to possibly consider is to keep the old lookup code, but only for an error “old version would have found and used $result; if you want to use it, use that name explicitly”. OTOH perhaps that would be more infuriating than just breaking without showing that we could continue to keep that command working but we chose not to? Either way, that would be a non-trivial amount of rarely-used code to adapt/maintain, a maintenance headache.)

(This is one of those times having a robust telemetry infrastructure that would allow us to gather statistics sounds attractive.)

@vrothberg
Copy link
Member

Thanks, @mtrmac. I hope to find time to tackle it next week.

vrothberg added a commit to vrothberg/common that referenced this issue Nov 22, 2021
Make sure to enforce the "latest" tag when looking up images in the
local storage.

Context: containers/podman/issues/11964
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
@vrothberg vrothberg removed the machine label Nov 22, 2021
vrothberg added a commit to vrothberg/common that referenced this issue Nov 22, 2021
Make sure to enforce the "latest" tag when looking up images in the
local storage.  Also make sure that digested short-names are subject
to the extended digest lookups.

Context: containers/podman/issues/11964
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
vrothberg added a commit to vrothberg/common that referenced this issue Nov 22, 2021
Make sure to enforce the "latest" tag when looking up images in the
local storage.  Also make sure that digested short-names are subject
to the extended digest lookups.

Context: containers/podman/issues/11964
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
vrothberg added a commit to vrothberg/common that referenced this issue Nov 22, 2021
Make sure to enforce the "latest" tag when looking up images in the
local storage.  Also make sure that digested short-names are subject
to the extended digest lookups.

Context: containers/podman/issues/11964
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
vrothberg added a commit to vrothberg/common that referenced this issue Nov 22, 2021
Make sure to enforce the "latest" tag when looking up images in the
local storage.  Also make sure that digested short-names are subject
to the extended digest lookups.

Context: containers/podman/issues/11964
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
vrothberg added a commit to vrothberg/libpod that referenced this issue Nov 23, 2021
For reasons buried in the history of Podman, looking up an untagged
image would match any tag of matching image. For instance, looking up
centos would match a local image centos:foobar.  Change that behavior
to only match the latest tag.

Fix: containers#11964
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
@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.

5 participants