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

Copy/Move images between users #2979

Closed
langdon opened this issue Apr 19, 2019 · 36 comments
Closed

Copy/Move images between users #2979

langdon opened this issue Apr 19, 2019 · 36 comments
Assignees
Labels
do-not-close kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue

Comments

@langdon
Copy link

langdon commented Apr 19, 2019

/kind feature

Description
I often accidentally build an image as the "wrong" user. For example, as me vs as root. I would really like a way to "tag" the image in to the other user so that a) I don't have to build it again; b) it takes up the same space on disk as only one copy. Given the nature of images, as soon as they start to diverge they could just create the new layers in the user that "forked."

I know I could build a container from the image, export it, then import the tarball as the other user but that is pretty tedious and doesn't let me only have "1" copy.

Output of podman version:

podman version 1.2.0
@openshift-ci-robot openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 19, 2019
@rhatdan
Copy link
Member

rhatdan commented Apr 20, 2019

So you want to copy it from the root storage to your user storage or from your storage to root?
We have the beginnings of support for this in Skopeo, which now can understand how to pull form non root.

@Fodoj
Copy link
Contributor

Fodoj commented Apr 20, 2019

Something I would love to have implemented as well

@kunalkushwaha
Copy link
Collaborator

Supporting this use-case will also fix major part of issue "redundant image pull shall be avoided (rootless & root)"

@langdon
Copy link
Author

langdon commented Apr 22, 2019

So you want to copy it from the root storage to your user storage or from your storage to root?
We have the beginnings of support for this in Skopeo, which now can understand how to pull form non root.

Correct. And/or user1 -> user2. However, it would nice if the implementation underneath was actually just "tagging" so I would only have one actual copy of the image on disk. But that is just a nicety.

@baude
Copy link
Member

baude commented May 29, 2019

theoretically this could be done with a secondary image store shared by everyone.

@rhatdan
Copy link
Member

rhatdan commented May 29, 2019

I think this might be a skopeo issue. Where we could easily copy between two different containers/storage.

@rhatdan
Copy link
Member

rhatdan commented Aug 5, 2019

No update on this issue. Would love to get someone looking at this in the community

@kunalkushwaha
Copy link
Collaborator

I would like to take this up.

/assign

@rhatdan
Copy link
Member

rhatdan commented Aug 30, 2019

@kunalkushwaha Thanks. I have been thinking a little about this. Basically allowing tools to grow the ability to copy between different containers storage. Either between users or between hosts.

I look forward to your ideas.

@kunalkushwaha
Copy link
Collaborator

I tried to think from couple of point of views, how to enable sharing of images between different users on a system.

  • UX:

    1. user should be able to list images of other users/all users
      • podman images -u <user-name>
      • podman images --all-users
    2. Copy/import image from other user.
      • podman image copy -u <user-name> <image-name>
  • Implementations:
    Each user should be configure, if they want to share images? (libpod.conf) default : false

    1. Sharing of content store

      • Change permission of content store folders to 755, so other users can read and extract the images.
      • extract the image in local graph folder
      • This will let, podman images -u|-a, work without sudo/su.
        • no duplication of tar files.
      • changing folder settings, will allow all images to be visible to other users.
      • do not break the existing user setup.
    2. Create a common share content folders.

    • while images search i.e. podman images -u|-a user need to use sudo | su.
    • while copy/import copy the contents of image into common content folder.
      • this will be required only once by any user, later will be available for all users.
      • images from common content folder can be listed along with users image.
    • drawback, sudo | su can be misused.
    • overhead of maintaining common content folder.
      - how/who can delete the images etc.
    1. For users opts for sharing image, keep all images in common content folder.
    • This will be single content folders for all users.
    • extract the images in local folder.
    • benefit:
      • no duplication of data.
      • management will be simple (all users can add/delete images)[a reference count can be used to delete for all users]
    • drawback:
      • enabling/disabling setting for sharing images, require extra tooling to sync data to common to private content store and vice versa.
      • ability to delete images

@rhatdan Please share your views on each approaches.

@matpen
Copy link

matpen commented Sep 6, 2019

I am just a regular user and by no means an expert of the internals of skopeo and podman, however I wanted to share a few ideas based on our use case.

We plan to have some images to be run by different system users on the same VPS. With the current implementation, this means that the images must be duplicated for each user, which is largely inefficient in terms of disk space.

So ideally there would be a way to set "permissions" on a image or blobs, so that other system users can base their images on those. The root user would share them in a way similar to unix permissions (users/groups).

It therefore seems to me that the major obstacle is the implementation of such a "permission system". With this in mind, here a few direct comments to the above list of ideas:

user should be able to list images of other users/all users

Would be nice to have, subject to the above "permission system". E.g an unprivileged user could want to list "system images" shared by root.

Copy/import image from other user.

A "copy" is quickly achievable with podman save/load. What would be nice to have is the disk-efficient "sharing" of blobs, so that some kind of "copy-on-write" is achieved (in the same way as it already is for the images of a single user).

Change permission of content store folders to 755, so other users can read and extract the images.

It would be nice to be able to share only some images, and not the whole store. I do understand that this means setting permissions on a lots of blobs, so I figure that some specialized interface is needed.

drawback, sudo | su can be misused.

With the "permission" idea, it would work like any other file.

overhead of maintaining common content folder.

  • how/who can delete the images etc.

This is where the "copy-on-write" would be nice. btrfs?

@rhatdan
Copy link
Member

rhatdan commented Sep 6, 2019

The big issue here is permissions on individual files. Using additionalstores will not work since we have to lock the storage. We could potentially fix this in storage to allow storage use without locks, perhaps if mounted read/only.

But we still have issues with the ownership of individual files within the image.

@rhatdan
Copy link
Member

rhatdan commented Sep 6, 2019

@nalind @vrothberg @giuseppe @mtrmac WDYT?

@giuseppe
Copy link
Member

giuseppe commented Sep 6, 2019

I thought about having the possibility to share images among users/system storage (owned by root), but that requires changes in the file system. This should be possible in fuse-overlayfs.

The idea is that every file is stored with permissions 0755, so only the owner can modify them, and the original permission is stored in an extended attribute.

fuse-overlayfs then could look into different storages, either owned root or inside other users and be able to read all the files/directories there. fuse-overlayfs would then use the data in the xattr to restore the file permissions back so that the container would not see any difference.

The main advantage is that there are no copies involved among different storages. if needed the image can still be copied locally to a different storage, but that should not be required.

We'd need to mark these shared images specially though, as they cannot be used normally otherwise the file permissions will be wrong (every file in the container has permissions 0755)

@mtrmac
Copy link
Collaborator

mtrmac commented Sep 6, 2019

The idea is that every file is stored with permissions 0755, so only the owner can modify them, and the original permission is stored in an extended attribute.

That doesn’t work in all the general ways some in here wish for (BTW the conversation seems to be wavering between just making fresh copies. If an unprivileged user can share a container layer to be used by other users, the unprivileged user owning the layer can maliciously modify the data (any of directory structure, metadata, contents of individual files), attacking at the very least others’ containers, and at least until a complete audit of the c/storage code base and possibly all callers, extremely likely the rest of the others’ accounts as well.

Storage that is truly opportunistically shared between unprivileged users requires a trusted (and probably privileged) third party, i.e. a daemon.

Or something like the “additional stores” facility, with a fixed and trusted set of content that can be consumed, but not modified or added to, by unprivileged users. I’m not sure what the “will not work since we have to lock the storage” part refers to, that may well be a blocker — but vaguely speaking at a high level, of all the random things mentioned in here, this one seems to at least have a chance of working.:

  • Truly shared is insecure as mentioned above
  • skopeo copy between two different non-root storages would require two different non-root permissions, i.e. an multiple-process architecture passing the image data though some kind of IPC — just using an intermediate dir: staging probably works acceptably enough without building all that special-purpose code.

@rhatdan
Copy link
Member

rhatdan commented Sep 6, 2019

I am also concerned about world writable content like CONTAINER/tmp, /var/tmp. Also access to setuid programs within these directories could cause issues.

@mtrmac
Copy link
Collaborator

mtrmac commented Sep 6, 2019

Oh, the contents of the containers (as opposed to the immutable parent layers) needs to definitely stay user-specific and inaccessible to other users; we can only be discussing the immutable layers of committed images (i.e. something pretty close to the “additional stores”); it’s just that even keeping the immutable layers immutable seems pretty difficult.

(So, if “additional stores” are unworkable because of some kind of locking problems, everything about the more general sharing concept is probably unworkable by extension.)

@giuseppe
Copy link
Member

giuseppe commented Sep 7, 2019

I was talking about the immutable layers of the image. Containers data cannot be shared.

Accessing another storage requires to trust the owner. I am not proposing to do/enable by default for all users. We would need to think of another model (checksumming each file?) if we want to use not trusted storages.

@rhatdan In such shared model, every file/dir become 0755 so there won't really be any setuid binary hanging around. The permissions will be restored by fuse-overlay on the fly when these files are used in a lower layer, so the container will see the original mask but that is then applied in the user running fuse-overlays context

The 0755 for all files makes sure only the owner can really modify these files.

@mtrmac
Copy link
Collaborator

mtrmac commented Sep 7, 2019

We would need to think of another model (checksumming each file?) if we want to use not trusted storages.

Checksumming at what point? The attacker can reshuffle the FS structure at any time, or modify a file that the victim has mmap()ed.

The only way a checksum/verification step would work sense if the verifier also made a private unmodifiable copy as a part of the process — but that’s just extra overhead on top of having unshared per-user stores; I suppose it would save some network bandwidth, but a local HTTP proxy would do that just as well at much lower cost.

@giuseppe
Copy link
Member

giuseppe commented Sep 7, 2019

Checksumming each file/directory. Then the checksum is checked after a local copy exists (or a reflink if possible). This could still be faster than duplicating the whole image as most likely not all files are accessed thus needed to be copied. Anyway I think we should look at the trusted case first, where only trusted storages are used

@vrothberg
Copy link
Member

Maybe we can take a few steps back and first define which problem(s) we're trying to solve?

Problem 1) The title of the issue suggests to allow for copying/moving images across storages. I think that's a valuable addition, assuming we find a way to avoid a complex CLI.

Problem 2) The comments are pointing to a truly sharing storage across users. Considering the comments, this seems like a rather controversial feature.

I prefer to first focus on Problem 1) to provide a simple solution for a rather complex problem.

I'd love to see support for moving/copying images across storages in Skopeo, Podman and maybe Buildah: skopeo migrate userA userB, {podman,buildah} image migrate userA userB?

  • It should be smart enough to figure out where the image storage of the source and destination users are.
  • Image storage paths could be force set via CLI options.

Given all the permission issues, maybe that's something that should be restricted to root?

@rhatdan
Copy link
Member

rhatdan commented Sep 9, 2019

I like @vrothberg simplification. I see this is more of a scp/cp issue for now. The additionalstores stuff would be nice,especially between root and non-root, so we don't have to worry about problems that @mtrmac is talking about.

I like to define containers/image as a library that allows you to move OCI images between different types of image storage. What I see us doing is enhancing it or using it to allow us to copy between two containers/storage. Root->dwalsh or vrothberg->dwalsh or dwalsh->root. Obviously this requires a privileged process to complete.

@matpen
Copy link

matpen commented Sep 9, 2019

I allow myself to pitch in again as "just a user". It seems to me that problem 1 is already solved with the current tools:

sudo podman save myimage | sudo -u myuser podman load` // copy
sudo podman rmi myimage // if a "move" is desired

Instead, the emphasis of the OP and in my previous comment is on the sharing of storage space

I know I could build a container from the image, export it, then import the tarball as the other user but that is pretty tedious and doesn't let me only have "1" copy.

Which is basically, as far as I understand, problem 2 (so that "copy+remove" is actually implemented as "share+remove", which would remove the requirement of double as much storage after the copy).

@mtrmac
Copy link
Collaborator

mtrmac commented Sep 9, 2019

I like to define containers/image as a library that allows you to move OCI images between different types of image storage. What I see us doing is enhancing it or using it to allow us to copy between two containers/storage. Root->dwalsh or vrothberg->dwalsh or dwalsh->root. Obviously this requires a privileged process to complete.

It’s not obvious how a library can get the necessary privileges, and whether it should be doing anything like that behind the caller’s back (also remember that there isn’t really a fork in Go, so this would require a c/image helper process to exist in a known path); doing this without an on-disk copy requires inventing a completely single-purpose RPC system.

Honestly all of this screams “corner case that people would like to dream about but no-one wants to write and maintain all of that complex code” to me, at this point.

Using an intermediate dir: is already possible without writing all that code, sure, at a performance cost.

As for accidentally forgetting to use sudo, alias podman='sudo podman', or an equivalent podman functionality (per-user “disable rootless” config file that redirects invocations to exec(sudo $@)) is so much easier than writing that RPC system and dealing with the privileges, that anyone contemplating native support for the cross-user copies should IMHO very strongly consider any of the alternatives.

@kunalkushwaha
Copy link
Collaborator

Hi @mtrmac @giuseppe, I have one doubt. Can we separate out image download logic with untar to file-system?

Downloaded tars + metadata (manifest) can be shared among users. I think that should not have any security concern.

vrothberg added a commit to vrothberg/storage that referenced this issue Sep 11, 2019
Wrap the ID or the digest to ErrImageUnknown errors to avoid ambiguity
which image is unknown.  Consumers of the storage library may have
multiple subsequent calls to the storage API where it can be unclear
which image is unknown.  Wrapping the ID and digest attempts to avoid
this ambiguity.

Related-to:    github.com/containers/podman/issues/2979
Signed-off-by: Valentin Rothberg <[email protected]>
@vrothberg
Copy link
Member

@kunalkushwaha, that's already being worked on in containers/image#611 but more unlikely to get merged any time soon. Note that avoiding such redundant downloads is orthogonal to the request of this issue but very different.

@mtrmac
Copy link
Collaborator

mtrmac commented Sep 11, 2019

Downloaded tars + metadata (manifest) can be shared among users. I think that should not have any security concern.

That would be a bit better because the compressed layers would never be mmap()ed, we already have some infrastructure to deal with untrusted registry storage, and it would be new code explicitly written for such a risky environment.

Still, it’s an attack vector (add a manifest.json/home/victim/.ssh/id_rsa symlink, convince the victim to pull the image, and hope to see the error messages), and it would require extra storage for all those images. Who is going to use this? (And who is going to set this up in advance and then use this, because it probably couldn’t be the default.)


@kunalkushwaha, that's already being worked on in containers/image#611

That tries to avoid downloads to a single store; this issue discusses sharing data between multiple container stores owned by multiple mutually untrusted users.

@kunalkushwaha
Copy link
Collaborator

One way I feel sharing of images can be done without much changes in current workflow is as below.

  • The .. .local/share/containers/storage/vfs-layers & .. .local/share/containers/storage/vfs-layers of users will be changed to 0755 mode.

  • When user foo wants to list the images of user bar too, shall use command e.g podman images -u bar

    • Here, based on parsing of files images.json, images from user bar can be displayed.
      • images from other user can be shown with prefix the <user-name>:
      • Since this is corner case, but yet very useful, parsing (slow opetation) may be acceptable.
      • Though not sure, how detailed information can be shown here.
      • e.g.
$ podman images -u bar
REPOSITORY                 TAG      IMAGE ID       CREATED         SIZE
docker.io/library/alpine   latest   961769676411   3 weeks ago     5.85 MB
k8s.gcr.io/pause           3.1      da86e6ba6ca1   21 months ago   747 kB
bar:localhost/kk/test      latest   bd8asdrfa4c1   --              --             
  • A user foo can import he image from user bar with command like
    podman image copy -u bar localhost/kk/test:latest

    • On this manifest and tar blobs can be read from bar user folders (which is read-only for user foo)
    • image will be extracted to .. .local/share/containers/storage/vfs folder.
    • Since each user can have different graph file-system, complexity of file-system to file-system copy will not be there
  • Few things we need to still think like

    • how to keep reference of such imported images in image store?
    • Will user foo be allowed to push the base image?

Please let me know WDYT on this.
I can try to implement as prototype to see any more practical difficulties that we may face while implementation.

@kunalkushwaha
Copy link
Collaborator

ping @mtrmac @vrothberg

@vrothberg
Copy link
Member

@giuseppe worked on fuse-overlayfs (containers/fuse-overlayfs#119) to enable initial support. I currently have too many things on my table to think the details through but try to come back to it asap.

@github-actions
Copy link

github-actions bot commented Nov 4, 2019

This issue had no activity for 30 days. In the absence of activity or the "do-not-close" label, the issue will be automatically closed within 7 days.

@rhatdan
Copy link
Member

rhatdan commented Nov 4, 2019

@giuseppe Any update on this?

@vrothberg
Copy link
Member

Friendly ping.

@rhatdan
Copy link
Member

rhatdan commented Jan 31, 2020

I don't think we have come up with a good solution for this, so I am thinking it might be time to close this issue.

@mikelpr
Copy link

mikelpr commented Feb 1, 2020

is there a manual way of moving an image to the root storage then? in the absence of support in the tooling

@vrothberg
Copy link
Member

is there a manual way of moving an image to the root storage then? in the absence of support in the tooling

I don't think we have come up with a good solution for this, so I am thinking it might be time to close this issue.

I agree. As mentioned above, the tools allow copying images with some intermediate steps. Doing that in one command while addressing all possible permission issues etc., however, is quite tough.

@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 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-close kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue
Projects
None yet
Development

No branches or pull requests