Stop storing keys in the nix store for storeKeysOnMachine. #650
Conversation
kevincox
force-pushed
the
keys-no-store
branch
2 times, most recently
from
ae36e61
to
5de6739
Compare
|
cc: @aszlig @ryanartecona @edolstra as you guys have touched this code last. |
There was a problem hiding this comment.
Nice! The existence of storeKeysOnMachine in its current form always made me nervous, and I never understood why it was an option anyway. This makes much more sense to me. I think this is roughly how I would've expected it to work when I first learned about it.
|
So really |
|
Yeah, it probably could have a better name. But that is orthogonal to this PR. |
There was a problem hiding this comment.
First of all, having /etc/nixops-keys sounds a bit dangerous to me, given that files in /etc are usually generated and might be overwritten if there is an environment.etc."nixops-keys" option definition. So I'd opt to use something in /var/lib instead.
But that aside, having different key directories based on whether storeKeysOnMachine is true or false is also not necessary here, because we can simply bind-mount the keys from let's say /var/lib/nixops-keys to /run/keys - even on a one by one basis.
|
On Apr 25, 2017 19:28, "aszlig" ***@***.***> wrote:
First of all, having /etc/nixops-keys sounds a bit dangerous to me, given that files in /etc are usually generated and might be overwritten if there is an environment.etc."nixops-keys" option definition. So I'd opt to use something in /var/lib instead.
How about setting up the symlink using Nix. That sounds like the best of
both worlds.
But that aside, having different key directories based on whether storeKeysOnMachine is true or false is also not necessary here, because we can simply bind-mount the keys from let's say /var/lib/nixops-keys to /run/keys - even on a one by one basis.
Call me whatever you will but I like the simplicity of a different
directory over bind mounting a disk directory over a ramfs. However if
people prefer a bind mount I'll galdly change it.
|
|
@kevincox: Instead of bind mounts, I guess symlinks are okay as well, since that's what's already done in the existing implementation, although a bind mount would have the advantage that it will work even if programs don't properly dereference symlinks or doing permission checks on the symlink. Having it in a different directory now means that existing deployments need to check Another problem I see here (which is problematic for |
Updated.
I didn't really want to remove the keys on deploy because the new keys are uploaded before the new config is activated. I could probably add it to the chmod activation script but since removing keys is pretty rare I propose pushing this to the next PR. |
This patch changes storeKeysOnMachine to work roughly the same way when true as when false. The only difference is that the keys are stored in /var/keys (which is usually a real disk) as opposed to /run/keys (which is RAM). This is an improvement on the previous version of storeKeysOnMachine where the keys were stored in the Nix store and linked from /run/keys. The previous version doesn't allow setting unix permissions on the keys, meaning any process on the server can read all keys. This solution has the downside that rolling back doesn't roll back the keys, however this is consistent with how storeKeysOnMachine=false works so shouldn't be a major concern. Furthermore if someone wants to have the keys rollback with the system they can use builtins.toFile instead of the keys mechanism which works the same way that the previous storeKeysOnMachine=true worked. Additionally a symlink /etc/nixops-keys has been added that will be set to the appropriate key location to make it easier to switch between the options.
|
Guys, how about adding an option for keys path? Defaulting to I used to have services for persisting keys: https://gist.github.com/ip1981/755f7bf71c157ebb1bd8f6d404cae5d3 And I'm really ready for configurable keys path: https://github.com/ip1981/nixsap/blob/49659d55878d09827566902d07d01f051926aff2/modules/deployment/keyrings.nix#L38 :) |
|
We clearly need to document motivations for these options. We are mixing different needs here. The way I see it:
|
|
A different take on this: #664 |
This is what the first version did basically. It was binary but changing the target would be trivial. It also added a
If you want to do this simply use builtins.toFile. I don't see any advantage to using the keys system.
I'm okay with that. |
|
Honestly, I think rollbacks are overestimated. First of all, it means discrepancy between the machine specification and actual state. Second, deploy with previous configuration is in fact a rollback. If you haven't done GC, it will be very fast. Quick rollback in case of f*ck up... should not happen :) Thanks to Nix, I can test configurations and migrations to death before pushing to production. |
|
I only see some value in "keys in the store" that if key content changes you will get activation/restart. There is a hopefully nice workaround: use new key name for new content, e .g. by appending a number to its name :) |
|
I think rollbacks aren't commonly useful to have for the key level. I would argue that they are very useful however when you need to get an ssh server running so that you can deploy the new version :) That being said I don't know how many people depend on keys for their ssh server and can't use a newer key. |
|
#664 is ready for review as an alternative implementation. It doesn't change behavior of |
|
Hello! Thank you for this PR. In the past several months, some major changes have taken place in
This is all accumulating in to what I hope will be a NixOps 2.0 My hope is that by adding types and more thorough automated testing, However, because of the major changes, it has become likely that this If you would like to see this merge, please bring it up to date with Thank you again for the work you've done here, I am sorry to be Graham |
This patch changes storeKeysOnMachine to work roughly the same way when
true as when false. The only difference is that the keys are stored in
/var/keys (which is usually a real disk) as opposed to /run/keys (which
is RAM).
This is an improvement on the previous version of storeKeysOnMachine
where the keys were stored in the Nix store and linked from /run/keys.
The previous version doesn't allow setting unix permissions on the keys,
meaning any process on the server can read all keys.
This solution has the downside that rolling back doesn't roll back the
keys, however this is consistent with how storeKeysOnMachine=false works
so shouldn't be a major concern. Furthermore if someone wants to have
the keys rollback with the system they can use builtins.toFile instead
of the keys mechanism which works the same way that the previous
storeKeysOnMachine=true worked.
Additionally a symlink /etc/nixos-keys has been added that will be set
to the appropriate key location to make it easier to switch between the
options.