Skip to content

Commit 4092fff

Browse files
felixfonteinfelixfontein
authored
Merge pull request #1733 from felixfontein/merge-aws-fix
Fix ToString for AWS KMS to include role, context, and profile
2 parents a8fb540 + 89fd098 commit 4092fff

File tree

3 files changed

+229
-53
lines changed

3 files changed

+229
-53
lines changed

‎config/config_test.go‎

Lines changed: 154 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package config
22

33
import (
4+
"fmt"
45
"os"
56
"path"
67
"testing"
78

9+
"github.com/getsops/sops/v3/keys"
810
"github.com/stretchr/testify/assert"
911
)
1012

@@ -94,6 +96,20 @@ creation_rules:
9496
- kms:
9597
- arn: foo
9698
aws_profile: bar
99+
- arn: foo
100+
context:
101+
baz: bam
102+
- arn: foo
103+
aws_profile: bar
104+
context:
105+
baz: bam
106+
- arn: foo
107+
role: '123'
108+
- arn: foo
109+
aws_profile: bar
110+
context:
111+
baz: bam
112+
role: '123'
97113
pgp:
98114
- bar
99115
gcp_kms:
@@ -129,113 +145,124 @@ creation_rules:
129145
- 'https://foo.vault:8200/v1/foo/keys/foo-key'
130146
- merge:
131147
- merge:
132-
- kms:
148+
- pgp:
133149
# key01
150+
- foo
151+
kms:
152+
# key02
134153
- arn: foo
135154
aws_profile: foo
136-
pgp:
137-
# key02
138-
- foo
139-
gcp_kms:
140155
# key03
156+
- arn: foo
157+
aws_profile: bar
158+
context:
159+
baz: bam
160+
role: '123'
161+
gcp_kms:
162+
# key04
141163
- resource_id: foo
142164
azure_keyvault:
143-
# key04
165+
# key05
144166
- vaultUrl: https://foo.vault.azure.net
145167
key: foo-key
146168
version: fooversion
147169
hc_vault:
148-
# key05
149-
- 'https://bar.vault:8200/v1/bar/keys/bar-key'
150-
- kms:
151170
# key06
152-
- arn: bar
153-
aws_profile: bar
154-
pgp:
171+
- 'https://bar.vault:8200/v1/bar/keys/bar-key'
172+
- pgp:
155173
# key07
156174
- bar
157-
gcp_kms:
175+
kms:
158176
# key08
159-
- resource_id: bar
177+
- arn: bar
178+
aws_profile: bar
179+
gcp_kms:
160180
# key09
181+
- resource_id: bar
182+
# key10
161183
- resource_id: baz
162184
azure_keyvault:
163-
# key10
185+
# key11
164186
- vaultUrl: https://bar.vault.azure.net
165187
key: bar-key
166188
version: barversion
167189
hc_vault:
168-
# key01 - duplicate#1
190+
# key12
169191
- 'https://baz.vault:8200/v1/baz/keys/baz-key'
192+
pgp:
193+
# key13
194+
- baz
170195
kms:
171-
# key11
196+
# key14
172197
- arn: baz
173198
aws_profile: baz
174-
pgp:
175-
# key12
176-
- baz
177199
gcp_kms:
178-
# key03 - duplicate#2
179-
# --> should be removed when loading config
200+
# duplicate of key09
180201
- resource_id: bar
181202
azure_keyvault:
182-
# key04 - duplicate#3
203+
# duplicate of key05
183204
- vaultUrl: https://foo.vault.azure.net
184205
key: foo-key
185206
version: fooversion
186207
hc_vault:
187-
# key13 - duplicate#4 - but from different key_group
188-
# --> should stay
208+
# key15 (duplicate of key00, but that's in a different key_group)
189209
- 'https://foo.vault:8200/v1/foo/keys/foo-key'
190-
- kms:
191-
# key14
210+
- pgp:
211+
# key16
212+
- qux
213+
kms:
214+
# key17
192215
- arn: qux
193216
aws_profile: qux
194-
# key14 - duplicate#5
217+
# key18
195218
- arn: baz
196219
aws_profile: bar
197-
pgp:
198-
# key15
199-
- qux
220+
# key19
221+
- arn: baz
222+
role: '123'
200223
gcp_kms:
201-
# key16
224+
# key20
202225
- resource_id: qux
203-
# key17
226+
# key21
204227
- resource_id: fnord
205228
azure_keyvault:
206-
# key18
229+
# key22
207230
- vaultUrl: https://baz.vault.azure.net
208231
key: baz-key
209232
version: bazversion
210233
hc_vault:
211-
# key19
234+
# key23
212235
- 'https://qux.vault:8200/v1/qux/keys/qux-key'
213-
# everything below this should be loaded,
214-
# since it is not in a merge block
236+
pgp:
237+
# duplicate of key07
238+
- bar
215239
kms:
216-
# duplicated key06
240+
# duplicate of key08
217241
- arn: bar
218242
aws_profile: bar
219-
# key20
243+
# key24
220244
- arn: fnord
221245
aws_profile: fnord
222-
pgp:
223-
# duplicated key07
224-
- bar
246+
# duplicate of key03
247+
- arn: foo
248+
aws_profile: bar
249+
context:
250+
baz: bam
251+
role: '123'
225252
gcp_kms:
226-
# duplicated key08
253+
# duplicate of key09
227254
- resource_id: bar
228-
# key21
255+
# duplicate of key21
229256
- resource_id: fnord
230257
azure_keyvault:
231-
# duplicated key10
258+
# duplicate of key11
232259
- vaultUrl: https://bar.vault.azure.net
233260
key: bar-key
234261
version: barversion
235262
hc_vault:
236-
# duplicated 'key01 - duplicate#2'
263+
# duplicate of key12
237264
- 'https://baz.vault:8200/v1/baz/keys/baz-key'
238-
# key22
265+
# key25
239266
- 'https://fnord.vault:8200/v1/fnord/keys/fnord-key'
240267
`)
241268

@@ -421,6 +448,7 @@ func TestLoadConfigFile(t *testing.T) {
421448
}
422449

423450
func TestLoadConfigFileWithGroups(t *testing.T) {
451+
bam := "bam"
424452
expected := configFile{
425453
CreationRules: []creationRule{
426454
{
@@ -432,7 +460,37 @@ func TestLoadConfigFileWithGroups(t *testing.T) {
432460
PathRegex: "",
433461
KeyGroups: []keyGroup{
434462
{
435-
KMS: []kmsKey{{Arn: "foo", AwsProfile: "bar"}},
463+
KMS: []kmsKey{
464+
{
465+
Arn: "foo",
466+
AwsProfile: "bar",
467+
},
468+
{
469+
Arn: "foo",
470+
Context: map[string]*string{
471+
"baz": &bam,
472+
},
473+
},
474+
{
475+
Arn: "foo",
476+
AwsProfile: "bar",
477+
Context: map[string]*string{
478+
"baz": &bam,
479+
},
480+
},
481+
{
482+
Arn: "foo",
483+
Role: "123",
484+
},
485+
{
486+
Arn: "foo",
487+
AwsProfile: "bar",
488+
Context: map[string]*string{
489+
"baz": &bam,
490+
},
491+
Role: "123",
492+
},
493+
},
436494
PGP: []string{"bar"},
437495
GCPKMS: []gcpKmsKey{{ResourceID: "foo"}},
438496
AzureKV: []azureKVKey{{VaultURL: "https://foo.vault.azure.net", Key: "foo-key", Version: "fooversion"}},
@@ -459,12 +517,52 @@ func TestLoadConfigFileWithGroups(t *testing.T) {
459517
assert.Equal(t, expected, conf)
460518
}
461519

520+
func id(key keys.MasterKey) string {
521+
return fmt.Sprintf("%s: %s", key.TypeToIdentifier(), key.ToString())
522+
}
523+
524+
func ids(keys []keys.MasterKey) []string {
525+
result := make([]string, 0, len(keys))
526+
for _, key := range keys {
527+
result = append(result, id(key))
528+
}
529+
return result
530+
}
531+
462532
func TestLoadConfigFileWithMerge(t *testing.T) {
463533
conf, err := parseCreationRuleForFile(parseConfigFile(sampleConfigWithMergeType, t), "/conf/path", "whatever", nil)
464534
assert.Nil(t, err)
465535
assert.Equal(t, 2, len(conf.KeyGroups))
466-
assert.Equal(t, 1, len(conf.KeyGroups[0]))
467-
assert.Equal(t, 22, len(conf.KeyGroups[1]))
536+
assert.Equal(t, []string{
537+
"hc_vault: https://foo.vault:8200/v1/foo/keys/foo-key",
538+
}, ids(conf.KeyGroups[0]))
539+
assert.Equal(t, []string{
540+
"pgp: foo", // key01
541+
"kms: foo||foo", //key02
542+
"kms: foo+123|baz:bam|bar", //key03
543+
"gcp_kms: foo", //key04
544+
"azure_kv: https://foo.vault.azure.net/keys/foo-key/fooversion", //key05
545+
"hc_vault: https://bar.vault:8200/v1/bar/keys/bar-key", //key06
546+
"pgp: bar", //key07
547+
"kms: bar||bar", //key08
548+
"gcp_kms: bar", //key09
549+
"gcp_kms: baz", //key10
550+
"azure_kv: https://bar.vault.azure.net/keys/bar-key/barversion", //key11
551+
"hc_vault: https://baz.vault:8200/v1/baz/keys/baz-key", //key12
552+
"pgp: baz", //key13
553+
"kms: baz||baz", //key14
554+
"hc_vault: https://foo.vault:8200/v1/foo/keys/foo-key", //key15
555+
"pgp: qux", //key16
556+
"kms: qux||qux", //key17
557+
"kms: baz||bar", //key18
558+
"kms: baz+123", //key19
559+
"gcp_kms: qux", //key20
560+
"gcp_kms: fnord", //key21
561+
"azure_kv: https://baz.vault.azure.net/keys/baz-key/bazversion", //key22
562+
"hc_vault: https://qux.vault:8200/v1/qux/keys/qux-key", //key23
563+
"kms: fnord||fnord", //key24
564+
"hc_vault: https://fnord.vault:8200/v1/fnord/keys/fnord-key", //key25
565+
}, ids(conf.KeyGroups[1]))
468566
}
469567

470568
func TestLoadConfigFileWithNoMatchingRules(t *testing.T) {
@@ -538,9 +636,13 @@ func TestKeyGroupsForFileWithGroups(t *testing.T) {
538636
conf, err := parseCreationRuleForFile(parseConfigFile(sampleConfigWithGroups, t), "/conf/path", "whatever", nil)
539637
assert.Nil(t, err)
540638
assert.Equal(t, "bar", conf.KeyGroups[0][0].ToString())
541-
assert.Equal(t, "foo", conf.KeyGroups[0][1].ToString())
639+
assert.Equal(t, "foo||bar", conf.KeyGroups[0][1].ToString())
640+
assert.Equal(t, "foo|baz:bam", conf.KeyGroups[0][2].ToString())
641+
assert.Equal(t, "foo|baz:bam|bar", conf.KeyGroups[0][3].ToString())
642+
assert.Equal(t, "foo+123", conf.KeyGroups[0][4].ToString())
643+
assert.Equal(t, "foo+123|baz:bam|bar", conf.KeyGroups[0][5].ToString())
542644
assert.Equal(t, "qux", conf.KeyGroups[1][0].ToString())
543-
assert.Equal(t, "baz", conf.KeyGroups[1][1].ToString())
645+
assert.Equal(t, "baz||foo", conf.KeyGroups[1][1].ToString())
544646
}
545647

546648
func TestLoadConfigFileWithUnencryptedSuffix(t *testing.T) {

‎kms/keysource.go‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"os"
1313
"regexp"
14+
"sort"
1415
"strings"
1516
"time"
1617

@@ -181,6 +182,38 @@ func ParseKMSContext(in interface{}) map[string]*string {
181182
return out
182183
}
183184

185+
// kmsContextToString converts a dictionary into a string that can be parsed
186+
// again with ParseKMSContext().
187+
func kmsContextToString(in map[string]*string) string {
188+
if len(in) == 0 {
189+
return ""
190+
}
191+
192+
// Collect the keys in a slice and compute the expected length
193+
keys := make([]string, 0, len(in))
194+
length := 0
195+
for key := range in {
196+
keys = append(keys, key)
197+
length += len(key) + len(*in[key]) + 2
198+
}
199+
200+
// Sort the keys
201+
sort.Strings(keys)
202+
203+
// Compose a comma-separated string of key-vale pairs
204+
var builder strings.Builder
205+
builder.Grow(length)
206+
for index, key := range keys {
207+
if index > 0 {
208+
builder.WriteString(",")
209+
}
210+
builder.WriteString(key)
211+
builder.WriteByte(':')
212+
builder.WriteString(*in[key])
213+
}
214+
return builder.String()
215+
}
216+
184217
// CredentialsProvider is a wrapper around aws.CredentialsProvider used for
185218
// authentication towards AWS KMS.
186219
type CredentialsProvider struct {
@@ -278,7 +311,18 @@ func (key *MasterKey) NeedsRotation() bool {
278311

279312
// ToString converts the key to a string representation.
280313
func (key *MasterKey) ToString() string {
281-
return key.Arn
314+
arnRole := key.Arn
315+
if key.Role != "" {
316+
arnRole = fmt.Sprintf("%s+%s", key.Arn, key.Role)
317+
}
318+
context := kmsContextToString(key.EncryptionContext)
319+
if key.AwsProfile != "" {
320+
return fmt.Sprintf("%s|%s|%s", arnRole, context, key.AwsProfile)
321+
}
322+
if context != "" {
323+
return fmt.Sprintf("%s|%s", arnRole, context)
324+
}
325+
return arnRole
282326
}
283327

284328
// ToMap converts the MasterKey to a map for serialization purposes.

0 commit comments

Comments
 (0)