diff --git a/lib/tbot/botfs/botfs.go b/lib/tbot/botfs/botfs.go index dd834200836bd..6007f04859dda 100644 --- a/lib/tbot/botfs/botfs.go +++ b/lib/tbot/botfs/botfs.go @@ -66,12 +66,7 @@ const ( // ACLRequired enables ACL support and fails if ACLs are unavailable. ACLRequired ACLMode = "required" -) - -// OpenMode is a mode for opening files. -type OpenMode int -const ( // DefaultMode is the preferred permissions mode for bot files. DefaultMode fs.FileMode = 0600 @@ -79,16 +74,32 @@ const ( // Directories need the execute bit set for most operations on their // contents to succeed. DefaultDirMode fs.FileMode = 0700 +) - // ReadMode is the mode with which files should be opened for reading and - // writing. - ReadMode OpenMode = OpenMode(os.O_CREATE | os.O_RDONLY) +// OpenFlags is a bitmask containing flags passed to `open()` +type OpenFlags int - // WriteMode is the mode with which files should be opened specifically +const ( + // ReadFlags contains `open()` flags to be used when opening files for + // reading. The file will be created if it does not exist, and reads should + // return an empty byte array. + ReadFlags = iota + + // WriteFlags is the mode with which files should be opened specifically // for writing. - WriteMode OpenMode = OpenMode(os.O_CREATE | os.O_WRONLY | os.O_TRUNC) + WriteFlags ) +// Flags returns opening flags for this OpenFlags variant. +func (f OpenFlags) Flags() int { + switch f { + case WriteFlags: + return os.O_CREATE | os.O_WRONLY | os.O_TRUNC + default: + return os.O_RDONLY + } +} + // ACLOptions contains parameters needed to configure ACLs type ACLOptions struct { // BotUser is the bot user that should have write access to this entry @@ -124,10 +135,10 @@ func (s *ACLSelector) CheckAndSetDefaults() error { return nil } -// openStandard attempts to open the given path for reading and writing with -// O_CREATE set. -func openStandard(path string, mode OpenMode) (*os.File, error) { - file, err := os.OpenFile(path, int(mode), DefaultMode) +// openStandard attempts to open the given path. The file may be writable +// depending on the provided `OpenFlags` value. +func openStandard(path string, flags OpenFlags) (*os.File, error) { + file, err := os.OpenFile(path, flags.Flags(), DefaultMode) if err != nil { return nil, trace.ConvertSystemError(err) } @@ -146,7 +157,7 @@ func createStandard(path string, isDir bool) error { return nil } - f, err := openStandard(path, WriteMode) + f, err := openStandard(path, WriteFlags) if err != nil { return trace.Wrap(err) } diff --git a/lib/tbot/botfs/fs_linux.go b/lib/tbot/botfs/fs_linux.go index ff1ae6e7239db..fe20fc85796a7 100644 --- a/lib/tbot/botfs/fs_linux.go +++ b/lib/tbot/botfs/fs_linux.go @@ -28,7 +28,6 @@ import ( "io/fs" "os" "os/user" - "path/filepath" "strconv" "sync" "syscall" @@ -62,45 +61,62 @@ const modeACLNone fs.FileMode = 0 // missingSyscallWarning is used to reduce log spam when a syscall is missing. var missingSyscallWarning sync.Once -// openSecure opens the given path for writing (with O_CREAT, mode 0600) -// with the RESOLVE_NO_SYMLINKS flag set. -func openSecure(path string, mode OpenMode) (*os.File, error) { +// openSecure opens the given path for either reading or writing with the +// RESOLVE_NO_SYMLINKS flag set. +func openSecure(path string, flags OpenFlags) (*os.File, error) { + var mode uint64 + if flags != ReadFlags { + // openat2() with a nonzero mode will raise EINVAL unless O_CREATE or + // O_TMPFILE is set, so only set this in non-read mode. + mode = uint64(DefaultMode.Perm()) + } + how := unix.OpenHow{ - // Equivalent to 0600. Unfortunately it's not worth reusing our - // default file mode constant here. - Mode: unix.O_RDONLY | unix.S_IRUSR | unix.S_IWUSR, - Flags: uint64(mode), + Mode: mode, + Flags: uint64(flags.Flags()) | unix.O_CLOEXEC, Resolve: unix.RESOLVE_NO_SYMLINKS, } - fd, err := unix.Openat2(unix.AT_FDCWD, path, &how) - if err != nil { - // note: returning the original error here for comparison purposes - return nil, err - } + for { + fd, err := unix.Openat2(unix.AT_FDCWD, path, &how) + if errors.Is(err, syscall.EINTR) { + // Per the stdlib's implementation, EINTR errors should be ignored + // and retried. + continue + } else if err != nil { + // note: returning the original error here for comparison purposes + return nil, err + } + + file := os.NewFile(uintptr(fd), path) + if file == nil { + // Probably useless since this implies the fd itself is invalid, but + // attempt to close the fd anyway. + _ = unix.Close(fd) + return nil, os.ErrInvalid + } - // os.File.Close() appears to close wrapped files sanely, so rely on that - // rather than relying on callers to use unix.Close(fd) - return os.NewFile(uintptr(fd), filepath.Base(path)), nil + return file, nil + } } // openSymlinks mode opens the file for read or write using the given symlink // mode, potentially failing or logging a warning if symlinks can't be // secured. -func openSymlinksMode(path string, mode OpenMode, symlinksMode SymlinksMode) (*os.File, error) { +func openSymlinksMode(path string, flags OpenFlags, symlinksMode SymlinksMode) (*os.File, error) { var file *os.File var err error switch symlinksMode { case SymlinksSecure: - file, err = openSecure(path, mode) + file, err = openSecure(path, flags) if errors.Is(err, unix.ENOSYS) { return nil, trace.Errorf("openSecure failed due to missing syscall; configure `symlinks: insecure` for %q", path) } else if err != nil { return nil, trace.Wrap(err) } case SymlinksTrySecure: - file, err = openSecure(path, mode) + file, err = openSecure(path, flags) if errors.Is(err, unix.ENOSYS) { missingSyscallWarning.Do(func() { log.DebugContext( @@ -110,7 +126,7 @@ func openSymlinksMode(path string, mode OpenMode, symlinksMode SymlinksMode) (*o ) }) - file, err = openStandard(path, mode) + file, err = openStandard(path, flags) if err != nil { return nil, trace.Wrap(err) } @@ -118,7 +134,7 @@ func openSymlinksMode(path string, mode OpenMode, symlinksMode SymlinksMode) (*o return nil, trace.Wrap(err) } case SymlinksInsecure: - file, err = openStandard(path, mode) + file, err = openStandard(path, flags) if err != nil { return nil, trace.Wrap(err) } @@ -144,7 +160,7 @@ func createSecure(path string, isDir bool) error { return nil } - f, err := openSecure(path, WriteMode) + f, err := openSecure(path, WriteFlags) if errors.Is(err, unix.ENOSYS) { // bubble up the original error for comparison return err @@ -217,7 +233,7 @@ func Create(path string, isDir bool, symlinksMode SymlinksMode) error { // Read reads the contents of the given file into memory. func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { - file, err := openSymlinksMode(path, ReadMode, symlinksMode) + file, err := openSymlinksMode(path, ReadFlags, symlinksMode) if err != nil { return nil, trace.Wrap(err) } @@ -234,7 +250,7 @@ func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { // Write stores the given data to the file at the given path. func Write(path string, data []byte, symlinksMode SymlinksMode) error { - file, err := openSymlinksMode(path, WriteMode, symlinksMode) + file, err := openSymlinksMode(path, WriteFlags, symlinksMode) if err != nil { return trace.Wrap(err) } diff --git a/lib/tbot/botfs/fs_unix.go b/lib/tbot/botfs/fs_unix.go index 5e8596f5e2f5f..71d56f8b72516 100644 --- a/lib/tbot/botfs/fs_unix.go +++ b/lib/tbot/botfs/fs_unix.go @@ -60,7 +60,7 @@ func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { }) } - file, err := openStandard(path, ReadMode) + file, err := openStandard(path, ReadFlags) if err != nil { return nil, trace.Wrap(err) } @@ -90,7 +90,7 @@ func Write(path string, data []byte, symlinksMode SymlinksMode) error { }) } - file, err := openStandard(path, WriteMode) + file, err := openStandard(path, WriteFlags) if err != nil { return trace.Wrap(err) } diff --git a/lib/tbot/botfs/fs_windows.go b/lib/tbot/botfs/fs_windows.go index 75b72ce2a6a9a..298566a465c15 100644 --- a/lib/tbot/botfs/fs_windows.go +++ b/lib/tbot/botfs/fs_windows.go @@ -61,7 +61,7 @@ func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { }) } - file, err := openStandard(path, ReadMode) + file, err := openStandard(path, ReadFlags) if err != nil { return nil, trace.Wrap(err) } @@ -91,7 +91,7 @@ func Write(path string, data []byte, symlinksMode SymlinksMode) error { }) } - file, err := openStandard(path, WriteMode) + file, err := openStandard(path, WriteFlags) if err != nil { return trace.Wrap(err) }