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

Skopeo leaks details about the host in oci-archives #1627

Closed
graywolf-at-work opened this issue Aug 9, 2022 · 10 comments · Fixed by #2202
Closed

Skopeo leaks details about the host in oci-archives #1627

graywolf-at-work opened this issue Aug 9, 2022 · 10 comments · Fixed by #2202
Labels
kind/bug A defect in an existing functionality (or a PR fixing it)

Comments

@graywolf-at-work
Copy link
Contributor

When running

skopeo copy docker://docker.io/library/ubuntu@sha256:b2339eee806d44d6a8adc0a790f824fb71f03366dd754d400316ae5a7e3ece3e oci-archive:/tmp/x.tar

the resulting /tmp/x.tar contains references to the current user. In particular
it seems that uname and gname are set to the current user and group. uid and
gid are set as well (I think they should simply be 0 since they are not
relevant on this level in the oci-archive). I don't think there is a good
reason to leak this information about the host running the skopeo-copy.

I think the same happening even with the layers inside the oci-archive when
doing podman save, but I assume root cause will be the same so I'm not filling
that separately (I can if it is desired).

@graywolf-at-work
Copy link
Contributor Author

Example:

+$ skopeo copy docker://alpine:3.16 oci-archive:alpine.tar
Getting image source signatures
Copying blob 213ec9aee27d done  
Copying config 29f453b10b done  
Writing manifest to image destination
Storing signatures
+$ ruby -rrubygems/package -e 'Gem::Package::TarReader.new(File.open "alpine.tar").each { |e| puts e.header.uname.size }'
4
4
4
4
4
4
4

You can just drop the .size to see the actual user name.

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 10, 2022

Thanks for your report.

Moving this to c/image, where it would be fixed.

Note to self: This does not refer to layer data, but to the outer archive containing the OCI image format.

Compare containers/podman#14978 .

@mtrmac mtrmac transferred this issue from containers/skopeo Aug 10, 2022
@mtrmac
Copy link
Collaborator

mtrmac commented Aug 10, 2022

Non-Ruby demonstration:

$ tar tvvf foo.tar       
drwxr-xr-x  0 $user   $group       0 Aug 10 17:32 blobs/
…

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 10, 2022

From a quick look, it currently seems possible to override the UID/GID via c/storage/pkg/archive.TarOptions.ChownOpts, but that only sets the numerical ID values, not the Uname/Gname values.

@graywolf-at-work
Copy link
Contributor Author

Note to self: This does not refer to layer data, but to the outer archive containing the OCI image format.

It actually applies to both, at least as far as I can tell. I just think the
root cause will be the same so I've made just one bug report.

+$ cat Containerfile 
from alpine:3.16
run touch /foo
run chown 1:1 /foo
+$ podman build -t x .
STEP 1/3: FROM alpine:3.16
STEP 2/3: run touch /foo
--> eb772b1c2d9
STEP 3/3: run chown 1:1 /foo
COMMIT x
--> c1faa4d1429
Successfully tagged localhost/x:latest
c1faa4d14290810bd939569182476abd0daf87afff91b899ba8273d32024f5ec
+$ skopeo copy containers-storage:localhost/x oci-archive:x.tar
Getting image source signatures
Copying blob ec34fcc1d526 done  
Copying blob 62034270b15d done  
Copying blob 1af89d6f9600 done  
Copying config c1faa4d142 done  
Writing manifest to image destination
Storing signatures
+$ tar -xvf x.tar 
blobs/
blobs/sha256/
blobs/sha256/00c2c0e38c7f6800842fa2ff4c2eeaeb29156681e6e8e0c69819920c7ecabef1
blobs/sha256/16d38a86e8c075aeb1c30d3da5382a013d2a430b55c482ef4e8de95f99166f17
blobs/sha256/38cbcb3460656f05fa15738be3350428a99dc2885e9a57dbfc5f6f122bbe15ad
blobs/sha256/afc1a9a81deb4915ea10d6f44f12255aad3b88f3f1bc9a6c1c06a59c3912ce0e
blobs/sha256/c1faa4d14290810bd939569182476abd0daf87afff91b899ba8273d32024f5ec
index.json
oci-layout
+$ zcat blobs/sha256/00c2c0e38c7f6800842fa2ff4c2eeaeb29156681e6e8e0c69819920c7ecabef1 | xxd | head -n 20
00000000: 666f 6f00 0000 0000 0000 0000 0000 0000  foo.............
00000010: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000020: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000030: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000040: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000050: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000060: 0000 0000 3031 3030 3634 3400 3030 3030  ....0100644.0000
00000070: 3030 3100 3030 3030 3030 3100 3030 3030  001.0000001.0000
00000080: 3030 3030 3030 3000 3134 3237 3437 3532  0000000.14274752
00000090: 3332 3400 3031 3131 3337 0020 3000 0000  324.011137. 0...
000000a0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
000000b0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
000000c0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
000000d0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
000000e0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
000000f0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000100: 0075 7374 6172 0030 3062 696e 0000 0000  .ustar.00bin....
00000110: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000120: 0000 0000 0000 0000 0062 696e 0000 0000  .........bin....
00000130: 0000 0000 0000 0000 0000 0000 0000 0000  ................

Notice the bin there, which corresponds to my machine's passwd file:

+$ cat /etc/passwd | grep bin:x
bin:x:1:1:bin:/bin:/sbin/nologin

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 10, 2022

It actually applies to both, at least as far as I can tell. I just think the root cause will be the same so I've made just one bug report.

It probably involves the same code but it’s very unlikely to be the same fix (because container images actually have various UID/GID values that must be preserved). So, please file the build part against Buildah.

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 10, 2022

(Also, that note to self was because I was very confused how a copy could modify layer contents, because c/image just doesn’t touch the underlying tar data when docker:// and oci-archive: are used.)

@graywolf-at-work
Copy link
Contributor Author

Ah, got it. Will file second issue.

@rhatdan
Copy link
Member

rhatdan commented Aug 10, 2022

Bin inside of the container is UID 1? Perhaps I don't understand the leak.

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 10, 2022

@rhatdan Let’s discuss the build case in the Buildah issue.

@mtrmac mtrmac added the kind/bug A defect in an existing functionality (or a PR fixing it) label Dec 9, 2022
mtrmac added a commit to mtrmac/storage that referenced this issue Nov 28, 2023
…count IDs

A prerequisite for fixing containers/image#1627 .

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
openshift-merge-bot bot pushed a commit to containers/podman that referenced this issue Nov 29, 2023
Instead of relying on the remote server to create tar files
with the right account IDs (which the remote server doesn't
even know, when the client and server run under different accounts),
have the remote client ignore the account IDs when unpacking.

Then just hard-code 0 in the remote server, so that the remote
server's account identity does not leak in the tar file contents.

Compare containers/image#1627 .

[NO NEW TESTS NEEDED] : #18563
suggests that existing tests already cover these code paths / properties.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A defect in an existing functionality (or a PR fixing it)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants