Fixing updatekeys command to respect configured shamir_threshold with groups

[jorn-ola-birkeland]

I noticed a bug when running sops updatekeys <filename> after changing the .sops.yaml configuration from a single master key to 3 groups with master keys and a shamir_threshold below the groups size. The threshold configuration was not respected, and the group size was always used. There was also a problem with increasing the shamir_threshold. If the threshold was still below the group size, the old threshold were used.

This PR aims to solve the following issues:

• Remove the unused argument shamir-secret-sharing-quorum, and the related GroupQuorum option that was always 0
• Correctly respect the shamir_threshold defined in the configuration
• Include changes to the Shamir threshold in the evaluation of whether or not the file will change
• Present the changes to the Shamir threshold to the user for confirmation

Example to reproduce the problem
Use the following .sops.yaml to encrypt a file

creation_rules:
  - path_regex: <pattern>
    gcp_kms: <gcp_resource>

Then change .sops.yaml to

creation_rules:
  - path_regex: <pattern>
    shamir_threshold: 2
    key_groups:
      - gcp_kms:
          - resource_id: <gcp_resource>
        age:
          - <age key 1>
      - age:
          - <age key 2>
          - <age key 3>
      - age:
          - <age key 4>

After running sops updatekeys <filename>, the resulting encrypted file has a metadata section starting with:

sops:
    shamir_threshold: 3
    key_groups:
        - gcp_kms:
           ...

I.e shamir_threshold in .sops.yaml was not respected, and the number of groups was used instead.

User experience after change
Message when running sops updatekeys <filename> after changing .sops.yaml from no groups to 3 groups, with shamir_threshold configured as 2

The following changes will be made to the file's groups:
+++ shamir_threshold: 2
Group 1
    <gcp_resource>
+++ <age key 1>
Group 2
+++ <age key 2>
+++ <age key 3>
Group 3
+++ <age key 4>
Is this okay? (y/n):

After confirmation the the resulting encrypted file has a metadata section starting with:

sops:
    shamir_threshold: 2
    key_groups:
        - gcp_kms:
           ...

Message when changing shamir_threshold from 2 to 3 (equal to removing shamir_threshold from configuration)

The following changes will be made to the file's groups:
+++ shamir_threshold: 3
--- shamir_threshold: 2
Group 1
    <gcp_resource>
    <age key 1>
Group 2
    <age key 2>
    <age key 3>
Group 3
    <age key 4>
Is this okay? (y/n):

After confirmation the the resulting encrypted file has a metadata section starting with:

sops:
    shamir_threshold: 3
    key_groups:
        - gcp_kms:
           ...

[felixfontein]

Thanks for your contribution!

[felixfontein]

I think this should have used the option shamir-secret-sharing-threshold. My guess is that this was simply forgotten in 63708c6, resp. that the option was renamed in parallel to a PR that added/modified this, and thus the old name got re-introduced here. (Turns out the PR was created after merging the rename: #255. If you look at the commits though, you can see that the first commit which already made use of the old name was created before the rename PR got merged.)

Please re-add GroupQuorum with the correct option value.

[felixfontein]

I don't think the logic to compute the new shamir theshold should be part of a diff function. I think it should be a separate function, that can be used in the diff function (though I would avoid that and simply pass old and new value on after the new value has been computed).

[felixfontein]

Also please note that you need to sign-off your commits for the DCO.