Skip to content

Commit e9bbc41

Browse files
committed
Remove local fork of archive/tar package
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 <[email protected]>
1 parent 54d35c0 commit e9bbc41

64 files changed

Lines changed: 88 additions & 7400 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Dockerfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
ARG CROSS="false"
44
ARG SYSTEMD="false"
5-
# IMPORTANT: When updating this please note that stdlib archive/tar pkg is vendored
65
ARG GO_VERSION=1.17.7
76
ARG DEBIAN_FRONTEND=noninteractive
87
ARG VPNKIT_VERSION=0.5.0

hack/vendor.sh

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,5 @@ set -x
1010
SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
1111
"${SCRIPTDIR}"/go-mod-prepare.sh
1212

13-
if [ $# -eq 0 ] || [ "$1" != "archive/tar" ]; then
14-
GO111MODULE=auto go mod tidy -modfile 'vendor.mod' -compat 1.17
15-
GO111MODULE=auto go mod vendor -modfile vendor.mod
16-
fi
17-
18-
if [ $# -eq 0 ] || [ "$1" = "archive/tar" ]; then
19-
echo "update vendored copy of archive/tar"
20-
: "${GO_VERSION:=$(awk -F '[ =]' '$1 == "ARG" && $2 == "GO_VERSION" { print $3; exit }' ./Dockerfile)}"
21-
rm -rf vendor/archive/tar
22-
mkdir -p vendor/archive/tar
23-
echo "downloading: https://golang.org/dl/go${GO_VERSION%.0}.src.tar.gz"
24-
curl -fsSL "https://golang.org/dl/go${GO_VERSION%.0}.src.tar.gz" \
25-
| tar --extract --gzip --directory=vendor/archive/tar --strip-components=4 go/src/archive/tar
26-
patch --strip=4 --directory=vendor/archive/tar --input="$PWD/patches/0001-archive-tar-do-not-populate-user-group-names.patch"
27-
fi
28-
13+
GO111MODULE=auto go mod tidy -modfile 'vendor.mod' -compat 1.17
14+
GO111MODULE=auto go mod vendor -modfile vendor.mod

patches/0001-archive-tar-do-not-populate-user-group-names.patch

Lines changed: 0 additions & 75 deletions
This file was deleted.

pkg/archive/archive.go

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -403,12 +403,64 @@ func (compression *Compression) Extension() string {
403403
return ""
404404
}
405405

406+
// nosysFileInfo hides the system-dependent info of the wrapped FileInfo to
407+
// prevent tar.FileInfoHeader from introspecting it and potentially calling into
408+
// glibc.
409+
type nosysFileInfo struct {
410+
os.FileInfo
411+
}
412+
413+
func (fi nosysFileInfo) Sys() interface{} {
414+
// A Sys value of type *tar.Header is safe as it is system-independent.
415+
// The tar.FileInfoHeader function copies the fields into the returned
416+
// header without performing any OS lookups.
417+
if sys, ok := fi.FileInfo.Sys().(*tar.Header); ok {
418+
return sys
419+
}
420+
return nil
421+
}
422+
423+
// sysStat, if non-nil, populates hdr from system-dependent fields of fi.
424+
var sysStat func(fi os.FileInfo, hdr *tar.Header) error
425+
426+
// FileInfoHeaderNoLookups creates a partially-populated tar.Header from fi.
427+
//
428+
// Compared to the archive/tar.FileInfoHeader function, this function is safe to
429+
// call from a chrooted process as it does not populate fields which would
430+
// require operating system lookups. It behaves identically to
431+
// tar.FileInfoHeader when fi is a FileInfo value returned from
432+
// tar.Header.FileInfo().
433+
//
434+
// When fi is a FileInfo for a native file, such as returned from os.Stat() and
435+
// os.Lstat(), the returned Header value differs from one returned from
436+
// tar.FileInfoHeader in the following ways. The Uname and Gname fields are not
437+
// set as OS lookups would be required to populate them. The AccessTime and
438+
// ChangeTime fields are not currently set (not yet implemented) although that
439+
// is subject to change. Callers which require the AccessTime or ChangeTime
440+
// fields to be zeroed should explicitly zero them out in the returned Header
441+
// value to avoid any compatibility issues in the future.
442+
func FileInfoHeaderNoLookups(fi os.FileInfo, link string) (*tar.Header, error) {
443+
hdr, err := tar.FileInfoHeader(nosysFileInfo{fi}, link)
444+
if err != nil {
445+
return nil, err
446+
}
447+
if sysStat != nil {
448+
return hdr, sysStat(fi, hdr)
449+
}
450+
return hdr, nil
451+
}
452+
406453
// FileInfoHeader creates a populated Header from fi.
407-
// Compared to archive pkg this function fills in more information.
408-
// Also, regardless of Go version, this function fills file type bits (e.g. hdr.Mode |= modeISDIR),
409-
// which have been deleted since Go 1.9 archive/tar.
454+
//
455+
// Compared to the archive/tar package, this function fills in less information
456+
// but is safe to call from a chrooted process. The AccessTime and ChangeTime
457+
// fields are not set in the returned header, ModTime is truncated to one-second
458+
// precision, and the Uname and Gname fields are only set when fi is a FileInfo
459+
// value returned from tar.Header.FileInfo(). Also, regardless of Go version,
460+
// this function fills file type bits (e.g. hdr.Mode |= modeISDIR), which have
461+
// been deleted since Go 1.9 archive/tar.
410462
func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, error) {
411-
hdr, err := tar.FileInfoHeader(fi, link)
463+
hdr, err := FileInfoHeaderNoLookups(fi, link)
412464
if err != nil {
413465
return nil, err
414466
}
@@ -418,9 +470,6 @@ func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, erro
418470
hdr.ChangeTime = time.Time{}
419471
hdr.Mode = fillGo18FileTypeBits(int64(chmodTarEntry(os.FileMode(hdr.Mode))), fi)
420472
hdr.Name = canonicalTarName(name, fi.IsDir())
421-
if err := setHeaderForSpecialDevice(hdr, name, fi.Sys()); err != nil {
422-
return nil, err
423-
}
424473
return hdr, nil
425474
}
426475

@@ -1251,7 +1300,7 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
12511300
}
12521301
defer srcF.Close()
12531302

1254-
hdr, err := tar.FileInfoHeader(srcSt, "")
1303+
hdr, err := FileInfoHeaderNoLookups(srcSt, "")
12551304
if err != nil {
12561305
return err
12571306
}

pkg/archive/archive_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,3 +1402,12 @@ func TestPigz(t *testing.T) {
14021402
assert.Equal(t, reflect.TypeOf(contextReaderCloserWrapper.Reader), reflect.TypeOf(&gzip.Reader{}))
14031403
}
14041404
}
1405+
1406+
func TestNosysFileInfo(t *testing.T) {
1407+
st, err := os.Stat("archive_test.go")
1408+
assert.NilError(t, err)
1409+
h, err := tar.FileInfoHeader(nosysFileInfo{st}, "")
1410+
assert.NilError(t, err)
1411+
assert.Check(t, h.Uname == "")
1412+
assert.Check(t, h.Gname == "")
1413+
}

pkg/archive/archive_unix.go

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ import (
1717
"golang.org/x/sys/unix"
1818
)
1919

20+
func init() {
21+
sysStat = statUnix
22+
}
23+
2024
// fixVolumePathPrefix does platform specific processing to ensure that if
2125
// the path being passed in is not in a volume path format, convert it to one.
2226
func fixVolumePathPrefix(srcPath string) string {
@@ -45,19 +49,24 @@ func chmodTarEntry(perm os.FileMode) os.FileMode {
4549
return perm // noop for unix as golang APIs provide perm bits correctly
4650
}
4751

48-
func setHeaderForSpecialDevice(hdr *tar.Header, name string, stat interface{}) (err error) {
49-
s, ok := stat.(*syscall.Stat_t)
52+
// statUnix populates hdr from system-dependent fields of fi without performing
53+
// any OS lookups.
54+
func statUnix(fi os.FileInfo, hdr *tar.Header) error {
55+
s, ok := fi.Sys().(*syscall.Stat_t)
56+
if !ok {
57+
return nil
58+
}
5059

51-
if ok {
52-
// Currently go does not fill in the major/minors
53-
if s.Mode&unix.S_IFBLK != 0 ||
54-
s.Mode&unix.S_IFCHR != 0 {
55-
hdr.Devmajor = int64(unix.Major(uint64(s.Rdev))) //nolint: unconvert
56-
hdr.Devminor = int64(unix.Minor(uint64(s.Rdev))) //nolint: unconvert
57-
}
60+
hdr.Uid = int(s.Uid)
61+
hdr.Gid = int(s.Gid)
62+
63+
if s.Mode&unix.S_IFBLK != 0 ||
64+
s.Mode&unix.S_IFCHR != 0 {
65+
hdr.Devmajor = int64(unix.Major(uint64(s.Rdev))) //nolint: unconvert
66+
hdr.Devminor = int64(unix.Minor(uint64(s.Rdev))) //nolint: unconvert
5867
}
5968

60-
return
69+
return nil
6170
}
6271

6372
func getInodeFromStat(stat interface{}) (inode uint64, err error) {

pkg/containerfs/archiver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (retErr error) {
137137
}
138138
defer srcF.Close()
139139

140-
hdr, err := tar.FileInfoHeader(srcSt, "")
140+
hdr, err := archive.FileInfoHeaderNoLookups(srcSt, "")
141141
if err != nil {
142142
return err
143143
}

0 commit comments

Comments
 (0)