Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Demonstration of how system activation fails if the template path is not "external" #660

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jfly
Copy link
Contributor

@jfly jfly commented Nov 11, 2024

This demonstrates one way of reproducing
#659. There may be others: I think you could achieve the same thing with a regular secret rather than a template.

Here's how this fails:

$ nix build .#checks.x86_64-linux.template-path-not-external -L
...
vm-test-run-sops-template-path-not-external> machine: must succeed: /run/current-system/bin/switch-to-configuration test
vm-test-run-sops-template-path-not-external> machine # [   11.028130] nixos[949]: switching to system configuration /nix/store/0jbh5rys5x1br66s4n777jq8ny2bq9nb-nixos-system-machine-test
vm-test-run-sops-template-path-not-external> machine # [   11.029653] systemd[1]: Stopped target Local File Systems.
vm-test-run-sops-template-path-not-external> machine # [   11.031619] systemd[1]: Stopped target Remote File Systems.
vm-test-run-sops-template-path-not-external> machine # activating the configuration...
vm-test-run-sops-template-path-not-external> machine # /nix/store/8788mz0w701ff12zpsgnbm7nzsfy290k-sops-install-secrets-0.0.1/bin/sops-install-secrets: failed to prepare new secrets directory: cannot access /run/secrets: readlink /run/secrets: invalid argument
vm-test-run-sops-template-path-not-external> machine # Failed to run activate script
...

The only fix I can think of is to check if the chosen path is inside of /run/secrets and reject it if it is (of course allowing for the default value for these paths, which actually is inside of /run/secrets).

…not "external"

This demonstrates one way of reproducing
Mic92#659. There may be others: I
think you could achieve the same thing with a regular secret rather than
a template.

Here's how this fails:

    $ nix build .#checks.x86_64-linux.template-path-not-external -L
    ...
    vm-test-run-sops-template-path-not-external> machine: must succeed: /run/current-system/bin/switch-to-configuration test
    vm-test-run-sops-template-path-not-external> machine # [   11.028130] nixos[949]: switching to system configuration /nix/store/0jbh5rys5x1br66s4n777jq8ny2bq9nb-nixos-system-machine-test
    vm-test-run-sops-template-path-not-external> machine # [   11.029653] systemd[1]: Stopped target Local File Systems.
    vm-test-run-sops-template-path-not-external> machine # [   11.031619] systemd[1]: Stopped target Remote File Systems.
    vm-test-run-sops-template-path-not-external> machine # activating the configuration...
    vm-test-run-sops-template-path-not-external> machine # /nix/store/8788mz0w701ff12zpsgnbm7nzsfy290k-sops-install-secrets-0.0.1/bin/sops-install-secrets: failed to prepare new secrets directory: cannot access /run/secrets: readlink /run/secrets: invalid argument
    vm-test-run-sops-template-path-not-external> machine # Failed to run activate script
    ...

The only fix I can think of is to check if the chosen path is inside of
`/run/secrets` and reject it if it is (of course allowing for the
default value for these paths, which actually *is* inside of
`/run/secrets`).
Test value: ${config.sops.placeholder.test_key}
'';
# This path is inside of `/run/secrets`, which isn't allowed.
path = "/run/secrets/foo-bar";
Copy link
Owner

Choose a reason for hiding this comment

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

If it's inside /run/secrets maybe it should name the template differently rather than doing the symlink? Alternative would be not allow this behavior and make it an eval error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, I like the "name it differently" idea. I think that'll actually clean up the implementation a bit, as it currently deals carefully (and verbosely) with the "rendered" subdir.

I'll give this a shot later today.

@Mic92
Copy link
Owner

Mic92 commented Nov 11, 2024

On the other hand this could also happen with any other invalid symlink as a target.

@jfly
Copy link
Contributor Author

jfly commented Nov 11, 2024

On the other hand this could also happen with any other invalid symlink as a target.

Sorry, what does "invalid symlink as a target" mean?

The issue I'm demonstrating here is showing how /run/secrets can end up being a directory rather than a symlink.

I suppose there are more subtle scenarios involving symlinks to /run/secrets that would not be handled by the fix. I'm not sure if that's what you're referring to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants