-
Notifications
You must be signed in to change notification settings - Fork 25
feat(branch-keys): add hv-2 spec #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rishav-karanjit
wants to merge
99
commits into
master
Choose a base branch
from
rishav/hv-2-spec
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,012
−105
Open
Changes from 33 commits
Commits
Show all changes
99 commits
Select commit
Hold shift + click to select a range
8db6855
CreateKeyInput changes
rishav-karanjit 464271c
Update branch-key-store.md
rishav-karanjit 5a46148
auto commit
rishav-karanjit 1ce8bf0
bkc
rishav-karanjit 3ebc886
auto commit
rishav-karanjit 178bd9a
auto commit
rishav-karanjit f246cc4
auto commit
rishav-karanjit 838e818
auto commit
rishav-karanjit 6bff806
auto commit
rishav-karanjit 413d74d
auto commit
rishav-karanjit ab82862
auto commit
rishav-karanjit 5f9db4c
auto commit
rishav-karanjit 8b54bac
auto commit
rishav-karanjit 8308fcb
auto commit
rishav-karanjit 8baeb72
auto commit
rishav-karanjit 879f13c
auto commit
rishav-karanjit 13aef97
auto commit
rishav-karanjit 6a35be8
auto commit
rishav-karanjit 1360dab
auto commit
rishav-karanjit 4550add
auto commit
rishav-karanjit 757de04
nits
rishav-karanjit fe9405c
Update framework/branch-key-store.md
rishav-karanjit 117b632
Update framework/branch-key-store.md
rishav-karanjit 9f7b93e
DQ1
rishav-karanjit 05a9cf7
formatting:
rishav-karanjit 95b70f3
auto commit
rishav-karanjit e8f5784
auto commit
rishav-karanjit 02ea9a3
Initial empty commit
rishav-karanjit 9635b60
auto commit
rishav-karanjit 686f21c
auto commit
rishav-karanjit b822360
chore: add hv-2 changes in ./changes directory
rishav-karanjit ebac8ac
auto commit
rishav-karanjit a2d8a48
auto commit
rishav-karanjit 01a22e6
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit ef6d1dc
Update branch-key-store.md
rishav-karanjit e8d54f2
auto commit
rishav-karanjit e768f46
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit 0481ed3
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit 4aa2261
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit db3300d
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit a48aabf
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit 02de72b
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit bbe76a8
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit 83bff87
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit 8963404
auto commit
rishav-karanjit 4eafe3d
Update changes/2025-06-30_branch_keys_version_2/background.md
rishav-karanjit d538a41
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit a341b0d
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 72d95fb
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit a16bcbb
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit d853d8f
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 55d690f
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 8ed6bfe
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit d3100a9
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 0593bdb
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 865c3ed
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 9858b49
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 7529099
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit bc6c700
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 47d1cdc
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 2b28735
Update changes/2025-06-30_branch_keys_version_2/designquestion.md
rishav-karanjit 9e4c9aa
Update framework/branch-key-store.md
rishav-karanjit 3ee3a0c
Update framework/branch-key-store.md
rishav-karanjit d827b21
Update framework/branch-key-store.md
rishav-karanjit 25c5b20
Update framework/branch-key-store.md
rishav-karanjit 55fccff
Apply suggestions from code review
rishav-karanjit 8095f24
auto commit
rishav-karanjit ca4d912
auto commit
rishav-karanjit 77c3833
Merge branch 'master' into rishav/hv-2-spec
rishav-karanjit 8d4eb7f
auto commit
rishav-karanjit 541c729
Update framework/branch-key-store.md
rishav-karanjit 82c1ad8
auto commit
rishav-karanjit 688c354
auto commit
rishav-karanjit 81b60e8
auto commit
rishav-karanjit 30e58d9
Update background.md
rishav-karanjit ec22bf4
auto commit
rishav-karanjit 8db8704
auto commit
rishav-karanjit d062762
formatting
rishav-karanjit b49dd3f
auto commit
rishav-karanjit 1ad7803
auto commit
rishav-karanjit 15bc50a
auto commit
rishav-karanjit aad0ee3
designq -> protect_branch_key_without_kms_ec
rishav-karanjit 2d105ea
resolve https://github.com/awslabs/aws-encryption-sdk-specification/p…
rishav-karanjit 7d10c1f
resolve https://github.com/awslabs/aws-encryption-sdk-specification/p…
rishav-karanjit a725bc0
nit fix
rishav-karanjit 79c5efd
resolve https://github.com/awslabs/aws-encryption-sdk-specification/p…
rishav-karanjit feaa9f0
Update framework/branch-key-store.md
rishav-karanjit e2f3eb8
Update framework/branch-key-store.md
rishav-karanjit 625ec56
auto commit
rishav-karanjit e818581
Update framework/branch-key-store.md
rishav-karanjit 3725032
Update framework/structures.md
rishav-karanjit d6fd990
Update framework/branch-key-store.md
rishav-karanjit 9c5c281
auto commit
rishav-karanjit 1ccbcbe
auto commit
rishav-karanjit e982ff0
auto commit
rishav-karanjit 24b76b8
auto commit
rishav-karanjit f300cd0
formatting
rishav-karanjit d3453f8
Merge branch 'master' into rishav/hv-2-spec
rishav-karanjit 7fa3363
chore: encryption-context changes (#300)
rishav-karanjit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,330 @@ | ||
[//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved." | ||
[//]: # "SPDX-License-Identifier: CC-BY-SA-4.0" | ||
|
||
# Customers should control encryption context | ||
|
||
# Definitions | ||
|
||
## 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). | ||
|
||
## HV-1 | ||
|
||
The Branch Key Store's Branch Keys are designated as `"hierarchy-version" : "1"`. | ||
|
||
This document proposes changes to theses Branch Keys; | ||
when HV-1 is written, | ||
we mean a Branch Key Item written by the Branch Key Store v0.2.0 to v0.7.0. | ||
|
||
## Branch Key's Cryptographic Material | ||
|
||
Cryptographic material (AES-256 bit key) generated and protected by KMS. | ||
|
||
## Branch Key's Properities | ||
|
||
These three values are stored on every | ||
Active, | ||
Version, | ||
and Beacon Key Item in the Branch Key Store's Storage. | ||
|
||
They are also present in the KMS Encryption Context. | ||
|
||
```json | ||
"kms-arn" : "arn:aws:kms:us-west-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab", | ||
"create-time" : "2023-06-03T19:03:29.358Z", | ||
"hierarchy-version" : "1", | ||
``` | ||
|
||
The Active has an additional field: | ||
|
||
```json | ||
"version": "branch:version:83eec007-5659-4554-bf11-699b90f41ac6" | ||
``` | ||
|
||
## Branch Key's Location | ||
|
||
These two values are stored on every | ||
Active, | ||
Version, | ||
and Beacon Key Item in the Branch Key Store's Storage. | ||
|
||
For Active: | ||
|
||
```json | ||
"branch-key-id" : "bbb9baf1-03e6-4716-a586-6bf29995314b", | ||
"type" : "branch:ACTIVE" | ||
``` | ||
|
||
For Version: | ||
|
||
```json | ||
"branch-key-id" : "bbb9baf1-03e6-4716-a586-6bf29995314b", | ||
"type": "branch:version:83eec007-5659-4554-bf11-699b90f41ac6" | ||
``` | ||
|
||
For Beacon: | ||
|
||
```json | ||
"branch-key-id" : "bbb9baf1-03e6-4716-a586-6bf29995314b", | ||
"type": "beacon:ACTIVE" | ||
``` | ||
|
||
However, | ||
the Logical Key Store Name is also included in the KMS Encryption Context. | ||
|
||
```json | ||
"branch-key-id" : "bbb9baf1-03e6-4716-a586-6bf29995314b", | ||
"type" : "branch:ACTIVE", | ||
"tablename": "KeyStore" | ||
``` | ||
|
||
These three values describe where the Branch Key is stored; however, the tablename is a logical description, not a physical description. | ||
At this time, no cryptography binds the Branch Key to the physical table. | ||
|
||
```json | ||
"branch-key-id" : "bbb9baf1-03e6-4716-a586-6bf29995314b", | ||
"tablename": "KeyStore" | ||
``` | ||
|
||
These two values label the Branch Key, seperating it from all other Branch Keys. | ||
|
||
## Branch Key's Encryption Context | ||
|
||
These values are determined by the Branch Key Creator | ||
or last Branch Key Mutator. | ||
|
||
In DynamoDB and in KMS Encryption Requests, | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
their keys are prefixed with `aws-crypto-ec:`. | ||
|
||
```json | ||
"aws-crypto-ec:department" : "admin" | ||
``` | ||
|
||
However, | ||
when returned to the requesters of the Branch Key Store's | ||
`Get*Key*` operations, | ||
the prefix is removed. | ||
|
||
## Branch Key's Context | ||
|
||
The union of the Branch Key's: | ||
|
||
- Properities | ||
- Location (including logical key store name) | ||
- Encryption Context | ||
|
||
# Background | ||
|
||
The current Branch Key Store's Branch Key's use KMS Encryption Context | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
to cryptographically bind the Branch Key's cryptographic material to | ||
the Branch Key's Context. | ||
|
||
This cryptographic binding mitigates a number of threats around | ||
storing the Branch Keys | ||
and is vital to the safe usage of the Hierarchal Keyring. | ||
|
||
However, | ||
it is not clear that this is the best use of KMS Encryption Context, | ||
as it interferes with customers ability to use KMS Key Policies to | ||
constrain Key Usage. | ||
|
||
Further more, | ||
Encryption Context evaluation can be customized by the calling principles authorization; | ||
KMS Key Grants can have restricted but unique conditions. | ||
|
||
Many potential Branch Key Customers are prevented from using | ||
the Hierarchy Keyring, | ||
as they have pre-existing Key Policies with their tenants | ||
that cannot be met if the KMS Encryption Context is populated | ||
by the Branch Key's Context. | ||
|
||
Finally, | ||
some of the users have insisted that | ||
Crypto Tools refactor the KMS Encryption Context | ||
to only be the Branch Key's Encryption Context. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Requirements | ||
|
||
1. Branch Key's Context be cryptographically bound to the Branch Key's Cryptographic Material | ||
|
||
2. Branch Key's Encryption Context, untransformed in any way, is the KMS Encryption Context | ||
|
||
3. Support for all of Behaviors of the current Branch Key Store (Create, Version, Get\*) | ||
|
||
# Out of Scope | ||
|
||
- Abstracting away from KMS | ||
- Supporting any Branch Key protection Scheme via the DB-ESDK, much like the DDBEC's MetaStore. | ||
|
||
## Why not Abstract away from KMS | ||
|
||
Over the past year, | ||
Crypto Tools has spent a significant amount of our time | ||
supporting services integrating with KMS for multi-tenant applications. | ||
|
||
While we MAY eventually want to support GCP or Azure, | ||
we MUST focus on the fasting growing customer base we have; | ||
AWS Services and Software-as-a-service providers integrating with KMS. | ||
|
||
## Why not the MetaStore approach? | ||
|
||
The MetaStore was the predecessor to the Branch Key Store; it is the "caching" solution for the legacy DynamoDB Encryption Client (DDBEC). The MetaStore used the DDBEC itself to protect the hierarchical material with KMS. | ||
|
||
Which affords for some flexibility, as the MetaStore was an interface. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
However, our customers have complained that the DB-ESDK is complicated; | ||
using the DB-ESDK to protect the materials used by the DB-ESDK is NOT | ||
a step towards simplification. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
While such a feature MAY provide the greatest flexibility to | ||
our customers, | ||
it is not a simplification of the Hierarchy Keyring, | ||
but a complication to it. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Design Questions | ||
|
||
## 1 How can the Branch Key's Context be protected without using KMS EC? | ||
|
||
See [DesignQuestion.md](./DesignQuestion.md). | ||
texastony marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## 2 How are we going to offer operation in a mixed mode? | ||
|
||
UPDATE: 2.1 was rejected. | ||
There will be no Policy (2.2). | ||
|
||
### 2.1 `hierarchy-version-policy` | ||
|
||
Much like [Key Commitment](https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/concepts.html#key-commitment), | ||
I suggest we introduce a policy that | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
would allow for mixed usage or restricting to one or the other. | ||
|
||
Something like `hierarchy-version-policy`: | ||
|
||
- ALLOW_V1 | ||
- ALLOW_V2 | ||
- ALLOW_V1_OR_V2 | ||
- ANY | ||
|
||
This policy would be part of (all) Branch Key Store configuration. | ||
|
||
(all: Branch Key Store, any future Branch Key Store Usage client.) | ||
|
||
This allows our customers to | ||
restrict to one KMS Encryption Context | ||
experience or another. | ||
|
||
The effort to implement and test this is very low, | ||
relative to other code paths in this effort. | ||
|
||
### 2.2 Do Nothing | ||
|
||
UPDATE: We are going with Do Nothing | ||
|
||
There is no immediate customer demand for a policy like this. | ||
In general, | ||
the Branch Key in the table, | ||
in combination with KMS, | ||
the authority of it's treatment. | ||
|
||
However, | ||
given the low effort required to implement/support this, | ||
and the aid it gives our customers in having | ||
a consistent KMS Encryption Context experience, | ||
I suggest we implement the policy. | ||
|
||
### 2.3 Pros of 2.1 | ||
|
||
**Update Readers, than Writers**: | ||
To use HV-2, | ||
customers already using the H-Keyring will need | ||
to update their readers before they start creating | ||
or mutating Branch Keys to HV-2. | ||
|
||
i.e: They need to update their readers before they update their writers. | ||
|
||
This alone is justification for the policy. | ||
|
||
**Consistent KMS Encryption Context**: | ||
The arguments for the HV-1 Encryption Context suggest | ||
users SHOULD be able to restrict their Branch Key Store clients | ||
to a HV-1 such that they know any IAM or KMS Key Policies | ||
they created that depend on HV-1's Encryption Context | ||
are still valid. | ||
|
||
### 2.4 Cons | ||
|
||
**Additional Development work and documentation**: | ||
Nothing is free; | ||
we would need to implement, document, test, and maintain this policy. | ||
|
||
Still, | ||
the migration benefit justifies this cost. | ||
|
||
## 3 Branch Key Creation | ||
|
||
UPDATE: The team decided that the Branch Key Store would create and version both HV-1 and HV-2 BKs. | ||
UPDATE: The team changed their mind; the Branch Key Store WILL support HV-2 Branch Key Creation, via a flag. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Customers MUST be able to chose which `hierarchy-version` | ||
new Branch Keys are created with. | ||
|
||
The question is what should the UX be. | ||
|
||
### 3.1 Specify the `hierarchy-version` at Creation | ||
|
||
Currently, our library has one Branch Key Creation operations: | ||
|
||
- `BranchKeyStore#CreateKey` | ||
|
||
To this operation, | ||
we could add a flag that dictates the `hierarchy-version` to be created with. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
If that flag conflicts with `hierarchy-version-policy`, | ||
then FAIL. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Otherwise, respect the flag. | ||
|
||
**Plumbing through `GenerateRandom`**: | ||
UPDATE: `kms:GenerateDataKey` closed this negative consequence. | ||
We will had to add an optional `AwsKms` input | ||
to supply the plain-text data key. | ||
rishav-karanjit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### 3.2 Follow the Configuration | ||
|
||
This does not work if the configuration is `ALLOW\_V1\_OR_V2`; | ||
this is not recommended. | ||
|
||
### 3.3 New Operation or Client | ||
|
||
We could leave the old Branch Key Store alone, | ||
and introduce a new Branch Key Store V2. | ||
|
||
The UX advantage here is that we can introduce new error types | ||
without breaking customers. | ||
|
||
For example, | ||
the creation of the `hierarchy-version-policy` implies | ||
that we will need an error type for rejecting a Branch Key | ||
that does not match. | ||
|
||
Particularly if we went down the additional KMS approaches (1.2, 1.3), | ||
new error types will need to be created to represent | ||
failures from the additional KMS key. | ||
|
||
But we have committed to 1.4; | ||
this limits the failure modes/additional errors | ||
to the point that I do not think new client | ||
or operation is needed. | ||
|
||
### 3.4 Author's conclusion | ||
|
||
Unless someone can think of something else, | ||
3.1. | ||
|
||
## 4 Branch Key Versioning (Rotation) | ||
|
||
UPDATE: The team elected for 3.5; we will Mirror 3.5. | ||
(BKS can VersionKey HV-1 or HV-2). | ||
texastony marked this conversation as resolved.
Show resolved
Hide resolved
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to readers: This was reviewed in design doc review of hv-2