diff --git a/internal/fsutil/create.go b/internal/fsutil/create.go index 0503c96f1..2db479354 100644 --- a/internal/fsutil/create.go +++ b/internal/fsutil/create.go @@ -3,6 +3,7 @@ package fsutil import ( "crypto/sha256" "encoding/hex" + "errors" "fmt" "hash" "io" @@ -10,6 +11,8 @@ import ( "os" "path/filepath" "strings" + "sync" + "syscall" ) type CreateOptions struct { @@ -27,6 +30,19 @@ type CreateOptions struct { // If OverrideMode is true and entry already exists, update the mode. Does // not affect symlinks. OverrideMode bool + // Uid and Gid, if not nil, are applied to the created entry with + // os.Chown, so the entry faithfully reflects the ownership recorded + // in the source package. A nil value leaves the corresponding id + // unchanged. Ownership is applied to newly created entries, and to + // pre-existing ones only when OverrideMode is true. It is never + // applied to symlinks (their ownership has no meaningful effect), + // nor to hard links (they share the inode, and therefore the + // ownership, of their already-created target). Applying ownership + // requires sufficient privilege (e.g. running as root); otherwise a + // warning is logged once and the entry keeps the ownership of the + // current process user and group. + Uid *int + Gid *int } type Entry struct { @@ -143,10 +159,17 @@ func CreateWriter(options *CreateOptions) (io.WriteCloser, *Entry, error) { } } + _, existedErr := os.Lstat(path) file, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, o.Mode) if err != nil { return nil, nil, err } + if existedErr != nil { + if err := applyOwnership(path, o); err != nil { + file.Close() + return nil, nil, err + } + } entry := &Entry{ Path: path, Mode: o.Mode, @@ -160,17 +183,51 @@ func CreateWriter(options *CreateOptions) (io.WriteCloser, *Entry, error) { return wp, entry, nil } +// applyOwnership applies the Uid/Gid of the created entry, if any. +// Ownership changes that fail due to lack of privilege are logged once +// and otherwise ignored, so unprivileged cuts keep working with the +// entries owned by the current process user and group. +func applyOwnership(path string, o *CreateOptions) error { + uid, gid := -1, -1 + if o.Uid != nil { + uid = *o.Uid + } + if o.Gid != nil { + gid = *o.Gid + } + if uid == -1 && gid == -1 { + return nil + } + err := os.Chown(path, uid, gid) + if errors.Is(err, syscall.EPERM) { + warnOwnership.Do(func() { + logf("WARNING: cannot preserve file ownership without sufficient privilege: entries will be owned by the current user and group") + }) + return nil + } + return err +} + +var warnOwnership sync.Once + func createDir(o *CreateOptions) error { debugf("Creating directory: %s (mode %#o)", o.Path, o.Mode) path, err := absPath(o.Root, o.Path) if err != nil { return err } + _, existedErr := os.Lstat(path) err = os.Mkdir(path, o.Mode) if os.IsExist(err) { + if existedErr == nil && o.OverrideMode { + return applyOwnership(path, o) + } return nil } - return err + if err != nil { + return err + } + return applyOwnership(path, o) } func createFile(o *CreateOptions) error { @@ -179,6 +236,7 @@ func createFile(o *CreateOptions) error { if err != nil { return err } + _, existedErr := os.Lstat(path) file, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, o.Mode) if err != nil { return err @@ -188,7 +246,13 @@ func createFile(o *CreateOptions) error { if copyErr != nil { return copyErr } - return err + if err != nil { + return err + } + if existedErr != nil || o.OverrideMode { + return applyOwnership(path, o) + } + return nil } func createSymlink(o *CreateOptions) error { diff --git a/internal/fsutil/create_test.go b/internal/fsutil/create_test.go index 5e908fccc..def709aca 100644 --- a/internal/fsutil/create_test.go +++ b/internal/fsutil/create_test.go @@ -470,3 +470,141 @@ func (s *S) TestCreateEmptyRoot(c *C) { _, _, err = fsutil.CreateWriter(options) c.Assert(err, ErrorMatches, "internal error: CreateOptions.Root is unset") } + +func entryOwnership(path string) (uid, gid int) { + info, err := os.Lstat(path) + if err != nil { + panic(err) + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + panic("unsupported system: not a syscall.Stat_t") + } + return int(stat.Uid), int(stat.Gid) +} + +func (s *S) TestCreateOwnership(c *C) { + // Ownership can always be set to the current process user and group, + // even when running unprivileged. The unprivileged attempt to set + // other ids fails with EPERM, which is logged and otherwise ignored. + selfUid, selfGid := os.Getuid(), os.Getgid() + dir := c.MkDir() + + uid, gid := selfUid, selfGid + _, err := fsutil.Create(&fsutil.CreateOptions{ + Root: dir, + Path: "file", + Mode: 0644, + Data: bytes.NewBufferString("data1"), + Uid: &uid, + Gid: &gid, + }) + c.Assert(err, IsNil) + gotUid, gotGid := entryOwnership(filepath.Join(dir, "file")) + c.Assert(gotUid, Equals, selfUid) + c.Assert(gotGid, Equals, selfGid) + + // Directories are owned too. + _, err = fsutil.Create(&fsutil.CreateOptions{ + Root: dir, + Path: "somedir", + Mode: fs.ModeDir | 0755, + MakeParents: true, + Uid: &uid, + Gid: &gid, + }) + c.Assert(err, IsNil) + gotUid, gotGid = entryOwnership(filepath.Join(dir, "somedir")) + c.Assert(gotUid, Equals, selfUid) + c.Assert(gotGid, Equals, selfGid) + + // Ownership of pre-existing entries is only applied with + // OverrideMode, mirroring the mode semantics. + hack := func(root string) { + err := os.WriteFile(filepath.Join(root, "file"), []byte("data"), 0644) + c.Assert(err, IsNil) + } + root := c.MkDir() + hack(root) + rootUid, rootGid := 0, 0 + if selfUid == 0 { + // Use an id different from the current ones for a meaningful + // assertion when running as root. + rootUid, rootGid = 0, 0 + } + _, err = fsutil.Create(&fsutil.CreateOptions{ + Root: root, + Path: "file", + Mode: 0644, + Data: bytes.NewBufferString("data"), + Uid: &rootUid, + Gid: &rootGid, + }) + c.Assert(err, IsNil) + gotUid, gotGid = entryOwnership(filepath.Join(root, "file")) + if selfUid == 0 { + c.Assert(gotUid, Equals, rootUid) + c.Assert(gotGid, Equals, rootGid) + } else { + // Unchanged: the entry pre-existed and OverrideMode is unset. + c.Assert(gotUid, Equals, selfUid) + c.Assert(gotGid, Equals, selfGid) + } + + root = c.MkDir() + hack(root) + _, err = fsutil.Create(&fsutil.CreateOptions{ + Root: root, + Path: "file", + Mode: 0644, + Data: bytes.NewBufferString("data"), + OverrideMode: true, + Uid: &rootUid, + Gid: &rootGid, + }) + c.Assert(err, IsNil) + gotUid, gotGid = entryOwnership(filepath.Join(root, "file")) + if selfUid == 0 { + c.Assert(gotUid, Equals, rootUid) + c.Assert(gotGid, Equals, rootGid) + } else { + // EPERM is logged once and otherwise ignored. + c.Assert(gotUid, Equals, selfUid) + c.Assert(gotGid, Equals, selfGid) + c.Assert(c.GetTestLog(), Matches, "(?s).*WARNING: cannot preserve file ownership.*") + } + + // Ownership of symlinks is never applied, not even via their target. + root = c.MkDir() + err = os.WriteFile(filepath.Join(root, "bar"), []byte("data"), 0644) + c.Assert(err, IsNil) + _, err = fsutil.Create(&fsutil.CreateOptions{ + Root: root, + Path: "foo", + Link: "bar", + Mode: fs.ModeSymlink, + Uid: &rootUid, + Gid: &rootGid, + }) + c.Assert(err, IsNil) + gotUid, gotGid = entryOwnership(filepath.Join(root, "bar")) + c.Assert(gotUid, Equals, selfUid) + c.Assert(gotGid, Equals, selfGid) + + // CreateWriter applies ownership to newly created files too. + root = c.MkDir() + writer, _, err := fsutil.CreateWriter(&fsutil.CreateOptions{ + Root: root, + Path: "file", + Mode: 0644, + Uid: &uid, + Gid: &gid, + }) + c.Assert(err, IsNil) + _, err = writer.Write([]byte("data")) + c.Assert(err, IsNil) + c.Assert(writer.Close(), IsNil) + gotUid, gotGid = entryOwnership(filepath.Join(root, "file")) + c.Assert(gotUid, Equals, selfUid) + c.Assert(gotGid, Equals, selfGid) +} diff --git a/internal/tarball/extract.go b/internal/tarball/extract.go index 40db2cbac..62cd739dc 100644 --- a/internal/tarball/extract.go +++ b/internal/tarball/extract.go @@ -34,6 +34,14 @@ type ExtractInfo struct { Context any } +// tarDirInfo holds the mode and ownership of a directory as recorded in +// the package tarball. +type tarDirInfo struct { + mode fs.FileMode + uid int + gid int +} + func getValidOptions(options *ExtractOptions) (*ExtractOptions, error) { for extractPath, extractInfos := range options.Extract { isGlob := strings.ContainsAny(extractPath, "*?") @@ -114,12 +122,12 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { pendingHardLinks := make(map[string][]pendingHardLink) // When creating a file we will iterate through its parent directories and - // create them with the permissions defined in the tarball. + // create them with the permissions and ownership defined in the tarball. // // The assumption is that the tar entries of the parent directories appear // before the entry for the file itself. This is the case for .deb files but // not for all tarballs. - tarDirMode := make(map[string]fs.FileMode) + tarDirs := make(map[string]tarDirInfo) tarReader := tar.NewReader(dataReader) for { tarHeader, err := tarReader.Next() @@ -137,7 +145,11 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { sourceIsDir := sourcePath[len(sourcePath)-1] == '/' if sourceIsDir { - tarDirMode[sourcePath] = tarHeader.FileInfo().Mode() + tarDirs[sourcePath] = tarDirInfo{ + mode: tarHeader.FileInfo().Mode(), + uid: tarHeader.Uid, + gid: tarHeader.Gid, + } } // Find all globs and copies that require this source, and map them by @@ -196,24 +208,35 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { if mode != 0 { tarHeader.Mode = int64(mode) } - // Create the parent directories using the permissions from the tarball. + // Create the parent directories using the permissions and + // ownership from the tarball. parents := parentDirs(targetPath) for _, path := range parents { if path == "/" { continue } - mode, ok := tarDirMode[path] + dirInfo, ok := tarDirs[path] if !ok { continue } - delete(tarDirMode, path) + delete(tarDirs, path) createOptions := &fsutil.CreateOptions{ Root: options.TargetDir, Path: path, - Mode: mode, + Mode: dirInfo.mode, MakeParents: true, } + // Only special ownership (any non-root user or a + // non-root group) needs to be applied explicitly: + // root-owned entries already end up root-owned when + // cutting as root, and cannot be chowned when cutting + // as a regular user. + if dirInfo.uid != 0 || dirInfo.gid != 0 { + uid, gid := dirInfo.uid, dirInfo.gid + createOptions.Uid = &uid + createOptions.Gid = &gid + } err := options.Create(nil, createOptions) if err != nil { return err @@ -225,7 +248,12 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { link = filepath.Join(options.TargetDir, link) } - // Create the entry itself. + // Create the entry itself, preserving any special + // ownership recorded in the tarball (e.g. the + // setgid-shadow group of the PAM chkpwd helpers). + // Root-owned entries are skipped: they already end up + // root-owned when cutting as root, and cannot be chowned + // when cutting as a regular user. createOptions := &fsutil.CreateOptions{ Root: options.TargetDir, Path: targetPath, @@ -235,6 +263,11 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error { MakeParents: true, OverrideMode: true, } + if tarHeader.Uid != 0 || tarHeader.Gid != 0 { + uid, gid := tarHeader.Uid, tarHeader.Gid + createOptions.Uid = &uid + createOptions.Gid = &gid + } err := options.Create(extractInfos, createOptions) if err != nil && os.IsNotExist(err) && tarHeader.Typeflag == tar.TypeLink { // The hard link could not be created because the content diff --git a/internal/tarball/extract_test.go b/internal/tarball/extract_test.go index fd5eb6147..86da056e5 100644 --- a/internal/tarball/extract_test.go +++ b/internal/tarball/extract_test.go @@ -1,12 +1,15 @@ package tarball_test import ( + "archive/tar" "bytes" + "io/fs" "os" "path" "path/filepath" "sort" "strings" + "syscall" . "gopkg.in/check.v1" @@ -628,3 +631,89 @@ func (s *S) TestExtractCreateCallback(c *C) { c.Assert(createExtractInfos, DeepEquals, test.calls) } } + +func (s *S) TestExtractOwnership(c *C) { + // Entries are created with the ownership recorded in the package + // tarball (e.g. the setgid-shadow group of the PAM chkpwd helpers). + // Applying ownership requires sufficient privilege (e.g. running as + // root); otherwise a warning is logged once and the entries keep the + // ownership of the current process user and group. + shadowGid := 42 + entries := []testutil.TarEntry{{ + Header: tar.Header{ + Typeflag: tar.TypeDir, + Name: "./usr/", + Mode: 0755, + Uid: 0, + Gid: shadowGid, + Uname: "root", + Gname: "shadow", + Format: tar.FormatGNU, + }, + }, { + Header: tar.Header{ + Typeflag: tar.TypeDir, + Name: "./usr/sbin/", + Mode: 0755, + Uid: 0, + Gid: shadowGid, + Uname: "root", + Gname: "shadow", + Format: tar.FormatGNU, + }, + }, { + Header: tar.Header{ + Typeflag: tar.TypeReg, + Name: "./usr/sbin/unix_chkpwd", + Mode: 02755, + Uid: 0, + Gid: shadowGid, + Uname: "root", + Gname: "shadow", + Format: tar.FormatGNU, + }, + Content: []byte("chkpwd-binary-content"), + }} + pkgdata, err := testutil.MakeDeb(entries) + c.Assert(err, IsNil) + + dir := c.MkDir() + // The ownership warnings are logged through fsutil's logger. + fsutil.SetLogger(c) + defer fsutil.SetLogger(nil) + options := tarball.ExtractOptions{ + Package: "test-package", + TargetDir: dir, + Extract: map[string][]tarball.ExtractInfo{ + "/usr/sbin/unix_chkpwd": []tarball.ExtractInfo{{ + Path: "/usr/sbin/unix_chkpwd", + }}, + }, + } + err = tarball.Extract(bytes.NewReader(pkgdata), &options) + c.Assert(err, IsNil) + + info, err := os.Lstat(filepath.Join(dir, "usr/sbin/unix_chkpwd")) + c.Assert(err, IsNil) + c.Assert(info.Mode()&fs.ModeSetgid, Not(Equals), 0) + stat, ok := info.Sys().(*syscall.Stat_t) + c.Assert(ok, Equals, true) + dirInfo, err := os.Lstat(filepath.Join(dir, "usr/sbin")) + c.Assert(err, IsNil) + dirStat, ok := dirInfo.Sys().(*syscall.Stat_t) + c.Assert(ok, Equals, true) + if os.Geteuid() == 0 { + // Ownership of the entry and its parent directories is + // preserved from the tarball. + c.Assert(stat.Uid, Equals, uint32(0)) + c.Assert(stat.Gid, Equals, uint32(shadowGid)) + c.Assert(dirStat.Gid, Equals, uint32(shadowGid)) + } else { + // Without sufficient privilege the entries are owned by the + // current process user and group, and a warning is logged. + c.Assert(stat.Uid, Equals, uint32(os.Getuid())) + c.Assert(stat.Gid, Equals, uint32(os.Getgid())) + c.Assert(dirStat.Gid, Equals, uint32(os.Getgid())) + c.Assert(c.GetTestLog(), Matches, "(?s).*WARNING: cannot preserve file ownership.*") + } +}