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

Possibly broken API key management for backends #627

Open
joepie91 opened this issue Mar 19, 2017 · 6 comments
Open

Possibly broken API key management for backends #627

joepie91 opened this issue Mar 19, 2017 · 6 comments

Comments

@joepie91
Copy link
Contributor

joepie91 commented Mar 19, 2017

While looking into an issue with the DigitalOcean backend (#628), I ran into a second issue that's more abstract in nature; for the authentication token to be available within the <DigitalOceanState>.destroy() call, it needs to be persisted in the auth_token state property. However, if the authentication token is ever rotated inbetween the create and destroy calls for a given machine, the destroy call will fail because the auth_token in the state file is no longer valid.

This can be worked around by overriding the active auth token using the DIGITAL_OCEAN_AUTH_TOKEN environment variable; but this is a usability stink, and not really a proper solution.

I'm not very familiar with NixOps - all I've described above and in my other issue is stuff that I've inferred from reading the codebase for a few hours, and I don't even have my first working deployment yet - but perhaps there's a better mechanism for accessing/maintaining auth tokens, that can be controlled outside of individual machines? Environment variables are a bit awkward for this purpose, as they can't be easily integrated into the network specification.

I'd imagine that the same issue applies to the Hetzner backend, and probably others as well. If no such mechanism exists yet, I'd perhaps suggest having "backend key management" - that is, something like the following:

  1. Store a named token or set of credentials in the network configuration.
  2. Store a reference to that name in the machine state.
  3. Resolve the credentials to use on runtime, based on that reference and the current set of credentials in the network configuration.

This would allow credential rotation without breaking destroy for currently existing machines. It wouldn't affect SSH key management; this suggestion is purely for things like API credentials.

(I may be entirely wrong about this issue; like I said, my experience with NixOps so far is very limited. Feedback is very welcome!)

@joepie91 joepie91 changed the title Possibly broken API key management for DigitalOcean backend and others Possibly broken API key management for backends Mar 19, 2017
@domenkozar domenkozar added the bug label Apr 12, 2017
@domenkozar
Copy link
Member

Good find, probably the cause for https://github.com/NixOS/nixops/issues/490

EC2 backend avoids this by storing API token in ~/.ec2-keys.

@nh2
Copy link
Contributor

nh2 commented Apr 15, 2018

I've written in https://github.com/NixOS/nixops/issues/490#issuecomment-381434938 what I think is the issue.

The solution appears easy:

Whenever making API requests, always check what's the correct, current API key by inspecting machine definition and environment variables.

I don't know why the API key is stored in the state (sqlite DB) at all. It seems unnecessary and the root cause of this problem.

@nh2
Copy link
Contributor

nh2 commented Apr 15, 2018

Also related: #925

@nh2
Copy link
Contributor

nh2 commented Apr 15, 2018

Whenever making API requests, always check what's the correct, current API key by inspecting machine definition and environment variables.

Hmm, that doesn't seem to be easily possible because destroy() doesn't take defn as an argument, only create() does. Maybe that's the root problem: Not all functions are given the information they need, thus the backends so far used unreliable hacks (storing it in the state in create()) that break in all but the standard life cycle (create-then-destroy).

Is there any reason not to make the contents of the .nix files visible to all functions, including e.g. destroy()? For that is static read-only information.

@nh2
Copy link
Contributor

nh2 commented Apr 15, 2018

Is there any reason not to make the contents of the .nix files visible to all functions

One possible reason might be that this requires evaluating the nix expressions, which can take very long (minutes if you have many servers), and that we don't want destroy() to take as long as deploy().

Is it possible to lazily evaluate only the deployment.* attributes conveniently, without evaluationg the entire machine definitions? Then we could do it without that drawback.

@nh2
Copy link
Contributor

nh2 commented Apr 15, 2018

Also as written in https://github.com/NixOS/nixops/issues/490#issuecomment-381434938, the Hetzner backend has a special case for updating the keys in create(). But that of course doesn't solve this issue entirely, only if create() is called before what you want to do with the rotated API token.

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

No branches or pull requests

4 participants