mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 21:47:20 +00:00
4fcbb57d23
## Summary - `checkPermFile` in `pkg/secure/secure.go` now self-heals incorrect file permissions via `os.Chmod` instead of returning a fatal error - Fixes orbit crash-looping indefinitely when `/opt/orbit/updates-metadata.json` has mode 755 instead of the expected 600 ## Problem Orbit refuses to start when `updates-metadata.json` has wrong permissions (e.g. 755 instead of 600), entering an infinite restart loop (`systemd` restart counter observed at 3447+). The manual workaround is `chmod 600 /opt/orbit/updates-metadata.json`, but the root cause — an external process changing file permissions — is intermittent and hard to track. The `checkPermFile` function in `pkg/secure/secure.go` was designed as a security check, but its behavior of fatally erroring on any permission mismatch causes a denial-of-service on the legitimate user. For comparison, `checkPermPath` (the directory equivalent) already tolerates permissions that are less permissive than expected. ## Fix When `checkPermFile` detects a permission mismatch, it now attempts `os.Chmod` to correct the permissions before proceeding. It only returns an error if the chmod itself fails (e.g. insufficient privileges). This preserves the security intent — files end up with correct permissions — while making orbit resilient to external permission drift. ## Test plan - [ ] `go test ./pkg/secure/ -v -run TestOpenFile` — verifies self-healing behavior - [ ] `go test ./pkg/secure/ -v -run TestMkdirAll` — unchanged, verifies directory checks still work - [ ] Manual: create `/opt/orbit/updates-metadata.json` with mode 755, start orbit, confirm it self-heals and starts normally --------- Co-authored-by: Bash Bandicoot <bash-bandicoot@users.noreply.github.com>
93 lines
2.3 KiB
Go
93 lines
2.3 KiB
Go
//go:build !windows
|
|
// +build !windows
|
|
|
|
package secure
|
|
|
|
import (
|
|
"errors"
|
|
"fmt"
|
|
"os"
|
|
"path"
|
|
"syscall"
|
|
)
|
|
|
|
func isMorePermissive(currentMode, newMode os.FileMode) bool {
|
|
currentGroup := currentMode & 070
|
|
newGroup := newMode & 070
|
|
currentAll := currentMode & 07
|
|
newAll := newMode & 07
|
|
|
|
return newGroup > currentGroup || newAll > currentAll
|
|
}
|
|
|
|
func checkPermPath(path string, perm os.FileMode) error {
|
|
if !perm.IsDir() {
|
|
perm ^= os.ModeDir
|
|
}
|
|
|
|
dir, err := os.Stat(path)
|
|
if err == nil {
|
|
if dir.IsDir() {
|
|
if isMorePermissive(dir.Mode(), perm) {
|
|
return fmt.Errorf(
|
|
"Path %s already exists with mode %o instead of the expected %o", path, dir.Mode(), perm)
|
|
}
|
|
return nil
|
|
}
|
|
return &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR}
|
|
}
|
|
|
|
// Look for the parent directory in the path and then check the permissions in that
|
|
// This is based on the logic from os.MkdirAll
|
|
i := len(path)
|
|
// This first loop skips over trailing path separators. Eg:
|
|
// /some/path//////
|
|
// ^ i will end up here
|
|
for i > 0 && os.IsPathSeparator(path[i-1]) { // Skip trailing path separator.
|
|
i--
|
|
}
|
|
|
|
j := i
|
|
// This loop starts from where i left off and finds the previous path separator
|
|
// /some/path//////
|
|
// ^ j will end up here
|
|
for j > 0 && !os.IsPathSeparator(path[j-1]) { // Scan backward over element.
|
|
j--
|
|
}
|
|
|
|
// if we are pointing at a path separator that is not the root separator, then check for the path before it
|
|
// /some
|
|
if j > 1 {
|
|
return checkPermPath(path[:j-1], perm)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func checkPermFile(filePath string, perm os.FileMode) error {
|
|
if f, err := os.Stat(filePath); !errors.Is(err, os.ErrNotExist) && f != nil && f.Mode() != perm {
|
|
if chmodErr := os.Chmod(filePath, perm); chmodErr != nil {
|
|
return fmt.Errorf(
|
|
"file %s has mode %o instead of the expected %o and chmod failed: %w", filePath, f.Mode(), perm, chmodErr)
|
|
}
|
|
}
|
|
if err := checkPermPath(path.Dir(filePath), perm); err != nil {
|
|
return err
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func MkdirAll(path string, perm os.FileMode) error {
|
|
if err := checkPermPath(path, perm); err != nil {
|
|
return err
|
|
}
|
|
return os.MkdirAll(path, perm)
|
|
}
|
|
|
|
func OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) {
|
|
if err := checkPermFile(name, perm); err != nil {
|
|
return nil, err
|
|
}
|
|
return os.OpenFile(name, flag, perm)
|
|
}
|