diff --git a/changes/2025-01-16_key-store-mitigate-update-race/background.md b/changes/2025-01-16_key-store-mitigate-update-race/background.md new file mode 100644 index 00000000..2ae0e647 --- /dev/null +++ b/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 = `, +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. diff --git a/framework/branch-key-store.md b/framework/branch-key-store.md index c1ac0b31..dd4bf480 100644 --- a/framework/branch-key-store.md +++ b/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. diff --git a/framework/key-store/dynamodb-key-storage.md b/framework/key-store/dynamodb-key-storage.md index 7c0635b3..a29bd2aa 100644 --- a/framework/key-store/dynamodb-key-storage.md +++ b/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 diff --git a/framework/key-store/key-storage.md b/framework/key-store/key-storage.md index 1fdf54c5..5640c877 100644 --- a/framework/key-store/key-storage.md +++ b/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. diff --git a/framework/structures.md b/framework/structures.md index 9ad3f325..3b4946a3 100644 --- a/framework/structures.md +++ b/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