Skip to content

Commit 136de57

Browse files
authored
feat(kms-keyring): KMS CMK alias is a valid identifier (aws#231)
CMK alias or keyId are valid identifiers for keyIds and generatorKeyId.
1 parent 4d8974c commit 136de57

File tree

5 files changed

+66
-10
lines changed

5 files changed

+66
-10
lines changed

modules/kms-keyring/src/kms_client_supplier.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export function getClient<Client extends AwsEsdkKMSInterface, Config> (
4242
* in this case, the Encryption SDK should find the default region
4343
* or the default region needs to be supplied to this function
4444
*/
45-
const config = { ...defaultConfig, region } as Config
45+
const config = (region ? { ...defaultConfig, region } : { ...defaultConfig }) as Config
4646
const client = new KMSClient(config)
4747

4848
/* Postcondition: A region must be configured.

modules/kms-keyring/src/kms_keyring.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ export function KmsKeyringClass<S extends SupportedAlgorithmSuites, Client exten
7878
needs(!(!discovery && !generatorKeyId && !keyIds.length), 'Noop keyring is not allowed: Set a keyId or discovery')
7979
/* Precondition: A keyring can be either a Discovery or have keyIds configured. */
8080
needs(!(discovery && (generatorKeyId || keyIds.length)), 'A keyring can be either a Discovery or have keyIds configured.')
81-
/* Precondition: All KMS key arns must be valid. */
82-
needs(!generatorKeyId || !!regionFromKmsKeyArn(generatorKeyId), 'Malformed arn.')
83-
needs(keyIds.every(keyarn => !!regionFromKmsKeyArn(keyarn)), 'Malformed arn.')
81+
/* Precondition: All KMS key identifiers must be valid. */
82+
needs(!generatorKeyId || typeof regionFromKmsKeyArn(generatorKeyId) === 'string', 'Malformed arn.')
83+
needs(keyIds.every(keyArn => typeof regionFromKmsKeyArn(keyArn) === 'string'), 'Malformed arn.')
8484
/* Precondition: clientProvider needs to be a callable function. */
8585
needs(typeof clientProvider === 'function', 'Missing clientProvider')
8686

modules/kms-keyring/src/region_from_kms_key_arn.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@
1515

1616
import { needs } from '@aws-crypto/material-management'
1717

18+
/* See: https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html#arn-syntax-kms
19+
* regex to match: 'resourceType/resourceId' || 'resourceType'
20+
* This is complicated because the `split(':')`.
21+
* The valid resourceType resourceId delimiters are `/`, `:`.
22+
* This means if the delimiter is a `:` it will be split out,
23+
* when splitting the whole arn.
24+
*/
25+
const aliasOrKeyResourceType = /^(alias|key)(\/.*)*$/
26+
1827
export function regionFromKmsKeyArn (kmsKeyArn: string): string {
1928
/* Precondition: A KMS key arn must be a string. */
2029
needs(typeof kmsKeyArn === 'string', 'KMS key arn must be a string.')
@@ -23,17 +32,30 @@ export function regionFromKmsKeyArn (kmsKeyArn: string): string {
2332
* arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012
2433
* arn:aws:kms:us-east-1:123456789012:alias/example-alias
2534
*/
26-
const [arnLiteral, partition, service, region] = kmsKeyArn.split(':')
35+
const [arnLiteral, partition, service, region = ''] = kmsKeyArn.split(':')
2736

2837
/* Postcondition: The ARN must be well formed.
2938
* The arn and kms section have defined values,
3039
* but the aws section does not.
40+
* It is also possible to have a a key or alias.
41+
* In this case the partition, service, region
42+
* will be empty.
43+
* In this case the arnLiteral should look like an alias.
3144
*/
3245
needs(
33-
arnLiteral === 'arn' &&
34-
partition &&
35-
service === 'kms' &&
36-
region,
46+
(arnLiteral === 'arn' &&
47+
partition &&
48+
service === 'kms' &&
49+
region) ||
50+
/* Partition may or may not have a value.
51+
* If the resourceType delimiter is /,
52+
* it will not have a value.
53+
* However if the delimiter is : it will
54+
* because of the split(':')
55+
*/
56+
(!service &&
57+
!region &&
58+
arnLiteral.match(aliasOrKeyResourceType)),
3759
'Malformed arn.')
3860

3961
return region

modules/kms-keyring/test/kms_keyring.constructor.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('KmsKeyring: constructor', () => {
7575
expect(() => new TestKmsKeyring({ clientProvider, generatorKeyId, keyIds, discovery })).to.throw()
7676
})
7777

78-
it('Precondition: All KMS key arns must be valid.', () => {
78+
it('Precondition: All KMS key identifiers must be valid.', () => {
7979
const clientProvider: any = () => {}
8080
class TestKmsKeyring extends KmsKeyringClass(Keyring as KeyRingConstructible<NodeAlgorithmSuite>) {}
8181

@@ -95,6 +95,18 @@ describe('KmsKeyring: constructor', () => {
9595
})).to.throw()
9696
})
9797

98+
it('An KMS CMK alias is a valid CMK identifier', () => {
99+
const clientProvider: any = () => {}
100+
class TestKmsKeyring extends KmsKeyringClass(Keyring as KeyRingConstructible<NodeAlgorithmSuite>) {}
101+
102+
const test = new TestKmsKeyring({
103+
clientProvider,
104+
generatorKeyId: 'alias/example-alias',
105+
keyIds: ['alias:example-alias']
106+
})
107+
expect(test).to.be.instanceOf(TestKmsKeyring)
108+
})
109+
98110
it('Precondition: clientProvider needs to be a callable function.', () => {
99111
class TestKmsKeyring extends KmsKeyringClass(Keyring as KeyRingConstructible<NodeAlgorithmSuite>) {}
100112
const clientProvider: any = 'not function'

modules/kms-keyring/test/region_from_kms_key_arn.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,19 @@ describe('regionFromKmsKeyArn', () => {
2525
expect(test1).to.equal('us-east-1')
2626
const test2 = regionFromKmsKeyArn('arn:aws:kms:us-east-1:123456789012:alias/example-alias')
2727
expect(test2).to.equal('us-east-1')
28+
const test3 = regionFromKmsKeyArn('arn:aws:kms:us-east-1:123456789012:12345678-1234-1234-1234-123456789012')
29+
expect(test3).to.equal('us-east-1')
30+
})
31+
32+
it('return empty string for an alias', () => {
33+
expect(regionFromKmsKeyArn('alias/example-alias')).to.equal('')
34+
expect(regionFromKmsKeyArn('alias:example-alias')).to.equal('')
35+
/* using a keyId is confusing.
36+
* It should work but is not recommended.
37+
* Figuring out what region they CMK exist in is difficult.
38+
*/
39+
expect(regionFromKmsKeyArn('key/12345678-1234-1234-1234-123456789012')).to.equal('')
40+
expect(regionFromKmsKeyArn('key:12345678-1234-1234-1234-123456789012')).to.equal('')
2841
})
2942

3043
it('Precondition: A KMS key arn must be a string.', () => {
@@ -40,5 +53,14 @@ describe('regionFromKmsKeyArn', () => {
4053
expect(() => regionFromKmsKeyArn('arn:aws:NOTkms:us-east-1:123456789012:alias/example-alias')).to.throw()
4154
// empty region
4255
expect(() => regionFromKmsKeyArn('arn:aws:kms::123456789012:alias/example-alias')).to.throw()
56+
57+
// no resource type
58+
expect(() => regionFromKmsKeyArn('example-alias')).to.throw()
59+
// invalid resource type
60+
expect(() => regionFromKmsKeyArn('something/example-alias')).to.throw()
61+
expect(() => regionFromKmsKeyArn('something:example-alias')).to.throw()
62+
// invalid delimiter
63+
expect(() => regionFromKmsKeyArn('alias_example-alias')).to.throw()
64+
expect(() => regionFromKmsKeyArn('key_12345678-1234-1234-1234-123456789012')).to.throw()
4365
})
4466
})

0 commit comments

Comments
 (0)