search

getsops / sops

Simple and flexible tool for managing secrets
https://getsops.io/
Mozilla Public License 2.0
16.97k stars 878 forks source link

`updatekeys` should trigger on `shamir_threshold` change #810

Open dimrozakis opened 3 years ago

dimrozakis commented 3 years ago

When using groups to split a key using shamir secret sharing, modifying a key in any group in .sops.yaml will cause the file to be reencrypted after running updatekeys. The same is not true when changing the shamir_threshold option in .sopsyaml. Do you think it'd make sense for this to also trigger reencryption?

autrilla commented 3 years ago

Hmm, probably not. When changing the Shamir threshold, you should rotate the data key entirely (since the old shards will exist forever, and be able to decrypt the file!).

dimrozakis commented 3 years ago

@autrilla rotating the key doesn't seem to work either:

  1. Modify shamir_threshold in creation_rules of .sops.yaml.
  2. Run sops -r -i path/to/file.
  3. File is updated but if inspected, the file still contains the old value for shamir_threshold.
  4. Trying to decrypt the file verifies that the old shamir_threshold is still in effect.

Key rotation doesn't seem to read .sops.yaml (see also #365).

autrilla commented 3 years ago

You're right, I don't think there's currently a way to do this. I see two ways of implementing this, either adding a shamir threshold flag to the rotate command, or making updatekeys take the shamir threshold into account, letting the user know that the data key must be changed (and doing that for them).

dimrozakis commented 3 years ago

Either would work, I'd prefer updatekeys taking care of this automatically instead of having to pass an extra flag to the rotate command. One could easily forget to pass said flag and end up with an insecure set up without realizing it (for example because the user would think that the threshold is higher than that which is in effect).

PS: In the same vein of doing the safe thing without expecting the user to run extra commands, perhaps updatekeys should always rotate the key, or at least if not doing so would result in an insecure setup as you mentioned in https://github.com/mozilla/sops/issues/365#issuecomment-400962200

in the case where you're removing a master key from the file, I think you have to rotate the data key, as you want people who previously had access to that key to stop having access to the file, so if you keep the same data key, they can still decrypt the file