From b33837ae3cfa012b65810891bebbee71fa4c0658 Mon Sep 17 00:00:00 2001 From: Darren Dormer Date: Mon, 10 Feb 2025 12:55:49 +0100 Subject: [PATCH] Fix symlink ownership change on Darwin systems. --- pkgs/sops-install-secrets/darwin.go | 25 ------------------ pkgs/sops-install-secrets/linux.go | 32 ---------------------- pkgs/sops-install-secrets/main.go | 41 ++++++++++++++++++++++++++--- 3 files changed, 37 insertions(+), 61 deletions(-) diff --git a/pkgs/sops-install-secrets/darwin.go b/pkgs/sops-install-secrets/darwin.go index 05cbf75..d4cb291 100644 --- a/pkgs/sops-install-secrets/darwin.go +++ b/pkgs/sops-install-secrets/darwin.go @@ -9,8 +9,6 @@ import ( "os" "os/exec" "strings" - - "golang.org/x/sys/unix" ) func RuntimeDir() (string, error) { @@ -23,29 +21,6 @@ func RuntimeDir() (string, error) { return strings.TrimSuffix(rundir, "/"), nil } -func SecureSymlinkChown(symlinkToCheck string, expectedTarget string, owner, group int) error { - // not sure what O_PATH is needed for anyways - fd, err := unix.Open(symlinkToCheck, unix.O_CLOEXEC|unix.O_SYMLINK|unix.O_NOFOLLOW, 0) - if err != nil { - return fmt.Errorf("failed to open %s: %w", symlinkToCheck, err) - } - defer unix.Close(fd) - - buf := make([]byte, len(expectedTarget)+1) // oversize by one to detect trunc - n, err := unix.Readlinkat(fd, "", buf) - if err != nil { - return fmt.Errorf("couldn't readlinkat %s", symlinkToCheck) - } - if n > len(expectedTarget) || string(buf[:n]) != expectedTarget { - return fmt.Errorf("symlink %s does not point to %s", symlinkToCheck, expectedTarget) - } - err = unix.Fchownat(fd, "", owner, group, unix.AT_SYMLINK_NOFOLLOW) - if err != nil { - return fmt.Errorf("cannot change owner of '%s' to %d/%d: %w", symlinkToCheck, owner, group, err) - } - return nil -} - // Does: // mkdir /tmp/mymount // NUMSECTORS=128000 # a sector is 512 bytes diff --git a/pkgs/sops-install-secrets/linux.go b/pkgs/sops-install-secrets/linux.go index b551e4b..ed3f8a2 100644 --- a/pkgs/sops-install-secrets/linux.go +++ b/pkgs/sops-install-secrets/linux.go @@ -18,38 +18,6 @@ func RuntimeDir() (string, error) { return rundir, nil } -func SecureSymlinkChown(symlinkToCheck, expectedTarget string, owner, group int) error { - // fd, err := unix.Open(symlinkToCheck, unix.O_CLOEXEC|unix.O_PATH|unix.O_NOFOLLOW, 0) - fd, err := unix.Open(symlinkToCheck, unix.O_CLOEXEC|unix.O_PATH|unix.O_NOFOLLOW, 0) - if err != nil { - return fmt.Errorf("failed to open %s: %w", symlinkToCheck, err) - } - defer unix.Close(fd) - - buf := make([]byte, len(expectedTarget)+1) // oversize by one to detect trunc - n, err := unix.Readlinkat(fd, "", buf) - if err != nil { - return fmt.Errorf("couldn't readlinkat %s", symlinkToCheck) - } - if n > len(expectedTarget) || string(buf[:n]) != expectedTarget { - return fmt.Errorf("symlink %s does not point to %s", symlinkToCheck, expectedTarget) - } - stat := unix.Stat_t{} - err = unix.Fstat(fd, &stat) - if err != nil { - return fmt.Errorf("cannot stat '%s': %w", symlinkToCheck, err) - } - if stat.Uid == uint32(owner) && stat.Gid == uint32(group) { - return nil // already correct - } - - err = unix.Fchownat(fd, "", owner, group, unix.AT_EMPTY_PATH) - if err != nil { - return fmt.Errorf("cannot change owner of '%s' to %d/%d: %w", symlinkToCheck, owner, group, err) - } - return nil -} - func MountSecretFs(mountpoint string, keysGID int, useTmpfs bool, userMode bool) error { if err := os.MkdirAll(mountpoint, 0o751); err != nil { return fmt.Errorf("cannot create directory '%s': %w", mountpoint, err) diff --git a/pkgs/sops-install-secrets/main.go b/pkgs/sops-install-secrets/main.go index d500ef8..ec80fa7 100644 --- a/pkgs/sops-install-secrets/main.go +++ b/pkgs/sops-install-secrets/main.go @@ -185,17 +185,50 @@ func linksAreEqual(linkTarget, targetFile string, info os.FileInfo, owner int, g return linkTarget == targetFile && validUG } +func SecureSymlinkChown(targetFile string, path string, owner int, group int) error { + // Create a temp directory to house the symlink while we change it's + // ownership. `os.MkdirTemp` creates a directory with the permissions 0700. + // The temp dir is created in the same parent directory of the final + // symlink, because the later `os.Rename` operation won't work across disk + // devices. + dir, err := os.MkdirTemp(filepath.Dir(path), "") + if err != nil { + return fmt.Errorf("cannot create temporary symlink directory: %w", err) + } + defer os.RemoveAll(dir) + + // Create symlink to `targetFile` in the temp dir, before chowning it. + var tmpSymlink = filepath.Join(dir, filepath.Base(path)) + if err = os.Symlink(targetFile, tmpSymlink); err != nil { + return fmt.Errorf( + "cannot create symlink '%s' (pointing to '%s'): %w", path, targetFile, err) + } + + err = os.Lchown(tmpSymlink, owner, group) + if err != nil { + return fmt.Errorf( + "cannot change owner of symlink '%s' (pointing to '%s') to owner/group: %d/%d: %w", + tmpSymlink, targetFile, owner, group, err) + } + + // Move the chowned symlink to it's final location. + err = os.Rename(tmpSymlink, path) + if err != nil { + return fmt.Errorf("cannot move symlink '%s' to '%s': %w", tmpSymlink, path, err) + } + return nil +} + func createSymlink(targetFile string, path string, owner int, group int, userMode bool) error { for { stat, err := os.Lstat(path) if os.IsNotExist(err) { - if err = os.Symlink(targetFile, path); err != nil { - return fmt.Errorf("cannot create symlink '%s': %w", path, err) - } if !userMode { - if err = SecureSymlinkChown(path, targetFile, owner, group); err != nil { + if err = SecureSymlinkChown(targetFile, path, owner, group); err != nil { return fmt.Errorf("cannot chown symlink '%s': %w", path, err) } + } else if err = os.Symlink(targetFile, path); err != nil { + return fmt.Errorf("cannot create symlink '%s': %w", path, err) } return nil } else if err != nil {