Fixing updatekeys command to respect configured shamir_threshold with…

… groups

cmd/sops/common/common.go

@@ -397,6 +397,13 @@ func max(a, b int) int {
 	return b
 }
 
+func min(a, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}
+
 // DiffKeyGroups returns the list of diffs found in two sops.keyGroup slices
 func DiffKeyGroups(ours, theirs []sops.KeyGroup) []Diff {
 	var diffs []Diff
@@ -447,3 +454,34 @@ func PrettyPrintDiffs(diffs []Diff) {
 		}
 	}
 }
+
+type ShamirThresholdDiff struct {
+	Old int
+	New int
+}
+
+// DiffShamirThreshold Use config (limited to number of key groups) if there are two or more groups
+func DiffShamirThreshold(metadata sops.Metadata, conf *config.Config) ShamirThresholdDiff {
+	var diff = ShamirThresholdDiff{}
+	diff.Old = metadata.ShamirThreshold
+	if conf.ShamirThreshold > 0 && len(conf.KeyGroups) > 1 {
+		diff.New = min(conf.ShamirThreshold, len(conf.KeyGroups))
+	} else if len(conf.KeyGroups) > 1 {
+		diff.New = len(conf.KeyGroups)
+	}
+
+	return diff
+}
+
+func PrettyPrintShamirThresholdDiffs(diff ShamirThresholdDiff) {
+	if diff.Old > 0 && diff.Old == diff.New {
+		fmt.Printf("    shamir_threshold: %d\n", diff.New)
+	} else {
+		if diff.New > 0 {
+			color.New(color.FgGreen).Printf("+++ shamir_threshold: %d\n", diff.New)
+		}
+		if diff.Old > 0 {
+			color.New(color.FgRed).Printf("--- shamir_threshold: %d\n", diff.Old)
+		}
+	}
+}

cmd/sops/common/common_test.go

@@ -0,0 +1,87 @@
+package common
+
+import (
+	"github.com/getsops/sops/v3"
+	"github.com/getsops/sops/v3/config"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestDiffShamirThresholdWhenIntroducingGroupsAndThresholdBelowGroupSize(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 0}
+	conf := MakeConfig(2, 3)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 0, diff.Old)
+	assert.Equal(t, 2, diff.New)
+}
+
+func TestDiffShamirThresholdWhenIntroducingGroupsAndThresholdAboveGroupSize(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 0}
+	conf := MakeConfig(4, 3)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 0, diff.Old)
+	assert.Equal(t, 3, diff.New)
+}
+
+func TestDiffShamirThresholdWhenIntroducingGroupsAndNoThresholdIsConfigured(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 0}
+	conf := MakeConfig(0, 3)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 0, diff.Old)
+	assert.Equal(t, 3, diff.New)
+}
+
+func TestDiffShamirThresholdWhenReducingThreshold(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 3}
+	conf := MakeConfig(2, 3)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 3, diff.Old)
+	assert.Equal(t, 2, diff.New)
+}
+
+func TestDiffShamirThresholdWhenIncreasingThreshold(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 2}
+	conf := MakeConfig(3, 4)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 2, diff.Old)
+	assert.Equal(t, 3, diff.New)
+}
+
+func TestDiffShamirThresholdWhenRemovingThresholdConfiguration(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 2}
+	conf := MakeConfig(0, 4)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 2, diff.Old)
+	assert.Equal(t, 4, diff.New)
+}
+
+func TestDiffShamirThresholdWhenIntroducingSingleGroup(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 0}
+	conf := MakeConfig(2, 1)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 0, diff.Old)
+	assert.Equal(t, 0, diff.New)
+}
+
+func TestDiffShamirThresholdWhenReducingToSingleGroup(t *testing.T) {
+	metadata := sops.Metadata{ShamirThreshold: 3}
+	conf := MakeConfig(2, 1)
+	diff := DiffShamirThreshold(metadata, &conf)
+
+	assert.Equal(t, 3, diff.Old)
+	assert.Equal(t, 0, diff.New)
+}
+
+func MakeConfig(shamirThreshold int, keygroups int) config.Config {
+	return config.Config{
+		ShamirThreshold: shamirThreshold,
+		KeyGroups:       make([]sops.KeyGroup, keygroups),
+	}
+}

cmd/sops/main.go

@@ -593,7 +593,6 @@ func main() {
 				for _, path := range c.Args() {
 					err := updatekeys.UpdateKeys(updatekeys.Opts{
 						InputPath:   path,
-						GroupQuorum: c.Int("shamir-secret-sharing-quorum"),
 						KeyServices: keyservices(c),
 						Interactive: !c.Bool("yes"),
 						ConfigPath:  configPath,

cmd/sops/subcommand/updatekeys/updatekeys.go

@@ -15,7 +15,6 @@ import (
 // Opts represents key operation options and config
 type Opts struct {
 	InputPath       string
-	GroupQuorum     int
 	KeyServices     []keyservice.KeyServiceClient
 	DecryptionOrder []string
 	Interactive     bool
@@ -59,18 +58,22 @@ func updateFile(opts Opts) error {
 		return fmt.Errorf("The config file %s does not contain any creation rule", opts.ConfigPath)
 	}
 
+	shamirThresholdDiff := common.DiffShamirThreshold(tree.Metadata, conf)
+	shamirThresholdWillChange := shamirThresholdDiff.Old != shamirThresholdDiff.New
+
 	diffs := common.DiffKeyGroups(tree.Metadata.KeyGroups, conf.KeyGroups)
 	keysWillChange := false
 	for _, diff := range diffs {
 		if len(diff.Added) > 0 || len(diff.Removed) > 0 {
 			keysWillChange = true
 		}
 	}
-	if !keysWillChange {
+	if !keysWillChange && !shamirThresholdWillChange {
 		log.Printf("File %s already up to date", opts.InputPath)
 		return nil
 	}
 	fmt.Printf("The following changes will be made to the file's groups:\n")
+	common.PrettyPrintShamirThresholdDiffs(shamirThresholdDiff)
 	common.PrettyPrintDiffs(diffs)
 
 	if opts.Interactive {
@@ -92,10 +95,7 @@ func updateFile(opts Opts) error {
 		return common.NewExitError(err, codes.CouldNotRetrieveKey)
 	}
 	tree.Metadata.KeyGroups = conf.KeyGroups
-	if opts.GroupQuorum != 0 {
-		tree.Metadata.ShamirThreshold = opts.GroupQuorum
-	}
-	tree.Metadata.ShamirThreshold = min(tree.Metadata.ShamirThreshold, len(tree.Metadata.KeyGroups))
+	tree.Metadata.ShamirThreshold = shamirThresholdDiff.New
 	errs := tree.Metadata.UpdateMasterKeysWithKeyServices(key, opts.KeyServices)
 	if len(errs) > 0 {
 		return fmt.Errorf("error updating one or more master keys: %s", errs)
@@ -116,10 +116,3 @@ func updateFile(opts Opts) error {
 	log.Printf("File %s synced with new keys", opts.InputPath)
 	return nil
 }
-
-func min(a, b int) int {
-	if a < b {
-		return a
-	}
-	return b
-}