diff --git a/asserts/gpgkeypairmgr.go b/asserts/gpgkeypairmgr.go index 9024975172e..1d1d0b00aed 100644 --- a/asserts/gpgkeypairmgr.go +++ b/asserts/gpgkeypairmgr.go @@ -26,7 +26,6 @@ import ( "os" "os/exec" "path/filepath" - "strconv" "strings" "github.com/snapcore/snapd/osutil" @@ -38,12 +37,7 @@ func ensureGPGHomeDirectory() (string, error) { return "", err } - uid, err := strconv.Atoi(real.Uid) - if err != nil { - return "", err - } - - gid, err := strconv.Atoi(real.Gid) + uid, gid, err := osutil.UidGid(real) if err != nil { return "", err } diff --git a/client/login.go b/client/login.go index 722ccf5d9b7..97b982cdf7b 100644 --- a/client/login.go +++ b/client/login.go @@ -25,7 +25,6 @@ import ( "fmt" "os" "path/filepath" - "strconv" "github.com/snapcore/snapd/osutil" ) @@ -112,12 +111,7 @@ func writeAuthData(user User) error { return err } - uid, err := strconv.Atoi(real.Uid) - if err != nil { - return err - } - - gid, err := strconv.Atoi(real.Gid) + uid, gid, err := osutil.UidGid(real) if err != nil { return err } diff --git a/cmd/snap-seccomp/main.go b/cmd/snap-seccomp/main.go index cc9187d791a..935b3098b43 100644 --- a/cmd/snap-seccomp/main.go +++ b/cmd/snap-seccomp/main.go @@ -651,7 +651,7 @@ func compile(content []byte, out string) error { } // write atomically - fout, err := osutil.NewAtomicFile(out, 0644, 0, -1, -1) + fout, err := osutil.NewAtomicFile(out, 0644, 0, osutil.NoChown, osutil.NoChown) if err != nil { return err } diff --git a/cmd/snap-update-ns/change.go b/cmd/snap-update-ns/change.go index d440f6365ec..ba05413452f 100644 --- a/cmd/snap-update-ns/change.go +++ b/cmd/snap-update-ns/change.go @@ -21,7 +21,6 @@ package main import ( "fmt" - "os" "path/filepath" "sort" "strings" @@ -66,9 +65,11 @@ var changePerform = (*Change).Perform // this change (such as mounted tmpfs or overlayfs). func (c *Change) Perform() ([]*Change, error) { if c.Action == Mount { - mode := os.FileMode(0755) - uid := 0 - gid := 0 + const ( + mode = 0755 + uid = 0 + gid = 0 + ) // Create target mount directory if needed. if err := ensureMountPoint(c.Entry.Dir, mode, uid, gid); err != nil { return nil, err diff --git a/cmd/snap-update-ns/export_test.go b/cmd/snap-update-ns/export_test.go index fd6a987213f..adeb9f1b653 100644 --- a/cmd/snap-update-ns/export_test.go +++ b/cmd/snap-update-ns/export_test.go @@ -27,6 +27,8 @@ import ( "time" . "gopkg.in/check.v1" + + "github.com/snapcore/snapd/osutil/sys" ) var ( @@ -129,7 +131,7 @@ type SystemCalls interface { Lstat(name string) (os.FileInfo, error) Close(fd int) error - Fchown(fd int, uid int, gid int) error + Fchown(fd int, uid sys.UserID, gid sys.GroupID) error Mkdirat(dirfd int, path string, mode uint32) error Mount(source string, target string, fstype string, flags uintptr, data string) (err error) Open(path string, flags int, mode uint32) (fd int, err error) @@ -220,7 +222,7 @@ func (sys *SyscallRecorder) Close(fd int) error { return sys.freeFd(fd) } -func (sys *SyscallRecorder) Fchown(fd int, uid int, gid int) error { +func (sys *SyscallRecorder) Fchown(fd int, uid sys.UserID, gid sys.GroupID) error { return sys.call(fmt.Sprintf("fchown %d %d %d", fd, uid, gid)) } diff --git a/cmd/snap-update-ns/utils.go b/cmd/snap-update-ns/utils.go index 39bbe16fed4..16f839898b8 100644 --- a/cmd/snap-update-ns/utils.go +++ b/cmd/snap-update-ns/utils.go @@ -29,6 +29,7 @@ import ( "github.com/snapcore/snapd/interfaces/mount" "github.com/snapcore/snapd/logger" + "github.com/snapcore/snapd/osutil/sys" ) // not available through syscall @@ -47,7 +48,7 @@ var ( sysOpen = syscall.Open sysOpenat = syscall.Openat sysUnmount = syscall.Unmount - sysFchown = syscall.Fchown + sysFchown = sys.Fchown ioutilReadDir = ioutil.ReadDir ) @@ -64,7 +65,7 @@ func (e *ReadOnlyFsError) Error() string { // Create directories for all but the last segments and return the file // descriptor to the leaf directory. This function is a base for secure // variants of mkdir, touch and symlink. -func secureMkPrefix(segments []string, perm os.FileMode, uid, gid int) (int, error) { +func secureMkPrefix(segments []string, perm os.FileMode, uid sys.UserID, gid sys.GroupID) (int, error) { logger.Debugf("secure-mk-prefix %q %v %d %d -> ...", segments, perm, uid, gid) // Declare var and don't assign-declare below to ensure we don't swallow @@ -105,7 +106,7 @@ func secureMkPrefix(segments []string, perm os.FileMode, uid, gid int) (int, err // by segments. This function can be used to construct subsequent elements of // the constructed path. The return value contains the newly created file // descriptor or -1 on error. -func secureMkDir(fd int, segments []string, i int, perm os.FileMode, uid, gid int) (int, error) { +func secureMkDir(fd int, segments []string, i int, perm os.FileMode, uid sys.UserID, gid sys.GroupID) (int, error) { logger.Debugf("secure-mk-dir %d %q %d %v %d %d -> ...", fd, segments, i, perm, uid, gid) segment := segments[i] @@ -152,7 +153,7 @@ func secureMkDir(fd int, segments []string, i int, perm os.FileMode, uid, gid in // segments. This function is meant to be used to create the leaf file as a // preparation for a mount point. Existing files are reused without errors. // Newly created files have the specified mode and ownership. -func secureMkFile(fd int, segments []string, i int, perm os.FileMode, uid, gid int) error { +func secureMkFile(fd int, segments []string, i int, perm os.FileMode, uid sys.UserID, gid sys.GroupID) error { logger.Debugf("secure-mk-file %d %q %d %v %d %d", fd, segments, i, perm, uid, gid) segment := segments[i] made := true @@ -219,7 +220,7 @@ func splitIntoSegments(name string) ([]string, error) { // The uid and gid are used for the fchown(2) system call which is performed // after each segment is created and opened. The special value -1 may be used // to request that ownership is not changed. -func secureMkdirAll(name string, perm os.FileMode, uid, gid int) error { +func secureMkdirAll(name string, perm os.FileMode, uid sys.UserID, gid sys.GroupID) error { logger.Debugf("secure-mkdir-all %q %v %d %d", name, perm, uid, gid) // Only support absolute paths to avoid bugs in snap-confine when @@ -258,7 +259,7 @@ func secureMkdirAll(name string, perm os.FileMode, uid, gid int) error { // This function is like secureMkdirAll but it creates an empty file instead of // a directory for the final path component. Each created directory component // is chowned to the desired user and group. -func secureMkfileAll(name string, perm os.FileMode, uid, gid int) error { +func secureMkfileAll(name string, perm os.FileMode, uid sys.UserID, gid sys.GroupID) error { logger.Debugf("secure-mkfile-all %q %q %d %d", name, perm, uid, gid) // Only support absolute paths to avoid bugs in snap-confine when @@ -362,7 +363,7 @@ func planWritableMimic(dir string) ([]*Change, error) { return changes, nil } -func ensureMountPoint(path string, mode os.FileMode, uid int, gid int) error { +func ensureMountPoint(path string, mode os.FileMode, uid sys.UserID, gid sys.GroupID) error { // If the mount point is not present then create a directory in its // place. This is very naive, doesn't handle read-only file systems // but it is a good starting point for people working with things like diff --git a/cmd/snap-update-ns/utils_test.go b/cmd/snap-update-ns/utils_test.go index 71b3bd06f38..2c51d73fc35 100644 --- a/cmd/snap-update-ns/utils_test.go +++ b/cmd/snap-update-ns/utils_test.go @@ -30,6 +30,7 @@ import ( update "github.com/snapcore/snapd/cmd/snap-update-ns" "github.com/snapcore/snapd/interfaces/mount" "github.com/snapcore/snapd/logger" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/testutil" ) @@ -298,7 +299,7 @@ func (s *realSystemSuite) TestSecureMkdirAllForReal(c *C) { // Create d (which already exists) with mode 0777 (but c.MkDir() used 0700 // internally and since we are not creating the directory we should not be // changing that. - c.Assert(update.SecureMkdirAll(d, 0777, -1, -1), IsNil) + c.Assert(update.SecureMkdirAll(d, 0777, sys.FlagID, sys.FlagID), IsNil) fi, err := os.Stat(d) c.Assert(err, IsNil) c.Check(fi.IsDir(), Equals, true) @@ -308,7 +309,7 @@ func (s *realSystemSuite) TestSecureMkdirAllForReal(c *C) { // check that it was applied. Note that default umask 022 is subtracted so // effective directory has different permissions. d1 := filepath.Join(d, "subdir") - c.Assert(update.SecureMkdirAll(d1, 0707, -1, -1), IsNil) + c.Assert(update.SecureMkdirAll(d1, 0707, sys.FlagID, sys.FlagID), IsNil) fi, err = os.Stat(d1) c.Assert(err, IsNil) c.Check(fi.IsDir(), Equals, true) @@ -317,7 +318,7 @@ func (s *realSystemSuite) TestSecureMkdirAllForReal(c *C) { // Create d2, which is a deeper subdirectory, with another distinct mode // and check that it was applied. d2 := filepath.Join(d, "subdir/subdir/subdir") - c.Assert(update.SecureMkdirAll(d2, 0750, -1, -1), IsNil) + c.Assert(update.SecureMkdirAll(d2, 0750, sys.FlagID, sys.FlagID), IsNil) fi, err = os.Stat(d2) c.Assert(err, IsNil) c.Check(fi.IsDir(), Equals, true) @@ -502,7 +503,7 @@ func (s *realSystemSuite) TestSecureMkfileAllForReal(c *C) { // check that it was applied. Note that default umask 022 is subtracted so // effective directory has different permissions. f1 := filepath.Join(d, "file") - c.Assert(update.SecureMkfileAll(f1, 0707, -1, -1), IsNil) + c.Assert(update.SecureMkfileAll(f1, 0707, sys.FlagID, sys.FlagID), IsNil) fi, err := os.Stat(f1) c.Assert(err, IsNil) c.Check(fi.Mode().IsRegular(), Equals, true) @@ -511,7 +512,7 @@ func (s *realSystemSuite) TestSecureMkfileAllForReal(c *C) { // Create f2, which is a deeper subdirectory, with another distinct mode // and check that it was applied. f2 := filepath.Join(d, "subdir/subdir/file") - c.Assert(update.SecureMkfileAll(f2, 0750, -1, -1), IsNil) + c.Assert(update.SecureMkfileAll(f2, 0750, sys.FlagID, sys.FlagID), IsNil) fi, err = os.Stat(f2) c.Assert(err, IsNil) c.Check(fi.Mode().IsRegular(), Equals, true) diff --git a/daemon/api.go b/daemon/api.go index ddb76484d4d..579d7f27690 100644 --- a/daemon/api.go +++ b/daemon/api.go @@ -2179,13 +2179,9 @@ func setupLocalUser(st *state.State, username, email string) error { if err != nil { return fmt.Errorf("cannot lookup user %q: %s", username, err) } - uid, err := strconv.Atoi(user.Uid) + uid, gid, err := osutil.UidGid(user) if err != nil { - return fmt.Errorf("cannot get uid of user %q: %s", username, err) - } - gid, err := strconv.Atoi(user.Gid) - if err != nil { - return fmt.Errorf("cannot get gid of user %q: %s", username, err) + return err } authDataFn := filepath.Join(user.HomeDir, ".snap", "auth.json") if err := osutil.MkdirAllChown(filepath.Dir(authDataFn), 0700, uid, gid); err != nil { diff --git a/osutil/export_test.go b/osutil/export_test.go index 6d1c8081762..504105a9ee0 100644 --- a/osutil/export_test.go +++ b/osutil/export_test.go @@ -26,6 +26,8 @@ import ( "os/user" "syscall" "time" + + "github.com/snapcore/snapd/osutil/sys" ) func MockUserLookup(mock func(name string) (*user.User, error)) func() { @@ -85,7 +87,7 @@ func WaitingReaderGuts(r io.Reader) (io.Reader, *exec.Cmd) { return wr.reader, wr.cmd } -func MockChown(f func(*os.File, int, int) error) func() { +func MockChown(f func(*os.File, sys.UserID, sys.GroupID) error) func() { oldChown := chown chown = f return func() { diff --git a/osutil/io.go b/osutil/io.go index 832af928bd2..90050403bf9 100644 --- a/osutil/io.go +++ b/osutil/io.go @@ -27,6 +27,7 @@ import ( "path/filepath" "strings" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/strutil" ) @@ -53,8 +54,8 @@ type AtomicFile struct { target string tmpname string - uid int - gid int + uid sys.UserID + gid sys.GroupID closed bool renamed bool } @@ -75,11 +76,7 @@ type AtomicFile struct { // Also note that there are a number of scenarios where Commit fails and then // Cancel also fails. In all these scenarios your filesystem was probably in a // rather poor state. Good luck. -func NewAtomicFile(filename string, perm os.FileMode, flags AtomicWriteFlags, uid, gid int) (aw *AtomicFile, err error) { - if (uid < 0) != (gid < 0) { - return nil, errors.New("internal error: AtomicFile needs none or both of uid and gid set") - } - +func NewAtomicFile(filename string, perm os.FileMode, flags AtomicWriteFlags, uid sys.UserID, gid sys.GroupID) (aw *AtomicFile, err error) { if flags&AtomicWriteFollow != 0 { if fn, err := os.Readlink(filename); err == nil || (fn != "" && os.IsNotExist(err)) { if filepath.IsAbs(fn) { @@ -136,7 +133,9 @@ func (aw *AtomicFile) Cancel() error { return e2 } -var chown = (*os.File).Chown +var chown = sys.Chown + +const NoChown = sys.FlagID // Commit the modification; make it permanent. // @@ -144,7 +143,7 @@ var chown = (*os.File).Chown // write will fail. If Commit fails, the writer _might_ be closed; // Cancel() needs to be called to clean up. func (aw *AtomicFile) Commit() error { - if aw.uid > -1 && aw.gid > -1 { + if aw.uid != NoChown || aw.gid != NoChown { if err := chown(aw.File, aw.uid, aw.gid); err != nil { return err } @@ -185,27 +184,27 @@ func (aw *AtomicFile) Commit() error { // file created is an AtomicWriter, which is Committed before returning. // // AtomicWriteChown and AtomicWriteFileChown take an uid and a gid that can be -// used to specify the ownership of the created file. They must be both -// non-negative (in which case chown is called), or both negative (in which -// case it isn't). +// used to specify the ownership of the created file. A special value of +// 0xffffffff (math.MaxUint32, or NoChown for convenience) can be used to +// request no change to that attribute. // // AtomicWriteFile and AtomicWriteFileChown take the content to be written as a // []byte, and so work exactly like io.WriteFile(); AtomicWrite and // AtomicWriteChown take an io.Reader which is copied into the file instead, // and so are more amenable to streaming. func AtomicWrite(filename string, reader io.Reader, perm os.FileMode, flags AtomicWriteFlags) (err error) { - return AtomicWriteChown(filename, reader, perm, flags, -1, -1) + return AtomicWriteChown(filename, reader, perm, flags, NoChown, NoChown) } func AtomicWriteFile(filename string, data []byte, perm os.FileMode, flags AtomicWriteFlags) (err error) { - return AtomicWriteChown(filename, bytes.NewReader(data), perm, flags, -1, -1) + return AtomicWriteChown(filename, bytes.NewReader(data), perm, flags, NoChown, NoChown) } -func AtomicWriteFileChown(filename string, data []byte, perm os.FileMode, flags AtomicWriteFlags, uid, gid int) (err error) { +func AtomicWriteFileChown(filename string, data []byte, perm os.FileMode, flags AtomicWriteFlags, uid sys.UserID, gid sys.GroupID) (err error) { return AtomicWriteChown(filename, bytes.NewReader(data), perm, flags, uid, gid) } -func AtomicWriteChown(filename string, reader io.Reader, perm os.FileMode, flags AtomicWriteFlags, uid, gid int) (err error) { +func AtomicWriteChown(filename string, reader io.Reader, perm os.FileMode, flags AtomicWriteFlags, uid sys.UserID, gid sys.GroupID) (err error) { aw, err := NewAtomicFile(filename, perm, flags, uid, gid) if err != nil { return err diff --git a/osutil/io_test.go b/osutil/io_test.go index 5be5227f353..234638bff54 100644 --- a/osutil/io_test.go +++ b/osutil/io_test.go @@ -27,6 +27,7 @@ import ( "path/filepath" "github.com/snapcore/snapd/osutil" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/strutil" . "gopkg.in/check.v1" @@ -177,19 +178,11 @@ func (ts *AtomicWriteTestSuite) TestAtomicWriteFileNoOverwriteTmpExisting(c *C) c.Assert(err, ErrorMatches, "open .*: file exists") } -func (ts *AtomicWriteTestSuite) TestAtomicFileUidGidError(c *C) { - d := c.MkDir() - p := filepath.Join(d, "foo") - - _, err := osutil.NewAtomicFile(p, 0644, 0, -1, 1) - c.Check(err, ErrorMatches, ".*needs none or both of uid and gid set") -} - func (ts *AtomicWriteTestSuite) TestAtomicFileChownError(c *C) { - eUid := 42 - eGid := 74 + eUid := sys.UserID(42) + eGid := sys.GroupID(74) eErr := errors.New("this didn't work") - defer osutil.MockChown(func(fd *os.File, uid int, gid int) error { + defer osutil.MockChown(func(fd *os.File, uid sys.UserID, gid sys.GroupID) error { c.Check(uid, Equals, eUid) c.Check(gid, Equals, eGid) return eErr @@ -211,7 +204,7 @@ func (ts *AtomicWriteTestSuite) TestAtomicFileChownError(c *C) { func (ts *AtomicWriteTestSuite) TestAtomicFileCancelError(c *C) { d := c.MkDir() p := filepath.Join(d, "foo") - aw, err := osutil.NewAtomicFile(p, 0644, 0, -1, -1) + aw, err := osutil.NewAtomicFile(p, 0644, 0, osutil.NoChown, osutil.NoChown) c.Assert(err, IsNil) c.Assert(aw.File.Close(), IsNil) @@ -222,7 +215,7 @@ func (ts *AtomicWriteTestSuite) TestAtomicFileCancelError(c *C) { func (ts *AtomicWriteTestSuite) TestAtomicFileCancelBadError(c *C) { d := c.MkDir() p := filepath.Join(d, "foo") - aw, err := osutil.NewAtomicFile(p, 0644, 0, -1, -1) + aw, err := osutil.NewAtomicFile(p, 0644, 0, osutil.NoChown, osutil.NoChown) c.Assert(err, IsNil) defer aw.Close() @@ -234,7 +227,7 @@ func (ts *AtomicWriteTestSuite) TestAtomicFileCancelBadError(c *C) { func (ts *AtomicWriteTestSuite) TestAtomicFileCancelNoClose(c *C) { d := c.MkDir() p := filepath.Join(d, "foo") - aw, err := osutil.NewAtomicFile(p, 0644, 0, -1, -1) + aw, err := osutil.NewAtomicFile(p, 0644, 0, osutil.NoChown, osutil.NoChown) c.Assert(err, IsNil) c.Assert(aw.Close(), IsNil) @@ -245,7 +238,7 @@ func (ts *AtomicWriteTestSuite) TestAtomicFileCancel(c *C) { d := c.MkDir() p := filepath.Join(d, "foo") - aw, err := osutil.NewAtomicFile(p, 0644, 0, -1, -1) + aw, err := osutil.NewAtomicFile(p, 0644, 0, osutil.NoChown, osutil.NoChown) c.Assert(err, IsNil) fn := aw.File.Name() c.Check(osutil.FileExists(fn), Equals, true) diff --git a/osutil/mkdirallchown.go b/osutil/mkdirallchown.go index 7dc1c17333a..7f6c6bddba3 100644 --- a/osutil/mkdirallchown.go +++ b/osutil/mkdirallchown.go @@ -23,11 +23,13 @@ import ( "os" "path/filepath" "syscall" + + "github.com/snapcore/snapd/osutil/sys" ) // MkdirAllChown is like os.MkdirAll but it calls os.Chown on any // directories it creates. -func MkdirAllChown(path string, perm os.FileMode, uid, gid int) error { +func MkdirAllChown(path string, perm os.FileMode, uid sys.UserID, gid sys.GroupID) error { if s, err := os.Stat(path); err == nil { if s.IsDir() { return nil @@ -54,7 +56,7 @@ func MkdirAllChown(path string, perm os.FileMode, uid, gid int) error { return err } - if err := os.Chown(cand, uid, gid); err != nil { + if err := sys.ChownPath(cand, uid, gid); err != nil { return err } diff --git a/osutil/sys/syscall.go b/osutil/sys/syscall.go new file mode 100644 index 00000000000..23a87445983 --- /dev/null +++ b/osutil/sys/syscall.go @@ -0,0 +1,97 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2017 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package sys + +import ( + "os" + "syscall" + "unsafe" +) + +// FlagID can be passed to chown-ish functions to mean "no change", +// and can be returned from getuid-ish functions to mean "not found". +const FlagID = 1<<32 - 1 + +// UserID is the type of the system's user identifiers (in C, uid_t). +// +// We give it its own explicit type so you don't have to remember that +// it's a uint32 (which lead to the bug this package fixes in the +// first place) +type UserID uint32 + +// GroupID is the type of the system's group identifiers (in C, gid_t). +type GroupID uint32 + +// uid_t is an unsigned 32-bit integer in linux right now. +// so syscall.Gete?[ug]id are wrong, and break in 32 bits +// (see https://github.com/golang/go/issues/22739) +func Getuid() UserID { + return UserID(getid(_SYS_GETUID)) +} + +func Geteuid() UserID { + return UserID(getid(_SYS_GETEUID)) +} + +func Getgid() GroupID { + return GroupID(getid(_SYS_GETGID)) +} + +func Getegid() GroupID { + return GroupID(getid(_SYS_GETEGID)) +} + +func getid(id uintptr) uint32 { + // these are documented as not failing, but see golang#22924 + r0, _, errno := syscall.RawSyscall(id, 0, 0, 0) + if errno != 0 { + return uint32(-errno) + } + return uint32(r0) +} + +func Chown(f *os.File, uid UserID, gid GroupID) error { + return Fchown(int(f.Fd()), uid, gid) +} + +func Fchown(fd int, uid UserID, gid GroupID) error { + _, _, errno := syscall.Syscall(syscall.SYS_FCHOWN, uintptr(fd), uintptr(uid), uintptr(gid)) + if errno == 0 { + return nil + } + return errno +} + +func ChownPath(path string, uid UserID, gid GroupID) error { + AT_FDCWD := -100 // also written as -0x64 in ztypes_linux_*.go (but -100 in sys_linux_*.s, and /usr/include/linux/fcntl.h) + return FchownAt(uintptr(AT_FDCWD), path, uid, gid, 0) +} + +func FchownAt(dirfd uintptr, path string, uid UserID, gid GroupID, flags int) error { + p0, err := syscall.BytePtrFromString(path) + if err != nil { + return err + } + _, _, errno := syscall.Syscall6(syscall.SYS_FCHOWNAT, dirfd, uintptr(unsafe.Pointer(p0)), uintptr(uid), uintptr(gid), uintptr(flags), 0) + if errno == 0 { + return nil + } + return errno +} diff --git a/osutil/sys/sysnum_32.go b/osutil/sys/sysnum_32.go new file mode 100644 index 00000000000..dd78cc4e6db --- /dev/null +++ b/osutil/sys/sysnum_32.go @@ -0,0 +1,30 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- +// +build arm 386 ppc + +/* + * Copyright (C) 2017 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package sys + +import "syscall" + +const ( + _SYS_GETUID = syscall.SYS_GETUID32 + _SYS_GETGID = syscall.SYS_GETGID32 + _SYS_GETEUID = syscall.SYS_GETEUID32 + _SYS_GETEGID = syscall.SYS_GETEGID32 +) diff --git a/osutil/sys/sysnum_64.go b/osutil/sys/sysnum_64.go new file mode 100644 index 00000000000..49e569ad694 --- /dev/null +++ b/osutil/sys/sysnum_64.go @@ -0,0 +1,30 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- +// +build arm64 amd64 ppc64le s390x + +/* + * Copyright (C) 2017 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package sys + +import "syscall" + +const ( + _SYS_GETUID = syscall.SYS_GETUID + _SYS_GETGID = syscall.SYS_GETGID + _SYS_GETEUID = syscall.SYS_GETEUID + _SYS_GETEGID = syscall.SYS_GETEGID +) diff --git a/osutil/user.go b/osutil/user.go index 087b35d93e8..5edd3a03e47 100644 --- a/osutil/user.go +++ b/osutil/user.go @@ -28,6 +28,8 @@ import ( "regexp" "strconv" "strings" + + "github.com/snapcore/snapd/osutil/sys" ) var userLookup = user.Lookup @@ -104,13 +106,9 @@ func AddUser(name string, opts *AddUserOptions) error { return fmt.Errorf("cannot find user %q: %s", name, err) } - uid, err := strconv.Atoi(u.Uid) - if err != nil { - return fmt.Errorf("cannot parse user id %s: %s", u.Uid, err) - } - gid, err := strconv.Atoi(u.Gid) + uid, gid, err := UidGid(u) if err != nil { - return fmt.Errorf("cannot parse group id %s: %s", u.Gid, err) + return err } sshDir := filepath.Join(u.HomeDir, ".ssh") @@ -161,3 +159,20 @@ func RealUser() (*user.User, error) { return real, nil } + +// UidGid returns the uid and gid of the given user, as uint32s +// +// XXX this should go away soon +func UidGid(u *user.User) (sys.UserID, sys.GroupID, error) { + // XXX this will be wrong for high uids on 32-bit arches (for now) + uid, err := strconv.Atoi(u.Uid) + if err != nil { + return sys.FlagID, sys.FlagID, fmt.Errorf("cannot parse user id %s: %s", u.Uid, err) + } + gid, err := strconv.Atoi(u.Gid) + if err != nil { + return sys.FlagID, sys.FlagID, fmt.Errorf("cannot parse group id %s: %s", u.Gid, err) + } + + return sys.UserID(uid), sys.GroupID(gid), nil +} diff --git a/osutil/user_test.go b/osutil/user_test.go index ddfd9b5cee7..cb0eaa74872 100644 --- a/osutil/user_test.go +++ b/osutil/user_test.go @@ -29,6 +29,7 @@ import ( "gopkg.in/check.v1" "github.com/snapcore/snapd/osutil" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/testutil" ) @@ -187,3 +188,25 @@ func (s *createUserSuite) TestRealUser(c *check.C) { c.Check(cur.Username, check.Equals, t.CurrentUsername) } } + +func (s *createUserSuite) TestUidGid(c *check.C) { + for k, t := range map[string]struct { + User *user.User + Uid sys.UserID + Gid sys.GroupID + Err string + }{ + "happy": {&user.User{Uid: "10", Gid: "10"}, 10, 10, ""}, + "bad uid": {&user.User{Uid: "x", Gid: "10"}, sys.FlagID, sys.FlagID, "cannot parse user id x"}, + "bad gid": {&user.User{Uid: "10", Gid: "x"}, sys.FlagID, sys.FlagID, "cannot parse group id x"}, + } { + uid, gid, err := osutil.UidGid(t.User) + c.Check(uid, check.Equals, t.Uid, check.Commentf(k)) + c.Check(gid, check.Equals, t.Gid, check.Commentf(k)) + if t.Err == "" { + c.Check(err, check.IsNil, check.Commentf(k)) + } else { + c.Check(err, check.ErrorMatches, ".*"+t.Err+".*", check.Commentf(k)) + } + } +} diff --git a/overlord/snapstate/catalogrefresh.go b/overlord/snapstate/catalogrefresh.go index fb42faf8662..4b75a3fc96f 100644 --- a/overlord/snapstate/catalogrefresh.go +++ b/overlord/snapstate/catalogrefresh.go @@ -89,7 +89,7 @@ func refreshCatalogs(st *state.State, theStore StoreService) error { return err } - namesFile, err := osutil.NewAtomicFile(dirs.SnapNamesFile, 0644, 0, -1, -1) + namesFile, err := osutil.NewAtomicFile(dirs.SnapNamesFile, 0644, 0, osutil.NoChown, osutil.NoChown) if err != nil { return err } diff --git a/snap/info.go b/snap/info.go index a8b1ad1ffca..039a1f221ea 100644 --- a/snap/info.go +++ b/snap/info.go @@ -29,6 +29,7 @@ import ( "strings" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/strutil" "github.com/snapcore/snapd/timeout" ) @@ -60,7 +61,7 @@ type PlaceInfo interface { UserCommonDataDir(home string) string // UserXdgRuntimeDir returns the per user XDG_RUNTIME_DIR directory - UserXdgRuntimeDir(userID int) string + UserXdgRuntimeDir(userID sys.UserID) string // DataHomeDir returns the a glob that matches all per user data directories of a snap. DataHomeDir() string @@ -296,7 +297,7 @@ func (s *Info) CommonDataHomeDir() string { } // UserXdgRuntimeDir returns the XDG_RUNTIME_DIR directory of the snap for a particular user. -func (s *Info) UserXdgRuntimeDir(euid int) string { +func (s *Info) UserXdgRuntimeDir(euid sys.UserID) string { return filepath.Join("/run/user", fmt.Sprintf("%d/snap.%s", euid, s.Name())) } diff --git a/snap/pack/pack.go b/snap/pack/pack.go index 39923a1a573..34179013f0c 100644 --- a/snap/pack/pack.go +++ b/snap/pack/pack.go @@ -30,6 +30,7 @@ import ( "syscall" "github.com/snapcore/snapd/osutil" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/squashfs" ) @@ -175,9 +176,9 @@ func copyToBuildDir(sourceDir, buildDir string) error { return err } // ensure that permissions are preserved - uid := int(info.Sys().(*syscall.Stat_t).Uid) - gid := int(info.Sys().(*syscall.Stat_t).Gid) - return os.Chown(dest, uid, gid) + uid := sys.UserID(info.Sys().(*syscall.Stat_t).Uid) + gid := sys.GroupID(info.Sys().(*syscall.Stat_t).Gid) + return sys.ChownPath(dest, uid, gid) } // handle char/block devices diff --git a/snap/snapenv/snapenv.go b/snap/snapenv/snapenv.go index cce12616ade..6e021656b17 100644 --- a/snap/snapenv/snapenv.go +++ b/snap/snapenv/snapenv.go @@ -28,6 +28,7 @@ import ( "github.com/snapcore/snapd/arch" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/snap" ) @@ -119,7 +120,7 @@ func userEnv(info *snap.Info, home string) map[string]string { result := map[string]string{ "SNAP_USER_COMMON": info.UserCommonDataDir(home), "SNAP_USER_DATA": info.UserDataDir(home), - "XDG_RUNTIME_DIR": info.UserXdgRuntimeDir(os.Geteuid()), + "XDG_RUNTIME_DIR": info.UserXdgRuntimeDir(sys.Geteuid()), } // For non-classic snaps, we set HOME but on classic allow snaps to see real HOME if !info.NeedsClassic() { diff --git a/snap/snapenv/snapenv_test.go b/snap/snapenv/snapenv_test.go index 13d89bca9a0..55eda8a995a 100644 --- a/snap/snapenv/snapenv_test.go +++ b/snap/snapenv/snapenv_test.go @@ -30,6 +30,7 @@ import ( "github.com/snapcore/snapd/arch" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/testutil" ) @@ -91,7 +92,7 @@ func (ts *HTestSuite) TestUser(c *C) { "HOME": "/root/snap/foo/17", "SNAP_USER_COMMON": "/root/snap/foo/common", "SNAP_USER_DATA": "/root/snap/foo/17", - "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.foo", os.Geteuid()), + "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.foo", sys.Geteuid()), }) } @@ -102,7 +103,7 @@ func (ts *HTestSuite) TestUserForClassicConfinement(c *C) { // NOTE HOME Is absent! we no longer override it "SNAP_USER_COMMON": "/root/snap/foo/common", "SNAP_USER_DATA": "/root/snap/foo/17", - "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.foo", os.Geteuid()), + "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.foo", sys.Geteuid()), }) } @@ -136,7 +137,7 @@ func (s *HTestSuite) TestSnapRunSnapExecEnv(c *C) { "SNAP_USER_COMMON": fmt.Sprintf("%s/snap/snapname/common", usr.HomeDir), "SNAP_USER_DATA": fmt.Sprintf("%s/snap/snapname/42", usr.HomeDir), "SNAP_VERSION": "1.0", - "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.snapname", os.Geteuid()), + "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.snapname", sys.Geteuid()), }) } } diff --git a/tests/main/high-user-handling/task.yaml b/tests/main/high-user-handling/task.yaml new file mode 100644 index 00000000000..9ceed8d56ff --- /dev/null +++ b/tests/main/high-user-handling/task.yaml @@ -0,0 +1,12 @@ +summary: Check that the refresh data copy works. + +systems: [-ubuntu-core-*] + +prepare: | + useradd --uid "$(( (1<<32)-2 ))" --shell /bin/sh hightest + +restore: | + userdel hightest + +execute: | + sudo -E -u hightest "$(which go)" run test.go diff --git a/tests/main/high-user-handling/test.go b/tests/main/high-user-handling/test.go new file mode 100644 index 00000000000..90d30f8ce74 --- /dev/null +++ b/tests/main/high-user-handling/test.go @@ -0,0 +1,16 @@ +package main + +import ( + "fmt" + "os" + + "github.com/snapcore/snapd/osutil/sys" +) + +func main() { + expected := sys.UserID((1 << 32) - 2) + if uid := sys.Getuid(); uid != expected { + fmt.Printf("*** getuid() returned %d (expecting %d)\n", uid, expected) + os.Exit(1) + } +}