Skip to content

Commit 2b6d411

Browse files
texastonytexastony
authored
Merge branch 'master' into tony/change-key-store-admin
2 parents 797335e + 8d6bb56 commit 2b6d411

File tree

5 files changed

+135
-8
lines changed

5 files changed

+135
-8
lines changed

‎changes/2025-01-16_key-store-mitigate-update-race/background.md‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
[//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved."
2+
[//]: # "SPDX-License-Identifier: CC-BY-SA-4.0"
3+
4+
# Mitigate Update Race in Branch Key Store
5+
6+
# Definitions
7+
8+
## MPL
9+
10+
Material Providers Library
11+
12+
## Conventions used in this document
13+
14+
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD",
15+
"SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be
16+
interpreted as described in [RFC 2119](https://tools.ietf.org/html/rfc2119).
17+
18+
# Background
19+
20+
The [branch key store](../../framework/branch-key-store.md) needs to persist branch key versions.
21+
DynamoDB was selected as an easy-to-use option,
22+
with an interface later introduced to allow customers
23+
to implement other storage options.
24+
25+
The behavior of the `WriteNewEncryptedBranchKeyVersion` operation
26+
leaves open a possibility for a normally benign overwrite
27+
of the cipher-text of a Branch Key,
28+
should two or more agents a Version a Branch Key simultaneously.
29+
30+
This change mitigates this.
31+
32+
## Detailed Explanation
33+
34+
The Key Store's `VersionKey` operation does NOT,
35+
at this time,
36+
validate that the ACTIVE item has NOT been modified
37+
since it read the item.
38+
39+
This allows the Key Store's `VersionKey` operation
40+
to race itself.
41+
42+
`VersionKey`'s self-race is benign;
43+
the only consequence is an additional
44+
but unneeded versions of the Branch Key.
45+
46+
However,
47+
Crypto Tools or it's customers may write logic
48+
that modify Branch Key items in other ways.
49+
50+
Such modifications,
51+
if overwritten due to a race,
52+
may break customers or methods Crypto Tools
53+
introduces to modify Branch Keys.
54+
55+
Thus,
56+
Crypto Tools should refactor the Storage interface
57+
to mitigate the unintended overwrite.
58+
59+
## Optimistic Lock
60+
61+
We will mitigate this via an Optimistic Lock on the cipher-text.
62+
63+
All writes to ACTIVE,
64+
except those by `CreateKey`,
65+
would include a condition expression of
66+
`attribute_exists(branch-key-id) AND enc = <old-cipher-text-value>`,
67+
as [expressed in DynamoDB Syntax](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.OperatorsAndFunctions.html).
68+
69+
`enc` gives an assertion on the state of:
70+
71+
- any custom encryption context
72+
- the creation date
73+
- the hierarchy-version
74+
- the Logical Key Store Name
75+
76+
`enc` contains the Auth Tag from
77+
the AES-GCM operation executed by KMS.
78+
79+
Thus, by asserting `enc` has not changed,
80+
the Key Store asserts that nothing has changed!
81+
82+
Since this _Optimistic Lock_ is only
83+
applied AFTER the `enc` value has
84+
been validated by KMS
85+
during the Version routine,
86+
the Key Store KNOWS `enc` is valid.
87+
88+
If `enc` has been changed,
89+
the write will fail with an error detailing the condition check failure.
90+
91+
# Changes
92+
93+
The change is to use an Optimistic Lock
94+
on the old cipher-text value.
95+
96+
This refactors:
97+
98+
- The [Branch Key Store's VersionKey](../../framework/branch-key-store.md#versionkey)
99+
- The [Key Storage's WriteNewEncryptedBranchKeyVersion](../../framework/key-store/key-storage.md#writenewencryptedbranchkeyversion)
100+
- The [Dynamodb Key Storage's WriteNewEncryptedBranchKeyVersion](../../framework/key-store/dynamodb-key-storage.md#writenewencryptedbranchkeyversion)
101+
102+
These refactors are to use the old Active's cipher-text
103+
as the optimistic lock.

‎framework/branch-key-store.md‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55

66
## Version
77

8-
0.6.0
8+
0.7.0
99

1010
### Changelog
1111

12+
- 0.7.0
13+
- [Mitigate Update Race in the Branch Key Store](../changes/2025-01-16_key-store-mitigate-update-race/background.md)
1214
- 0.6.0
1315
- Introduce configurable storage options
1416
- 0.5.0
@@ -453,8 +455,11 @@ The wrapped Branch Keys, DECRYPT_ONLY and ACTIVE, MUST be created according to [
453455

454456
If creation of the keys are successful,
455457
then the key store MUST call the configured [KeyStorage interface's](./key-store/key-storage.md#interface)
456-
[WriteNewEncryptedBranchKeyVersion](./key-store/key-storage.md##writenewencryptedbranchkeyversion)
457-
with these 2 [EncryptedHierarchicalKeys](./key-store/key-storage.md##encryptedhierarchicalkey).
458+
[WriteNewEncryptedBranchKeyVersion](./key-store/key-storage.md#writenewencryptedbranchkeyversion)
459+
with an [OverWriteEncryptedHierarchicalKey](./key-store/key-storage.md#overwriteencryptedhierarchicalkey)
460+
with an `Item` that is the new ACTIVE
461+
and an `Old` that is the original ACTIVE,
462+
along with DECRYPT_ONLY.
458463

459464
If the [WriteNewEncryptedBranchKeyVersion](./key-store/key-storage.md##writenewencryptedbranchkeyversion) is successful,
460465
this operation MUST return a successful response containing no additional data.

‎framework/key-store/dynamodb-key-storage.md‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55

66
## Version
77

8-
0.1.0
8+
0.2.0
99

1010
### Changelog
1111

12+
- 0.2.0
13+
- [Mitigate Update Race in the Branch Key Store](../../changes/2025-01-16_key-store-mitigate-update-race/background.md)
1214
- 0.1.0
1315
- Initial record
1416

@@ -102,13 +104,21 @@ List of TransactWriteItem:
102104
- TableName: the configured Table Name
103105
- PUT:
104106
- Item: A [record formatted item](#record-format) constructed from the active input
105-
- ConditionExpression: `attribute_exists(branch-key-id)`
107+
- ConditionExpression: `attribute_exists(branch-key-id) AND enc = :encOld`
108+
- ExpressionAttributeValues: `{":encOld" := DDB.AttributeValue.B(oldCiphertextBlob)}`
106109
- TableName: the configured Table Name
107110

108111
TransactWriteItemRequest:
109112

110113
- TransactWriteItems: List of TransactWriteItem
111114

115+
The condition expression for the Active Input ensures
116+
the Active Item in storage has not changed since it was read.
117+
This prevents overwrites due to a race in updating the Active Item.
118+
119+
If the Write fails because of the Active Item's condition expression,
120+
the Storage Layer SHOULD throw a Version Race Exception.
121+
112122
### GetEncryptedActiveBranchKey
113123

114124
To get the active version for the branch key id from the keystore

‎framework/key-store/key-storage.md‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55

66
## Version
77

8-
0.1.0
8+
0.2.0
99

1010
### Changelog
1111

12+
- 0.2.0
13+
- [Mitigate Update Race in the Branch Key Store](../../changes/2025-01-16_key-store-mitigate-update-race/background.md)
1214
- 0.1.0
1315
- Initial record
1416

@@ -65,6 +67,13 @@ the UTF8 Encoded value of the version of the branch key.
6567
A structure that MUST have one member,
6668
the UTF8 Encoded value of the version of the branch key.
6769

70+
### OverWriteEncryptedHierarchicalKey
71+
72+
A structure that holds two related [EncryptedHierarchicalKeys](#encryptedhierarchicalkey):
73+
74+
- Item: the [EncryptedHierarchicalKey](#encryptedhierarchicalkey) that will be written
75+
- Old: the [EncryptedHierarchicalKey](#encryptedhierarchicalkey) that was read and is presumed to be the currently persisted item that will be replaced by `Item`.
76+
6877
## Interface
6978

7079
The KeyStorageInterface MUST support the following operations:
@@ -91,7 +100,7 @@ See the [default key stores's write new key to store specification](./default-ke
91100

92101
The WriteNewEncryptedBranchKeyVersion caller MUST provide:
93102

94-
- An [EncryptedHierarchicalKey](#encryptedhierarchicalkey) with a [type](#type) of ActiveHierarchicalSymmetricVersion
103+
- An [OverWriteEncryptedHierarchicalKey](#overwriteencryptedhierarchicalkey) with both `Item` and `Old` with [type](#type) of ActiveHierarchicalSymmetricVersion
95104
- An [EncryptedHierarchicalKey](#encryptedhierarchicalkey) with a [type](#type) of HierarchicalSymmetricVersion
96105

97106
Both keys need to be written together with a consistent transactional write.

‎framework/structures.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ This value MUST be a version 4 [UUID](https://www.ietf.org/rfc/rfc4122.txt).
404404

405405
##### Encryption Context
406406

407-
The [encryption context](#encryption-context) associated with this branch key.
407+
The [custom encryption context](#encryption-context) associated with this branch key.
408408

409409
## Beacon Key Materials
410410

0 commit comments

Comments
 (0)