chore(.NET): update hierarchy examples #618

josecorella · 2023-10-17T21:54:44Z

Issue #, if available:

Description of changes:
Updates Hierarchy Keyring examples to include more comments and to have an explicit example on how to create a key store, create a branch key, version a branch key, and encrypt and decrypt using a statically configured hierarchy keyring

Squash/merge commit message, if applicable:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

texastony · 2023-10-17T22:29:54Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        //    It may take a couple minutes for the table to become ACTIVE,
+        //    at which point it is ready to store branch and beacon keys.
+
+        // keyStore.CreateKeyStore(new CreateKeyStoreInput());


Question: Why should we not run this?

The Description of in 2 states that IF the table already exists,
it will validate the table.

Therefore, it SHOULD BE safe to run this,
as it will NOT create a table.

Though we may need to grant the CI Role Describe Table permission.

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

texastony · 2023-10-17T22:35:51Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        //    Only the branch key will be rotated. 
+        //    This rotation is eventually consistent. See the VersionBranchKey
+        //    Example for how to version a branch key.


Issue: Can you describe Version as Rotate?
It is implied here, but not explicitly said.

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

texastony · 2023-10-17T22:43:23Z

...ntimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyringBranchKeySupplier.cs

+/// This example requires access to the DDB Table where you are storing the Branch Keys.
+/// This table must be configured with the following
+/// primary key configuration:
+/// - Partition key is named "partition_key" with type (S)
+/// - Sort key is named "sort_key" with type (S)


Suggestion:
If we are not going to create or version branch keys,
we do not need write permission.

Suggested change

/// This example requires access to the DDB Table where you are storing the Branch Keys.

/// This table must be configured with the following

/// primary key configuration:

/// - Partition key is named "partition_key" with type (S)

/// - Sort key is named "sort_key" with type (S)

/// This example requires read access to the DDB Table

/// where you are storing the Branch Keys.

/// primary key configuration:

/// - Partition key is named "partition_key" with type (S)

/// - Sort key is named "sort_key" with type (S)

Although the example won't create or version, someone reading this example and trying to run it won't know what permissions they need to successfully run the example

someone reading this example and trying to run it won't know what permissions they need to successfully run the example

Yes. Hence my suggestion that we document it.
In meetings, we have celebrated the Permission Boundary of the H-Keyring
and Key Store.

The Key Store implicitly supports two levels of Actors:

Keyring Users : Read the Table, Decrypt from KMS

Key Store Admins : Read and Write to the table; Generate, ReEncrypt, Decrypt from KMS

I do not want to bother documenting all of that here.
We could, and we SHOULD, one day, document all of this.

For now, we can imply this Actor distinction to our customers in this example.

After all,
this Example does NOT include any Branch Key Creation or Version calls,
commented out or otherwise.

...ntimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyringBranchKeySupplier.cs

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

texastony · 2023-10-18T16:19:59Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        // 4. Create the Hierarchical Keyring, using the Branch Key ID Supplier above.
+        //    With this configuration, the AWS SDK Client ultimately configured will be capable
+        //    of encrypting or decrypting items for either tenant (assuming correct KMS access).
+        //    We are restricting the keyring to only branch key by statically configuring
+        //    it with a branch key id. For an examples on how to decide on which branch key to 
+        //    use see the `AwsKmsHierarchicalKeyringBranhcKeySupplier` example in this directory.


Suggested change

// 4. Create the Hierarchical Keyring, using the Branch Key ID Supplier above.

// With this configuration, the AWS SDK Client ultimately configured will be capable

// of encrypting or decrypting items for either tenant (assuming correct KMS access).

// We are restricting the keyring to only branch key by statically configuring

// it with a branch key id. For an examples on how to decide on which branch key to

// use see the `AwsKmsHierarchicalKeyringBranhcKeySupplier` example in this directory.

// 4. Create the Hierarchical Keyring, with a static Branch Key ID.

// With this configuration, the AWS SDK Client ultimately configured will be capable

// of encrypting or decrypting items for either tenant (assuming correct KMS access).

// We are restricting the keyring to only branch key by statically configuring

// it with a branch key id. For an examples on how to decide on which branch key to

// use see the `AwsKmsHierarchicalKeyringBranhcKeySupplier` example in this directory.

texastony · 2023-10-18T17:31:19Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

Suggestion: Link to our docs.
https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/use-hierarchical-keyring.html

texastony · 2023-10-18T17:34:13Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        // 8. Rotate the Branch Key in our KeyStore.
+        //    Only the branch key will be rotated. 
+        //    This rotation is eventually consistent. See the VersionBranchKey
+        //    Example for how to rotate a branch key.
+        //    To rotate a branch key you need:
+        //    - "dynamodb:GetItem"
+        //    - "kms:ReEncrypt*"
+        //    - "kms:GenerateDataKeyWithoutPlaintext"


Suggestion: Link to our docs.
The VersionBranchKey file and these comments do not explain
what rotation or an active branch key is.
We could add all that content here or in the other file.

Or we could link to our docs.

Suggested change

// 8. Rotate the Branch Key in our KeyStore.

// Only the branch key will be rotated.

// This rotation is eventually consistent. See the VersionBranchKey

// Example for how to rotate a branch key.

// To rotate a branch key you need:

// - "dynamodb:GetItem"

// - "kms:ReEncrypt*"

// - "kms:GenerateDataKeyWithoutPlaintext"

// 8. Rotate the Branch Key in our KeyStore.

// Only the branch key will be rotated.

// This rotation is eventually consistent.

// See the VersionBranchKey for a narrower example.

// See the Developer Guide for details: https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/use-hierarchical-keyring.html#rotate-branch-key

// Example for how to rotate a branch key.

// To rotate a branch key you need:

// - "dynamodb:GetItem"

// - "kms:ReEncrypt*"

// - "kms:GenerateDataKeyWithoutPlaintext"

texastony · 2023-10-18T17:39:56Z

...ntimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyringBranchKeySupplier.cs

+/// This example requires access to the DDB Table where you are storing the Branch Keys.
+/// This table must be configured with the following
+/// primary key configuration:
+/// - Partition key is named "partition_key" with type (S)
+/// - Sort key is named "sort_key" with type (S)


someone reading this example and trying to run it won't know what permissions they need to successfully run the example

Yes. Hence my suggestion that we document it.
In meetings, we have celebrated the Permission Boundary of the H-Keyring
and Key Store.

The Key Store implicitly supports two levels of Actors:

Keyring Users : Read the Table, Decrypt from KMS

Key Store Admins : Read and Write to the table; Generate, ReEncrypt, Decrypt from KMS

I do not want to bother documenting all of that here.
We could, and we SHOULD, one day, document all of this.

For now, we can imply this Actor distinction to our customers in this example.

After all,
this Example does NOT include any Branch Key Creation or Version calls,
commented out or otherwise.

texastony · 2023-10-20T17:41:41Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        // This will NOT create a Key Store but check that the configured table name exists and it validates
+        // its construction. In order to check the construction is correct the configured IAM role
+        // MUST have `DescribeTable` permission on the Key Store resource.


Suggestion:
Semantic Line Breaks, grammar fixes,
& tighter phrasing to remove the other comment.

Suggested change

// This will NOT create a Key Store but check that the configured table name exists and it validates

// its construction. In order to check the construction is correct the configured IAM role

// MUST have `DescribeTable` permission on the Key Store resource.

// Since AWS Crypto Tool's Example table already exists,

// this will NOT create a Key Store

// but check that the configured table name exists

// and validate the table's index configuration.

// In order to check this configuration,

// the IAM role MUST have `DescribeTable` permission on

// the Key Store table.

texastony · 2023-10-20T17:42:09Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        // We have already created a Table with the specified configuration.
+        // For testing purposes we will not create a table when we run this example.


Suggested change

// We have already created a Table with the specified configuration.

// For testing purposes we will not create a table when we run this example.

texastony · 2023-10-20T17:47:00Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        // 4. Create the Hierarchical Keyring, with a static Branch Key ID.
+        //    With this configuration, the AWS SDK Client ultimately configured will be capable
+        //    of encrypting or decrypting items for either tenant (assuming correct KMS access).
+        //    We are restricting the keyring to only branch key by statically configuring
+        //    it with a branch key id. For an examples on how to decide on which branch key to 
+        //    use see the `AwsKmsHierarchicalKeyringBranhcKeySupplier` example in this directory.


Suggestion/Issue:
Do not mention tenants here.
This is a static configuration of one Branch Key ID.
It is not really a Multi-tenant scenario.

Suggested change

// 4. Create the Hierarchical Keyring, with a static Branch Key ID.

// With this configuration, the AWS SDK Client ultimately configured will be capable

// of encrypting or decrypting items for either tenant (assuming correct KMS access).

// We are restricting the keyring to only branch key by statically configuring

// it with a branch key id. For an examples on how to decide on which branch key to

// use see the `AwsKmsHierarchicalKeyringBranhcKeySupplier` example in this directory.

// 4. Create the Hierarchical Keyring, with a static Branch Key ID.

// This restricts the keyring to

// only one branch key by

// statically configuring it with a Branch Key ID.

// For a demonstration of Multi Branch Key support,

// see the `AwsKmsHierarchicalKeyringBranhcKeySupplier` example in this directory.

texastony · 2023-10-20T17:48:30Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

-            // KMS config in the keystore and therefore MUST have the right permissions.
-            BranchKeyIdSupplier = branchKeySupplier,
+            KeyStore = keyStore,
+            BranchKeyId = branchKeyId,
            // The value provided to `EntryCapacity` dictates how many branch keys will be held locally


Issue: Is this true?
I think that multiple of versions of the same Branch Key
count towards the Entry Capacity.

Suggested change

// The value provided to `EntryCapacity` dictates how many branch keys will be held locally

// The value provided to `EntryCapacity` dictates how many branch key versions will be held locally

texastony · 2023-10-20T17:49:35Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        //    See the VersionBranchKey for a narrower example.
+        //    See the Developer Guide for details:
+        //    https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/use-hierarchical-keyring.html#rotate-branch-key
+        //    Example for how to rotate a branch key.


Suggested change

// Example for how to rotate a branch key.

texastony · 2023-10-20T18:03:51Z

AwsEncryptionSDK/runtimes/net/Examples/Keyring/AwsKmsHierarchical/AwsKmsHierarchicalKeyring.cs

+        //    To rotate a branch key you need:
+        //    - "dynamodb:GetItem"
+        //    - "kms:ReEncrypt*"
+        //    - "kms:GenerateDataKeyWithoutPlaintext" 


Suggested change

// To rotate a branch key you need:

// - "dynamodb:GetItem"

// - "kms:ReEncrypt*"

// - "kms:GenerateDataKeyWithoutPlaintext"

// To rotate a branch key you need:

// - "dynamodb:ConditionCheckItem"

// - "dynamodb:GetItem"

// - "dynamodb:PutItem"

// - "kms:ReEncrypt*"

// - "kms:GenerateDataKeyWithoutPlaintext"

chore(.NET): update hierarchy examples

f270718

josecorella temporarily deployed to Duvet CI October 17, 2023 21:54 — with GitHub Actions Inactive

josecorella temporarily deployed to MPL_DAFNY October 17, 2023 21:54 — with GitHub Actions Inactive

Merge branch 'mainline' into jocorell/update-examples

9b92215

josecorella temporarily deployed to Duvet CI October 17, 2023 22:27 — with GitHub Actions Inactive

josecorella temporarily deployed to MPL_DAFNY October 17, 2023 22:27 — with GitHub Actions Inactive

josecorella marked this pull request as ready for review October 17, 2023 22:27

josecorella requested a review from a team as a code owner October 17, 2023 22:27

josecorella temporarily deployed to MPL_DAFNY October 17, 2023 22:27 — with GitHub Actions Inactive

texastony requested changes Oct 17, 2023

View reviewed changes

address comments

af38cd9

josecorella temporarily deployed to Duvet CI October 17, 2023 23:18 — with GitHub Actions Inactive

josecorella temporarily deployed to MPL_DAFNY October 17, 2023 23:18 — with GitHub Actions Inactive

texastony requested changes Oct 18, 2023

View reviewed changes

address comments

15e378a

josecorella temporarily deployed to Duvet CI October 19, 2023 18:51 — with GitHub Actions Inactive

josecorella temporarily deployed to MPL_DAFNY October 19, 2023 18:51 — with GitHub Actions Inactive

josecorella temporarily deployed to MPL_DAFNY October 19, 2023 18:52 — with GitHub Actions Inactive

josecorella requested a review from texastony October 19, 2023 18:52

texastony requested changes Oct 20, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(.NET): update hierarchy examples #618

chore(.NET): update hierarchy examples #618

josecorella commented Oct 17, 2023 •

edited

Loading

texastony Oct 17, 2023

texastony Oct 17, 2023

texastony Oct 17, 2023

josecorella Oct 17, 2023

texastony Oct 18, 2023

texastony Oct 18, 2023

texastony Oct 18, 2023

texastony Oct 18, 2023

texastony Oct 18, 2023

texastony Oct 20, 2023

texastony Oct 20, 2023

texastony Oct 20, 2023

texastony Oct 20, 2023

texastony Oct 20, 2023

texastony Oct 20, 2023

-        // This will NOT create a Key Store but check that the configured table name exists and it validates
-        // its construction. In order to check the construction is correct the configured IAM role
-        // MUST have `DescribeTable` permission on the Key Store resource.
+        // Since AWS Crypto Tool's Example table already exists,
+        // this will NOT create a Key Store
+        // but check that the configured table name exists
+        // and validate the table's index configuration.
+        // In order to check this configuration,
+        // the IAM role MUST have `DescribeTable` permission on
+        // the Key Store table.

		// We have already created a Table with the specified configuration.
		// For testing purposes we will not create a table when we run this example.

	// The value provided to `EntryCapacity` dictates how many branch keys will be held locally
	// The value provided to `EntryCapacity` dictates how many branch key versions will be held locally

chore(.NET): update hierarchy examples #618

Are you sure you want to change the base?

chore(.NET): update hierarchy examples #618

Conversation

josecorella commented Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josecorella commented Oct 17, 2023 •

edited

Loading