"net/http"
"os"
"os/user"
+ slashpath "path"
"path/filepath"
"sort"
"strconv"
"strings"
- "syscall"
"time"
"github.com/ianbruene/go-difflib/difflib"
+ "golang.org/x/sys/unix"
"ruderich.org/simon/safcm"
)
+// NOTE: Don't use plain os.Open, os.OpenFile, os.Remove or any other function
+// which uses absolute paths. These are vulnerable to symlink attacks. Always
+// use *at syscalls. See below for details.
+
+// openReadonlyFlags are flags for open* syscalls to safely read a file or
+// directory.
+//
+// O_NOFOLLOW prevents symlink attacks
+// O_NONBLOCK is necessary to prevent blocking on FIFOs
+const openReadonlyFlags = unix.O_RDONLY | unix.O_NOFOLLOW | unix.O_NONBLOCK
+
func (s *Sync) syncFiles() error {
// To create random file names for symlinks
rand.Seed(time.Now().UnixNano())
// The implementation is careful not to follow any symlinks to prevent
// possible race conditions which can be exploited and are especially
// dangerous when running with elevated privileges (which will most
- // likely be the case).
+ // likely be the case). This includes not using absolute paths in
+ // syscalls to prevent symlink attacks when a directory is writable by
+ // other users (e.g. when syncing a file to /home/user/dir/file the
+ // user could create dir as symlink to another directory and file
+ // would be written there). To prevent this *at syscalls are used and
+ // all symlinks in the path are rejected. This still permits the user
+ // to move dir during the sync but only to places which are writable
+ // by the user which cannot be prevented.
err := s.fileResolveIds(file)
if err != nil {
file.Path, file.OrigGroup, fmt.Sprintf(format, a...))
}
- var oldStat fs.FileInfo
+ parentFd, baseName, err := OpenParentDirectoryNoSymlinks(file.Path)
+ if err != nil {
+ return err
+ }
+ defer unix.Close(parentFd)
+
+ var oldStat unix.Stat_t
reopen:
- oldFh, err := OpenFileNoFollow(file.Path)
+ oldFh, err := OpenAtNoFollow(parentFd, baseName)
if err != nil {
- err := err.(*fs.PathError)
- if err.Err == syscall.ELOOP || err.Err == syscall.EMLINK {
- // Check if ELOOP was caused not by O_NOFOLLOW but by
- // too many nested symlinks before the final path
- // component.
- x, err := os.Lstat(file.Path)
+ if err == unix.ELOOP || err == unix.EMLINK {
+ // ELOOP from symbolic link, this is fine
+ err := unix.Fstatat(parentFd, baseName, &oldStat,
+ unix.AT_SYMLINK_NOFOLLOW)
if err != nil {
return err
}
- if x.Mode().Type() != fs.ModeSymlink {
- debugf("type changed from symlink to %s, retry",
- x.Mode().Type())
+ if oldStat.Mode&unix.S_IFMT != unix.S_IFLNK {
+ debugf("type changed from symlink, retrying")
goto reopen
}
- // ELOOP from symbolic link, this is fine
- oldStat = x
} else if os.IsNotExist(err) {
change.Created = true
debugf("will create")
} else {
defer oldFh.Close()
- x, err := oldFh.Stat()
+ err := unix.Fstat(int(oldFh.Fd()), &oldStat)
if err != nil {
return err
}
- oldStat = x
}
var oldData []byte
var changeType, changePerm, changeUserOrGroup, changeData bool
if !change.Created {
+ // Manually convert to FileMode; from src/os/stat_linux.go in
+ // Go's sources (stat_*.go for other UNIX systems are
+ // identical, except for stat_darwin.go which has an extra
+ // S_IFWHT)
+ mode := fs.FileMode(oldStat.Mode & 0777)
+ switch oldStat.Mode & unix.S_IFMT {
+ case unix.S_IFBLK:
+ mode |= fs.ModeDevice
+ case unix.S_IFCHR:
+ mode |= fs.ModeDevice | fs.ModeCharDevice
+ case unix.S_IFDIR:
+ mode |= fs.ModeDir
+ case unix.S_IFIFO:
+ mode |= fs.ModeNamedPipe
+ case unix.S_IFLNK:
+ mode |= fs.ModeSymlink
+ case unix.S_IFREG:
+ // nothing to do
+ case unix.S_IFSOCK:
+ mode |= fs.ModeSocket
+ // Guard against unknown file types (e.g. on darwin); not in
+ // stat_*.go
+ default:
+ return fmt.Errorf("unexpected file mode %v",
+ oldStat.Mode&unix.S_IFMT)
+ }
+ if oldStat.Mode&unix.S_ISGID != 0 {
+ mode |= fs.ModeSetgid
+ }
+ if oldStat.Mode&unix.S_ISUID != 0 {
+ mode |= fs.ModeSetuid
+ }
+ if oldStat.Mode&unix.S_ISVTX != 0 {
+ mode |= fs.ModeSticky
+ }
+
// Compare permissions
- change.Old.Mode = oldStat.Mode()
+ change.Old.Mode = mode
if change.Old.Mode.Type() == fs.ModeSymlink {
// Some BSD systems permit changing permissions of
// symlinks but ignore them on traversal. To keep it
}
// Compare user/group
- x, ok := oldStat.Sys().(*syscall.Stat_t)
- if !ok {
- return fmt.Errorf("unsupported Stat().Sys()")
- }
- change.Old.Uid = int(x.Uid)
- change.Old.Gid = int(x.Gid)
+ change.Old.Uid = int(oldStat.Uid)
+ change.Old.Gid = int(oldStat.Gid)
if change.Old.Uid != file.Uid || change.Old.Gid != file.Gid {
changeUserOrGroup = true
debugf("uid/gid differs %d/%d -> %d/%d",
}
oldData = x
case fs.ModeSymlink:
- x, err := os.Readlink(file.Path)
+ buf := make([]byte, unix.PathMax)
+ n, err := unix.Readlinkat(parentFd, baseName, buf)
if err != nil {
return fmt.Errorf("reading old content: %v",
err)
}
- oldData = []byte(x)
+ oldData = buf[:n]
}
if !changeType && file.Mode.Type() != fs.ModeDir {
if !bytes.Equal(oldData, file.Data) {
}
}
}
- oldStat = nil // prevent accidental use
// No changes
if !change.Created && !changeType &&
// We cannot rename over directories and vice versa
if changeType && (change.Old.Mode.IsDir() || file.Mode.IsDir()) {
debugf("removing (due to type change)")
- err := os.RemoveAll(file.Path)
- if err != nil {
- return err
+ // In the past os.RemoveAll() was used here. However, this is
+ // difficult to implement manually with *at syscalls. To keep
+ // it simple only permit removing files and empty directories
+ // here. This also has the bonus of preventing data loss when
+ // (accidentally) replacing a directory tree with a file.
+ const msg = "will not replace non-empty directory, " +
+ "please remove manually"
+ err := unix.Unlinkat(parentFd, baseName, 0)
+ if err != nil && !os.IsNotExist(err) {
+ err2 := unix.Unlinkat(parentFd, baseName,
+ AT_REMOVEDIR)
+ if err2 != nil && !os.IsNotExist(err2) {
+ // See src/os/file_unix.go in Go's sources
+ if err2 == unix.ENOTDIR {
+ return err
+ } else if err2 == unix.ENOTEMPTY {
+ return fmt.Errorf(msg)
+ } else {
+ return err2
+ }
+ }
}
}
// Directory: create new directory, also type change to directory
if file.Mode.IsDir() && (change.Created || changeType) {
debugf("creating directory")
- err := os.Mkdir(file.Path, 0700)
+ err := unix.Mkdirat(parentFd, baseName, 0700)
if err != nil {
return err
}
// We must be careful not to chmod arbitrary files. If the
// target directory is writable then it might have changed to
- // a symlink at this point. There's no lchmod so open the
- // directory.
+ // a symlink at this point. There's no lchmod and fchmodat is
+ // incomplete so open the directory.
debugf("chmodding %s", file.Mode)
- dh, err := OpenFileNoFollow(file.Path)
+ dh, err := OpenAtNoFollow(parentFd, baseName)
if err != nil {
return err
}
return nil
}
- dir := filepath.Dir(file.Path)
+ dir := slashpath.Dir(file.Path) // only used in debug messages
// Create hidden file which should be ignored by most other tools and
// thus not affect anything during creation
- base := "." + filepath.Base(file.Path)
+ tmpBase := "." + baseName
- var tmpPath string
switch file.Mode.Type() {
case 0: // regular file
debugf("creating temporary file %q",
- filepath.Join(dir, base+"*"))
- tmpPath, err = WriteTemp(dir, base, file.Data,
+ slashpath.Join(dir, tmpBase+"*"))
+ x, err := WriteTempAt(parentFd, tmpBase, file.Data,
file.Uid, file.Gid, file.Mode)
if err != nil {
return err
}
+ tmpBase = x
case fs.ModeSymlink:
i := 0
retry:
- // Similar to os.CreateTemp() but for symlinks which we cannot
- // open as file
- tmpPath = filepath.Join(dir,
- base+strconv.Itoa(rand.Int()))
- debugf("creating temporary symlink %q", tmpPath)
- err := os.Symlink(string(file.Data), tmpPath)
+ x := tmpBase + strconv.Itoa(rand.Int())
+ debugf("creating temporary symlink %q",
+ slashpath.Join(dir, x))
+ err := unix.Symlinkat(string(file.Data), parentFd, x)
if err != nil {
if os.IsExist(err) && i < 10000 {
i++
}
return err
}
- err = os.Lchown(tmpPath, file.Uid, file.Gid)
+ tmpBase = x
+
+ err = unix.Fchownat(parentFd, tmpBase, file.Uid, file.Gid,
+ unix.AT_SYMLINK_NOFOLLOW)
if err != nil {
- os.Remove(tmpPath)
+ unix.Unlinkat(parentFd, tmpBase, 0)
return err
}
// Permissions are irrelevant for symlinks (on most systems)
panic(fmt.Sprintf("invalid file type %s", file.Mode))
}
- debugf("renaming %q", tmpPath)
- err = os.Rename(tmpPath, file.Path)
+ debugf("renaming %q", slashpath.Join(dir, tmpBase))
+ err = unix.Renameat(parentFd, tmpBase, parentFd, baseName)
if err != nil {
- os.Remove(tmpPath)
+ unix.Unlinkat(parentFd, tmpBase, 0)
return err
}
- err = SyncPath(dir)
+ err = unix.Fsync(parentFd)
if err != nil {
return err
}
return nil
}
+// OpenParentDirectoryNoSymlinks opens the dirname of path without following
+// any symlinks and returns a file descriptor to it and the basename of path.
+// To prevent symlink attacks in earlier path components when these are
+// writable by other users it starts at the root (or current directory for
+// relative paths) and uses openat (with O_NOFOLLOW) for each path component.
+// If a symlink is encountered it returns an error. However, it's impossible
+// to guarantee that the returned descriptor refers to the same location as
+// given in path because users with write access can rename path components.
+// But this is not required to prevent the mentioned attacks.
+func OpenParentDirectoryNoSymlinks(path string) (int, string, error) {
+ // Slash separated paths are used for the configuration
+ parts := strings.Split(path, "/")
+
+ var dir string
+ if path == "/" {
+ // Root: use root itself as base name because root is the
+ // parent of itself
+ dir = "/"
+ parts = []string{"/"}
+ } else if parts[0] == "" {
+ // Absolute path
+ dir = "/"
+ parts = parts[1:]
+ } else if path == "." {
+ // Current directory: open parent directory and use current
+ // directory name as base name
+ wd, err := os.Getwd()
+ if err != nil {
+ return -1, "", fmt.Errorf(
+ "failed to get working directory: %w", err)
+ }
+ dir = ".."
+ parts = []string{filepath.Base(wd)}
+ } else if parts[0] != "." {
+ // Relative path: start at the current directory
+ dir = "."
+ }
+
+ dirFd, err := unix.Openat(unix.AT_FDCWD, dir, openReadonlyFlags, 0)
+ if err != nil {
+ return -1, "", err
+ }
+ // Walk path one directory at a time to ensure there are no symlinks
+ // in the path. This prevents users with write access to change the
+ // path to point to arbitrary locations. O_NOFOLLOW when opening the
+ // path is not enough as only the last path component is checked.
+ for i, name := range parts[:len(parts)-1] {
+ fd, err := unix.Openat(dirFd, name, openReadonlyFlags, 0)
+ if err != nil {
+ unix.Close(dirFd)
+ if err == unix.ELOOP || err == unix.EMLINK {
+ x := filepath.Join(append([]string{dir},
+ parts[:i+1]...)...)
+ return -1, "", fmt.Errorf(
+ "symlink not permitted in path: %q",
+ x)
+ }
+ return -1, "", err
+ }
+ unix.Close(dirFd)
+ dirFd = fd
+ }
+
+ return dirFd, parts[len(parts)-1], nil
+}
+
func resolveIdsToNames(uid, gid int) (string, string, error) {
u, err := user.LookupId(strconv.Itoa(uid))
if err != nil {
return result, nil
}
-func OpenFileNoFollow(path string) (*os.File, error) {
- return os.OpenFile(path,
- // O_NOFOLLOW prevents symlink attacks
- // O_NONBLOCK is necessary to prevent blocking on FIFOs
- os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
+func OpenFileNoSymlinks(path string) (*os.File, error) {
+ parentFd, baseName, err := OpenParentDirectoryNoSymlinks(path)
+ if err != nil {
+ return nil, err
+ }
+ defer unix.Close(parentFd)
+ return OpenAtNoFollow(parentFd, baseName)
+}
+
+func OpenAtNoFollow(dirFd int, base string) (*os.File, error) {
+ fd, err := unix.Openat(dirFd, base, openReadonlyFlags, 0)
+ if err != nil {
+ return nil, err
+ }
+ return os.NewFile(uintptr(fd), ""), nil
}
-func WriteTemp(dir, base string, data []byte, uid, gid int, mode fs.FileMode) (
- string, error) {
+func WriteTempAt(dirFd int, base string, data []byte, uid, gid int,
+ mode fs.FileMode) (string, error) {
- fh, err := os.CreateTemp(dir, base)
+ fh, tmpBase, err := createTempAt(dirFd, base)
if err != nil {
return "", err
}
- tmpPath := fh.Name()
_, err = fh.Write(data)
if err != nil {
fh.Close()
- os.Remove(tmpPath)
+ unix.Unlinkat(dirFd, tmpBase, 0)
return "", err
}
- // CreateTemp() creates the file with 0600
+ // createTempAt() creates the file with 0600
err = fh.Chown(uid, gid)
if err != nil {
fh.Close()
- os.Remove(tmpPath)
+ unix.Unlinkat(dirFd, tmpBase, 0)
return "", err
}
err = fh.Chmod(mode)
if err != nil {
fh.Close()
- os.Remove(tmpPath)
+ unix.Unlinkat(dirFd, tmpBase, 0)
return "", err
}
err = fh.Sync()
if err != nil {
fh.Close()
- os.Remove(tmpPath)
+ unix.Unlinkat(dirFd, tmpBase, 0)
return "", err
}
err = fh.Close()
if err != nil {
- os.Remove(tmpPath)
+ unix.Unlinkat(dirFd, tmpBase, 0)
return "", err
}
- return tmpPath, nil
+ return tmpBase, nil
+}
+
+// createTempAt works similar to os.CreateTemp but uses unix.Openat to create
+// the temporary file.
+func createTempAt(dirFd int, base string) (*os.File, string, error) {
+ var tmpBase string
+
+ i := 0
+retry:
+ tmpBase = base + strconv.Itoa(rand.Int())
+
+ fd, err := unix.Openat(dirFd, tmpBase,
+ unix.O_RDWR|unix.O_CREAT|unix.O_EXCL, 0600)
+ if err != nil {
+ if os.IsExist(err) && i < 10000 {
+ i++
+ goto retry
+ }
+ return nil, "", err
+ }
+
+ return os.NewFile(uintptr(fd), ""), tmpBase, nil
}
// SyncPath syncs path, which should be a directory. To guarantee durability
`4: files: "file" (group): will create`,
`3: files: "file" (group): creating`,
`4: files: "file" (group): creating temporary file ".file*"`,
- `4: files: "file" (group): renaming "./.fileRND"`,
+ `4: files: "file" (group): renaming ".fileRND"`,
},
nil,
},
`4: files: "file" (group): permission differs -rwxr-xr-x -> urwxr-xr-x`,
`3: files: "file" (group): updating`,
`4: files: "file" (group): creating temporary file ".file*"`,
- `4: files: "file" (group): renaming "./.fileRND"`,
+ `4: files: "file" (group): renaming ".fileRND"`,
},
nil,
},
`4: files: "file" (group): content differs`,
`3: files: "file" (group): updating`,
`4: files: "file" (group): creating temporary file ".file*"`,
- `4: files: "file" (group): renaming "./.fileRND"`,
+ `4: files: "file" (group): renaming ".fileRND"`,
},
nil,
},
`4: files: "path" (group): type differs L--------- -> ----------`,
`3: files: "path" (group): updating`,
`4: files: "path" (group): creating temporary file ".path*"`,
- `4: files: "path" (group): renaming "./.pathRND"`,
+ `4: files: "path" (group): renaming ".pathRND"`,
},
nil,
},
`3: files: "path" (group): updating`,
`4: files: "path" (group): removing (due to type change)`,
`4: files: "path" (group): creating temporary file ".path*"`,
- `4: files: "path" (group): renaming "./.pathRND"`,
+ `4: files: "path" (group): renaming ".pathRND"`,
},
nil,
},
+ {
+ "change: directory to file (non-empty)",
+ safcm.MsgSyncReq{},
+ &safcm.File{
+ Path: "path",
+ Mode: 0666,
+ Uid: -1,
+ Gid: -1,
+ OrigGroup: "group",
+ Data: []byte("content\n"),
+ },
+ func() {
+ ft.CreateDirectory("path", 0777)
+ ft.CreateFile("path/file", "content\n", 0644)
+ },
+ true,
+ []ft.File{
+ root,
+ {
+ Path: "path",
+ Mode: fs.ModeDir | 0777,
+ },
+ {
+ Path: "path/file",
+ Mode: 0644,
+ Data: []byte("content\n"),
+ },
+ },
+ safcm.MsgSyncResp{
+ FileChanges: []safcm.FileChange{
+ {
+ Path: "path",
+ Old: safcm.FileChangeInfo{
+ Mode: fs.ModeDir | 0777,
+ User: user,
+ Uid: uid,
+ Group: group,
+ Gid: gid,
+ },
+ New: safcm.FileChangeInfo{
+ Mode: 0666,
+ User: user,
+ Uid: uid,
+ Group: group,
+ Gid: gid,
+ },
+ },
+ },
+ },
+ []string{
+ `4: files: "path" (group): type differs d--------- -> ----------`,
+ `3: files: "path" (group): updating`,
+ `4: files: "path" (group): removing (due to type change)`,
+ },
+ fmt.Errorf("will not replace non-empty directory, please remove manually"),
+ },
{
"change: directory to symlink",
`4: files: "path" (group): type differs p--------- -> ----------`,
`3: files: "path" (group): updating`,
`4: files: "path" (group): creating temporary file ".path*"`,
- `4: files: "path" (group): renaming "./.pathRND"`,
+ `4: files: "path" (group): renaming ".pathRND"`,
},
nil,
},
nil,
},
+ // Symlink "attacks"
+
+ {
+ "symlink in earlier path component",
+ safcm.MsgSyncReq{},
+ &safcm.File{
+ Path: "dir/file",
+ Mode: 0644,
+ Uid: -1,
+ Gid: -1,
+ OrigGroup: "group",
+ Data: []byte("content"),
+ },
+ func() {
+ ft.CreateDirectory("tmp", 0755)
+ ft.CreateSymlink("dir", "tmp")
+ },
+ false,
+ []ft.File{
+ root,
+ {
+ Path: "dir",
+ Mode: fs.ModeSymlink | 0777,
+ Data: []byte("tmp"),
+ },
+ {
+ Path: "tmp",
+ Mode: fs.ModeDir | 0755,
+ },
+ },
+ safcm.MsgSyncResp{},
+ nil,
+ fmt.Errorf("symlink not permitted in path: \"dir\""),
+ },
+
// Diffs
{
ruderich.org/simon / safcm / safcm.git / commitdiff