1
0
Fork 0
mirror of https://github.com/Mic92/sops-nix.git synced 2024-12-14 11:57:52 +00:00

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/<foo>` to
   `/run/secrets-for-users/rendered/<foo>`, 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
   <https://github.com/Mic92/sops-nix/issues/659>.

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).
This commit is contained in:
Jeremy Fleischman 2024-11-11 00:18:56 -06:00 committed by mergify[bot]
parent 47fc1d8c72
commit eee831aadb
6 changed files with 32 additions and 21 deletions

View file

@ -3,7 +3,7 @@
let let
cfg = config.sops; cfg = config.sops;
sops-install-secrets = (pkgs.callPackage ../.. {}).sops-install-secrets; sops-install-secrets = (pkgs.callPackage ../.. {}).sops-install-secrets;
secretType = lib.types.submodule ({ config, name, ... }: { secretType = lib.types.submodule ({ name, ... }: {
options = { options = {
name = lib.mkOption { name = lib.mkOption {
type = lib.types.str; type = lib.types.str;
@ -71,10 +71,11 @@ let
merge = lib.mergeEqualOption; merge = lib.mergeEqualOption;
}; };
manifestFor = suffix: secrets: pkgs.writeTextFile { manifestFor = suffix: secrets: templates: pkgs.writeTextFile {
name = "manifest${suffix}.json"; name = "manifest${suffix}.json";
text = builtins.toJSON { text = builtins.toJSON {
secrets = builtins.attrValues secrets; secrets = builtins.attrValues secrets;
templates = builtins.attrValues templates;
secretsMountPoint = cfg.defaultSecretsMountPoint; secretsMountPoint = cfg.defaultSecretsMountPoint;
symlinkPath = cfg.defaultSymlinkPath; symlinkPath = cfg.defaultSymlinkPath;
keepGenerations = cfg.keepGenerations; keepGenerations = cfg.keepGenerations;
@ -93,11 +94,11 @@ let
''; '';
}; };
manifest = manifestFor "" cfg.secrets; manifest = manifestFor "" cfg.secrets cfg.templates;
escapedAgeKeyFile = lib.escapeShellArg cfg.age.keyFile; 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 if [[ ! -f ${escapedAgeKeyFile} ]]; then
echo generating machine-specific age key... echo generating machine-specific age key...
${pkgs.coreutils}/bin/mkdir -p $(${pkgs.coreutils}/bin/dirname ${escapedAgeKeyFile}) ${pkgs.coreutils}/bin/mkdir -p $(${pkgs.coreutils}/bin/dirname ${escapedAgeKeyFile})
@ -106,7 +107,7 @@ let
fi fi
'' + '' '' + ''
${sops-install-secrets}/bin/sops-install-secrets -ignore-passwd ${manifest} ${sops-install-secrets}/bin/sops-install-secrets -ignore-passwd ${manifest}
''))); ''));
in { in {
options.sops = { options.sops = {
secrets = lib.mkOption { secrets = lib.mkOption {

View file

@ -8,7 +8,7 @@ let
inherit cfg; inherit cfg;
inherit (pkgs) writeTextFile; inherit (pkgs) writeTextFile;
}; };
manifest = manifestFor "" regularSecrets {}; manifest = manifestFor "" regularSecrets regularTemplates {};
pathNotInStore = lib.mkOptionType { pathNotInStore = lib.mkOptionType {
name = "pathNotInStore"; name = "pathNotInStore";
@ -20,6 +20,9 @@ let
regularSecrets = lib.filterAttrs (_: v: !v.neededForUsers) cfg.secrets; 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) || useSystemdActivation = (options.systemd ? sysusers && config.systemd.sysusers.enable) ||
(options.services ? userborn && config.services.userborn.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"); 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) { systemd.services.sops-install-secrets = lib.mkIf (regularSecrets != { } && useSystemdActivation) {
wantedBy = [ "sysinit.target" ]; wantedBy = [ "sysinit.target" ];
after = [ "systemd-sysusers.service" ]; after = [ "systemd-sysusers.service" ];

View file

@ -1,11 +1,12 @@
{ writeTextFile, cfg }: { writeTextFile, cfg }:
suffix: secrets: extraJson: suffix: secrets: templates: extraJson:
writeTextFile { writeTextFile {
name = "manifest${suffix}.json"; name = "manifest${suffix}.json";
text = builtins.toJSON ({ text = builtins.toJSON ({
secrets = builtins.attrValues secrets; secrets = builtins.attrValues secrets;
templates = builtins.attrValues templates;
# Does this need to be configurable? # Does this need to be configurable?
secretsMountPoint = "/run/secrets.d"; secretsMountPoint = "/run/secrets.d";
symlinkPath = "/run/secrets"; symlinkPath = "/run/secrets";
@ -15,7 +16,6 @@ writeTextFile {
ageKeyFile = cfg.age.keyFile; ageKeyFile = cfg.age.keyFile;
ageSshKeyPaths = cfg.age.sshKeyPaths; ageSshKeyPaths = cfg.age.sshKeyPaths;
useTmpfs = cfg.useTmpfs; useTmpfs = cfg.useTmpfs;
templates = cfg.templates;
placeholderBySecretName = cfg.placeholder; placeholderBySecretName = cfg.placeholder;
userMode = false; userMode = false;
logging = { logging = {

View file

@ -2,6 +2,7 @@
let let
cfg = config.sops; cfg = config.sops;
secretsForUsers = lib.filterAttrs (_: v: v.neededForUsers) cfg.secrets; secretsForUsers = lib.filterAttrs (_: v: v.neededForUsers) cfg.secrets;
templatesForUsers = {}; # We do not currently support `neededForUsers` for templates.
manifestFor = pkgs.callPackage ../manifest-for.nix { manifestFor = pkgs.callPackage ../manifest-for.nix {
inherit cfg; inherit cfg;
inherit (pkgs) writeTextFile; inherit (pkgs) writeTextFile;
@ -9,7 +10,7 @@ let
withEnvironment = import ../with-environment.nix { withEnvironment = import ../with-environment.nix {
inherit cfg lib; inherit cfg lib;
}; };
manifestForUsers = manifestFor "-for-users" secretsForUsers { manifestForUsers = manifestFor "-for-users" secretsForUsers templatesForUsers {
secretsMountPoint = "/run/secrets-for-users.d"; secretsMountPoint = "/run/secrets-for-users.d";
symlinkPath = "/run/secrets-for-users"; symlinkPath = "/run/secrets-for-users";
}; };

View file

@ -71,7 +71,7 @@ type template struct {
type manifest struct { type manifest struct {
Secrets []secret `json:"secrets"` Secrets []secret `json:"secrets"`
Templates map[string]*template `json:"templates"` Templates []template `json:"templates"`
PlaceholderBySecretName map[string]string `json:"placeholderBySecretName"` PlaceholderBySecretName map[string]string `json:"placeholderBySecretName"`
SecretsMountPoint string `json:"secretsMountPoint"` SecretsMountPoint string `json:"secretsMountPoint"`
SymlinkPath string `json:"symlinkPath"` SymlinkPath string `json:"symlinkPath"`
@ -185,7 +185,7 @@ func linksAreEqual(linkTarget, targetFile string, info os.FileInfo, owner int, g
return linkTarget == targetFile && validUG 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 { for {
stat, err := os.Lstat(path) stat, err := os.Lstat(path)
if os.IsNotExist(err) { 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 { for _, secret := range secrets {
targetFile := filepath.Join(targetDir, secret.Name) targetFile := filepath.Join(targetDir, secret.Name)
if targetFile == secret.Path { 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 { if err := os.MkdirAll(parent, os.ModePerm); err != nil {
return fmt.Errorf("cannot create parent directory of '%s': %w", secret.Path, err) 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) 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 { if err := os.MkdirAll(parent, os.ModePerm); err != nil {
return fmt.Errorf("cannot create parent directory of '%s': %w", template.Path, err) 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) 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) return app.validateSopsFile(secret, &file)
} }
func renderTemplates(templates map[string]*template, secretByPlaceholder map[string]*secret) { func renderTemplates(templates []template, secretByPlaceholder map[string]*secret) {
for _, template := range templates { for i := range templates {
template := &templates[i]
rendered := renderTemplate(&template.content, secretByPlaceholder) rendered := renderTemplate(&template.content, secretByPlaceholder)
template.value = []byte(rendered) 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 { if err := app.validateTemplate(template); err != nil {
return err return err
} }
@ -893,7 +895,7 @@ func symlinkWalk(filename string, linkDirname string, walkFn filepath.WalkFunc)
return filepath.Walk(filename, symWalkFunc) 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 restart []string
var reload []string var reload []string
@ -1148,7 +1150,7 @@ func replaceRuntimeDir(path, rundir string) (ret string) {
return 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 { for _, template := range templates {
fp := filepath.Join(targetDir, template.Name) fp := filepath.Join(targetDir, template.Name)
@ -1302,7 +1304,7 @@ func installSecrets(args []string) error {
if isDry { if isDry {
return nil 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) return fmt.Errorf("failed to prepare symlinks to secret store: %w", err)
} }
if err := atomicSymlink(*secretDir, manifest.SymlinkPath); err != nil { if err := atomicSymlink(*secretDir, manifest.SymlinkPath); err != nil {

View file

@ -266,6 +266,10 @@ in {
age.keyFile = "/run/age-keys.txt"; age.keyFile = "/run/age-keys.txt";
defaultSopsFile = ./test-assets/secrets.yaml; defaultSopsFile = ./test-assets/secrets.yaml;
secrets.test_key = { }; secrets.test_key = { };
# Verify that things work even with `neededForUsers` secrets. See
# <https://github.com/Mic92/sops-nix/issues/659>.
secrets."nested/test/file".neededForUsers = true;
}; };
# must run before sops sets up keys # must run before sops sets up keys