From eee831aadbc25f72fa9d214d57bddfd847e026b2 Mon Sep 17 00:00:00 2001 From: Jeremy Fleischman Date: Mon, 11 Nov 2024 00:18:56 -0600 Subject: [PATCH] Do not render templates when decrypting `neededForUsers` secrets This fixes https://github.com/Mic92/sops-nix/issues/659 In https://github.com/Mic92/sops-nix/pull/649, we started rendering templates twice: 1. When rendering `neededForUsers` secrets (if there are any `neededForUsers` secrets). 2. When decrypting "regular" secrets. This alone was weird and wrong, but didn't cause issues for people until https://github.com/Mic92/sops-nix/pull/655, which triggered https://github.com/Mic92/sops-nix/issues/659. The cause is not super obvious: 1. When rendering `neededForUsers` secrets, we'd generate templates in `/run/secrets-for-users/rendered`. 2. However, the `path` for these templates is in `/run/secrets/rendered`, which is not inside of the `/run/secrets-for-users` directory we're dealing with, so we'd generate a symlink from `/run/secrets/rendered/` to `/run/secrets-for-users/rendered/`, which required making the parent directory of the symlink (`/run/secrets/rendered/`). 3. This breaks sops-nix's assumption that `/run/secrets` either doesn't exist, or is a symlink, and you get the symptoms described in . Reproducing this in a test was straightforward: just expand our existing template test to also have a `neededForUsers` secret. Fixing this was also straightforward: don't render templates during the `neededForUsers` phase (if we want to add support for `neededForUsers` templates in the future, that would be straightforward to do, but I opted not do that here). --- modules/home-manager/sops.nix | 11 +++++----- modules/sops/default.nix | 7 +++++-- modules/sops/manifest-for.nix | 4 ++-- modules/sops/secrets-for-users/default.nix | 3 ++- pkgs/sops-install-secrets/main.go | 24 ++++++++++++---------- pkgs/sops-install-secrets/nixos-test.nix | 4 ++++ 6 files changed, 32 insertions(+), 21 deletions(-) diff --git a/modules/home-manager/sops.nix b/modules/home-manager/sops.nix index e1bb449..9432c41 100644 --- a/modules/home-manager/sops.nix +++ b/modules/home-manager/sops.nix @@ -3,7 +3,7 @@ let cfg = config.sops; sops-install-secrets = (pkgs.callPackage ../.. {}).sops-install-secrets; - secretType = lib.types.submodule ({ config, name, ... }: { + secretType = lib.types.submodule ({ name, ... }: { options = { name = lib.mkOption { type = lib.types.str; @@ -71,10 +71,11 @@ let merge = lib.mergeEqualOption; }; - manifestFor = suffix: secrets: pkgs.writeTextFile { + manifestFor = suffix: secrets: templates: pkgs.writeTextFile { name = "manifest${suffix}.json"; text = builtins.toJSON { secrets = builtins.attrValues secrets; + templates = builtins.attrValues templates; secretsMountPoint = cfg.defaultSecretsMountPoint; symlinkPath = cfg.defaultSymlinkPath; keepGenerations = cfg.keepGenerations; @@ -93,11 +94,11 @@ let ''; }; - manifest = manifestFor "" cfg.secrets; + manifest = manifestFor "" cfg.secrets cfg.templates; escapedAgeKeyFile = lib.escapeShellArg cfg.age.keyFile; - script = toString (pkgs.writeShellScript "sops-nix-user" ((lib.optionalString cfg.age.generateKey '' + script = toString (pkgs.writeShellScript "sops-nix-user" (lib.optionalString cfg.age.generateKey '' if [[ ! -f ${escapedAgeKeyFile} ]]; then echo generating machine-specific age key... ${pkgs.coreutils}/bin/mkdir -p $(${pkgs.coreutils}/bin/dirname ${escapedAgeKeyFile}) @@ -106,7 +107,7 @@ let fi '' + '' ${sops-install-secrets}/bin/sops-install-secrets -ignore-passwd ${manifest} - ''))); + '')); in { options.sops = { secrets = lib.mkOption { diff --git a/modules/sops/default.nix b/modules/sops/default.nix index bdd2f80..72cb94e 100644 --- a/modules/sops/default.nix +++ b/modules/sops/default.nix @@ -8,7 +8,7 @@ let inherit cfg; inherit (pkgs) writeTextFile; }; - manifest = manifestFor "" regularSecrets {}; + manifest = manifestFor "" regularSecrets regularTemplates {}; pathNotInStore = lib.mkOptionType { name = "pathNotInStore"; @@ -20,6 +20,9 @@ let regularSecrets = lib.filterAttrs (_: v: !v.neededForUsers) cfg.secrets; + # Currently, all templates are "regular" (there's no support for `neededForUsers` for templates.) + regularTemplates = cfg.templates; + useSystemdActivation = (options.systemd ? sysusers && config.systemd.sysusers.enable) || (options.services ? userborn && config.services.userborn.enable); @@ -354,7 +357,7 @@ in { sops.environment.SOPS_GPG_EXEC = lib.mkIf (cfg.gnupg.home != null || cfg.gnupg.sshKeyPaths != []) (lib.mkDefault "${pkgs.gnupg}/bin/gpg"); - # When using sysusers we no longer be started as an activation script because those are started in initrd while sysusers is started later. + # When using sysusers we no longer are started as an activation script because those are started in initrd while sysusers is started later. systemd.services.sops-install-secrets = lib.mkIf (regularSecrets != { } && useSystemdActivation) { wantedBy = [ "sysinit.target" ]; after = [ "systemd-sysusers.service" ]; diff --git a/modules/sops/manifest-for.nix b/modules/sops/manifest-for.nix index bdefaee..de62f08 100644 --- a/modules/sops/manifest-for.nix +++ b/modules/sops/manifest-for.nix @@ -1,11 +1,12 @@ { writeTextFile, cfg }: -suffix: secrets: extraJson: +suffix: secrets: templates: extraJson: writeTextFile { name = "manifest${suffix}.json"; text = builtins.toJSON ({ secrets = builtins.attrValues secrets; + templates = builtins.attrValues templates; # Does this need to be configurable? secretsMountPoint = "/run/secrets.d"; symlinkPath = "/run/secrets"; @@ -15,7 +16,6 @@ writeTextFile { ageKeyFile = cfg.age.keyFile; ageSshKeyPaths = cfg.age.sshKeyPaths; useTmpfs = cfg.useTmpfs; - templates = cfg.templates; placeholderBySecretName = cfg.placeholder; userMode = false; logging = { diff --git a/modules/sops/secrets-for-users/default.nix b/modules/sops/secrets-for-users/default.nix index 7c316c4..dc49aa4 100644 --- a/modules/sops/secrets-for-users/default.nix +++ b/modules/sops/secrets-for-users/default.nix @@ -2,6 +2,7 @@ let cfg = config.sops; secretsForUsers = lib.filterAttrs (_: v: v.neededForUsers) cfg.secrets; + templatesForUsers = {}; # We do not currently support `neededForUsers` for templates. manifestFor = pkgs.callPackage ../manifest-for.nix { inherit cfg; inherit (pkgs) writeTextFile; @@ -9,7 +10,7 @@ let withEnvironment = import ../with-environment.nix { inherit cfg lib; }; - manifestForUsers = manifestFor "-for-users" secretsForUsers { + manifestForUsers = manifestFor "-for-users" secretsForUsers templatesForUsers { secretsMountPoint = "/run/secrets-for-users.d"; symlinkPath = "/run/secrets-for-users"; }; diff --git a/pkgs/sops-install-secrets/main.go b/pkgs/sops-install-secrets/main.go index 391eb7d..f4764a0 100644 --- a/pkgs/sops-install-secrets/main.go +++ b/pkgs/sops-install-secrets/main.go @@ -71,7 +71,7 @@ type template struct { type manifest struct { Secrets []secret `json:"secrets"` - Templates map[string]*template `json:"templates"` + Templates []template `json:"templates"` PlaceholderBySecretName map[string]string `json:"placeholderBySecretName"` SecretsMountPoint string `json:"secretsMountPoint"` SymlinkPath string `json:"symlinkPath"` @@ -185,7 +185,7 @@ func linksAreEqual(linkTarget, targetFile string, info os.FileInfo, owner int, g return linkTarget == targetFile && validUG } -func symlinkSecret(targetFile string, path string, owner int, group int, userMode bool) error { +func createSymlink(targetFile string, path string, owner int, group int, userMode bool) error { for { stat, err := os.Lstat(path) if os.IsNotExist(err) { @@ -217,7 +217,7 @@ func symlinkSecret(targetFile string, path string, owner int, group int, userMod } } -func symlinkSecrets(targetDir string, secrets []secret, templates map[string]*template, userMode bool) error { +func symlinkSecretsAndTemplates(targetDir string, secrets []secret, templates []template, userMode bool) error { for _, secret := range secrets { targetFile := filepath.Join(targetDir, secret.Name) if targetFile == secret.Path { @@ -227,7 +227,7 @@ func symlinkSecrets(targetDir string, secrets []secret, templates map[string]*te if err := os.MkdirAll(parent, os.ModePerm); err != nil { return fmt.Errorf("cannot create parent directory of '%s': %w", secret.Path, err) } - if err := symlinkSecret(targetFile, secret.Path, secret.owner, secret.group, userMode); err != nil { + if err := createSymlink(targetFile, secret.Path, secret.owner, secret.group, userMode); err != nil { return fmt.Errorf("failed to symlink secret '%s': %w", secret.Path, err) } } @@ -241,7 +241,7 @@ func symlinkSecrets(targetDir string, secrets []secret, templates map[string]*te if err := os.MkdirAll(parent, os.ModePerm); err != nil { return fmt.Errorf("cannot create parent directory of '%s': %w", template.Path, err) } - if err := symlinkSecret(targetFile, template.Path, template.owner, template.group, userMode); err != nil { + if err := createSymlink(targetFile, template.Path, template.owner, template.group, userMode); err != nil { return fmt.Errorf("failed to symlink template '%s': %w", template.Path, err) } } @@ -610,8 +610,9 @@ func (app *appContext) validateSecret(secret *secret) error { return app.validateSopsFile(secret, &file) } -func renderTemplates(templates map[string]*template, secretByPlaceholder map[string]*secret) { - for _, template := range templates { +func renderTemplates(templates []template, secretByPlaceholder map[string]*secret) { + for i := range templates { + template := &templates[i] rendered := renderTemplate(&template.content, secretByPlaceholder) template.value = []byte(rendered) } @@ -702,7 +703,8 @@ func (app *appContext) validateManifest() error { } } - for _, template := range m.Templates { + for i := range m.Templates { + template := &m.Templates[i] if err := app.validateTemplate(template); err != nil { return err } @@ -893,7 +895,7 @@ func symlinkWalk(filename string, linkDirname string, walkFn filepath.WalkFunc) return filepath.Walk(filename, symWalkFunc) } -func handleModifications(isDry bool, logcfg loggingConfig, symlinkPath string, secretDir string, secrets []secret, templates map[string]*template) error { +func handleModifications(isDry bool, logcfg loggingConfig, symlinkPath string, secretDir string, secrets []secret, templates []template) error { var restart []string var reload []string @@ -1148,7 +1150,7 @@ func replaceRuntimeDir(path, rundir string) (ret string) { return } -func writeTemplates(targetDir string, templates map[string]*template, keysGID int, userMode bool) error { +func writeTemplates(targetDir string, templates []template, keysGID int, userMode bool) error { for _, template := range templates { fp := filepath.Join(targetDir, template.Name) @@ -1302,7 +1304,7 @@ func installSecrets(args []string) error { if isDry { return nil } - if err := symlinkSecrets(manifest.SymlinkPath, manifest.Secrets, manifest.Templates, manifest.UserMode); err != nil { + if err := symlinkSecretsAndTemplates(manifest.SymlinkPath, manifest.Secrets, manifest.Templates, manifest.UserMode); err != nil { return fmt.Errorf("failed to prepare symlinks to secret store: %w", err) } if err := atomicSymlink(*secretDir, manifest.SymlinkPath); err != nil { diff --git a/pkgs/sops-install-secrets/nixos-test.nix b/pkgs/sops-install-secrets/nixos-test.nix index 765d8be..dc246b6 100644 --- a/pkgs/sops-install-secrets/nixos-test.nix +++ b/pkgs/sops-install-secrets/nixos-test.nix @@ -266,6 +266,10 @@ in { age.keyFile = "/run/age-keys.txt"; defaultSopsFile = ./test-assets/secrets.yaml; secrets.test_key = { }; + + # Verify that things work even with `neededForUsers` secrets. See + # . + secrets."nested/test/file".neededForUsers = true; }; # must run before sops sets up keys