feat(KeyStore): Mitigate Update Race in Branch Key Store

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

@@ -0,0 +1,103 @@
+[//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved."
+[//]: # "SPDX-License-Identifier: CC-BY-SA-4.0"
+
+# Mitigate Update Race in Branch Key Store
+
+# Definitions
+
+## MPL
+
+Material Providers Library
+
+## Conventions used in this document
+
+The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD",
+"SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be
+interpreted as described in [RFC 2119](https://tools.ietf.org/html/rfc2119).
+
+# Background
+
+The [branch key store](../../framework/branch-key-store.md) needs to persist branch key versions.
+DynamoDB was selected as an easy-to-use option,
+with an interface later introduced to allow customers
+to implement other storage options.
+
+The behavior of the `WriteNewEncryptedBranchKeyVersion` operation
+leaves open a possibility for a normally benign overwrite
+of the cipher-text of a Branch Key,
+should two or more agents a Version a Branch Key simultaneously.
+
+This change mitigates this.
+
+## Detailed Explanation
+
+The Key Store's `VersionKey` operation does NOT,
+at this time,
+validate that the ACTIVE item has NOT been modified
+since it read the item.
+
+This allows the Key Store's `VersionKey` operation
+to race itself.
+
+`VersionKey`'s self-race is benign;
+the only consequence is an additional
+but unneeded versions of the Branch Key.
+
+However,
+Crypto Tools or it's customers may write logic
+that modify Branch Key items in other ways.
+
+Such modifications,
+if overwritten due to a race,
+may break customers or methods Crypto Tools
+introduces to modify Branch Keys.
+
+Thus,
+Crypto Tools should refactor the Storage interface
+to mitigate the unintended overwrite.
+
+## Optimistic Lock
+
+We will mitigate this via an Optimistic Lock on the cipher-text.
+
+All writes to ACTIVE,
+except those by `CreateKey`,
+would include a condition expression of
+`attribute_exists(branch-key-id) AND enc = <old-cipher-text-value>`,
+as [expressed in DynamoDB Syntax](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.OperatorsAndFunctions.html).
+
+`enc` gives an assertion on the state of:
+
+- any custom encryption context
+- the creation date
+- the hierarchy-version
+- the Logical Key Store Name
+
+`enc` contains the Auth Tag from
+the AES-GCM operation executed by KMS.
+
+Thus, by asserting `enc` has not changed,
+the Key Store asserts that nothing has changed!
+
+Since this _Optimistic Lock_ is only
+applied AFTER the `enc` value has
+been validated by KMS
+during the Version routine,
+the Key Store KNOWS `enc` is valid.
+
+If `enc` has been changed,
+the write will fail with an error detailing the condition check failure.
+
+# Changes
+
+The change is to use an Optimistic Lock
+on the old cipher-text value.
+
+This refactors:
+
+- The [Branch Key Store's VersionKey](../../framework/branch-key-store.md#versionkey)
+- The [Key Storage's WriteNewEncryptedBranchKeyVersion](../../framework/key-store/key-storage.md#writenewencryptedbranchkeyversion)
+- The [Dynamodb Key Storage's WriteNewEncryptedBranchKeyVersion](../../framework/key-store/dynamodb-key-storage.md#writenewencryptedbranchkeyversion)
+
+These refactors are to use the old Active's cipher-text
+as the optimistic lock.

framework/branch-key-store.md

@@ -5,10 +5,12 @@
 
 ## Version
 
-0.6.0
+0.7.0
 
 ### Changelog
 
+- 0.7.0
+  - [Mitigate Update Race in the Branch Key Store](../changes/2025-01-16_key-store-mitigate-update-race/background.md)
 - 0.6.0
   - Introduce configurable storage options
 - 0.5.0
@@ -453,8 +455,11 @@ The wrapped Branch Keys, DECRYPT_ONLY and ACTIVE, MUST be created according to [
 
 If creation of the keys are successful,
 then the key store MUST call the configured [KeyStorage interface's](./key-store/key-storage.md#interface)
-[WriteNewEncryptedBranchKeyVersion](./key-store/key-storage.md##writenewencryptedbranchkeyversion)
-with these 2 [EncryptedHierarchicalKeys](./key-store/key-storage.md##encryptedhierarchicalkey).
+[WriteNewEncryptedBranchKeyVersion](./key-store/key-storage.md#writenewencryptedbranchkeyversion)
+with an [OverWriteEncryptedHierarchicalKey](./key-store/key-storage.md#overwriteencryptedhierarchicalkey)
+with an `Item` that is the new ACTIVE
+and an `Old` that is the original ACTIVE,
+along with DECRYPT_ONLY.
 
 If the [WriteNewEncryptedBranchKeyVersion](./key-store/key-storage.md##writenewencryptedbranchkeyversion) is successful,
 this operation MUST return a successful response containing no additional data.

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

@@ -5,10 +5,12 @@
 
 ## Version
 
-0.1.0
+0.2.0
 
 ### Changelog
 
+- 0.2.0
+  - [Mitigate Update Race in the Branch Key Store](../../changes/2025-01-16_key-store-mitigate-update-race/background.md)
 - 0.1.0
   - Initial record
 
@@ -102,13 +104,21 @@ List of TransactWriteItem:
   - TableName: the configured Table Name
 - PUT:
   - Item: A [record formatted item](#record-format) constructed from the active input
-  - ConditionExpression: `attribute_exists(branch-key-id)`
+  - ConditionExpression: `attribute_exists(branch-key-id) AND enc = :encOld`
+  - ExpressionAttributeValues: `{":encOld" := DDB.AttributeValue.B(oldCiphertextBlob)}`
   - TableName: the configured Table Name
 
 TransactWriteItemRequest:
 
 - TransactWriteItems: List of TransactWriteItem
 
+The condition expression for the Active Input ensures
+the Active Item in storage has not changed since it was read.
+This prevents overwrites due to a race in updating the Active Item.
+
+If the Write fails because of the Active Item's condition expression,
+the Storage Layer SHOULD throw a Version Race Exception.
+
 ### GetEncryptedActiveBranchKey
 
 To get the active version for the branch key id from the keystore

framework/key-store/key-storage.md

@@ -5,10 +5,12 @@
 
 ## Version
 
-0.1.0
+0.2.0
 
 ### Changelog
 
+- 0.2.0
+  - [Mitigate Update Race in the Branch Key Store](../../changes/2025-01-16_key-store-mitigate-update-race/background.md)
 - 0.1.0
   - Initial record
 
@@ -65,6 +67,13 @@ the UTF8 Encoded value of the version of the branch key.
 A structure that MUST have one member,
 the UTF8 Encoded value of the version of the branch key.
 
+### OverWriteEncryptedHierarchicalKey
+
+A structure that holds two related [EncryptedHierarchicalKeys](#encryptedhierarchicalkey):
+
+- Item: the [EncryptedHierarchicalKey](#encryptedhierarchicalkey) that will be written
+- Old: the [EncryptedHierarchicalKey](#encryptedhierarchicalkey) that was read and is presumed to be the currently persisted item that will be replaced by `Item`.
+
 ## Interface
 
 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
 
 The WriteNewEncryptedBranchKeyVersion caller MUST provide:
 
-- An [EncryptedHierarchicalKey](#encryptedhierarchicalkey) with a [type](#type) of ActiveHierarchicalSymmetricVersion
+- An [OverWriteEncryptedHierarchicalKey](#overwriteencryptedhierarchicalkey) with both `Item` and `Old` with [type](#type) of ActiveHierarchicalSymmetricVersion
 - An [EncryptedHierarchicalKey](#encryptedhierarchicalkey) with a [type](#type) of HierarchicalSymmetricVersion
 
 Both keys need to be written together with a consistent transactional write.

framework/structures.md

@@ -404,7 +404,7 @@ This value MUST be a version 4 [UUID](https://www.ietf.org/rfc/rfc4122.txt).
 
 ##### Encryption Context
 
-The [encryption context](#encryption-context) associated with this branch key.
+The [custom encryption context](#encryption-context) associated with this branch key.
 
 ## Beacon Key Materials