From 1b6806cd56a2107534137cbab2b653327445d1e7 Mon Sep 17 00:00:00 2001 From: James Tunnicliffe Date: Mon, 15 Aug 2022 15:17:40 +0100 Subject: [PATCH 1/2] Add manage command Prior to this change, if we wanted to add or remove a master key, we had to rotate the data key. While there is nothing wrong with rotating the data key, having to do so at the same time as adding or removing a master key makes git history noisy. This change adds a manage command, which allows separate master key management to data key rotation. --- cmd/sops/main.go | 45 ++++++++++++++++++-------- cmd/sops/manage.go | 79 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 cmd/sops/manage.go diff --git a/cmd/sops/main.go b/cmd/sops/main.go index 80f1f132e..221e391a3 100644 --- a/cmd/sops/main.go +++ b/cmd/sops/main.go @@ -552,6 +552,10 @@ func main() { Name: "rotate, r", Usage: "generate a new data encryption key and reencrypt all values with the new key", }, + cli.BoolFlag{ + Name: "manage, m", + Usage: "manage master keys without reencrypting all values with a new data key", + }, cli.StringFlag{ Name: "kms, k", Usage: "comma separated list of KMS ARNs", @@ -719,7 +723,7 @@ func main() { c.String("rm-kms") != "" || c.String("rm-pgp") != "" || c.String("rm-gcp-kms") != "" || c.String("rm-hc-vault-transit") != "" || c.String("rm-azure-kv") != "" || c.String("rm-age") != "" { return common.NewExitError("Error: cannot add or remove keys on non-existent files, use `--kms` and `--pgp` instead.", codes.CannotChangeKeysFromNonExistentFile) } - if c.Bool("encrypt") || c.Bool("decrypt") || c.Bool("rotate") { + if c.Bool("encrypt") || c.Bool("decrypt") || c.Bool("rotate") || c.Bool("manage") { return common.NewExitError("Error: cannot operate on non-existent file", codes.NoFileSpecified) } } @@ -818,7 +822,7 @@ func main() { IgnoreMAC: c.Bool("ignore-mac"), }) } - if c.Bool("rotate") { + if c.Bool("rotate") || c.Bool("manage") { var addMasterKeys []keys.MasterKey kmsEncryptionContext := kms.ParseKMSContext(c.String("encryption-context")) for _, k := range kms.MasterKeysFromArnString(c.String("add-kms"), kmsEncryptionContext, c.String("aws-profile")) { @@ -884,16 +888,31 @@ func main() { rmMasterKeys = append(rmMasterKeys, k) } - output, err = rotate(rotateOpts{ - OutputStore: outputStore, - InputStore: inputStore, - InputPath: fileName, - Cipher: aes.NewCipher(), - KeyServices: svcs, - IgnoreMAC: c.Bool("ignore-mac"), - AddMasterKeys: addMasterKeys, - RemoveMasterKeys: rmMasterKeys, - }) + if c.Bool("rotate") { + output, err = rotate(rotateOpts{ + OutputStore: outputStore, + InputStore: inputStore, + InputPath: fileName, + Cipher: aes.NewCipher(), + KeyServices: svcs, + IgnoreMAC: c.Bool("ignore-mac"), + AddMasterKeys: addMasterKeys, + RemoveMasterKeys: rmMasterKeys, + }) + } else if c.Bool("manage") { + output, err = manage(manageOpts{ + OutputStore: outputStore, + InputStore: inputStore, + InputPath: fileName, + Cipher: aes.NewCipher(), + KeyServices: svcs, + IgnoreMAC: c.Bool("ignore-mac"), + AddMasterKeys: addMasterKeys, + RemoveMasterKeys: rmMasterKeys, + }) + } else { + return common.NewExitError(fmt.Errorf("unexpected command in branch"), codes.ErrorGeneric) + } } if c.String("set") != "" { @@ -915,7 +934,7 @@ func main() { }) } - isEditMode := !c.Bool("encrypt") && !c.Bool("decrypt") && !c.Bool("rotate") && c.String("set") == "" + isEditMode := !c.Bool("encrypt") && !c.Bool("decrypt") && !c.Bool("rotate") && c.String("set") == "" && !c.Bool("manage") if isEditMode { _, statErr := os.Stat(fileName) fileExists := statErr == nil diff --git a/cmd/sops/manage.go b/cmd/sops/manage.go new file mode 100644 index 000000000..545ce4667 --- /dev/null +++ b/cmd/sops/manage.go @@ -0,0 +1,79 @@ +package main + +import ( + "fmt" + + "go.mozilla.org/sops/v3" + "go.mozilla.org/sops/v3/audit" + "go.mozilla.org/sops/v3/cmd/sops/codes" + "go.mozilla.org/sops/v3/cmd/sops/common" + "go.mozilla.org/sops/v3/keys" + "go.mozilla.org/sops/v3/keyservice" +) + +type manageOpts struct { + Cipher sops.Cipher + InputStore sops.Store + OutputStore sops.Store + InputPath string + IgnoreMAC bool + AddMasterKeys []keys.MasterKey + RemoveMasterKeys []keys.MasterKey + KeyServices []keyservice.KeyServiceClient +} + +func manage(opts manageOpts) ([]byte, error) { + tree, err := common.LoadEncryptedFileWithBugFixes(common.GenericDecryptOpts{ + Cipher: opts.Cipher, + InputStore: opts.InputStore, + InputPath: opts.InputPath, + IgnoreMAC: opts.IgnoreMAC, + KeyServices: opts.KeyServices, + }) + if err != nil { + return nil, err + } + + audit.SubmitEvent(audit.RotateEvent{ + File: tree.FilePath, + }) + + dataKey, err := common.DecryptTree(common.DecryptTreeOpts{ + Cipher: opts.Cipher, IgnoreMac: opts.IgnoreMAC, Tree: tree, + KeyServices: opts.KeyServices, + }) + if err != nil { + return nil, err + } + + // Add new master keys + for _, key := range opts.AddMasterKeys { + tree.Metadata.KeyGroups[0] = append(tree.Metadata.KeyGroups[0], key) + } + // Remove master keys + for _, rmKey := range opts.RemoveMasterKeys { + for i := range tree.Metadata.KeyGroups { + for j, groupKey := range tree.Metadata.KeyGroups[i] { + if rmKey.ToString() == groupKey.ToString() { + tree.Metadata.KeyGroups[i] = append(tree.Metadata.KeyGroups[i][:j], tree.Metadata.KeyGroups[i][j+1:]...) + } + } + } + } + + tree.Metadata.UpdateMasterKeysWithKeyServices(dataKey, opts.KeyServices) + + // Reencrypt the file with the same data key + err = common.EncryptTree(common.EncryptTreeOpts{ + DataKey: dataKey, Tree: tree, Cipher: opts.Cipher, + }) + if err != nil { + return nil, err + } + + encryptedFile, err := opts.OutputStore.EmitEncryptedFile(*tree) + if err != nil { + return nil, common.NewExitError(fmt.Sprintf("Could not marshal tree: %s", err), codes.ErrorDumpingTree) + } + return encryptedFile, nil +} From 3fcbb3ab8a762c0a4dafbfd036f87024257b4f1e Mon Sep 17 00:00:00 2001 From: James Tunnicliffe Date: Tue, 16 Aug 2022 16:33:28 +0100 Subject: [PATCH 2/2] Only allow adding master keys in the key management command Previously we allowed removing master keys, but that implies that the removed key is still secure. Better to assume that it isn't secure so it can no longer open the file. --- cmd/sops/main.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/sops/main.go b/cmd/sops/main.go index 221e391a3..02480fd04 100644 --- a/cmd/sops/main.go +++ b/cmd/sops/main.go @@ -900,6 +900,11 @@ func main() { RemoveMasterKeys: rmMasterKeys, }) } else if c.Bool("manage") { + if len(rmMasterKeys) > 0 { + // Assume that when removing a master key, it isn't being securely stored, so shouldn't be trusted + return common.NewExitError(fmt.Errorf("you must rotate the data key when removing a master key"), codes.ErrorGeneric) + } + output, err = manage(manageOpts{ OutputStore: outputStore, InputStore: inputStore,