From a27e1bddf2a33572e2670420492b626fa6d90b48 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 29 Sep 2023 13:08:36 +0200 Subject: [PATCH 1/4] Ignore irrelevant return value (it is always nil). Signed-off-by: Felix Fontein --- stores/yaml/store.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stores/yaml/store.go b/stores/yaml/store.go index 1481dff8f..51ce25426 100644 --- a/stores/yaml/store.go +++ b/stores/yaml/store.go @@ -204,9 +204,9 @@ func (store *Store) appendSequence(in []interface{}, sequence *yaml.Node) { } if len(comments) > 0 { if beginning { - comments = store.addCommentsHead(sequence, comments) + store.addCommentsHead(sequence, comments) } else { - comments = store.addCommentsFoot(sequence.Content[len(sequence.Content)-1], comments) + store.addCommentsFoot(sequence.Content[len(sequence.Content)-1], comments) } } } @@ -231,9 +231,9 @@ func (store *Store) appendTreeBranch(branch sops.TreeBranch, mapping *yaml.Node) } if len(comments) > 0 { if beginning { - comments = store.addCommentsHead(mapping, comments) + store.addCommentsHead(mapping, comments) } else { - comments = store.addCommentsFoot(mapping.Content[len(mapping.Content)-2], comments) + store.addCommentsFoot(mapping.Content[len(mapping.Content)-2], comments) } } } From 38ec3f7a524335564eac2a64620608b2d357a064 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 29 Sep 2023 13:17:14 +0200 Subject: [PATCH 2/4] Handle unhandled errors. Signed-off-by: Felix Fontein --- cmd/sops/common/common.go | 3 ++ cmd/sops/main.go | 5 ++++ stores/yaml/store.go | 4 +++ stores/yaml/store_test.go | 59 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+) diff --git a/cmd/sops/common/common.go b/cmd/sops/common/common.go index 907125d28..7beecb8c1 100644 --- a/cmd/sops/common/common.go +++ b/cmd/sops/common/common.go @@ -89,6 +89,9 @@ func DecryptTree(opts DecryptTreeOpts) (dataKey []byte, err error) { } fileMac, err := opts.Cipher.Decrypt(opts.Tree.Metadata.MessageAuthenticationCode, dataKey, opts.Tree.Metadata.LastModified.Format(time.RFC3339)) if !opts.IgnoreMac { + if err != nil { + return nil, NewExitError(fmt.Sprintf("Cannot decrypt MAC: %s", err), codes.MacMismatch) + } if fileMac != computedMac { // If the file has an empty MAC, display "no MAC" instead of not displaying anything if fileMac == "" { diff --git a/cmd/sops/main.go b/cmd/sops/main.go index b902c186f..d481781b4 100644 --- a/cmd/sops/main.go +++ b/cmd/sops/main.go @@ -1234,6 +1234,11 @@ func extractSetArguments(set string) (path []interface{}, valueToInsert interfac fullPath := strings.TrimRight(pathValuePair[0], " ") jsonValue := pathValuePair[1] valueToInsert, err = jsonValueToTreeInsertableValue(jsonValue) + if err != nil { + // All errors returned by jsonValueToTreeInsertableValue are created by common.NewExitError(), + // so we can simply pass them on + return nil, nil, err + } path, err = parseTreePath(fullPath) if err != nil { diff --git a/stores/yaml/store.go b/stores/yaml/store.go index 51ce25426..29fe2652a 100644 --- a/stores/yaml/store.go +++ b/stores/yaml/store.go @@ -131,6 +131,10 @@ func (store Store) appendYamlNodeToTreeBranch(node *yaml.Node, branch sops.TreeB return nil, fmt.Errorf("YAML documents that are values are not supported") case yaml.AliasNode: branch, err = store.appendYamlNodeToTreeBranch(node.Alias, branch, false) + if err != nil { + // This should never happen since node.Alias was already successfully decoded before + return nil, err + } } if !commentsWereHandled { branch = store.appendCommentToMap(node.FootComment, branch) diff --git a/stores/yaml/store_test.go b/stores/yaml/store_test.go index f37e3deb4..4851068a3 100644 --- a/stores/yaml/store_test.go +++ b/stores/yaml/store_test.go @@ -48,6 +48,59 @@ var BRANCHES = sops.TreeBranches{ }, } +var ALIASES = []byte(`--- +key1: &foo + - foo +key2: *foo +key3: &bar + foo: bar + baz: bam +key4: *bar +`) + +var ALIASES_BRANCHES = sops.TreeBranches{ + sops.TreeBranch{ + sops.TreeItem{ + Key: "key1", + Value: []interface{}{ + "foo", + }, + }, + sops.TreeItem{ + Key: "key2", + Value: []interface{}{ + "foo", + }, + }, + sops.TreeItem{ + Key: "key3", + Value: sops.TreeBranch{ + sops.TreeItem{ + Key: "foo", + Value: "bar", + }, + sops.TreeItem{ + Key: "baz", + Value: "bam", + }, + }, + }, + sops.TreeItem{ + Key: "key4", + Value: sops.TreeBranch{ + sops.TreeItem{ + Key: "foo", + Value: "bar", + }, + sops.TreeItem{ + Key: "baz", + Value: "bam", + }, + }, + }, + }, +} + var COMMENT_1 = []byte(`# test a: b: null @@ -170,6 +223,12 @@ func TestLoadPlainFile(t *testing.T) { assert.Equal(t, BRANCHES, branches) } +func TestLoadAliasesPlainFile(t *testing.T) { + branches, err := (&Store{}).LoadPlainFile(ALIASES) + assert.Nil(t, err) + assert.Equal(t, ALIASES_BRANCHES, branches) +} + func TestComment1(t *testing.T) { // First iteration: load and store branches, err := (&Store{}).LoadPlainFile(COMMENT_1) From f668c71545c18bcb193730cdc42e28ca4ac782c9 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 29 Sep 2023 13:17:22 +0200 Subject: [PATCH 3/4] Defer only after checking err. Signed-off-by: Felix Fontein --- cmd/sops/common/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/sops/common/common.go b/cmd/sops/common/common.go index 7beecb8c1..fa54ecc7d 100644 --- a/cmd/sops/common/common.go +++ b/cmd/sops/common/common.go @@ -321,10 +321,10 @@ func FixAWSKMSEncryptionContextBug(opts GenericDecryptOpts, tree *sops.Tree) (*s } file, err := os.Create(opts.InputPath) - defer file.Close() if err != nil { return nil, NewExitError(fmt.Sprintf("Could not open file for writing: %s", err), codes.CouldNotWriteOutputFile) } + defer file.Close() _, err = file.Write(encryptedFile) if err != nil { return nil, err From dd59dc109672ad1fd3d902c62da4ce1d8483f407 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 29 Sep 2023 13:17:30 +0200 Subject: [PATCH 4/4] Check err for nil in tests. Signed-off-by: Felix Fontein --- kms/keysource_test.go | 1 + pgp/keysource_test.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/kms/keysource_test.go b/kms/keysource_test.go index 81cd9bbd8..4bed28621 100644 --- a/kms/keysource_test.go +++ b/kms/keysource_test.go @@ -446,6 +446,7 @@ func TestMasterKey_createKMSConfig(t *testing.T) { assert.NoError(t, err) creds, err := cfg.Credentials.Retrieve(context.TODO()) + assert.Nil(t, err) assert.Equal(t, "id", creds.AccessKeyID) assert.Equal(t, "secret", creds.SecretAccessKey) assert.Equal(t, "token", creds.SessionToken) diff --git a/pgp/keysource_test.go b/pgp/keysource_test.go index b6145fcef..b72bf8a92 100644 --- a/pgp/keysource_test.go +++ b/pgp/keysource_test.go @@ -332,6 +332,7 @@ func TestMasterKey_Decrypt(t *testing.T) { fingerprint, "--no-encrypt-to", }, bytes.NewReader(data)) + assert.Nil(t, err) assert.NoErrorf(t, gnuPGHome.ImportFile(mockPrivateKey), stderr.String()) encryptedData := stdout.String() @@ -414,6 +415,7 @@ func TestMasterKey_decryptWithOpenPGP(t *testing.T) { fingerprint, "--no-encrypt-to", }, bytes.NewReader(data)) + assert.Nil(t, err) assert.NoErrorf(t, gnuPGHome.ImportFile(mockPrivateKey), stderr.String()) encryptedData := stdout.String() @@ -462,6 +464,7 @@ func TestMasterKey_decryptWithGnuPG(t *testing.T) { fingerprint, "--no-encrypt-to", }, bytes.NewReader(data)) + assert.Nil(t, err) assert.NoErrorf(t, gnuPGHome.ImportFile(mockPrivateKey), stderr.String()) encryptedData := stdout.String()