[20.10 backport] Remove local fork of archive/tar package #46695
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neersighted
approved these changes
Oct 20, 2023
|
Linter is complaining; you probably need to cherry-pick 65e1adc (part of #43765) |
thaJeztah
added
status/2-code-review
status/failing-ci
corhere
removed
the
status/failing-ci
|
The only CI failures are the usual Windows Server hcsshim timeouts. PTAL @thaJeztah |
thaJeztah
reviewed
Oct 23, 2023
hack/vendor.sh
Outdated
| @@ -1,28 +0,0 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Looks like you removed a bit too much here; the hack/vendor.sh script itself should probably still be there (or Jenkins to be updated);
[2023-10-23T17:06:17.709Z] + docker run --rm -t --privileged -v /home/ubuntu/workspace/moby_PR-46695/.git:/go/src/github.com/docker/docker/.git --name docker-pr2 -e DOCKER_EXPERIMENTAL -e DOCKER_GITCOMMIT=d6cbc6b37d407d5bc8573bcfbbcf803b8fcb91e4 -e DOCKER_GRAPHDRIVER -e TEST_FORCE_VALIDATE -e VALIDATE_REPO=https://github.com/moby/moby.git -e VALIDATE_BRANCH=20.10 docker:d6cbc6b37d407d5bc8573bcfbbcf803b8fcb91e4 hack/validate/vendor
[2023-10-23T17:06:17.969Z] hack/validate/vendor: line 14: ./hack/vendor.sh: No such file or directory
script returned exit code 127
There was a problem hiding this comment.
Whoops, missed that one. Those pesky horizontal scroll bars...
A copy of Go's archive/tar packge was vendored with a patch applied to mitigate CVE-2019-14271. Vendoring standard library packages is not supported by Go in module-aware mode, which is getting in the way of maintenance. A different approach to mitigate the vulnerability is needed which does not involve vendoring parts of the standard library. glibc implements name service lookups such as users, groups and DNS using a scheme known as Name Service Switch. The services are implemented as modules, shared libraries which glibc dynamically links into the process the first time a function requiring the module is called. This is the crux of the vulnerability: if a process linked against glibc chroots, then calls one of the functions implemented with NSS for the first time, glibc may load NSS modules out of the chrooted filesystem. The API underlying the `docker cp` command is implemented by forking a new process which chroots into the container's rootfs and writes a tar stream of files from the container over standard output. It utilizes the Go standard library's archive/tar package to write the tar stream. It makes use of the tar.FileInfoHeader function to construct a tar.Header value from an fs.FileInfo value. In modern versions of Go on *nix platforms, FileInfoHeader will attempt to resolve the file's UID and GID to their respective user and group names by calling the os/user functions LookupId and LookupGroupId. The cgo implementation of os/user on *nix performs lookups by calling the corresponding libc functions. So when linked against glibc, calls to tar.FileInfoHeader after the process has chrooted into the container's rootfs can have the side effect of loading NSS modules from the container! Without any mitigations, a malicious container image author can trivially get arbitrary code execution by leveraging this vulnerability and escape the chroot (which is not a sandbox) into the host. Mitigate the vulnerability without patching or forking archive/tar by hiding the OS-dependent file info from tar.FileInfoHeader which it needs to perform the lookups. Without that information available it falls back to populating the tar.Header with only the information obtainable directly from the FileInfo value without making any calls into os/user. Fixes moby#42402 Signed-off-by: Cory Snider <csnider@mirantis.com> (cherry picked from commit e9bbc41) Signed-off-by: Cory Snider <csnider@mirantis.com>
The recently-upgraded gosec linter has a rule for archive extraction code which may be vulnerable to directory traversal attacks, a.k.a. Zip Slip. Gosec's detection is unfortunately prone to false positives, however: it flags any filepath.Join call with an argument derived from a tar.Header value, irrespective of whether the resultant path is used for filesystem operations or if directory traversal attacks are guarded against. All of the lint errors reported by gosec appear to be false positives. Signed-off-by: Cory Snider <csnider@mirantis.com> (cherry picked from commit 833139f) Signed-off-by: Cory Snider <csnider@mirantis.com>
corhere
force-pushed
the
backport-20.10/safer-fileinfo
branch
from
6c3e402
to
6c523aa
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The hack of forking archive/tar is incompatible with the version of the package in Go 1.20.
Backport the proper fix rather than hacking on the hack.
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)