Skip to content

Commit

Permalink
handle /run/secrets more gracefully if its a directory
Browse files Browse the repository at this point in the history
  • Loading branch information
Mic92 committed Jan 10, 2025
1 parent 74b9fe5 commit f214c1b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 25 deletions.
3 changes: 3 additions & 0 deletions checks/nixos-test.nix
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ in
boot.initrd.postDeviceCommands = ''
cp -r ${testAssets + "/age-keys.txt"} /run/age-keys.txt
chmod -R 700 /run/age-keys.txt
# if the directory exists, sops-nix should replace it with a symlink
mkdir /run/secrets
'';
};

Expand Down
52 changes: 27 additions & 25 deletions pkgs/sops-install-secrets/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,19 @@ type template struct {
}

type manifest struct {
Secrets []secret `json:"secrets"`
Templates []template `json:"templates"`
PlaceholderBySecretName map[string]string `json:"placeholderBySecretName"`
SecretsMountPoint string `json:"secretsMountPoint"`
SymlinkPath string `json:"symlinkPath"`
KeepGenerations int `json:"keepGenerations"`
SSHKeyPaths []string `json:"sshKeyPaths"`
GnupgHome string `json:"gnupgHome"`
AgeKeyFile string `json:"ageKeyFile"`
AgeSSHKeyPaths []string `json:"ageSshKeyPaths"`
UseTmpfs bool `json:"useTmpfs"`
UserMode bool `json:"userMode"`
Logging loggingConfig `json:"logging"`
Secrets []secret `json:"secrets"`
Templates []template `json:"templates"`
PlaceholderBySecretName map[string]string `json:"placeholderBySecretName"`
SecretsMountPoint string `json:"secretsMountPoint"`
SymlinkPath string `json:"symlinkPath"`
KeepGenerations int `json:"keepGenerations"`
SSHKeyPaths []string `json:"sshKeyPaths"`
GnupgHome string `json:"gnupgHome"`
AgeKeyFile string `json:"ageKeyFile"`
AgeSSHKeyPaths []string `json:"ageSshKeyPaths"`
UseTmpfs bool `json:"useTmpfs"`
UserMode bool `json:"userMode"`
Logging loggingConfig `json:"logging"`
}

type secretFile struct {
Expand Down Expand Up @@ -379,7 +379,13 @@ func prepareSecretsDir(secretMountpoint string, linkName string, keysGID int, us
}
}
} else if !os.IsNotExist(err) {
return nil, fmt.Errorf("cannot access %s: %w", linkName, err)
if _, err2 := os.Lstat(linkName); err2 != nil {
return nil, fmt.Errorf("cannot access %s: %w", linkName, err)
}
// if `/run/secrets` exists, but is not a symlink, we need to remove it
if err = os.RemoveAll(linkName); err != nil {
return nil, fmt.Errorf("cannot remove %s: %w", linkName, err)
}
}
generation++
dir := filepath.Join(secretMountpoint, strconv.Itoa(int(generation)))
Expand Down Expand Up @@ -718,39 +724,35 @@ func (app *appContext) validateManifest() error {

func atomicSymlink(oldname, newname string) error {
if err := os.MkdirAll(filepath.Dir(newname), 0o755); err != nil {
return err
return fmt.Errorf("cannot create directory %s: %w", filepath.Dir(newname), err)
}

// Fast path: if newname does not exist yet, we can skip the whole dance
// below.
if err := os.Symlink(oldname, newname); err == nil || !os.IsExist(err) {
return err
return fmt.Errorf("cannot create symlink %s: %w", newname, err)
}

// We need to use ioutil.TempDir, as we cannot overwrite a ioutil.TempFile,
// and removing+symlinking creates a TOCTOU race.
d, err := os.MkdirTemp(filepath.Dir(newname), "."+filepath.Base(newname))
if err != nil {
return err
return fmt.Errorf("cannot create temporary directory: %w", err)
}
cleanup := true
defer func() {
if cleanup {
os.RemoveAll(d)
}
os.RemoveAll(d)
}()

symlink := filepath.Join(d, "tmp.symlink")
if err := os.Symlink(oldname, symlink); err != nil {
return err
return fmt.Errorf("cannot create symlink %s: %w", symlink, err)
}

if err := os.Rename(symlink, newname); err != nil {
return err
return fmt.Errorf("cannot rename %s to %s: %w", symlink, newname, err)
}

cleanup = false
return os.RemoveAll(d)
return nil
}

func pruneGenerations(secretsMountPoint, secretsDir string, keepGenerations int) error {
Expand Down

3 comments on commit f214c1b

@nyabinary
Copy link

@nyabinary nyabinary commented on f214c1b Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm this broke something for me

Jan 10 18:02:39 nyan sops-install-secrets[438]: /nix/store/hjkqf3hbl2fl5b9k5mxhxgi4zi5fhdi4-sops-install-secrets-0.0.1/bin/sops-install-secrets: cannot update secrets symlink: cannot create symlink /run/secrets-for-users: %!>
Jan 10 18:02:39 nyan systemd[1]: sops-install-secrets-for-users.service: Main process exited, code=exited, status=1/FAILURE
Jan 10 18:02:39 nyan systemd[1]: sops-install-secrets-for-users.service: Failed with result 'exit-code'.
Jan 10 18:02:39 nyan systemd[1]: Failed to start sops-install-secrets-for-users.service.

@Mic92
Copy link
Owner Author

@Mic92 Mic92 commented on f214c1b Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do comment in commits because it's very easily missed and open issues instead.

@Mic92
Copy link
Owner Author

@Mic92 Mic92 commented on f214c1b Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is also doesn't seem to be cut off.

Please sign in to comment.