Repository navigation
Conversation
extractFile() extracts a tar entry's content, mode, and ownership, but never reads hdr.Xattrs/hdr.PAXRecords -- so any file carrying a real Linux capability (e.g. cap_net_raw via setcap) or other extended attribute is silently stripped during extraction, with no error or warning. Matches issue ForAllSecure#8: the root cause was already correctly identified there (no Header.Xattrs/PAXRecords handling anywhere in the file), but no fix had been proposed. Fix: applyXattrs() reads hdr.PAXRecords for "SCHILY.xattr."-prefixed keys and restores each one via unix.Lsetxattr, called after chown/chmod succeeds for regular files, directories, and symlinks. Verified two ways: - rootfs/xattr_repro_test.go: builds a real tar stream (Go's own archive/tar writer) with a PAX xattr record, runs it through the actual handleFiles() extraction path, and checks the resulting file's xattr via the real Lgetxattr syscall. Fails on the pre-fix code (ENODATA), passes after. - End-to-end against a real registry: built a Docker image with a file carrying a genuine cap_net_raw capability (via setcap), independently confirmed the capability's raw bytes are present in the pushed image's real layer tar (fetched directly via the registry HTTP API, inspected with Python's stdlib tarfile -- no dependency on this tool or Docker's own extraction), then ran both the pre-fix and fixed binaries against that same image via a real local registry pull. Pre-fix: xattr absent (os.getxattr -> ENODATA). Fixed: xattr present, byte-for-byte identical to the real layer's own record. Also fixes an unrelated, independently-discovered bug this change surfaced: rootfs/pull_test.go declared `package rootfs` while importing its own package path, an import cycle that has prevented `go test ./...` from compiling on any modern Go toolchain. Confirmed pre-existing (via git stash) and unrelated to this change. The project's own CI never catches this because its "Test" step runs a bare `docker run` with no command, not `go test`. Fixed by dropping the self-import; both TestPull and TestPullTimeout now compile and pass against the real registry. golang.org/x/sys is added as a new (small, stdlib-adjacent) dependency for the Lsetxattr syscall; go.mod/go.sum changes beyond that line are `go mod tidy`'s standard housekeeping on a long-untouched (2019-era) dependency set, not an intentional bump. I used AI assistance for this, but not entirely.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Extended attributes (e.g. Linux capabilities set via
setcap, SELinux labels) are silently dropped during extraction --extractFile()never readshdr.Xattrs/hdr.PAXRecordsanywhere. Matches #8: the root cause was already correctly identified there ("no signs ofHeader.XattrsorHeader.PAXRecords"), but no fix had been proposed yet.Fix: a new
applyXattrs()readshdr.PAXRecordsforSCHILY.xattr.-prefixed keys and restores each one viaunix.Lsetxattr, called after chown/chmod succeeds for regular files, directories, and symlinks.Verified two ways:
rootfs/xattr_repro_test.go: builds a real tar stream (Go'''s ownarchive/tarwriter) carrying a PAX xattr record, runs it through the actualhandleFiles()extraction path, and checks the resulting file'''s xattr via the realLgetxattrsyscall. Fails on the pre-fix code (ENODATA), passes after.cap_net_rawcapability (viasetcap), independently confirmed the capability'''s raw bytes are present in the pushed image'''s real layer tar (fetched directly via the registry HTTP API, inspected with Python'''s stdlibtarfile-- no dependency on this tool or Docker'''s own extraction), then ran both the pre-fix and fixed binaries against that same image via a real local registry pull. Pre-fix: xattr absent (os.getxattr-> ENODATA). Fixed: xattr present, byte-for-byte identical to the real layer'''s own record.Also fixes an unrelated, independently-discovered bug this change surfaced:
rootfs/pull_test.godeclaredpackage rootfswhile importing its own package path -- an import cycle that has preventedgo test ./...from compiling on any modern Go toolchain. Confirmed pre-existing (viagit stash, unrelated to this change) -- the project'''s CI never catches it because its "Test" step runs a baredocker runwith no command, notgo test. Fixed by dropping the self-import;TestPull/TestPullTimeoutnow compile and pass against the real registry for the first time.golang.org/x/sysis added as a small new dependency for theLsetxattrsyscall; the rest of thego.mod/go.sumdiff isgo mod tidy'''s standard housekeeping on a long-untouched (2019-era) dependency set, not an intentional version bump.I used AI assistance for this, but not entirely.