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]

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

[felixfontein]

@jorn-ola-birkeland is there a reason you closed this instead of adjusting the PR? Are you planning to still work on this?

[jorn-ola-birkeland]

Yes, I'll create a new branch and PR. (Re-)adding the option is quite a different fix, so I'd rather just start from scratch.

BR, jobi

tir. 11. jun. 2024, 23:41 skrev Felix Fontein @.***>:

@jorn-ola-birkeland https://github.com/jorn-ola-birkeland is there a reason you closed this instead of adjusting the PR? Are you planning to still work on this?

— Reply to this email directly, view it on GitHub https://github.com/getsops/sops/pull/1509#issuecomment-2161645651, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARQTTNKXJCXYPCTGAIPHNBLZG5VINAVCNFSM6AAAAABHY44B76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRRGY2DKNRVGE . You are receiving this because you were mentioned.Message ID: @.***>