opencontainers / image-tools

OCI Image Tooling
https://opencontainers.org
Apache License 2.0
266 stars 83 forks source link

Rootless extraction and non-writable directories #208

Open besnardjb opened 6 years ago

besnardjb commented 6 years ago

Dear maintainers,

Problem

I'm currently using 'oci-image-tool' to extract OCI image in a rootless configuration to explore scenarios in multi-user systems involving containers. I've encountered an issue where the parent directory was not user writable (particularly in the centos image) and therefore as a regular user the extraction failed. Here are the steps leading me to the issue (with master @ c95f76c):

$ skopeo copy docker://centos oci:centos_image:latest
$ oci-image-tool create --ref name=latest ./centos_image ./centos_bundle/
open centos_bundle/rootfs/root/.bash_logout: permission denied
unable to open file [...]/opencontainers/image-tools/image/manifest.go:296
(...)

Here are the rights for /root in the Centos image extracted as root:

dr-xr-x---. 2 root root 4,0K 5 août 00:05 root

Note that the same happens for /usr/bin in this image.

My Solution

Given my limited knowledge of the code itself, I've devised a small fix which simply consists in checking that the parent directory is user-writable, and setting it so before manipulating files. The original permission is then restored afterward.

I'm not sure it is the most elegant approach (or maybe I missed someting obvious) but at least this small patch solved my issue, allowing me to unpack the centos image without being root.

Thanks!

Jean-Baptiste.

cyphar commented 6 years ago

I would recommend looking at https://github.com/openSUSE/umoci. It already has supported rootless unpacking for almost two years now (it's done in a somewhat similar fashion to what you do -- but it's a bit more complicated than just touching the parent). If you really want to change oci-image-tool to support this, then consider using the package I implemented as part of umoci -- github.com/openSUSE/umoci/pkg/unpriv.

umoci has also historically been far more rigorously tested, and has far more assurances that the image extract is the one you wanted. In addition, umoci supports generating new layers without having users do any form of copying (it uses manifests to calculate layer differences -- without needing special filesystems).

besnardjb commented 6 years ago

Hi Cyphar,

Thank you very much for taking the time to answer my question and for your advice. Indeed, my patch is somewhat hacky. In fact, I started with umoci which appeared to be the way to go. However, I've encountered some small issues that I still have to dig a little more. For example, umoci tries to remove the "system.nfs4_acl" attribute leading to EIO errors when extracting on a shared FS (works in /tmp):

$ strace -e lremovexattr umoci --log debug  unpack --rootless --image ./centos_image ./centos_bundle
   • parsed mappings           map.gid=[{1000 0 1}] map.uid=[{1000 0 1}]
  [...]
   • unpack layer: sha256:256b176beaff7815db2a93ee2071621ae88f451bb1e198ca73010ed5bba79b65
   • unpacking entry           path=anaconda-post.log root=centos_bundle/rootfs type=48
lremovexattr("centos_bundle/rootfs/anaconda-post.log", "system.nfs4_acl") = -1 EIO (Input/output error)
lremovexattr("centos_bundle/rootfs", "system.nfs4_acl") = -1 EIO (Input/output error)
   ⨯ input/output error
unpriv.lremovexattr
github.com/openSUSE/umoci/pkg/unpriv.Lremovexattr
    GOPATH/openSUSE/umoci/pkg/unpriv/unpriv.go:481

I'm going to further look into it (it is probably a matter of an if), I'll report to the UMOCI repo.

Thanks!

cyphar commented 6 years ago

However, I've encountered some small issues that I still have to dig a little more. For example, umoci tries to remove the "system.nfs4_acl" attribute leading to EIO errors when extracting on a shared FS (works in /tmp):

Feel free to report an issue with umoci (it looks like a real issue) -- but I already have an idea what the fix would be. For context, the reason why we remove xattrs is to make it so that the extracted rootfs is as close to the image as possible. However some xattrs cause issues when removing them -- or even including them in images (security.selinux is a good example). I guess system.nfs4_acl should be added to the same list.