From d5b59337a74d8f6083c242742b890ecb3f1ef221 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 11:18:47 -0600 Subject: [PATCH 01/10] docs(examples): design negative caching for multi-tenant H-Keyrings Add negative_caching.md: how a Tenant Hierarchical Keyring stops calling Amazon DynamoDB and AWS KMS for a Branch Key ID that AWS KMS has denied, for a configurable time, using only the public extension interfaces. - Trace the Hierarchical Keyring's cache and KMS paths (spec, MPL 1.11.3). - Detect Denials in the Tenant KMS Client; enforce them in a per keyring Cache Guard in front of the shared cache, checked before it. - Key Denials by Branch Key ID, not KMS Key ARN, as KMS can deny some Branch Keys of one KMS Key by encryption context; supports Discovery. - A Tenant Keyring Factory replaces the Branch Key ID Supplier. - Revisit the Partition ID: refreshed credentials share no constant part, so it should derive from a stable Tenant identity. Note in design.md 10.2 that the Partition ID decision is revisited; the example's fix is deferred. Co-Authored-By: Claude Opus 5.5 --- examples/src/multi_tenant/design.md | 8 + examples/src/multi_tenant/negative_caching.md | 603 ++++++++++++++++++ 2 files changed, 611 insertions(+) create mode 100644 examples/src/multi_tenant/negative_caching.md diff --git a/examples/src/multi_tenant/design.md b/examples/src/multi_tenant/design.md index 4a912441e..28d0d3050 100644 --- a/examples/src/multi_tenant/design.md +++ b/examples/src/multi_tenant/design.md @@ -755,6 +755,14 @@ and additionally by their Partition IDs. ### 10.2 The Partition ID +UPDATE: This decision is revisited in +[negative_caching.md 8](./negative_caching.md#8-the-partition-id-revisited). +No part of refreshed credentials is constant, +so a Partition ID derived from them changes on every refresh, +and every Tenant misses the shared cache once per refresh. +The Partition ID SHOULD instead be derived from a stable Tenant identity; +the example will be changed in a later change. + The Partition ID is the SHA-384 digest of the Tenant's secret access key. **It is distinct per Tenant**: diff --git a/examples/src/multi_tenant/negative_caching.md b/examples/src/multi_tenant/negative_caching.md new file mode 100644 index 000000000..9d7ce7911 --- /dev/null +++ b/examples/src/multi_tenant/negative_caching.md @@ -0,0 +1,603 @@ +[//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved." +[//]: # "SPDX-License-Identifier: Apache-2.0" + +# Negative Caching for Multi-Tenant AWS KMS Hierarchical Keyrings + +# 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). + +This document builds on [design.md](./design.md); +its definitions of Tenant, Tenant Credentials, Heavy Object, +Shared Transport, Tenant KMS Client, and AWS KMS Key Store apply here. + +## Branch Key ID (BKID) + +The identifier of a Branch Key in an AWS KMS Key Store; +the `branch-key-id` attribute of every Branch Key item, +and the partition key of the AWS KMS Key Store's DynamoDB table. + +A Branch Key ID identifies every version of a Branch Key. + +## Denial + +A failure, returned by AWS KMS, +to decrypt a Branch Key on behalf of a Tenant, +that is caused by the Tenant owned resources +(the KMS Key, and its Key Policy or Grants) +and that will not resolve by retrying. + +See [Design Question 5](#5-what-is-a-denial). + +## Negative Cache Entry + +A record that a Branch Key ID was Denied, +with the time it was Denied +and the time it expires. + +## Negative TTL + +How long a Negative Cache Entry is honored. +While it is honored, +no request is made to Amazon DynamoDB or AWS KMS for that Branch Key ID. + +## Cache Guard + +A [Cryptographic Materials Cache](https://github.com/awslabs/aws-encryption-sdk-specification/blob/master/framework/cryptographic-materials-cache.md) +that wraps the cache shared by every Tenant, +and fails a lookup for a Branch Key ID with an honored Negative Cache Entry, +rather than returning a cache miss. + +One per Tenant Hierarchical Keyring. + +## Tenant Keyring Factory + +An object, one per process, +that holds everything every Tenant Hierarchical Keyring shares +(the Shared Transport, the shared cache, the DynamoDB client, and the AWS KMS Key Store configuration), +and that vends a Tenant Hierarchical Keyring +given a Branch Key ID and Tenant Credentials. + +See [Design Question 7](#7-who-builds-a-tenant-hierarchical-keyring). + +# Background + +[design.md](./design.md) lets an application call AWS KMS +with per-Tenant credentials over one connection pool, +and gives every Tenant's Hierarchical Keyring one shared cache. + +It does not address a Tenant whose KMS Key stops being available: +the Tenant revokes the application's access, +disables the KMS Key, +or schedules it for deletion. + +The Hierarchical Keyring only records a Branch Key in its cache on success. +Thus, +every request for that Tenant's Branch Key +misses the cache, +reads the Branch Key from Amazon DynamoDB, +calls `kms:Decrypt`, +and fails; +every time, +for as long as the KMS Key is unavailable. + +These requests are not free: + +- They consume the application's DynamoDB capacity, though the cause is the Tenant's. +- They consume KMS request quota, and the latency of a round trip that is known to fail. +- With the storm tracking cache of [design.md 10.1](./design.md#101-one-shared-cache), + a failed fetch holds one of the fan out slots every Tenant shares for the in flight TTL + ([design.md 10.1.1](./design.md#1011-why-not-the-mpls-defaultcache)), + and concurrent callers for the failing Branch Key block, + rather than fail. + +Negative caching records the failure, +and fails subsequent requests for that Branch Key ID immediately, +for a configurable time, +without calling Amazon DynamoDB or AWS KMS. + +This document records the options considered, +and the decisions made, +to implement negative caching +without changing the source of the ESDK-Python or the MPL; +only by implementing the public, extendable interfaces +(Keyrings, Master Key Providers, Cryptographic Materials Managers, and Cryptographic Materials Caches). + +# Requirements + +1. After a Denial for a Branch Key ID, + the Tenant Hierarchical Keyring MUST NOT call Amazon DynamoDB or AWS KMS for that Branch Key ID + until the Negative TTL expires. + +2. A Denial for a Branch Key ID MUST NOT affect any other Branch Key ID, + including Branch Key IDs protected by the same KMS Key. + +3. While a Negative Cache Entry is honored, + the Branch Key ID's cached materials MUST NOT be used, + for encrypt or decrypt, + for any version of the Branch Key. + +4. Only a [Denial](#5-what-is-a-denial) MUST create a Negative Cache Entry; + transient failures (throttling, server errors, timeouts) + and failures of the Tenant's credentials MUST NOT. + +5. The ESDK-Python and MPL source MUST NOT be changed; + only their public, extendable interfaces MAY be implemented. + +6. Every AWS KMS Configuration of the AWS KMS Key Store MUST be supported, + including `Discovery` and `MRDiscovery`. + +7. Every Tenant's Hierarchical Keyring MUST still share one cache + ([design.md 10.1](./design.md#101-one-shared-cache)). + +# Out of Scope + +- Keyrings other than the AWS KMS Hierarchical Keyring, + the KMS Master Key Providers, + and the Caching Cryptographic Materials Manager. +- Failures of Amazon DynamoDB, including a missing Branch Key item. +- Propagating a `NotFoundException` to every Branch Key ID protected by the same KMS Key. +- A Branch Key ID Supplier selecting among Branch Key IDs within one keyring; + see [Design Question 7](#7-who-builds-a-tenant-hierarchical-keyring). + +## Why not Amazon DynamoDB failures? + +The DynamoDB table is owned by the application, not the Tenant. +A DynamoDB failure is the application's to resolve, +and is not a statement about any one Tenant. + +## Why not propagate `NotFoundException` across Branch Key IDs? + +A KMS Key that does not exist does not exist for every Branch Key it protects; +a `NotFoundException` for one Branch Key ID could create a Negative Cache Entry +for every Branch Key ID protected by that KMS Key. + +That requires mapping KMS Keys to Branch Key IDs, +which the Cache Guard does not otherwise need +([Design Question 6](#6-why-the-branch-key-id-and-not-the-kms-key-arn)). +It is an optimization; +each Branch Key ID is Denied on its own first failure regardless. + +# Design Questions + +## 1 Where does the Hierarchical Keyring call AWS KMS? + +Traced through the specification +(`framework/aws-kms/aws-kms-hierarchical-keyring.md`, +`framework/branch-key-store.md`, +`framework/cryptographic-materials-cache.md`, +`framework/storm-tracking-cryptographic-materials-cache.md`) +and the MPL +(`AwsKmsHierarchicalKeyring.dfy`, `v1.11.3`): + +``` +OnEncrypt OnDecrypt (each matching EDK; errors collected) + cacheId = SHA384(0x02, 0x01, partition, cacheId = SHA384(0x02, 0x02, partition, + logicalKeyStoreName, BKID) logicalKeyStoreName, BKID, version) + cache.GetCacheEntry(cacheId) cache.GetCacheEntry(cacheId) + |- error, not EntryDoesNotExist -> FAIL |- error, not EntryDoesNotExist -> FAIL + |- hit, within TTL -> wrap |- hit, within TTL -> unwrap + `- miss, or expired `- miss, or expired + KeyStore.GetActiveBranchKey(BKID) KeyStore.GetBranchKeyVersion(BKID, version) + 1. DynamoDB GetItem (branch:ACTIVE) 1. DynamoDB GetItem (branch:version:) + 2. kms:Decrypt 2. kms:Decrypt + |- failure -> FAIL; the cache is not told |- failure -> FAIL; the cache is not told + `- success -> PutCacheEntry `- success -> PutCacheEntry +``` + +Thus: + +**AWS KMS is only called on a cache miss**: +and only through the AWS KMS Key Store. + +**Amazon DynamoDB is always called first**: +A failing KMS Key costs a DynamoDB read on every attempt. + +**The cache never learns of a failure**: +`PutCacheEntry` is only called on success +(`AwsKmsHierarchicalKeyring.dfy`, `GetActiveHierarchicalMaterials`; +the Key Store's failure returns through `:-` before the cache is called). + +**A cache error other than a miss fails the keyring immediately**: +If `GetCacheEntry` fails with any error other than `EntryDoesNotExist`, +the keyring returns that error, +without calling the AWS KMS Key Store +(`AwsKmsHierarchicalKeyring.dfy`, both `GetCacheEntry` call sites). + +**The cache identifier is opaque**: +It is a SHA-384 digest; +it does not include the KMS Key ARN, +and a cache cannot recover the Branch Key ID from it. + +**The storm tracking cache is an accidental, and poor, rate limit**: +A failed fetch never calls `PutCacheEntry`, +so the Branch Key stays in flight. +While it is, +concurrent callers block, +and at most one retry is made per grace interval; +once a caller has waited the in flight TTL, +`GetCacheEntry` fails. +Failing Tenants slow every Tenant, +rather than failing fast. + +## 2 Which interfaces can be extended? + +| Interface | Native Python implementation accepted? | Sees a KMS failure? | Knows the Branch Key ID? | Can prevent the DynamoDB and KMS calls? | +| --------- | -------------------------------------- | ------------------- | ------------------------ | --------------------------------------- | +| Cryptographic Materials Cache (`ICryptographicMaterialsCache`, passed as `CacheTypeShared`) | Yes | No | No; the cache identifier is a digest | Yes; by failing `GetCacheEntry` | +| AWS KMS Key Store | **No** | — | — | — | +| KMS client (the Tenant KMS Client) | Yes; duck typed ([design.md 4.1](./design.md#41-keyrings-client-suppliers-and-the-aws-kms-key-store)) | **Yes**; the error code, exactly | Only if bound to it ([6](#6-why-the-branch-key-id-and-not-the-kms-key-arn)) | No; DynamoDB is called first | +| Keyring (an `IKeyring` delegating to the Hierarchical Keyring) | Yes | Indirectly; nested in Key Store and collection errors | Yes | Yes; but before the cache, so cache hits are lost | + +**The cache accepts native implementations**: +`aws_cryptography_materialproviders_CryptographicMaterialsCacheReference` +passes any object without an `_impl` attribute to Dafny unchanged; +`ICryptographicMaterialsCache` provides the Dafny facing methods, +which call the native `get_cache_entry` and friends. + +**Native errors keep their meaning**: +Those Dafny facing methods map native exceptions with `_smithy_error_to_dafny_error`: +a native `EntryDoesNotExist` becomes a Dafny `EntryDoesNotExist`, +which the keyring treats as a miss; +any other exception becomes another error, +which the keyring returns immediately. + +**The AWS KMS Key Store does not**: +`aws_cryptography_materialproviders_KeyStoreReference` +requires the MPL's `KeyStore` +(`native_input._config.dafnyImplInterface.impl`); +there is no Python interface to implement. + +**Keyrings accept native implementations**: +`aws_cryptography_materialproviders_KeyringReference` +passes any object without an `_impl` attribute unchanged. + +## 3 Where is a Denial detected? + +### 3.1 The cache + +Not possible; +the cache is never told of a failure ([1](#1-where-does-the-hierarchical-keyring-call-aws-kms)). + +A cache could infer one from a miss that is not followed by `PutCacheEntry`; +it cannot distinguish a failure from a slow fetch, +nor a Denial from throttling. + +### 3.2 A keyring wrapper + +A keyring delegating to the Hierarchical Keyring +receives the failure as an exception, +after the fact. + +**Con, the error is nested**: +The KMS error is wrapped by the AWS KMS Key Store's error, +and on decrypt, +in a collection of every encrypted data key's errors. +Classifying it means unpicking MPL error structures +that are not a stable interface. + +### 3.3 The Tenant KMS Client + +The Tenant KMS Client receives every KMS response, +before the MPL wraps it. + +**Pro, exact**: +The KMS error code is in hand, +unwrapped, +and the request that caused it. + +**Pro, already ours**: +The example already supplies every KMS client the AWS KMS Key Store uses. + +**Con, coupling**: +Negative caching depends on the Tenant KMS Client; +an application using boto3's KMS client would not get it. +For an example built on [design.md](./design.md), +this is acceptable. + +### 3.4 Author's conclusion + +3.3. + +## 4 Where is a Negative Cache Entry enforced? + +### 4.1 The Tenant KMS Client + +It could fail a `Decrypt` immediately, +without a network request. + +**Con, Amazon DynamoDB is still called**: +The AWS KMS Key Store reads the Branch Key before calling KMS, +violating Requirement 1. + +### 4.2 A keyring wrapper + +It could fail `OnEncrypt` and `OnDecrypt` before the Hierarchical Keyring is called. + +**Con, a second interface for a single behavior**: +The Cache Guard ([4.3](#43-a-cache-guard)) achieves the same at the cache, +which the Hierarchical Keyring already consults before any remote call. + +### 4.3 A Cache Guard + +A cache, +one per Tenant Hierarchical Keyring, +wrapping the cache every Tenant shares. + +`GetCacheEntry` fails, +with an error other than `EntryDoesNotExist`, +while its Branch Key ID has an honored Negative Cache Entry; +the Hierarchical Keyring then fails without calling the AWS KMS Key Store. +Otherwise, +every operation is delegated to the shared cache. + +**The Cache Guard checks before the shared cache**: +Requirement 3 means a Denied Branch Key ID's cached materials MUST NOT be used; +so the Negative Cache Entry is checked first, +and a hit in the shared cache is not returned. + +This also avoids the storm tracking cache: +checked after, +a miss would mark the Branch Key in flight, +and block concurrent callers, +before the Cache Guard could fail them. + +A Denial for one Branch Key ID does not affect another (Requirement 2), +even one protected by the same KMS Key; +see [6](#6-why-the-branch-key-id-and-not-the-kms-key-arn). + +**One Cache Guard per keyring**: +The cache identifier is opaque; +a cache shared by every Tenant cannot tell which Branch Key ID a lookup is for. +A Cache Guard per Tenant Hierarchical Keyring is bound to that keyring's Branch Key ID, +and delegates to the one shared cache, +satisfying Requirement 7. + +### 4.4 Author's conclusion + +4.3. + +## 5 What is a Denial? + +These KMS errors are Denials: + +| KMS error | Cause | +| --------- | ----- | +| `AccessDeniedException` | The Key Policy, a Grant, or an IAM Policy does not allow the request; including conditions on the encryption context | +| `DisabledException` | The KMS Key is disabled | +| `KMSInvalidStateException` | The KMS Key is pending deletion, pending import, or otherwise unusable | +| `NotFoundException` | The KMS Key does not exist | +| `IncorrectKeyException` | The Branch Key was not encrypted under the KMS Key requested | + +These are not: + +**Transient failures**: +Throttling (`ThrottlingException`), +KMS server errors (`KMSInternalException`, `DependencyTimeoutException`, HTTP 5xx), +and transport failures. +These are retried by the Shared Transport +([design.md 6](./design.md#6-retries)), +and say nothing about the Tenant's resources. + +**Failures of the Tenant's credentials**: +`UnrecognizedClientException` SHOULD never happen; +the credentials were issued for the Tenant. +An expired security token SHOULD cause the Tenant's credentials to be refreshed, +not the Branch Key ID to be Denied. + +## 6 Why the Branch Key ID, and not the KMS Key ARN? + +One KMS Key MAY protect many Branch Keys; +each Branch Key has its own encryption context. + +KMS evaluates the encryption context of each request: +a Key Policy or Grant MAY allow `kms:Decrypt` +only for requests whose encryption context matches a condition +(for example, `kms:EncryptionContext:branch-key-id`, +or a custom `aws-crypto-ec:` attribute). +Thus, +KMS MAY deny a fraction of the Branch Keys protected by one KMS Key, +and allow the rest. + +A Denial is therefore a statement about a Branch Key, +not about a KMS Key; +keyed by KMS Key ARN, +one Denied Branch Key would deny every Branch Key protected by that KMS Key, +violating Requirement 2. + +Keying by Branch Key ID also satisfies Requirement 6: +the Cache Guard never needs the KMS Key ARN, +which `Discovery` and `MRDiscovery` only learn from the DynamoDB item. + +**The Branch Key ID is bound, not parsed**: +For hierarchy version 1, +the KMS encryption context includes `branch-key-id` +(`framework/branch-key-store.md`, Encryption Context); +the Tenant KMS Client could read it from each `Decrypt` request. + +However, +hierarchy version 2 uses the Branch Key's encryption context, untransformed, +as the KMS encryption context; +it need not include `branch-key-id`. + +Thus, +the Tenant Keyring Factory binds the Branch Key ID +to the Tenant KMS Client it vends with each keyring, +and the Tenant KMS Client records a Denial against that Branch Key ID. +Where the request's encryption context includes `branch-key-id`, +the Tenant KMS Client SHOULD check it matches. + +As a Tenant KMS Client is a few references +([design.md, Tenant KMS Client](./design.md#tenant-kms-client)), +one per Tenant Hierarchical Keyring, +rather than one per Tenant, +costs nothing of note. + +## 7 Who builds a Tenant Hierarchical Keyring? + +The Cache Guard, +the Tenant KMS Client, +and the Negative Cache Entries they share, +MUST be wired together per Branch Key ID; +and every Tenant Hierarchical Keyring MUST share +the Shared Transport, +the shared cache, +and the DynamoDB client. + +### 7.1 A Branch Key ID Supplier + +The [Branch Key ID Supplier](https://github.com/awslabs/aws-encryption-sdk-specification/blob/master/framework/aws-kms/aws-kms-hierarchical-keyring.md#branch-key-supplier) +selects a Branch Key ID per request, +from the encryption context, +within one keyring. + +**Con, one keyring, many Branch Key IDs**: +The Cache Guard and the Tenant KMS Client are bound to one Branch Key ID ([6](#6-why-the-branch-key-id-and-not-the-kms-key-arn)); +a keyring whose Branch Key ID varies per request cannot bind them. + +**Con, one AWS KMS Key Store, one KMS client**: +A keyring has one AWS KMS Key Store, +and an AWS KMS Key Store one KMS client; +so a keyring serving many Tenants calls KMS with one set of credentials, +which [design.md](./design.md) exists to avoid. + +### 7.2 A Tenant Keyring Factory + +One per process. +It holds: + +- the Shared Transport +- the shared cache +- the DynamoDB client +- the AWS KMS Key Store configuration, + other than the KMS client + (table name, Logical Key Store Name, AWS KMS Configuration, grant tokens) +- the Negative TTL + +Given a Branch Key ID, +the Tenant's Credentials, +and a stable Tenant identity ([8](#8-the-partition-id-revisited)), +it vends a Tenant Hierarchical Keyring, +built with: + +- a Tenant KMS Client, bound to the Branch Key ID +- an AWS KMS Key Store, using that Tenant KMS Client +- a Cache Guard, bound to the Branch Key ID, delegating to the shared cache +- the Branch Key ID, statically +- a Partition ID derived from the stable Tenant identity + +**This changes the guidance of [design.md 10](./design.md#10-the-hierarchical-keyring)**: +which offers a Branch Key ID Supplier as the simpler multi-tenant design +when Tenants do not require distinct KMS credentials. +With negative caching, +the Branch Key ID MUST be static per keyring, +and the factory takes the Branch Key ID Supplier's place: +the application selects the Branch Key ID, +and asks the factory for that Branch Key ID's keyring. + +### 7.3 Author's conclusion + +7.2. + +## 8 The Partition ID, revisited + +[design.md 10.2](./design.md#102-the-partition-id) derives the Partition ID +from the SHA-384 digest of the Tenant's secret access key. + +**No part of refreshed credentials is constant**: +botocore's `RefreshableCredentials` replaces +the access key, +the secret key, +and the session token +on every refresh +(`botocore/credentials.py`, `_set_from_data`); +every STS issuance +(`AssumeRole`, `AssumeRoleWithWebIdentity`, and the like) +returns a new temporary access key id, secret, and session token. +Only long term IAM user keys are constant, +until they are rotated. + +Thus, +with refreshable credentials, +every Tenant's Partition ID changes on every refresh, +roughly hourly with STS's default session duration; +and every Tenant misses the shared cache once per refresh. +[design.md 10.2](./design.md#102-the-partition-id) called this miss intended; +in practice it is a periodic, synchronized cache miss for every Tenant, +which the shared cache exists to soften. + +**What the Partition ID protects**: +A keyring with Tenant B's credentials and Tenant A's Branch Key ID, +through misconfiguration, +MUST NOT be served Tenant A's cached materials +without KMS authorizing Tenant B. +A Partition ID per Tenant identity provides this; +a Partition ID per credential issuance is not needed for it. + +**Decision**: +The Partition ID SHOULD be derived from a stable Tenant identity +(for example, the Tenant's identifier, or the ARN of the role its credentials assume) +supplied to the Tenant Keyring Factory, +rather than from the Tenant's Credentials. + +As with any cache, +a revoked KMS authorization is not noticed until a cached entry expires; +once a fetch is Denied, +the Cache Guard enforces it (Requirement 3). + +UPDATE: The fix to the existing example is deferred to a later change; +[design.md 10.2](./design.md#102-the-partition-id) records this. + +## 9 The storm tracking cache + +The Cache Guard sits in front of the storm tracking cache, +and does not change its configuration +([design.md 10.1.1](./design.md#1011-why-not-the-mpls-defaultcache)). + +**The first Denial still holds a fan out slot**: +The fetch that is Denied +was made after the storm tracking cache returned a miss, +and so holds its fan out slot until the in flight TTL; +the Cache Guard prevents every later fetch, +so it holds no more. + +**Blocked callers are not released early**: +Callers that were already blocked in the storm tracking cache, +on the same Branch Key, +when the Denial occurred, +remain blocked until the in flight TTL. +Callers after the Denial are failed by the Cache Guard, +without reaching the storm tracking cache. + +# Open Questions + +1. **The Negative TTL**: + its default, and whether it is configured per factory or per Branch Key ID. + +2. **The error a Cache Guard raises**: + its type, + and whether it carries the original KMS error code + (for example, as an MPL `ComAmazonawsKms` error, or with the code in its message). + +3. **Thread safety**: + Negative Cache Entries are written by Tenant KMS Clients + and read by Cache Guards, + on any thread. + +4. **A Denial on encrypt versus decrypt**: + whether a Denial on `GetActiveBranchKey` and on `GetBranchKeyVersion` + are treated alike; + Requirement 3 suggests they are. + +# Consequences + +- The fail fast behavior depends on the MPL returning a cache error, + other than `EntryDoesNotExist`, without calling the AWS KMS Key Store + ([1](#1-where-does-the-hierarchical-keyring-call-aws-kms)); + this is behavior, not a documented interface. +- Negative caching requires the Tenant KMS Client ([3.3](#33-the-tenant-kms-client)). +- Every Tenant Hierarchical Keyring has a static Branch Key ID ([7.2](#72-a-tenant-keyring-factory)). From 0585e3a48a2029c2e362d1a8a452b0ce59e10201 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 13:58:10 -0600 Subject: [PATCH 02/10] docs(examples): address review of the negative caching design - Requirement 4: expired credentials are a transient failure. - Requirement 8: the factory and what it vends MUST be thread safe. - Negative TTL: configured per factory, default 5 minutes. - The Cache Guard passes through the KMS error it received, as is, and is a lightweight object despite being a cache. - Encrypt and decrypt Denials are treated the same. - Move NotFoundException propagation to Design Question 10, noting that a Branch Key's KMS Key is mutable (spec PR #287; MPL mutations/mutations TerminalKmsArn). - Resolve the open questions with UPDATEs. Co-Authored-By: Claude Opus 5.5 --- examples/src/multi_tenant/negative_caching.md | 113 +++++++++++++++--- 1 file changed, 95 insertions(+), 18 deletions(-) diff --git a/examples/src/multi_tenant/negative_caching.md b/examples/src/multi_tenant/negative_caching.md index 9d7ce7911..adf138148 100644 --- a/examples/src/multi_tenant/negative_caching.md +++ b/examples/src/multi_tenant/negative_caching.md @@ -36,7 +36,8 @@ See [Design Question 5](#5-what-is-a-denial). ## Negative Cache Entry A record that a Branch Key ID was Denied, -with the time it was Denied +with the error AWS KMS returned, +the time it was Denied, and the time it expires. ## Negative TTL @@ -45,6 +46,9 @@ How long a Negative Cache Entry is honored. While it is honored, no request is made to Amazon DynamoDB or AWS KMS for that Branch Key ID. +The Negative TTL is configured per [Tenant Keyring Factory](#tenant-keyring-factory), +and defaults to 5 minutes. + ## Cache Guard A [Cryptographic Materials Cache](https://github.com/awslabs/aws-encryption-sdk-specification/blob/master/framework/cryptographic-materials-cache.md) @@ -122,8 +126,7 @@ only by implementing the public, extendable interfaces for any version of the Branch Key. 4. Only a [Denial](#5-what-is-a-denial) MUST create a Negative Cache Entry; - transient failures (throttling, server errors, timeouts) - and failures of the Tenant's credentials MUST NOT. + transient failures (throttling, server errors, timeouts, expired credentials) MUST NOT. 5. The ESDK-Python and MPL source MUST NOT be changed; only their public, extendable interfaces MAY be implemented. @@ -134,13 +137,15 @@ only by implementing the public, extendable interfaces 7. Every Tenant's Hierarchical Keyring MUST still share one cache ([design.md 10.1](./design.md#101-one-shared-cache)). +8. The Tenant Keyring Factory, and every object it vends, MUST be thread safe, + and this SHOULD be tested. + # Out of Scope - Keyrings other than the AWS KMS Hierarchical Keyring, the KMS Master Key Providers, and the Caching Cryptographic Materials Manager. - Failures of Amazon DynamoDB, including a missing Branch Key item. -- Propagating a `NotFoundException` to every Branch Key ID protected by the same KMS Key. - A Branch Key ID Supplier selecting among Branch Key IDs within one keyring; see [Design Question 7](#7-who-builds-a-tenant-hierarchical-keyring). @@ -150,18 +155,6 @@ The DynamoDB table is owned by the application, not the Tenant. A DynamoDB failure is the application's to resolve, and is not a statement about any one Tenant. -## Why not propagate `NotFoundException` across Branch Key IDs? - -A KMS Key that does not exist does not exist for every Branch Key it protects; -a `NotFoundException` for one Branch Key ID could create a Negative Cache Entry -for every Branch Key ID protected by that KMS Key. - -That requires mapping KMS Keys to Branch Key IDs, -which the Cache Guard does not otherwise need -([Design Question 6](#6-why-the-branch-key-id-and-not-the-kms-key-arn)). -It is an optimization; -each Branch Key ID is Denied on its own first failure regardless. - # Design Questions ## 1 Where does the Hierarchical Keyring call AWS KMS? @@ -264,7 +257,7 @@ Not possible; the cache is never told of a failure ([1](#1-where-does-the-hierarchical-keyring-call-aws-kms)). A cache could infer one from a miss that is not followed by `PutCacheEntry`; -it cannot distinguish a failure from a slow fetch, +but it cannot distinguish a failure from a slow fetch, nor a Denial from throttling. ### 3.2 A keyring wrapper @@ -357,6 +350,39 @@ A Cache Guard per Tenant Hierarchical Keyring is bound to that keyring's Branch and delegates to the one shared cache, satisfying Requirement 7. +**The Cache Guard is a lightweight object**: +A cache is normally a Heavy Object +([design.md, Heavy Object](./design.md#heavy-object)); +it holds every entry. +While the Cache Guard is a cache, +it holds only its Branch Key ID's Negative Cache Entry, if any, +and a reference to the shared cache; +one per Tenant Hierarchical Keyring costs nothing of note. + +**The Cache Guard passes through the error it received**: +The Cache Guard fails `GetCacheEntry` +by raising the error the Tenant KMS Client received from AWS KMS, +as is, +from the Negative Cache Entry. + +_Note_: The MPL maps an error raised by a native cache +with `_smithy_error_to_dafny_error`; +an error that is not an MPL error, +such as botocore's `ClientError`, +becomes an opaque error carrying it. +Thus, +the first Denial surfaces wrapped by the AWS KMS Key Store's error, +and later failures as an opaque error carrying the original; +the original error is the same in both. + +**Encrypt and decrypt are treated the same**: +A Denial from `GetActiveBranchKey` (encrypt) +and from `GetBranchKeyVersion` (decrypt) +create the same Negative Cache Entry for the Branch Key ID, +and the Cache Guard fails every lookup for that Branch Key ID, +for encrypt and decrypt, +and for every version (Requirement 3). + ### 4.4 Author's conclusion 4.3. @@ -476,7 +502,7 @@ It holds: - the AWS KMS Key Store configuration, other than the KMS client (table name, Logical Key Store Name, AWS KMS Configuration, grant tokens) -- the Negative TTL +- the Negative TTL (default 5 minutes) Given a Branch Key ID, the Tenant's Credentials, @@ -573,26 +599,77 @@ remain blocked until the in flight TTL. Callers after the Denial are failed by the Cache Guard, without reaching the storm tracking cache. +## 10 Should a `NotFoundException` propagate across Branch Key IDs? + +A KMS Key that does not exist does not exist for every Branch Key it protects; +a `NotFoundException` for one Branch Key ID could create a Negative Cache Entry +for every Branch Key ID protected by that KMS Key. + +**It requires mapping KMS Keys to Branch Key IDs**: +which the Cache Guard does not otherwise need +([6](#6-why-the-branch-key-id-and-not-the-kms-key-arn)). +Under `Discovery` and `MRDiscovery`, +the KMS Key of a Branch Key is only known from its DynamoDB item. + +**The KMS Key of a Branch Key is mutable**: +A Branch Key's KMS Key is not fixed at creation. +The Branch Key Storage changes for Mutations +([aws-encryption-sdk-specification#287](https://github.com/awslabs/aws-encryption-sdk-specification/pull/287), +branch `tony/bks-storage-mutations`, +`changes/2025-01-17_key-store-storage-mutations/background.md`) +add operations that "facilitate changing the properties of all items of a Branch Key"; +the MPL's `mutations/mutations` branch changes the KMS Key ARN of all items of a Branch Key +(`Mutations(TerminalKmsArn := ...)`; +`AwsCryptographyKeyStoreAdmin/test/Mutations/TestKmsArnChanged.dfy`). +Thus, +a mapping from KMS Keys to Branch Key IDs can become stale, +and would need to be kept consistent with mutations. + +This is also why a Negative Cache Entry is bounded by the Negative TTL: +a Denied Branch Key MAY be mutated to a KMS Key the Tenant does allow, +and the Branch Key ID is retried once the Negative TTL expires. + +**It is an optimization**: +Each Branch Key ID is Denied on its own first failure regardless. + +### 10.1 Author's conclusion + +No; +each Branch Key ID is Denied on its own. + # Open Questions 1. **The Negative TTL**: its default, and whether it is configured per factory or per Branch Key ID. + UPDATE: Configured per Tenant Keyring Factory; the default is 5 minutes. + 2. **The error a Cache Guard raises**: its type, and whether it carries the original KMS error code (for example, as an MPL `ComAmazonawsKms` error, or with the code in its message). + UPDATE: The Cache Guard passes through the error it received, as is + ([4.3](#43-a-cache-guard)). + 3. **Thread safety**: Negative Cache Entries are written by Tenant KMS Clients and read by Cache Guards, on any thread. + UPDATE: There is no reason the Tenant Keyring Factory, + and the objects it vends, + are not thread safe; + they MUST be (Requirement 8), + and this SHOULD be tested. + 4. **A Denial on encrypt versus decrypt**: whether a Denial on `GetActiveBranchKey` and on `GetBranchKeyVersion` are treated alike; Requirement 3 suggests they are. + UPDATE: They are treated the same ([4.3](#43-a-cache-guard)). + # Consequences - The fail fast behavior depends on the MPL returning a cache error, From 0eeae487f44ea12540723b801a209e072ccc622e Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 14:10:31 -0600 Subject: [PATCH 03/10] docs(examples): credential failures are Denials in negative caching botocore only refreshes credentials proactively (advisory and mandatory windows before expiry) and never in response to an expired token error, so an expired, unrecognized, or mis-signed credential reaching AWS KMS will not resolve by retrying; left undenied, every request would call Amazon DynamoDB and AWS KMS. - Add ExpiredTokenException, UnrecognizedClientException, and InvalidSignatureException as Denials (5.1, 5.2). - Record a probe of AWS KMS through the Tenant KMS Client, varying one part of the credentials at a time. - Note these are usually the application's credential handling, not the Tenant's; the passed through error keeps the cause visible. Co-Authored-By: Claude Opus 5.5 --- examples/src/multi_tenant/negative_caching.md | 134 +++++++++++++++++- 1 file changed, 128 insertions(+), 6 deletions(-) diff --git a/examples/src/multi_tenant/negative_caching.md b/examples/src/multi_tenant/negative_caching.md index adf138148..091a61ab9 100644 --- a/examples/src/multi_tenant/negative_caching.md +++ b/examples/src/multi_tenant/negative_caching.md @@ -29,6 +29,8 @@ A failure, returned by AWS KMS, to decrypt a Branch Key on behalf of a Tenant, that is caused by the Tenant owned resources (the KMS Key, and its Key Policy or Grants) +or by the Tenant's Credentials +(expired, unrecognized, or not matching their signature), and that will not resolve by retrying. See [Design Question 5](#5-what-is-a-denial). @@ -126,7 +128,9 @@ only by implementing the public, extendable interfaces for any version of the Branch Key. 4. Only a [Denial](#5-what-is-a-denial) MUST create a Negative Cache Entry; - transient failures (throttling, server errors, timeouts, expired credentials) MUST NOT. + transient failures (throttling, server errors, timeouts) MUST NOT. + Expired, unrecognized, and mis-signed credentials are Denials + ([5.1](#51-expired-credentials-are-a-denial), [5.2](#52-unrecognized-and-mis-signed-credentials-are-denials)). 5. The ESDK-Python and MPL source MUST NOT be changed; only their public, extendable interfaces MAY be implemented. @@ -398,6 +402,9 @@ These KMS errors are Denials: | `KMSInvalidStateException` | The KMS Key is pending deletion, pending import, or otherwise unusable | | `NotFoundException` | The KMS Key does not exist | | `IncorrectKeyException` | The Branch Key was not encrypted under the KMS Key requested | +| `ExpiredTokenException` | The Tenant's Credentials have expired; see [5.1](#51-expired-credentials-are-a-denial) | +| `UnrecognizedClientException` | KMS does not recognize the access key id or the session token; see [5.2](#52-unrecognized-and-mis-signed-credentials-are-denials) | +| `InvalidSignatureException` | The request's signature does not match the credentials' secret, or its timestamp; see [5.2](#52-unrecognized-and-mis-signed-credentials-are-denials) | These are not: @@ -409,11 +416,126 @@ These are retried by the Shared Transport ([design.md 6](./design.md#6-retries)), and say nothing about the Tenant's resources. -**Failures of the Tenant's credentials**: -`UnrecognizedClientException` SHOULD never happen; -the credentials were issued for the Tenant. -An expired security token SHOULD cause the Tenant's credentials to be refreshed, -not the Branch Key ID to be Denied. +### 5.1 Expired credentials are a Denial + +UPDATE: An earlier revision of this document, +and of Requirement 4, +treated expired credentials as transient, +expecting them to cause the Tenant's Credentials to be refreshed. +Nothing does so; +they are a Denial. + +**botocore only refreshes credentials proactively**: +`RefreshableCredentials` checks its expiry whenever its credentials are read +(`get_frozen_credentials()`), +and refreshes them before they expire +(`botocore/credentials.py`, `_refresh` and `_protected_refresh`): + +- within the advisory refresh window (15 minutes before expiry), + one caller refreshes without blocking others; + if the refresh fails, the current credentials keep being used +- within the mandatory refresh window (10 minutes before expiry), + every caller blocks until a refresh succeeds; + if the refresh fails, the error is raised to the caller, + and no request is made + +**botocore never refreshes credentials reactively**: +No error code for an expired token +(`ExpiredToken`, `ExpiredTokenException`, `RequestExpired`, `InvalidClientTokenId`) +appears in botocore's source or its retry configuration (`data/_retry.json`); +the standard retry mode treats none of them as transient or throttling. +A request rejected for an expired token is neither retried nor followed by a refresh. + +The Tenant KMS Client reads the Tenant's Credentials on every attempt +([design.md 8](./design.md#8-tenant-credentials)), +and so is refreshed exactly as a boto3 client would be. + +**Thus, an expired token that reaches AWS KMS will not resolve by retrying**: +It means botocore's proactive refresh is absent or failing: + +- the credentials cannot be refreshed + (for example, `Credentials` constructed from an STS response, without a refresh function) +- the refresh source has been failing through the advisory refresh window +- the local clock disagrees with AWS, + so botocore believes the credentials have not expired + +In each case the next request fails identically, +and, +were it not a Denial, +every request would call Amazon DynamoDB and AWS KMS, +which this design exists to prevent. + +**Recovery is bounded by the Negative TTL**: +Once the cause is resolved +(the refresh source recovers, the clock is corrected, the credentials are replaced), +the Branch Key ID remains Denied until its Negative Cache Entry expires; +at most the Negative TTL. + +**Each Branch Key ID is Denied on its own**: +Expired credentials affect every Branch Key ID the Tenant uses; +as with [10](#10-should-a-notfoundexception-propagate-across-branch-key-ids), +each Branch Key ID is Denied on its own first failure, +rather than propagating the Denial across a Tenant's Branch Key IDs. + +_Note_: `ExpiredTokenException` (HTTP 400) is the error AWS's common errors documentation lists +for an expired security token; +the error code AWS KMS returns for one SHOULD be confirmed during implementation. + +### 5.2 Unrecognized and mis-signed credentials are Denials + +UPDATE: An earlier revision of this document +excluded `UnrecognizedClientException`, +as it SHOULD never happen for credentials issued for the Tenant. +That remains true of correctly wired credentials; +but when it does happen, +it will not resolve by retrying, +which is the property that makes a failure a Denial. + +The author probed AWS KMS (`us-west-2`, `GenerateDataKey`) +through the Tenant KMS Client, +varying one part of the credentials at a time: + +| Credentials | AWS KMS error | +| ----------- | ------------- | +| An access key id that does not exist | `UnrecognizedClientException` | +| Long term keys, with a bogus session token | `UnrecognizedClientException` | +| Temporary credentials, without their session token | `UnrecognizedClientException` | +| Temporary credentials, with a corrupted session token | `UnrecognizedClientException` | +| A real access key id, with the wrong secret | `InvalidSignatureException` | +| Temporary credentials, with the wrong secret | `InvalidSignatureException` | +| Valid temporary credentials (`sts:GetSessionToken`) | success | + +**`UnrecognizedClientException`**: +AWS KMS does not recognize the access key id or the session token: +an access key id that was never issued, or was deleted; +a session token that is bogus, corrupted, or attached to long term keys; +or temporary credentials whose session token was dropped. +AWS also documents this error for session tokens issued by the global STS endpoint +(version 1 tokens, by default) +when used in a Region that must be enabled (opt-in Regions); +the author did not test this. + +**`InvalidSignatureException`**: +The request's signature does not match the credentials' secret access key. +AWS also documents this error for a request whose timestamp is too far from AWS's clock +(clock skew); +the author did not test this. + +**These are usually the application's, not the Tenant's**: +Unlike the KMS Key Denials, +these are most often caused by how the application obtains, +stores, +or passes the Tenant's Credentials, +or by its clock. +Negative caching does not fix them; +it stops every request from calling Amazon DynamoDB and AWS KMS, +and, +as the Cache Guard passes through the error it received +([4.3](#43-a-cache-guard)), +the cause remains visible. + +**Each Branch Key ID is Denied on its own**: +as in [5.1](#51-expired-credentials-are-a-denial). ## 6 Why the Branch Key ID, and not the KMS Key ARN? From 51f79a74993fe168b2d4023a1898d7ed6cbcfd46 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 14:29:34 -0600 Subject: [PATCH 04/10] docs(examples): confirm AWS KMS returns ExpiredTokenException A GenerateDataKey request signed with sts:GetSessionToken credentials one minute after they expired returned HTTP 400 ExpiredTokenException; replace the note in negative_caching.md 5.1 that asked for this to be confirmed. Co-Authored-By: Claude Opus 5.5 --- examples/src/multi_tenant/negative_caching.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/examples/src/multi_tenant/negative_caching.md b/examples/src/multi_tenant/negative_caching.md index 091a61ab9..180d517c9 100644 --- a/examples/src/multi_tenant/negative_caching.md +++ b/examples/src/multi_tenant/negative_caching.md @@ -477,9 +477,14 @@ as with [10](#10-should-a-notfoundexception-propagate-across-branch-key-ids), each Branch Key ID is Denied on its own first failure, rather than propagating the Denial across a Tenant's Branch Key IDs. -_Note_: `ExpiredTokenException` (HTTP 400) is the error AWS's common errors documentation lists -for an expired security token; -the error code AWS KMS returns for one SHOULD be confirmed during implementation. +**AWS KMS returns `ExpiredTokenException`**: +The author signed an AWS KMS `GenerateDataKey` request, +through the Tenant KMS Client, +with temporary credentials from `sts:GetSessionToken` +one minute after they expired; +AWS KMS returned HTTP 400, +`ExpiredTokenException`: +"The security token included in the request is expired". ### 5.2 Unrecognized and mis-signed credentials are Denials From 912ae02e200c2e27b5f2cb49c62efe9c045e8c56 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 14:41:26 -0600 Subject: [PATCH 05/10] ci: skip CI that cannot run on forks; test only Python 3.12 on Ubuntu Forks have no CI_AWS_ROLE_ARN, CodeBuild project, or Slack webhooks, so those jobs could only fail there; and the ubuntu-latest runner no longer provides Python 3.8 or 3.9, whose failures cancelled most of the test matrix (fail-fast). - Skip codebuild_batch, codebuild_tests, and decrypt_oracle unless the repository owner is aws (pull.yml, push.yml, daily_ci.yml), and guard ci_codebuild-tests.yml's own push and schedule triggers. daily_ci.yml's guards previously covered only its schedule. - Allow those skips in pull.yml's alls-green gate. - Skip the Slack notifications on forks. - ci_tests.yaml: Python 3.12 on ubuntu-latest only (3.8-3.10 are end of life); nocmk on 3.12; drop test-upstream-requirements-py311, which pins Python 3.11. fail-fast stays true. Static analysis stays on 3.10, as the pinned linters predate 3.12. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci_codebuild-tests.yml | 3 ++ .github/workflows/ci_tests.yaml | 59 ++++-------------------- .github/workflows/daily_ci.yml | 15 +++--- .github/workflows/issue-notification.yml | 6 ++- .github/workflows/pull.yml | 8 ++++ .github/workflows/push.yml | 6 +++ 6 files changed, 37 insertions(+), 60 deletions(-) diff --git a/.github/workflows/ci_codebuild-tests.yml b/.github/workflows/ci_codebuild-tests.yml index 323a8ca48..bfd212b55 100644 --- a/.github/workflows/ci_codebuild-tests.yml +++ b/.github/workflows/ci_codebuild-tests.yml @@ -19,6 +19,9 @@ permissions: jobs: codebuild-tests: name: AWS CodeBuild CI + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there, + # including this workflow's own push and schedule triggers. + if: github.repository_owner == 'aws' runs-on: ubuntu-latest strategy: matrix: diff --git a/.github/workflows/ci_tests.yaml b/.github/workflows/ci_tests.yaml index 607b5c9e1..760351159 100644 --- a/.github/workflows/ci_tests.yaml +++ b/.github/workflows/ci_tests.yaml @@ -21,21 +21,16 @@ jobs: runs-on: ${{ matrix.os }} strategy: fail-fast: true + # To keep CI cheap, only Python 3.12 on Ubuntu is tested. + # Python 3.8, 3.9, and 3.10 are end of life; + # the ubuntu-latest runner no longer provides 3.8 or 3.9. matrix: os: - ubuntu-latest - - windows-latest - - macos-latest python: - - 3.8 - - 3.9 - - "3.10" - - "3.11" - "3.12" - - 3.x architecture: - x64 - - x86 category: - local - accept @@ -50,45 +45,6 @@ jobs: - "" - -mpl exclude: - # Python <3.11 incompatible with macOS ARM64 runners - # https://github.com/actions/setup-python/issues/948 - - python: 3.7 - os: macos-latest - - python: 3.8 - os: macos-latest - - python: 3.9 - os: macos-latest - - python: 3.10 - os: macos-latest - - # x86 builds are only meaningful for Windows - - os: ubuntu-latest - architecture: x86 - - os: macos-latest - architecture: x86 - - # `cryptography` no longer publishes 32-bit Windows wheels for - # any CPython version, so pip falls back to building from sdist. - # That build is Rust/maturin-based and fails on the hosted - # windows-latest runner because only the x86_64 Rust target is - # installed while the interpreter reports platform `win32` - # (i686), so maturin refuses to use it. Drop x86 on Windows - # entirely; the only path back would be host-side toolchain - # provisioning (i686 Rust target + 32-bit OpenSSL) per cell, - # which isn't worth it for a configuration upstream no longer - # supports. - - os: windows-latest - architecture: x86 - - # MPL is not supported on <3.11 - - python: 3.7 - optional_mpl_dependency: -mpl - - python: 3.8 - optional_mpl_dependency: -mpl - - python: 3.9 - optional_mpl_dependency: -mpl - - python: 3.10 - optional_mpl_dependency: -mpl # mpllocal requires the MPL to be installed - category: mpllocal optional_mpl_dependency: "" @@ -111,23 +67,24 @@ jobs: env: TOXENV: ${{ matrix.category }}${{ matrix.optional_mpl_dependency }} run: tox -- -vv - upstream-py311: + nocmk: + # Python 3.12 only, as above. test-upstream-requirements-py311 is not run: + # it pins Python 3.11 and frozen 3.11 requirements, with no 3.12 equivalent. runs-on: ubuntu-latest strategy: fail-fast: true matrix: category: - nocmk - - test-upstream-requirements-py311 steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v4 with: - python-version: "3.11" + python-version: "3.12" - run: | python -m pip install --upgrade pip pip install --upgrade -r dev_requirements/ci-requirements.txt - name: run test env: - TOXENV: ${{ matrix.category }}${{ matrix.optional_mpl_dependency }} + TOXENV: ${{ matrix.category }} run: tox -- -vv diff --git a/.github/workflows/daily_ci.yml b/.github/workflows/daily_ci.yml index 981c911b0..02684e5ff 100644 --- a/.github/workflows/daily_ci.yml +++ b/.github/workflows/daily_ci.yml @@ -14,20 +14,20 @@ permissions: jobs: codebuild_batch: - # Don't run the cron builds on forks - if: github.event_name != 'schedule' || github.repository_owner == 'aws' + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there on every trigger. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild_batch.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} codebuild_tests: - # Don't run the cron builds on forks - if: github.event_name != 'schedule' || github.repository_owner == 'aws' + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there on every trigger. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild-tests.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} decrypt_oracle: - # Don't run the cron builds on forks - if: github.event_name != 'schedule' || github.repository_owner == 'aws' + # Pins Python 3.9, which the ubuntu-latest runner no longer provides; skip on forks. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_decrypt-oracle.yaml static_analysis: # Don't run the cron builds on forks @@ -55,7 +55,8 @@ jobs: test_vector_handler, tests ] - if: ${{ failure() }} + # Forks have no Slack webhook. + if: ${{ failure() && github.repository_owner == 'aws' }} uses: aws/aws-cryptographic-material-providers-library/.github/workflows/slack-notification.yml@main with: message: "Daily CI failed on `${{ github.repository }}`. View run: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" diff --git a/.github/workflows/issue-notification.yml b/.github/workflows/issue-notification.yml index b784580ed..fe8b92026 100644 --- a/.github/workflows/issue-notification.yml +++ b/.github/workflows/issue-notification.yml @@ -10,7 +10,8 @@ permissions: jobs: notify-issue: - if: github.event_name == 'issues' + # Forks have no Slack webhook. + if: github.repository_owner == 'aws' && github.event_name == 'issues' uses: aws/aws-cryptographic-material-providers-library/.github/workflows/slack-notification.yml@main with: message: "New github issue `${{ github.event.issue.title }}`. Link: ${{ github.event.issue.html_url }}" @@ -18,7 +19,8 @@ jobs: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL_GHI }} notify-comment: - if: github.event_name == 'issue_comment' && !github.event.issue.pull_request + # Forks have no Slack webhook. + if: github.repository_owner == 'aws' && github.event_name == 'issue_comment' && !github.event.issue.pull_request uses: aws/aws-cryptographic-material-providers-library/.github/workflows/slack-notification.yml@main with: message: "New comment on issue `${{ github.event.issue.title }}`. Link: ${{ github.event.comment.html_url }}" diff --git a/.github/workflows/pull.yml b/.github/workflows/pull.yml index 01caa4904..58f339ecf 100644 --- a/.github/workflows/pull.yml +++ b/.github/workflows/pull.yml @@ -16,14 +16,20 @@ permissions: jobs: # Call each workflow with appropriate parameters codebuild_batch: + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild_batch.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} codebuild_tests: + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild-tests.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} decrypt_oracle: + # Pins Python 3.9, which the ubuntu-latest runner no longer provides; skip on forks. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_decrypt-oracle.yaml static_analysis: uses: ./.github/workflows/ci_static-analysis.yaml @@ -48,4 +54,6 @@ jobs: - name: Verify all required jobs passed uses: re-actors/alls-green@release/v1 with: + # Skipped on forks; see each job's condition. + allowed-skips: codebuild_batch, codebuild_tests, decrypt_oracle jobs: ${{ toJSON(needs) }} diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index e2d7d70d6..9da15523e 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -11,16 +11,22 @@ permissions: jobs: # Call each workflow with appropriate parameters codebuild_batch: + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild_batch.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} codebuild_tests: + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild-tests.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} decrypt_oracle: + # Pins Python 3.9, which the ubuntu-latest runner no longer provides; skip on forks. + if: github.repository_owner == 'aws' uses: ./.github/workflows/ci_decrypt-oracle.yaml static_analysis: From 978742999041675ef903b7308ebaea2b3db018d3 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 14:43:33 -0600 Subject: [PATCH 06/10] ci: run the offline examples tests in CI Examples tests only ran in CodeBuild, which forks cannot use, and all were marked examples, so the offline tests could not be selected apart from the integration tests. - Mark the offline examples tests pytest.mark.local, alongside examples; they need no network access or credentials. - Add tox factors exampleslocal (examples/test/legacy/, without the MPL) and mplexampleslocal (examples/test/, with the MPL), each -m local. - Run exampleslocal and mplexampleslocal-mpl in ci_tests.yaml. Locally with tox 4.25.0: exampleslocal 11 passed; mplexampleslocal-mpl 52 passed; the integration tests are deselected. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci_tests.yaml | 9 +++++++++ .../legacy/test_multi_tenant_kms_master_key_provider.py | 3 ++- examples/test/multi_tenant/test_branch_key_cleanup.py | 3 ++- examples/test/multi_tenant/test_kms_transport.py | 3 ++- examples/test/multi_tenant/test_multi_tenant_keyrings.py | 3 ++- examples/test/multi_tenant/test_tenant_kms_client.py | 3 ++- tox.ini | 9 ++++++--- 7 files changed, 25 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci_tests.yaml b/.github/workflows/ci_tests.yaml index 760351159..baf4571ce 100644 --- a/.github/workflows/ci_tests.yaml +++ b/.github/workflows/ci_tests.yaml @@ -35,6 +35,9 @@ jobs: - local - accept - mpllocal + # Offline examples tests (pytest.mark.local); these need no credentials. + - exampleslocal + - mplexampleslocal # These require credentials. # Enable them once we sort how to provide them. # - integ @@ -48,6 +51,12 @@ jobs: # mpllocal requires the MPL to be installed - category: mpllocal optional_mpl_dependency: "" + # mplexampleslocal requires the MPL to be installed + - category: mplexampleslocal + optional_mpl_dependency: "" + # exampleslocal is the legacy (Master Key Provider) examples, which do not use the MPL + - category: exampleslocal + optional_mpl_dependency: -mpl steps: # Support long Dafny filenames (used in MPL and DBESDK repos) - name: Support longpaths diff --git a/examples/test/legacy/test_multi_tenant_kms_master_key_provider.py b/examples/test/legacy/test_multi_tenant_kms_master_key_provider.py index a997de880..97ee42055 100644 --- a/examples/test/legacy/test_multi_tenant_kms_master_key_provider.py +++ b/examples/test/legacy/test_multi_tenant_kms_master_key_provider.py @@ -23,7 +23,8 @@ from ...src.multi_tenant import KmsTransport, TenantKmsClient from ..multi_tenant import fake_kms -pytestmark = [pytest.mark.examples] +# Offline: no network access, so these also run as local tests (tox: exampleslocal, mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] KEY_A = "arn:aws:kms:us-west-2:111122223333:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" MRK_A = "arn:aws:kms:us-west-2:111122223333:key/mrk-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" diff --git a/examples/test/multi_tenant/test_branch_key_cleanup.py b/examples/test/multi_tenant/test_branch_key_cleanup.py index 139d2dbf6..a6fe62c2c 100644 --- a/examples/test/multi_tenant/test_branch_key_cleanup.py +++ b/examples/test/multi_tenant/test_branch_key_cleanup.py @@ -8,7 +8,8 @@ from ..branch_key_cleanup import cleanup_branch_keys, delete_branch_key -pytestmark = [pytest.mark.examples] +# Offline: no network access, so these also run as local tests (tox: exampleslocal, mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] TABLE = "KeyStoreDdbTable" BKID = "bbb9baf1-03e6-4716-a586-6bf29995314b" diff --git a/examples/test/multi_tenant/test_kms_transport.py b/examples/test/multi_tenant/test_kms_transport.py index 04134e70d..881b8bd07 100644 --- a/examples/test/multi_tenant/test_kms_transport.py +++ b/examples/test/multi_tenant/test_kms_transport.py @@ -12,7 +12,8 @@ from ...src.multi_tenant.kms_transport import KmsTransport, default_endpoint_url from . import fake_kms -pytestmark = [pytest.mark.examples] +# Offline: no network access, so these also run as local tests (tox: exampleslocal, mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] KEY_ARN = "arn:aws:kms:us-west-2:111122223333:key/11111111-1111-1111-1111-111111111111" TENANT = Credentials("AKIATENANT", "secret") diff --git a/examples/test/multi_tenant/test_multi_tenant_keyrings.py b/examples/test/multi_tenant/test_multi_tenant_keyrings.py index be75f9fcb..a24bc4b4e 100644 --- a/examples/test/multi_tenant/test_multi_tenant_keyrings.py +++ b/examples/test/multi_tenant/test_multi_tenant_keyrings.py @@ -25,7 +25,8 @@ from ...src.multi_tenant_kms_keyring_example import TenantClientSupplier from . import fake_kms -pytestmark = [pytest.mark.examples] +# Offline: no network access, so these also run as local tests (tox: exampleslocal, mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] KEY_A = "arn:aws:kms:us-west-2:111122223333:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" KEY_B = "arn:aws:kms:us-east-1:444455556666:key/bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" diff --git a/examples/test/multi_tenant/test_tenant_kms_client.py b/examples/test/multi_tenant/test_tenant_kms_client.py index aa9c773b6..b614f44c2 100644 --- a/examples/test/multi_tenant/test_tenant_kms_client.py +++ b/examples/test/multi_tenant/test_tenant_kms_client.py @@ -10,7 +10,8 @@ from ...src.multi_tenant.tenant_kms_client import METHOD_TO_API_MAPPING from . import fake_kms -pytestmark = [pytest.mark.examples] +# Offline: no network access, so these also run as local tests (tox: exampleslocal, mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] KEY_A = "arn:aws:kms:us-west-2:111122223333:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" KEY_A2 = "arn:aws:kms:us-west-2:111122223333:key/a2a2a2a2-a2a2-a2a2-a2a2-a2a2a2a2a2a2" diff --git a/tox.ini b/tox.ini index a6666e3b1..7658340a6 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,7 @@ [tox] envlist = # <3.11: run all non-MPL tests - py{38,39,310}-{local,integ,accept,examples}, + py{38,39,310}-{local,integ,accept,examples,exampleslocal}, # >=3.11: run all tests with MPL installed and without MPL installed # The `-mpl` suffix tells tox to install the MPL. # In the case where the suffix IS NOT appended, @@ -10,14 +10,14 @@ envlist = # this runs tests for the target version WITH the MPL installed. # This does not run any MPL-specific tests; it only runs non-MPL-specific # tests in a test environment that also has the MPL. - py{311,312}-{local,integ,accept,examples}{,-mpl}, + py{311,312}-{local,integ,accept,examples,exampleslocal}{,-mpl}, # >=3.11: Run ONLY the MPL-specific tests. # These must be separate from the above target, since # these require the `-mpl` suffix. # The `mpl` prefix specifies a separate target, # i.e. `mpllocal` instead of `local`. # `mplXXX` contains tests using MPL components. - py{311,312}-mpl{local,examples}-mpl + py{311,312}-mpl{local,examples,exampleslocal}-mpl nocmk, bandit, doc8, readme, docs, {flake8,pylint}{,-tests,-examples}, @@ -94,6 +94,9 @@ commands = examples: {[testenv:base-command]commands} examples/test/legacy/ -m examples # MPL keyring examples require a special IAM role; run these separately under a separate set of permissions mplexamples: {[testenv:base-command]commands} examples/test/ -m examples --ignore examples/test/legacy/ + # Offline examples tests: no network access or credentials, so they can run anywhere, including forks' CI + exampleslocal: {[testenv:base-command]commands} examples/test/legacy/ -m local + mplexampleslocal: {[testenv:base-command]commands} examples/test/ -m local --ignore examples/test/legacy/ all: {[testenv:base-command]commands} test/ examples/test/legacy/ --ignore test/mpl/ mplall: {[testenv:base-command]commands} test/ examples/test/ manual: {[testenv:base-command]commands} From 46359a2eecf874042b1e45e5dd063fae449ba6a2 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 15:03:06 -0600 Subject: [PATCH 07/10] ci: run Hierarchical Keyring examples tests against DynamoDB Local The Hierarchical Keyring paths through the AWS KMS Key Store (CreateKey, VersionKey, and cache misses: DynamoDB plus GenerateDataKeyWithoutPlaintext, ReEncrypt, and Decrypt) had only run against AWS, with permissions the test identity lacks. DynamoDB Local, AWS's emulator, with the fake KMS, runs them with no AWS credentials, so they can run on forks. - Register a ddblocal pytest marker; the new tests are marked only ddblocal, so the examples (CodeBuild) and local runs do not select them. - examples/test/multi_tenant/test_ddblocal_hierarchical.py: CreateKey items and KMS calls; the multi-tenant example end to end, per tenant signing; VersionKey and cleanup of every version; and, without negative caching, every denied attempt calls DynamoDB GetItem and kms:Decrypt. - tox: mplddblocal factor; pass DDB_LOCAL_ENDPOINT through. - ci_tests.yaml: a ddblocal job on Python 3.12 with amazon/dynamodb-local 3.3.2 as a service container (its default command is -inMemory). Locally with podman and tox 4.25.0: mplddblocal-mpl 4 passed; no tables left behind; the local and examples markers select none of them. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci_tests.yaml | 32 +++ .../test_ddblocal_hierarchical.py | 208 ++++++++++++++++++ setup.cfg | 1 + tox.ini | 6 +- 4 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 examples/test/multi_tenant/test_ddblocal_hierarchical.py diff --git a/.github/workflows/ci_tests.yaml b/.github/workflows/ci_tests.yaml index baf4571ce..470ceba9e 100644 --- a/.github/workflows/ci_tests.yaml +++ b/.github/workflows/ci_tests.yaml @@ -76,6 +76,38 @@ jobs: env: TOXENV: ${{ matrix.category }}${{ matrix.optional_mpl_dependency }} run: tox -- -vv + ddblocal: + # Examples tests against DynamoDB Local (pytest -m ddblocal), with the fake KMS; + # no AWS credentials are needed, so this also runs on forks. + runs-on: ubuntu-latest + services: + dynamodb: + # Pinned, so a new DynamoDB Local release cannot change CI unnoticed. + # The image's default command is `-jar DynamoDBLocal.jar -inMemory`. + image: amazon/dynamodb-local:3.3.2 + ports: + - 8000:8000 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v4 + with: + python-version: "3.12" + - run: | + python -m pip install --upgrade pip + pip install --upgrade -r dev_requirements/ci-requirements.txt + - name: Wait for DynamoDB Local + run: | + for attempt in $(seq 1 30); do + curl -s -o /dev/null http://localhost:8000 && exit 0 + sleep 1 + done + echo "DynamoDB Local did not start" + exit 1 + - name: run test + env: + TOXENV: mplddblocal-mpl + DDB_LOCAL_ENDPOINT: http://localhost:8000 + run: tox -- -vv nocmk: # Python 3.12 only, as above. test-upstream-requirements-py311 is not run: # it pins Python 3.11 and frozen 3.11 requirements, with no 3.12 equivalent. diff --git a/examples/test/multi_tenant/test_ddblocal_hierarchical.py b/examples/test/multi_tenant/test_ddblocal_hierarchical.py new file mode 100644 index 000000000..f7a1b8f66 --- /dev/null +++ b/examples/test/multi_tenant/test_ddblocal_hierarchical.py @@ -0,0 +1,208 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Tests of the multi-tenant Hierarchical Keyring against DynamoDB Local, with the fake KMS. + +These need DynamoDB Local, but no AWS credentials or network access to AWS: + + podman run --rm -p 8000:8000 docker.io/amazon/dynamodb-local:3.3.2 + tox -e mplddblocal-mpl + +``DDB_LOCAL_ENDPOINT`` (default ``http://localhost:8000``) points the tests at DynamoDB Local. +They are marked only ``ddblocal``, so the ``examples`` and ``local`` test runs do not select them. +""" +# pylint: disable=redefined-outer-name +import collections +import os +import uuid + +import boto3 +import pytest +from aws_cryptographic_material_providers.keystore import KeyStore +from aws_cryptographic_material_providers.keystore.config import KeyStoreConfig +from aws_cryptographic_material_providers.keystore.models import ( + CreateKeyInput, + CreateKeyStoreInput, + KMSConfigurationKmsKeyArn, + VersionKeyInput, +) +from aws_cryptographic_material_providers.mpl import AwsCryptographicMaterialProviders +from aws_cryptographic_material_providers.mpl.config import MaterialProvidersConfig +from aws_cryptographic_material_providers.mpl.models import CreateAwsKmsHierarchicalKeyringInput +from botocore.credentials import Credentials + +import aws_encryption_sdk +from aws_encryption_sdk import CommitmentPolicy + +from ...src import multi_tenant_hierarchical_keyring_example as example +from ...src.multi_tenant import KmsTransport, TenantKmsClient +from ..branch_key_cleanup import cleanup_branch_keys, delete_branch_key +from . import fake_kms + +pytestmark = [pytest.mark.ddblocal] + +ENDPOINT = os.environ.get("DDB_LOCAL_ENDPOINT", "http://localhost:8000") +REGION = "us-west-2" +KEY_A = "arn:aws:kms:us-west-2:111122223333:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" +KEY_B = "arn:aws:kms:us-west-2:444455556666:key/bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" +TENANT_A = Credentials("AKIATENANTA", "secret-a") +TENANT_B = Credentials("AKIATENANTB", "secret-b") + + +@pytest.fixture +def ddb_env(monkeypatch): + """Point every DynamoDB client, including the examples' own, at DynamoDB Local. + + One set of fake credentials is used throughout: without -sharedDb, + DynamoDB Local keeps a separate database per access key id and region. + """ + monkeypatch.setenv("AWS_ENDPOINT_URL_DYNAMODB", ENDPOINT) + monkeypatch.setenv("AWS_ACCESS_KEY_ID", "ddblocal") + monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "ddblocal") + monkeypatch.setenv("AWS_DEFAULT_REGION", REGION) + monkeypatch.delenv("AWS_SESSION_TOKEN", raising=False) + monkeypatch.delenv("AWS_PROFILE", raising=False) + + +@pytest.fixture +def ddb(ddb_env): # pylint: disable=unused-argument + """Yield a DynamoDB Local client, and every DynamoDB operation made with it.""" + client = boto3.client("dynamodb", region_name=REGION) + calls = collections.Counter() + client.meta.events.register("before-call.dynamodb", lambda **kwargs: calls.update([kwargs["model"].name])) + yield client, calls + client.close() + + +@pytest.fixture +def kms(): + """Yield a transport to the fake KMS, which authorizes each tenant for its own KMS key only.""" + transport = KmsTransport() + fake = fake_kms.install(transport) + fake.add_key(KEY_A, TENANT_A.access_key) + fake.add_key(KEY_B, TENANT_B.access_key) + yield transport, fake + transport.close() + + +def key_store(ddb_client, table, transport, credentials, kms_key): + return KeyStore( + config=KeyStoreConfig( + ddb_client=ddb_client, + ddb_table_name=table, + logical_key_store_name=table, + kms_client=TenantKmsClient(transport, REGION, credentials), + kms_configuration=KMSConfigurationKmsKeyArn(value=kms_key), + ) + ) + + +@pytest.fixture +def table(ddb, kms): + """Create a Key Store table, unique to the test, and delete it afterwards.""" + client, calls = ddb + name = "KeyStore-" + uuid.uuid4().hex[:12] + key_store(client, name, kms[0], TENANT_A, KEY_A).create_key_store(input=CreateKeyStoreInput()) + calls.clear() + yield name + client.delete_table(TableName=name) + + +def item_types(ddb_client, table, branch_key_id): + items = ddb_client.query( + TableName=table, + KeyConditionExpression="#pk = :pk", + ExpressionAttributeNames={"#pk": "branch-key-id"}, + ExpressionAttributeValues={":pk": {"S": branch_key_id}}, + )["Items"] + return sorted(item["type"]["S"].rsplit(":", 1)[0] if item["type"]["S"].startswith("branch:version:") + else item["type"]["S"] for item in items) + + +def test_create_key_writes_active_version_and_beacon_items(ddb, kms, table): + client, _ = ddb + transport, fake = kms + branch_key_id = key_store(client, table, transport, TENANT_A, KEY_A).create_key( + input=CreateKeyInput() + ).branch_key_identifier + assert item_types(client, table, branch_key_id) == ["beacon:ACTIVE", "branch:ACTIVE", "branch:version"] + # The Key Store's KMS operations, through the Tenant KMS Client, signed by the tenant. + assert [(r["operation"], r["access_key"]) for r in fake.requests] == [ + ("GenerateDataKeyWithoutPlaintext", "AKIATENANTA"), + ("ReEncrypt", "AKIATENANTA"), + ("GenerateDataKeyWithoutPlaintext", "AKIATENANTA"), + ] + assert delete_branch_key(client, table, branch_key_id) is True + + +def test_multi_tenant_example_end_to_end(monkeypatch, ddb, kms, table): + client, _ = ddb + transport, fake = kms + monkeypatch.setattr(example, "KmsTransport", lambda: transport) + branch_key_ids = {} + try: + example.encrypt_and_decrypt_with_keyring( + key_store_table_name=table, + logical_key_store_name=table, + tenant_kms_key_ids={"TenantA": KEY_A, "TenantB": KEY_B}, + tenant_credentials={"TenantA": TENANT_A, "TenantB": TENANT_B}, + branch_key_ids=branch_key_ids, + ) + finally: + assert not cleanup_branch_keys(client, table, branch_key_ids) + + assert set(branch_key_ids) == {"TenantA", "TenantB"} + # Per tenant: create its Branch Key, then a cache miss on encrypt (active) and on decrypt (version); + # every request signed with that tenant's credentials. + per_tenant = [ + "GenerateDataKeyWithoutPlaintext", "ReEncrypt", "GenerateDataKeyWithoutPlaintext", "Decrypt", "Decrypt", + ] + assert [(r["operation"], r["access_key"]) for r in fake.requests] == ( + [(operation, "AKIATENANTA") for operation in per_tenant] + + [(operation, "AKIATENANTB") for operation in per_tenant] + ) + for branch_key_id in branch_key_ids.values(): + assert item_types(client, table, branch_key_id) == [] + + +def test_version_key_and_cleanup_of_every_version(ddb, kms, table): + client, _ = ddb + transport, _ = kms + store = key_store(client, table, transport, TENANT_A, KEY_A) + branch_key_id = store.create_key(input=CreateKeyInput()).branch_key_identifier + store.version_key(input=VersionKeyInput(branch_key_identifier=branch_key_id)) + assert item_types(client, table, branch_key_id) == [ + "beacon:ACTIVE", "branch:ACTIVE", "branch:version", "branch:version" + ] + assert delete_branch_key(client, table, branch_key_id) is True + assert item_types(client, table, branch_key_id) == [] + + +def test_without_negative_caching_every_denied_attempt_calls_dynamodb_and_kms(ddb, kms, table): + """Record the behavior negative caching exists to prevent (negative_caching.md, Background).""" + client, calls = ddb + transport, fake = kms + branch_key_id = key_store(client, table, transport, TENANT_A, KEY_A).create_key( + input=CreateKeyInput() + ).branch_key_identifier + fake.requests.clear() + calls.clear() + try: + # Tenant B's credentials, tenant A's Branch Key: the fake KMS denies every Decrypt. + mat_prov = AwsCryptographicMaterialProviders(config=MaterialProvidersConfig()) + keyring = mat_prov.create_aws_kms_hierarchical_keyring( + input=CreateAwsKmsHierarchicalKeyringInput( + key_store=key_store(client, table, transport, TENANT_B, KEY_A), + branch_key_id=branch_key_id, + ttl_seconds=600, + ) + ) + esdk = aws_encryption_sdk.EncryptionSDKClient( + commitment_policy=CommitmentPolicy.REQUIRE_ENCRYPT_REQUIRE_DECRYPT + ) + for _ in range(3): + with pytest.raises(Exception, match="AccessDeniedException"): # pylint: disable=broad-except + esdk.encrypt(source=b"tenant data", keyring=keyring) + assert calls["GetItem"] == 3 + assert [(r["operation"], r["access_key"]) for r in fake.requests] == [("Decrypt", "AKIATENANTB")] * 3 + finally: + assert delete_branch_key(client, table, branch_key_id) is True diff --git a/setup.cfg b/setup.cfg index 1f83b2654..1002992d4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -20,6 +20,7 @@ markers = integ: mark a test as an integration test (requires network access) accept: mark a test as an acceptance test (requires network access) examples: mark a test as an examples test (requires network access) + ddblocal: mark a test as needing DynamoDB Local (no AWS credentials or network access to AWS) # Flake8 Configuration [flake8] diff --git a/tox.ini b/tox.ini index 7658340a6..23a6147cf 100644 --- a/tox.ini +++ b/tox.ini @@ -17,7 +17,7 @@ envlist = # The `mpl` prefix specifies a separate target, # i.e. `mpllocal` instead of `local`. # `mplXXX` contains tests using MPL components. - py{311,312}-mpl{local,examples,exampleslocal}-mpl + py{311,312}-mpl{local,examples,exampleslocal,ddblocal}-mpl nocmk, bandit, doc8, readme, docs, {flake8,pylint}{,-tests,-examples}, @@ -80,6 +80,8 @@ passenv = AWS_PROFILE # Pass through custom pip config file settings PIP_CONFIG_FILE + # Where DynamoDB Local listens, for the ddblocal tests + DDB_LOCAL_ENDPOINT sitepackages = False deps = -rdev_requirements/test-requirements.txt @@ -97,6 +99,8 @@ commands = # Offline examples tests: no network access or credentials, so they can run anywhere, including forks' CI exampleslocal: {[testenv:base-command]commands} examples/test/legacy/ -m local mplexampleslocal: {[testenv:base-command]commands} examples/test/ -m local --ignore examples/test/legacy/ + # Examples tests against DynamoDB Local, with the fake KMS; see examples/test/multi_tenant/test_ddblocal_hierarchical.py + mplddblocal: {[testenv:base-command]commands} examples/test/ -m ddblocal --ignore examples/test/legacy/ all: {[testenv:base-command]commands} test/ examples/test/legacy/ --ignore test/mpl/ mplall: {[testenv:base-command]commands} test/ examples/test/ manual: {[testenv:base-command]commands} From ded3702a68c5cb6b74925c8c2af13c3b88b4f0ab Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 15:13:59 -0600 Subject: [PATCH 08/10] ci: only run the Pull Request Workflow on the fork's PRs - Revert daily_ci.yml to examples/credentials. Its pull_request trigger only fires for a PR that edits daily_ci.yml, so editing it re-ran Daily CI on every push; and on a fork's schedule, its original guards already skip every job. - ci_codebuild-tests.yml: drop the push and schedule triggers, keeping workflow_call; it is only called by pull.yml, push.yml, and daily_ci.yml. Temporary, for development on a fork. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci_codebuild-tests.yml | 10 +++------- .github/workflows/daily_ci.yml | 15 +++++++-------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci_codebuild-tests.yml b/.github/workflows/ci_codebuild-tests.yml index bfd212b55..f42cc5967 100644 --- a/.github/workflows/ci_codebuild-tests.yml +++ b/.github/workflows/ci_codebuild-tests.yml @@ -1,11 +1,8 @@ name: AWS CodeBuild CI on: - push: - # Run once a day - schedule: - - cron: "0 0 * * *" - # Add workflow_call trigger + # Only called by pull.yml, push.yml, and daily_ci.yml. + # Its own push and schedule triggers are removed, temporarily, for development on a fork. workflow_call: # Define any secrets that need to be passed from the caller secrets: @@ -19,8 +16,7 @@ permissions: jobs: codebuild-tests: name: AWS CodeBuild CI - # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there, - # including this workflow's own push and schedule triggers. + # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there. if: github.repository_owner == 'aws' runs-on: ubuntu-latest strategy: diff --git a/.github/workflows/daily_ci.yml b/.github/workflows/daily_ci.yml index 02684e5ff..981c911b0 100644 --- a/.github/workflows/daily_ci.yml +++ b/.github/workflows/daily_ci.yml @@ -14,20 +14,20 @@ permissions: jobs: codebuild_batch: - # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there on every trigger. - if: github.repository_owner == 'aws' + # Don't run the cron builds on forks + if: github.event_name != 'schedule' || github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild_batch.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} codebuild_tests: - # Forks have no CI_AWS_ROLE_ARN, nor the CodeBuild project; skip there on every trigger. - if: github.repository_owner == 'aws' + # Don't run the cron builds on forks + if: github.event_name != 'schedule' || github.repository_owner == 'aws' uses: ./.github/workflows/ci_codebuild-tests.yml secrets: CI_AWS_ROLE_ARN: ${{ secrets.CI_AWS_ROLE_ARN }} decrypt_oracle: - # Pins Python 3.9, which the ubuntu-latest runner no longer provides; skip on forks. - if: github.repository_owner == 'aws' + # Don't run the cron builds on forks + if: github.event_name != 'schedule' || github.repository_owner == 'aws' uses: ./.github/workflows/ci_decrypt-oracle.yaml static_analysis: # Don't run the cron builds on forks @@ -55,8 +55,7 @@ jobs: test_vector_handler, tests ] - # Forks have no Slack webhook. - if: ${{ failure() && github.repository_owner == 'aws' }} + if: ${{ failure() }} uses: aws/aws-cryptographic-material-providers-library/.github/workflows/slack-notification.yml@main with: message: "Daily CI failed on `${{ github.repository }}`. View run: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" From a0179837c5d49feddc9a874678d248677504cd98 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Thu, 5 Nov 2026 16:22:48 -0600 Subject: [PATCH 09/10] feat(examples): Cache Guard and Negative Cache Entries for negative caching Negative caching (negative_caching.md): while AWS KMS has Denied a Branch Key ID, its Tenant Hierarchical Keyring fails fast, without calling Amazon DynamoDB or AWS KMS, for a Negative TTL. negative_cache.py (no MPL import): - DENIAL_ERROR_CODES, is_denial, and error_code (Design Question 5). - BranchKeyNegativeCache: the Negative Cache Entry of one Branch Key ID, with record, get, and clear. One lock is held across checking expiry and discarding, so get() never erases a Denial recorded concurrently. Rejects an empty Branch Key ID and a non-positive or NaN Negative TTL. - record() logs a warning naming the Branch Key ID, the KMS error code, the Negative TTL, and the error's text. cache_guard.py: - CacheGuard, an ICryptographicMaterialsCache wrapping the cache shared by every Tenant. While Denied, it raises OpaqueWithTextError(obj=error, obj_message=str(error)) before consulting the shared cache; the Hierarchical Keyring then fails without calling the AWS KMS Key Store. Otherwise it delegates. - OpaqueWithTextError keeps the original error, the same object, and its code through the MPL's conversion to and from Dafny, for all eight Denial codes; a ComAmazonawsKms error loses both for the KMS errors the MPL models. A new wrapper per raise stops the error's traceback growing. Tests (pytest.mark.local), in examples/test/multi_tenant/: classification, expiry, replacement, logging, validation; a deterministic expiry race test; thread safety; the guard's delegation and refusal of cached hits; and end to end, a real Hierarchical Keyring on the Cache Guard, seeded with the specification's cache identifiers, failing encrypt and decrypt with zero DynamoDB and KMS calls while Denied. FakeClock and kms_error are shared in helpers.py. negative_caching.md 4.3 and 5.2 record the error wrapping, and that the first Denial (a real cache miss, confirmed against DynamoDB Local) loses the code of the KMS errors the MPL models, so the code is logged. Co-Authored-By: Claude Opus 5.5 --- examples/src/multi_tenant/cache_guard.py | 108 +++++ examples/src/multi_tenant/negative_cache.py | 188 ++++++++ examples/src/multi_tenant/negative_caching.md | 103 ++++- examples/test/multi_tenant/helpers.py | 24 + .../test/multi_tenant/test_cache_guard.py | 421 ++++++++++++++++++ .../test/multi_tenant/test_negative_cache.py | 270 +++++++++++ 6 files changed, 1097 insertions(+), 17 deletions(-) create mode 100644 examples/src/multi_tenant/cache_guard.py create mode 100644 examples/src/multi_tenant/negative_cache.py create mode 100644 examples/test/multi_tenant/helpers.py create mode 100644 examples/test/multi_tenant/test_cache_guard.py create mode 100644 examples/test/multi_tenant/test_negative_cache.py diff --git a/examples/src/multi_tenant/cache_guard.py b/examples/src/multi_tenant/cache_guard.py new file mode 100644 index 000000000..ba2c0ca8a --- /dev/null +++ b/examples/src/multi_tenant/cache_guard.py @@ -0,0 +1,108 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""The Cache Guard: fails a Hierarchical Keyring's cache lookups for a Denied Branch Key ID. + +See negative_caching.md in this directory, Design Question 4.3. + +A ``CacheGuard`` is a Cryptographic Materials Cache, one per Tenant Hierarchical Keyring, +wrapping the cache shared by every Tenant. It is passed to the keyring as ``CacheTypeShared``. + +While its ``BranchKeyNegativeCache`` has an honored Negative Cache Entry, +``get_cache_entry`` raises the error AWS KMS returned, the same object, +wrapped so that it survives the MPL's conversion to and from Dafny (negative_caching.md 4.3), +before consulting the shared cache; so a cached hit is not returned (Requirement 3). +The MPL treats any error other than ``EntryDoesNotExist`` as a failure, +and the Hierarchical Keyring fails without calling Amazon DynamoDB or AWS KMS (Requirement 1). +Otherwise, every operation is delegated to the shared cache. + +The Cache Guard is a lightweight object: it holds a reference to the shared cache +and a reference to its Branch Key ID's negative cache. + +This module imports the MPL; ``negative_cache.py`` does not. +""" +from aws_cryptographic_material_providers.mpl.errors import OpaqueWithTextError +from aws_cryptographic_material_providers.mpl.references import ICryptographicMaterialsCache + +from .negative_cache import BranchKeyNegativeCache + + +class CacheGuard(ICryptographicMaterialsCache): + """A Cryptographic Materials Cache that fails lookups for a Denied Branch Key ID. + + :param delegate: The cache shared by every Tenant; an MPL cache + (from ``create_cryptographic_materials_cache``) or any ``ICryptographicMaterialsCache`` + :type delegate: aws_cryptographic_material_providers.mpl.references.ICryptographicMaterialsCache + :param negative_cache: The negative cache of this keyring's Branch Key ID + :type negative_cache: BranchKeyNegativeCache + :raises TypeError: if ``delegate`` is not an ``ICryptographicMaterialsCache``, + or ``negative_cache`` is not a ``BranchKeyNegativeCache`` + """ + + def __init__(self, delegate, negative_cache): # pylint: disable=super-init-not-called + """Validates and stores the shared cache and the negative cache.""" + if not isinstance(delegate, ICryptographicMaterialsCache): + raise TypeError("delegate must be an ICryptographicMaterialsCache") + if not isinstance(negative_cache, BranchKeyNegativeCache): + raise TypeError("negative_cache must be a BranchKeyNegativeCache") + self._delegate = delegate + self._negative_cache = negative_cache + + @property + def negative_cache(self): + """The negative cache of this keyring's Branch Key ID. + + :rtype: BranchKeyNegativeCache + """ + return self._negative_cache + + def get_cache_entry(self, param): + """Raises the Denial's error, wrapped, while it is honored; otherwise delegates. + + :param param: The lookup + :type param: aws_cryptographic_material_providers.mpl.models.GetCacheEntryInput + :rtype: aws_cryptographic_material_providers.mpl.models.GetCacheEntryOutput + :raises aws_cryptographic_material_providers.mpl.errors.OpaqueWithTextError: + while Denied; ``obj`` is the Negative Cache Entry's error, the same object, + and ``obj_message`` is its text, which names the AWS KMS error code + :raises aws_cryptographic_material_providers.mpl.errors.EntryDoesNotExist: from the shared cache, on a miss + """ + # Checked before the shared cache: a Denied Branch Key ID's cached materials MUST NOT be used, + # and a miss in a storm tracking cache would block concurrent callers (negative_caching.md 4.3). + entry = self._negative_cache.get() + if entry is not None: + # OpaqueWithTextError, because it survives the MPL's conversion to and from Dafny intact: + # Dafny's opaque errors carry the Python object itself, so `obj` is the error AWS KMS returned, + # the same object; and `obj_message` keeps its text, which names the KMS error code. + # A ComAmazonawsKms error would lose both: Dafny keeps only its data, and for the KMS errors + # the MPL models (DisabledException, KMSInvalidStateException, NotFoundException, + # IncorrectKeyException) only their message, without the code (negative_caching.md 4.3). + # A new wrapper per raise also keeps the error's own traceback from growing on every raise. + raise OpaqueWithTextError(obj=entry.error, obj_message=str(entry.error)) + return self._delegate.get_cache_entry(param) + + def put_cache_entry(self, param): + """Delegates to the shared cache. + + :param param: The entry + :type param: aws_cryptographic_material_providers.mpl.models.PutCacheEntryInput + """ + return self._delegate.put_cache_entry(param) + + def update_usage_metadata(self, param): + """Delegates to the shared cache. + + :param param: The usage + :type param: aws_cryptographic_material_providers.mpl.models.UpdateUsageMetadataInput + """ + return self._delegate.update_usage_metadata(param) + + def delete_cache_entry(self, param): + """Delegates to the shared cache. + + :param param: The entry to delete + :type param: aws_cryptographic_material_providers.mpl.models.DeleteCacheEntryInput + """ + return self._delegate.delete_cache_entry(param) + + +__all__ = ("CacheGuard", "BranchKeyNegativeCache") diff --git a/examples/src/multi_tenant/negative_cache.py b/examples/src/multi_tenant/negative_cache.py new file mode 100644 index 000000000..48222e60b --- /dev/null +++ b/examples/src/multi_tenant/negative_cache.py @@ -0,0 +1,188 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Negative Cache Entries: records that AWS KMS Denied a Branch Key ID, honored for a Negative TTL. + +See negative_caching.md in this directory for the options considered and the decisions made. + +A ``BranchKeyNegativeCache`` holds the Negative Cache Entry, if any, of exactly one Branch Key ID. +One is shared by the Tenant KMS Client that records a Denial +and the Cache Guard that enforces it, for the same Tenant Hierarchical Keyring. + +This module MUST NOT import the AWS Cryptographic Material Providers Library (MPL), +as the Tenant KMS Client, which records Denials, does not. +""" +import collections +import logging +import math +import threading +import time + +import botocore.exceptions + +_LOGGER = logging.getLogger(__name__) + +#: Default Negative TTL, in seconds (negative_caching.md, Definitions). +DEFAULT_NEGATIVE_TTL_SECONDS = 300 + +#: AWS KMS error codes that are Denials (negative_caching.md, Design Question 5). +DENIAL_ERROR_CODES = frozenset( + [ + # The Tenant's KMS Key, and its Key Policy or Grants + "AccessDeniedException", + "DisabledException", + "KMSInvalidStateException", + "NotFoundException", + "IncorrectKeyException", + # The Tenant's Credentials (negative_caching.md 5.1, 5.2) + "ExpiredTokenException", + "UnrecognizedClientException", + "InvalidSignatureException", + ] +) + + +class NegativeCacheEntry( + collections.namedtuple("NegativeCacheEntry", ("branch_key_id", "error", "denied_at", "expires_at")) +): + """A record that a Branch Key ID was Denied. + + Times are from the ``BranchKeyNegativeCache``'s clock (by default ``time.monotonic``), in seconds. + + :param str branch_key_id: The Denied Branch Key ID + :param Exception error: The error AWS KMS returned, passed through as is by the Cache Guard + :param float denied_at: When the Denial was recorded + :param float expires_at: When the entry stops being honored; ``denied_at`` plus the Negative TTL + """ + + def is_honored(self, now): + """Returns True if the entry is honored at ``now``; that is, ``now < expires_at``. + + :param float now: A time from the same clock as ``denied_at`` + :rtype: bool + """ + return now < self.expires_at + + +def error_code(error): + """Returns the AWS KMS error code of ``error``, or its type name if it is not a ``ClientError``. + + :param Exception error: Any error + :rtype: str + """ + if isinstance(error, botocore.exceptions.ClientError): + return error.response.get("Error", {}).get("Code") or type(error).__name__ + return type(error).__name__ + + +def is_denial(error): + """Returns True if ``error`` is an AWS KMS error that is a Denial. + + Only a ``botocore.exceptions.ClientError`` whose ``response["Error"]["Code"]`` + is in ``DENIAL_ERROR_CODES`` is a Denial; every other error, including transport failures, is not. + + :param Exception error: Any error + :rtype: bool + """ + if not isinstance(error, botocore.exceptions.ClientError): + return False + return error_code(error) in DENIAL_ERROR_CODES + + +class BranchKeyNegativeCache(object): + """The Negative Cache Entry, if any, of one Branch Key ID. + + Thread safe: Denials are recorded by a Tenant KMS Client and read by a Cache Guard, on any thread. + + :param str branch_key_id: The Branch Key ID this negative cache is for + :param float negative_ttl: Seconds a Negative Cache Entry is honored; MUST be greater than zero + :param clock: Returns the current time in seconds; for tests (default ``time.monotonic``) + :raises ValueError: if ``branch_key_id`` is empty, or ``negative_ttl`` is not greater than zero + """ + + def __init__(self, branch_key_id, negative_ttl=DEFAULT_NEGATIVE_TTL_SECONDS, clock=time.monotonic): + """Validates the Branch Key ID and Negative TTL.""" + if not isinstance(branch_key_id, str) or not branch_key_id: + raise ValueError("branch_key_id must be a non-empty string") + # NaN is rejected explicitly: every comparison with NaN is False, so `negative_ttl <= 0` would accept it. + if ( + isinstance(negative_ttl, bool) + or not isinstance(negative_ttl, (int, float)) + or math.isnan(negative_ttl) + or negative_ttl <= 0 + ): + raise ValueError("negative_ttl must be a number greater than zero") + self._branch_key_id = branch_key_id + self._negative_ttl = negative_ttl + self._clock = clock + # Guards _entry. get() holds it across checking expiry and discarding, + # so it never discards a Denial recorded after the entry it found expired. + self._lock = threading.Lock() + self._entry = None + + @property + def branch_key_id(self): + """The Branch Key ID this negative cache is for. + + :rtype: str + """ + return self._branch_key_id + + @property + def negative_ttl(self): + """Seconds a Negative Cache Entry is honored. + + :rtype: float + """ + return self._negative_ttl + + def record(self, error): + """Records a Denial, replacing any existing Negative Cache Entry. + + The caller decides whether ``error`` is a Denial (see ``is_denial``); this does not. + + Logs a warning naming the Branch Key ID, the AWS KMS error code, the Negative TTL, + and the error's text, AWS KMS's message included; which MAY name account ids or role ARNs, + neither of which is secret. + The Cache Guard's error does not name the code in its ``str()`` (negative_caching.md 4.3), + so this is where an operator sees it. + + :param Exception error: The error AWS KMS returned + :returns: The new Negative Cache Entry + :rtype: NegativeCacheEntry + """ + with self._lock: + now = self._clock() + entry = NegativeCacheEntry( + branch_key_id=self._branch_key_id, error=error, denied_at=now, expires_at=now + self._negative_ttl + ) + self._entry = entry + _LOGGER.warning( + "AWS KMS Denied Branch Key ID %s (%s); failing its requests for %s seconds: %s", + self._branch_key_id, + error_code(error), + self._negative_ttl, + error, + ) + return entry + + def get(self): + """Returns the honored Negative Cache Entry, if any. + + An entry that is no longer honored is discarded. + + :returns: The honored entry, or None + :rtype: NegativeCacheEntry or None + """ + with self._lock: + entry = self._entry + if entry is None: + return None + if entry.is_honored(self._clock()): + return entry + self._entry = None + return None + + def clear(self): + """Discards the Negative Cache Entry, if any.""" + with self._lock: + self._entry = None diff --git a/examples/src/multi_tenant/negative_caching.md b/examples/src/multi_tenant/negative_caching.md index 180d517c9..57e834679 100644 --- a/examples/src/multi_tenant/negative_caching.md +++ b/examples/src/multi_tenant/negative_caching.md @@ -365,19 +365,81 @@ one per Tenant Hierarchical Keyring costs nothing of note. **The Cache Guard passes through the error it received**: The Cache Guard fails `GetCacheEntry` -by raising the error the Tenant KMS Client received from AWS KMS, -as is, -from the Negative Cache Entry. - -_Note_: The MPL maps an error raised by a native cache -with `_smithy_error_to_dafny_error`; -an error that is not an MPL error, -such as botocore's `ClientError`, -becomes an opaque error carrying it. +with the error the Tenant KMS Client received from AWS KMS, +the same object, +from the Negative Cache Entry; +wrapped, +so it survives the MPL's conversion to and from Dafny: +it raises `OpaqueWithTextError(obj=error, obj_message=str(error))`. + +UPDATE: An earlier revision raised the error itself. +The MPL converts an error raised by a native cache +with `_smithy_error_to_dafny_error`, +and converts it back with `_deserialize_error` +before the ESDK-Python raises it. +Raised as is, +a botocore `ClientError` became an `OpaqueError`, +whose `str()` is empty; +the code was only reachable inside it. + +The author compared the wrappers for all eight Denial codes, +with a realistic AWS KMS message that does not contain its code, +through a real Hierarchical Keyring on encrypt and decrypt: + +| Wrapper | The code survives | The original error, the same object | The code in the outermost `str()` | +| ------- | ----------------- | ----------------------------------- | --------------------------------- | +| `ComAmazonawsKms(message=error)` | 4 of 8 | never | 4 of 8, on encrypt only | +| `OpaqueWithTextError(obj=error, obj_message=str(error))` | 8 of 8 | 8 of 8 | never | + +**A `ComAmazonawsKms` error loses the code of the KMS errors the MPL models**: +It is converted by the MPL's AWS KMS shim, +as a real AWS KMS failure would be; +the KMS errors the MPL models +(`DisabledException`, `KMSInvalidStateException`, `NotFoundException`, `IncorrectKeyException`) +become typed errors that keep only their message, +and the code is lost. +Dafny keeps only the error's data, +so the original object is lost for every code. + +**An `OpaqueWithTextError` keeps both**: +Dafny's opaque errors carry the Python object itself; +`obj` is the original error, the same object, +and `obj_message` its text, which names the code. + +**But not in `str()`**: +The MPL generates `OpaqueWithTextError` with an empty message, +so its `str()` is always empty; +its `repr()`, and its `obj_message`, name the code. +On decrypt, +the ESDK-Python raises its own error +("No Configured KMS Key was able to decrypt the Data Key"), +with the Cache Guard's error within the MPL's `CollectionOfErrors`, +whichever wrapper is raised. Thus, -the first Denial surfaces wrapped by the AWS KMS Key Store's error, -and later failures as an opaque error carrying the original; -the original error is the same in both. +the Branch Key ID, the code, and the error's text are logged when a Denial is recorded, +which is where an operator sees them. +AWS KMS's message MAY name account ids or role ARNs; +neither is secret. + +**A new wrapper per raise**: +Re-raising one exception object appends each raise's frames to its traceback; +it would grow without bound, +keeping every frame, +and its locals, +alive for as long as the Denial is honored. +A new wrapper per raise does not. + +**The first Denial loses more**: +The first Denial is a real cache miss: +AWS KMS's error passes through the MPL's AWS KMS shim, +and surfaces as the AWS KMS Key Store's error. +The author confirmed, against DynamoDB Local with the fake KMS, +that for the four KMS errors the MPL models +no `str()`, no `repr()`, and no object in the error names the code; +and that the original error is never kept. +Thus, +the Cache Guard's failures carry more than the first Denial's; +the code of the first Denial is in the log. **Encrypt and decrypt are treated the same**: A Denial from `GetActiveBranchKey` (encrypt) @@ -533,11 +595,11 @@ stores, or passes the Tenant's Credentials, or by its clock. Negative caching does not fix them; -it stops every request from calling Amazon DynamoDB and AWS KMS, -and, -as the Cache Guard passes through the error it received -([4.3](#43-a-cache-guard)), -the cause remains visible. +it stops every request from calling Amazon DynamoDB and AWS KMS. +The cause remains available: +the Branch Key ID and the code are logged when the Denial is recorded, +and the Cache Guard's error carries the original error, the same object +([4.3](#43-a-cache-guard)). **Each Branch Key ID is Denied on its own**: as in [5.1](#51-expired-credentials-are-a-denial). @@ -779,6 +841,10 @@ each Branch Key ID is Denied on its own. UPDATE: The Cache Guard passes through the error it received, as is ([4.3](#43-a-cache-guard)). + UPDATE: As is, but wrapped in an `OpaqueWithTextError`, + so the original error, the same object, and its code survive the MPL's conversion to and from Dafny; + the code is also logged when the Denial is recorded ([4.3](#43-a-cache-guard)). + 3. **Thread safety**: Negative Cache Entries are written by Tenant KMS Clients and read by Cache Guards, @@ -805,3 +871,6 @@ each Branch Key ID is Denied on its own. this is behavior, not a documented interface. - Negative caching requires the Tenant KMS Client ([3.3](#33-the-tenant-kms-client)). - Every Tenant Hierarchical Keyring has a static Branch Key ID ([7.2](#72-a-tenant-keyring-factory)). +- The Cache Guard's error depends on how the MPL converts native cache errors to and from Dafny, + which is generated code, not a documented interface; + its outermost `str()` does not name the code, and an operator relies on the log ([4.3](#43-a-cache-guard)). diff --git a/examples/test/multi_tenant/helpers.py b/examples/test/multi_tenant/helpers.py new file mode 100644 index 000000000..485d07fc7 --- /dev/null +++ b/examples/test/multi_tenant/helpers.py @@ -0,0 +1,24 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Test helpers shared by the negative caching tests. + +This module MUST NOT import the MPL; the negative cache tests do not need it. +""" +import botocore.exceptions + + +class FakeClock: # pylint: disable=too-few-public-methods + """A clock the test advances by hand.""" + + def __init__(self, now=1000.0): + """Start at ``now``.""" + self.now = now + + def __call__(self): + """Return the current fake time.""" + return self.now + + +def kms_error(code="AccessDeniedException", operation="Decrypt"): + """Return the botocore ``ClientError`` AWS KMS would raise with ``code``.""" + return botocore.exceptions.ClientError({"Error": {"Code": code, "Message": "test " + code}}, operation) diff --git a/examples/test/multi_tenant/test_cache_guard.py b/examples/test/multi_tenant/test_cache_guard.py new file mode 100644 index 000000000..832287324 --- /dev/null +++ b/examples/test/multi_tenant/test_cache_guard.py @@ -0,0 +1,421 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Offline tests for the Cache Guard (negative_caching.md, Design Question 4.3). + +The end-to-end tests seed the shared cache under the cache identifiers the specification defines +(aws-kms-hierarchical-keyring.md, Appendix A), so a real Hierarchical Keyring encrypts and decrypts +from cache hits; any call to Amazon DynamoDB or AWS KMS is counted, and asserted to be zero. +""" +# pylint: disable=redefined-outer-name +import hashlib +import os +import threading +import time +import uuid + +import boto3 +import pytest +from aws_cryptographic_material_providers.keystore import KeyStore +from aws_cryptographic_material_providers.keystore.config import KeyStoreConfig +from aws_cryptographic_material_providers.keystore.models import BranchKeyMaterials, KMSConfigurationKmsKeyArn +from aws_cryptographic_material_providers.mpl import AwsCryptographicMaterialProviders +from aws_cryptographic_material_providers.mpl.config import MaterialProvidersConfig +from aws_cryptographic_material_providers.mpl.errors import EntryDoesNotExist, OpaqueWithTextError +from aws_cryptographic_material_providers.mpl.models import ( + CacheTypeDefault, + CacheTypeShared, + CreateAwsKmsHierarchicalKeyringInput, + CreateCryptographicMaterialsCacheInput, + DefaultCache, + DeleteCacheEntryInput, + GetCacheEntryInput, + MaterialsBranchKey, + PutCacheEntryInput, + UpdateUsageMetadataInput, +) +from aws_cryptographic_material_providers.mpl.references import ICryptographicMaterialsCache +from botocore.credentials import Credentials +from botocore.stub import Stubber + +import aws_encryption_sdk +from aws_encryption_sdk import CommitmentPolicy + +from ...src.multi_tenant import KmsTransport, TenantKmsClient +from ...src.multi_tenant.cache_guard import CacheGuard +from ...src.multi_tenant.negative_cache import BranchKeyNegativeCache +from . import fake_kms +from .helpers import FakeClock, kms_error + +# Offline: no network access, so these also run as local tests (tox: mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] + +KMS_KEY = "arn:aws:kms:us-west-2:111122223333:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" +TABLE = "KeyStoreTable" +TENANT = Credentials("AKIATENANTA", "secret-a") + + +class SpyCache(ICryptographicMaterialsCache): + """A native cache that records every call, and misses unless given an output.""" + + def __init__(self, output=None): + """Return ``output`` from get_cache_entry, or miss.""" + self.output = output + self.calls = [] + + def get_cache_entry(self, param): + """Record, then hit or miss.""" + self.calls.append(("get", param)) + if self.output is None: + raise EntryDoesNotExist(message="miss") + return self.output + + def put_cache_entry(self, param): + """Record.""" + self.calls.append(("put", param)) + + def update_usage_metadata(self, param): + """Record.""" + self.calls.append(("update", param)) + + def delete_cache_entry(self, param): + """Record.""" + self.calls.append(("delete", param)) + + +def branch_key_materials(branch_key_id, version): + return BranchKeyMaterials( + branch_key_identifier=branch_key_id, + branch_key_version=version, + encryption_context={}, + branch_key=os.urandom(32), + ) + + +def put_input(identifier, materials, now=None): + now = int(time.time()) if now is None else now + return PutCacheEntryInput( + identifier=identifier, + materials=MaterialsBranchKey(value=materials), + creation_time=now, + expiry_time=now + 600, + messages_used=0, + bytes_used=0, + ) + + +def denial_within(raised): + """Return the Cache Guard's OpaqueWithTextError within ``raised``, or None. + + On encrypt, the ESDK raises it; on decrypt, it is within the MPL's CollectionOfErrors, + within the ESDK's AWSEncryptionSDKClientError (negative_caching.md 4.3). + """ + pending, seen = [raised], set() + while pending: + error = pending.pop() + if id(error) in seen: + continue + seen.add(id(error)) + if isinstance(error, OpaqueWithTextError): + return error + for attribute in ("list", "args"): + value = getattr(error, attribute, None) or () + pending.extend(child for child in value if isinstance(child, BaseException)) + return None + + +@pytest.fixture +def mat_prov(): + return AwsCryptographicMaterialProviders(config=MaterialProvidersConfig()) + + +@pytest.fixture +def shared_cache(mat_prov): + return mat_prov.create_cryptographic_materials_cache( + input=CreateCryptographicMaterialsCacheInput(cache=CacheTypeDefault(value=DefaultCache(entry_capacity=100))) + ) + + +@pytest.fixture +def clock(): + return FakeClock() + + +# Construction + + +def test_construction(shared_cache): + negative = BranchKeyNegativeCache("bk-a") + guard = CacheGuard(delegate=shared_cache, negative_cache=negative) + assert isinstance(guard, ICryptographicMaterialsCache) + assert guard.negative_cache is negative + + +@pytest.mark.parametrize("kwargs", [ + {"delegate": object()}, + {"negative_cache": object()}, +]) +def test_invalid_construction(shared_cache, kwargs): + arguments = {"delegate": shared_cache, "negative_cache": BranchKeyNegativeCache("bk-a")} + arguments.update(kwargs) + with pytest.raises(TypeError): + CacheGuard(**arguments) + + +# Native interface, with no Denial: every operation is delegated. + + +def test_hit_passes_through_the_real_shared_cache(shared_cache): + guard = CacheGuard(delegate=shared_cache, negative_cache=BranchKeyNegativeCache("bk-a")) + materials = branch_key_materials("bk-a", str(uuid.uuid4())) + guard.put_cache_entry(put_input(b"\x01" * 48, materials)) + output = guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x01" * 48)) + assert output.materials.value.branch_key == materials.branch_key + + +def test_miss_passes_through_as_entry_does_not_exist(shared_cache): + guard = CacheGuard(delegate=shared_cache, negative_cache=BranchKeyNegativeCache("bk-a")) + with pytest.raises(EntryDoesNotExist): + guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x02" * 48)) + + +def test_put_update_and_delete_are_delegated_as_is(): + spy = SpyCache() + guard = CacheGuard(delegate=spy, negative_cache=BranchKeyNegativeCache("bk-a")) + put = put_input(b"\x03" * 48, branch_key_materials("bk-a", str(uuid.uuid4()))) + update = UpdateUsageMetadataInput(identifier=b"\x03" * 48, bytes_used=1) + delete = DeleteCacheEntryInput(identifier=b"\x03" * 48) + guard.put_cache_entry(put) + guard.update_usage_metadata(update) + guard.delete_cache_entry(delete) + assert spy.calls == [("put", put), ("update", update), ("delete", delete)] + + +# Native interface, with a Denial. + + +def test_denied_raises_the_recorded_error_wrapped_without_consulting_the_shared_cache(): + spy = SpyCache() + negative = BranchKeyNegativeCache("bk-a") + guard = CacheGuard(delegate=spy, negative_cache=negative) + error = kms_error() + negative.record(error) + with pytest.raises(OpaqueWithTextError) as excinfo: + guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x04" * 48)) + assert excinfo.value.obj is error # the same object, as is + assert excinfo.value.obj_message == str(error) + assert "AccessDeniedException" in excinfo.value.obj_message + assert not spy.calls + + +def test_each_denied_lookup_raises_a_new_wrapper(): + # Re-raising one exception object grows its traceback on every raise; a new wrapper per raise does not. + negative = BranchKeyNegativeCache("bk-a") + guard = CacheGuard(delegate=SpyCache(), negative_cache=negative) + error = kms_error() + negative.record(error) + raised = [] + for _ in range(3): + with pytest.raises(OpaqueWithTextError) as excinfo: + guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x04" * 48)) + raised.append(excinfo.value) + assert len({id(wrapper) for wrapper in raised}) == 3 + assert all(wrapper.obj is error for wrapper in raised) + assert error.__traceback__ is None + + +def test_denied_refuses_a_cached_hit(shared_cache): + # Requirement 3: while Denied, the Branch Key ID's cached materials MUST NOT be used. + negative = BranchKeyNegativeCache("bk-a") + guard = CacheGuard(delegate=shared_cache, negative_cache=negative) + guard.put_cache_entry(put_input(b"\x05" * 48, branch_key_materials("bk-a", str(uuid.uuid4())))) + negative.record(kms_error()) + with pytest.raises(OpaqueWithTextError): + guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x05" * 48)) + + +def test_delegates_again_once_the_negative_ttl_expires(shared_cache, clock): + negative = BranchKeyNegativeCache("bk-a", negative_ttl=60, clock=clock) + guard = CacheGuard(delegate=shared_cache, negative_cache=negative) + guard.put_cache_entry(put_input(b"\x06" * 48, branch_key_materials("bk-a", str(uuid.uuid4())))) + negative.record(kms_error()) + clock.now += 60 + assert guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x06" * 48)) is not None + + +def test_a_denial_does_not_affect_another_branch_key_on_the_same_shared_cache(shared_cache): + # Requirement 2. + negative_a = BranchKeyNegativeCache("bk-a") + guard_a = CacheGuard(delegate=shared_cache, negative_cache=negative_a) + guard_b = CacheGuard(delegate=shared_cache, negative_cache=BranchKeyNegativeCache("bk-b")) + guard_b.put_cache_entry(put_input(b"\x07" * 48, branch_key_materials("bk-b", str(uuid.uuid4())))) + negative_a.record(kms_error()) + with pytest.raises(OpaqueWithTextError): + guard_a.get_cache_entry(GetCacheEntryInput(identifier=b"\x08" * 48)) + assert guard_b.get_cache_entry(GetCacheEntryInput(identifier=b"\x07" * 48)) is not None + + +def _lookup_once(guard, error, failures): + """Look up once; anything but the spy's miss or the recorded error is a failure.""" + try: + guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x09" * 48)) + except EntryDoesNotExist: + pass # not Denied: the spy's miss + except OpaqueWithTextError as raised: + if raised.obj is not error: + failures.append(raised) + except Exception as unexpected: # pylint: disable=broad-except + failures.append(unexpected) + + +def _toggle(negative, error, times): + """Alternately record and clear the Denial.""" + for i in range(times): + if i % 2: + negative.record(error) + else: + negative.clear() + + +def test_thread_safety(): + # Requirement 8: lookups on any thread while Denials are recorded and cleared. + negative = BranchKeyNegativeCache("bk-a") + guard = CacheGuard(delegate=SpyCache(), negative_cache=negative) + error = kms_error() + failures = [] + start = threading.Barrier(9) + + def lookups(): + start.wait() + for _ in range(1000): + _lookup_once(guard, error, failures) + + def toggles(): + start.wait() + _toggle(negative, error, 1000) + + threads = [threading.Thread(target=lookups) for _ in range(8)] + [threading.Thread(target=toggles)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + assert not failures + + # The toggler's last action (i == 999, odd) records the Denial; so the guard ends Denied. + with pytest.raises(OpaqueWithTextError) as excinfo: + guard.get_cache_entry(GetCacheEntryInput(identifier=b"\x09" * 48)) + assert excinfo.value.obj is error + + +# End to end: a real Hierarchical Keyring, with the Cache Guard as its CacheTypeShared cache. + + +def encrypt_cache_id(partition_id, logical_key_store_name, branch_key_id): + """aws-kms-hierarchical-keyring.md, Appendix A, Encryption Materials.""" + return hashlib.sha384( + b"\x02\x00\x01\x00" + partition_id.encode() + b"\x00" + logical_key_store_name.encode() + + b"\x00" + branch_key_id.encode() + ).digest() + + +def decrypt_cache_id(partition_id, logical_key_store_name, branch_key_id, version): + """aws-kms-hierarchical-keyring.md, Appendix A, Decryption Materials.""" + return hashlib.sha384( + b"\x02\x00\x02\x00" + partition_id.encode() + b"\x00" + logical_key_store_name.encode() + + b"\x00" + branch_key_id.encode() + b"\x00" + version.encode() + ).digest() + + +@pytest.fixture +def remote_calls(): + """Count every DynamoDB and KMS request the keyring makes; the tests assert there are none.""" + ddb = boto3.client("dynamodb", region_name="us-west-2", aws_access_key_id="AKIAFAKE", aws_secret_access_key="x") + ddb_calls = [] + ddb.meta.events.register("before-call.dynamodb", lambda **kwargs: ddb_calls.append(kwargs["model"].name)) + transport = KmsTransport() + kms = fake_kms.install(transport) + with Stubber(ddb): # no responses queued: a DynamoDB call fails, and is counted + yield {"ddb": ddb, "ddb_calls": ddb_calls, "transport": transport, "kms": kms} + + +@pytest.fixture +def guarded(mat_prov, shared_cache, remote_calls): + """Build a Tenant Hierarchical Keyring with a Cache Guard, and seed its Branch Key in the shared cache.""" + branch_key_id, version, partition_id = "bk-a", str(uuid.uuid4()), "tenant-a" + materials = branch_key_materials(branch_key_id, version) + shared_cache.put_cache_entry(put_input(encrypt_cache_id(partition_id, TABLE, branch_key_id), materials)) + shared_cache.put_cache_entry(put_input(decrypt_cache_id(partition_id, TABLE, branch_key_id, version), materials)) + + negative = BranchKeyNegativeCache(branch_key_id) + key_store = KeyStore( + config=KeyStoreConfig( + ddb_client=remote_calls["ddb"], + ddb_table_name=TABLE, + logical_key_store_name=TABLE, + kms_client=TenantKmsClient(remote_calls["transport"], "us-west-2", TENANT), + kms_configuration=KMSConfigurationKmsKeyArn(value=KMS_KEY), + ) + ) + keyring = mat_prov.create_aws_kms_hierarchical_keyring( + input=CreateAwsKmsHierarchicalKeyringInput( + key_store=key_store, + branch_key_id=branch_key_id, + ttl_seconds=600, + cache=CacheTypeShared(value=CacheGuard(delegate=shared_cache, negative_cache=negative)), + partition_id=partition_id, + ) + ) + esdk = aws_encryption_sdk.EncryptionSDKClient(commitment_policy=CommitmentPolicy.REQUIRE_ENCRYPT_REQUIRE_DECRYPT) + return {"keyring": keyring, "negative": negative, "esdk": esdk} + + +def assert_no_remote_calls(remote_calls): + assert remote_calls["ddb_calls"] == [] + assert remote_calls["kms"].requests == [] + + +def test_keyring_accepts_the_cache_guard_and_uses_cached_hits(guarded, remote_calls): + esdk, keyring = guarded["esdk"], guarded["keyring"] + ciphertext, _ = esdk.encrypt(source=b"tenant data", keyring=keyring) + plaintext, _ = esdk.decrypt(source=ciphertext, keyring=keyring) + assert plaintext == b"tenant data" + assert_no_remote_calls(remote_calls) + + +@pytest.mark.parametrize("code", ["AccessDeniedException", "DisabledException"]) +def test_denied_keyring_fails_encrypt_without_calling_dynamodb_or_kms(guarded, remote_calls, code): + # Requirement 1, and Requirement 3: the cached hit is refused. + # DisabledException is a KMS error the MPL models; it would lose its code as a ComAmazonawsKms error. + error = kms_error(code) + guarded["negative"].record(error) + with pytest.raises(OpaqueWithTextError) as excinfo: + guarded["esdk"].encrypt(source=b"tenant data", keyring=guarded["keyring"]) + # The cause is reachable, the same object, after the MPL's conversion to and from Dafny (negative_caching.md 4.3). + assert excinfo.value.obj is error + assert code in excinfo.value.obj_message + assert_no_remote_calls(remote_calls) + + +@pytest.mark.parametrize("code", ["AccessDeniedException", "DisabledException"]) +def test_denied_keyring_fails_decrypt_without_calling_dynamodb_or_kms(guarded, remote_calls, code): + # Encrypt and decrypt are treated the same (negative_caching.md 4.3). + ciphertext, _ = guarded["esdk"].encrypt(source=b"tenant data", keyring=guarded["keyring"]) + error = kms_error(code) + guarded["negative"].record(error) + with pytest.raises(aws_encryption_sdk.exceptions.AWSEncryptionSDKClientError) as excinfo: + guarded["esdk"].decrypt(source=ciphertext, keyring=guarded["keyring"]) + denial = denial_within(excinfo.value) + assert denial is not None, "the Cache Guard's error is not within the ESDK's" + assert denial.obj is error + assert code in denial.obj_message + assert_no_remote_calls(remote_calls) + + +def test_keyring_recovers_when_the_denial_is_cleared(guarded, remote_calls): + guarded["negative"].record(kms_error()) + with pytest.raises(OpaqueWithTextError): + guarded["esdk"].encrypt(source=b"tenant data", keyring=guarded["keyring"]) + guarded["negative"].clear() + ciphertext, _ = guarded["esdk"].encrypt(source=b"tenant data", keyring=guarded["keyring"]) + plaintext, _ = guarded["esdk"].decrypt(source=ciphertext, keyring=guarded["keyring"]) + assert plaintext == b"tenant data" + assert_no_remote_calls(remote_calls) diff --git a/examples/test/multi_tenant/test_negative_cache.py b/examples/test/multi_tenant/test_negative_cache.py new file mode 100644 index 000000000..48298dc44 --- /dev/null +++ b/examples/test/multi_tenant/test_negative_cache.py @@ -0,0 +1,270 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Offline tests for Negative Cache Entries (negative_caching.md).""" +# pylint: disable=redefined-outer-name +import logging +import threading + +import botocore.exceptions +import pytest + +from ...src.multi_tenant.negative_cache import ( + DEFAULT_NEGATIVE_TTL_SECONDS, + DENIAL_ERROR_CODES, + BranchKeyNegativeCache, + NegativeCacheEntry, + error_code, + is_denial, +) +from .helpers import FakeClock, kms_error + +# Offline: no network access, so these also run as local tests (tox: mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] + +BKID = "bbb9baf1-03e6-4716-a586-6bf29995314b" + + +@pytest.fixture +def clock(): + return FakeClock() + + +@pytest.fixture +def negative(clock): + return BranchKeyNegativeCache(BKID, negative_ttl=60, clock=clock) + + +# Design Question 5: what is a Denial. + + +def test_denial_codes_are_exactly_the_design(): + assert DENIAL_ERROR_CODES == { + "AccessDeniedException", + "DisabledException", + "KMSInvalidStateException", + "NotFoundException", + "IncorrectKeyException", + "ExpiredTokenException", + "UnrecognizedClientException", + "InvalidSignatureException", + } + + +@pytest.mark.parametrize("code", sorted(DENIAL_ERROR_CODES)) +def test_denials(code): + assert is_denial(kms_error(code)) is True + + +@pytest.mark.parametrize("code", ["ThrottlingException", "KMSInternalException", "DependencyTimeoutException", "500"]) +def test_transient_kms_errors_are_not_denials(code): + assert is_denial(kms_error(code)) is False + + +@pytest.mark.parametrize("error", [ + botocore.exceptions.EndpointConnectionError(endpoint_url="https://kms.us-west-2.amazonaws.com"), + botocore.exceptions.ReadTimeoutError(endpoint_url="https://kms.us-west-2.amazonaws.com"), + botocore.exceptions.NoCredentialsError(), + ValueError("not a KMS error"), +]) +def test_other_errors_are_not_denials(error): + assert is_denial(error) is False + + +def test_error_code(): + assert error_code(kms_error("DisabledException")) == "DisabledException" + assert error_code(ValueError("not a KMS error")) == "ValueError" + + +# NegativeCacheEntry + + +def test_entry_is_honored_until_it_expires(): + error = kms_error("AccessDeniedException") + entry = NegativeCacheEntry(branch_key_id=BKID, error=error, denied_at=10.0, expires_at=70.0) + assert entry.is_honored(10.0) + assert entry.is_honored(69.999) + assert not entry.is_honored(70.0) + assert not entry.is_honored(100.0) + + +# BranchKeyNegativeCache + + +def test_defaults(): + negative = BranchKeyNegativeCache(BKID) + assert negative.branch_key_id == BKID + assert negative.negative_ttl == DEFAULT_NEGATIVE_TTL_SECONDS == 300 + assert negative.get() is None + + +@pytest.mark.parametrize("kwargs", [ + {"branch_key_id": ""}, + {"branch_key_id": BKID, "negative_ttl": 0}, + {"branch_key_id": BKID, "negative_ttl": -1}, + {"branch_key_id": BKID, "negative_ttl": float("nan")}, +]) +def test_invalid_construction(kwargs): + with pytest.raises(ValueError): + BranchKeyNegativeCache(**kwargs) + + +def test_nothing_recorded(negative): + assert negative.get() is None + + +def test_record_then_get(negative): + error = kms_error("AccessDeniedException") + entry = negative.record(error) + assert entry == NegativeCacheEntry(branch_key_id=BKID, error=error, denied_at=1000.0, expires_at=1060.0) + assert negative.get() == entry + assert negative.get().error is error # the same object, passed through as is + + +def test_entry_expires_after_the_negative_ttl(negative, clock): + negative.record(kms_error("AccessDeniedException")) + clock.now = 1059.9 + assert negative.get() is not None + clock.now = 1060.0 + assert negative.get() is None + # Discarded, not merely hidden: it stays gone even if the clock went backwards. + clock.now = 1000.0 + assert negative.get() is None + + +def test_record_replaces_the_entry_and_restarts_the_negative_ttl(negative, clock): + negative.record(kms_error("AccessDeniedException")) + clock.now = 1050.0 + second = kms_error("DisabledException") + entry = negative.record(second) + assert entry.denied_at == 1050.0 and entry.expires_at == 1110.0 + clock.now = 1100.0 + assert negative.get().error is second + + +def test_record_logs_one_warning_naming_the_branch_key_code_and_message(negative, caplog): + error = botocore.exceptions.ClientError( + {"Error": {"Code": "AccessDeniedException", "Message": "arn:aws:sts::111122223333:assumed-role/Tenant is not"}}, + "Decrypt", + ) + with caplog.at_level(logging.WARNING, logger="examples.src.multi_tenant.negative_cache"): + negative.record(error) + negative.get() # reads do not log + records = [record for record in caplog.records if record.name.endswith("multi_tenant.negative_cache")] + assert len(records) == 1 + assert records[0].levelno == logging.WARNING + assert BKID in records[0].getMessage() and "AccessDeniedException" in records[0].getMessage() + assert "60" in records[0].getMessage() + # AWS KMS's message, account id and role ARN included; neither is secret. + assert "arn:aws:sts::111122223333:assumed-role/Tenant is not" in records[0].getMessage() + + +def test_clear(negative): + negative.record(kms_error("AccessDeniedException")) + negative.clear() + assert negative.get() is None + negative.clear() # clearing nothing is fine + + +def test_record_does_not_classify(negative): + # The Tenant KMS Client decides what is a Denial (is_denial); record() trusts its caller. + negative.record(kms_error("ThrottlingException")) + assert negative.get() is not None + + +def test_thread_safety(): + # Requirement 8: Denials are recorded and read on any thread. + negative = BranchKeyNegativeCache(BKID, negative_ttl=60) + errors = [kms_error(code) for code in sorted(DENIAL_ERROR_CODES)] + failures = [] + start = threading.Barrier(16) + + def worker(index): + try: + start.wait() + for i in range(2000): + action = (index + i) % 3 + if action == 0: + entry = negative.record(errors[(index + i) % len(errors)]) + # The real clock: times are floats, so the TTL is compared approximately. + assert entry.branch_key_id == BKID and entry.expires_at - entry.denied_at == pytest.approx(60) + elif action == 1: + entry = negative.get() + assert entry is None or entry.error in errors + else: + negative.clear() + except Exception as error: # pylint: disable=broad-except + failures.append(error) + + threads = [threading.Thread(target=worker, args=(n,)) for n in range(16)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + assert not failures + final = negative.record(errors[0]) + assert negative.get() == final + + +class RacingClock(FakeClock): # pylint: disable=too-few-public-methods + """A fake clock that, once armed, runs ``on_tick`` the next time the arming thread reads it. + + ``get()`` reads the clock to decide whether an entry has expired; + so ``on_tick`` runs inside ``get()``, between reading the entry and discarding it. + """ + + def __init__(self, now=1000.0): + """Start disarmed.""" + super().__init__(now) + self.on_tick = None + self.armed_thread = None + + def arm(self, on_tick): + """Run ``on_tick`` on this thread's next read of the clock.""" + self.on_tick = on_tick + self.armed_thread = threading.current_thread() + + def __call__(self): + """Return the current fake time, after running ``on_tick`` if armed for this thread.""" + if self.on_tick is not None and threading.current_thread() is self.armed_thread: + on_tick, self.on_tick = self.on_tick, None + on_tick() + return self.now + + +def test_discarding_an_expired_entry_does_not_erase_a_concurrent_denial(): + """A get() that discards an expired entry MUST NOT erase a Denial recorded concurrently. + + Deterministic, rather than a stress test: while get() is deciding the old entry has expired, + another thread records a new Denial. An implementation that discards unconditionally, + or without holding its lock across the check and the discard, erases the new Denial, + and a denied Branch Key ID would be retried. With a lock held across both, + record() blocks until get() returns, the bounded wait below times out, and the new Denial survives. + + This catches the pattern of reading the entry, then the clock, then discarding; + an unlocked implementation that reads the clock before the entry has a window this cannot reach. + """ + clock = RacingClock() + negative = BranchKeyNegativeCache(BKID, negative_ttl=60, clock=clock) + negative.record(kms_error("AccessDeniedException")) + clock.now = 2000.0 # the first Denial has expired + new_error = kms_error("DisabledException") + recorded = threading.Event() + + def record_concurrently(): + negative.record(new_error) + recorded.set() + + racer = threading.Thread(target=record_concurrently) + + def start_racer(): + racer.start() + recorded.wait(timeout=0.2) # bounded: a correctly locked record() is still waiting for get() + + clock.arm(start_racer) + negative.get() # discards the expired entry; its result depends on the interleaving + racer.join(timeout=5) + assert not racer.is_alive() + + survivor = negative.get() + assert survivor is not None, "a concurrent Denial was erased by the discard of an expired entry" + assert survivor.error is new_error From 927966e531c4bf0d6bf7368638b13d829e15f595 Mon Sep 17 00:00:00 2001 From: Tony Knapp Date: Fri, 6 Nov 2026 10:54:05 -0600 Subject: [PATCH 10/10] feat(examples): Denial recording and the Tenant Keyring Factory Completes negative caching (examples/src/multi_tenant/negative_caching.md, Design Questions 3.3, 6, 7.2, and 8). TenantKmsClient(..., negative_cache=None): - Records a KMS ClientError in its BranchKeyNegativeCache, as the same object, only for a Decrypt, only for a Denial (is_denial), and only if the encryption context has no branch-key-id or names the negative cache's Branch Key ID. Assumes hierarchy version 1 Branch Keys, the only version shipped. The error is always re-raised unchanged. TenantKeyringFactory: - Holds what every Tenant Hierarchical Keyring shares: the KmsTransport, the DynamoDB client, the AWS KMS Key Store configuration, the cache limit TTL, the Negative TTL, and the storm tracking cache, which it creates from settings (default_storm_tracking_cache(), design.md 10.1.1) so it can require the grace period be less than ttl_seconds. - negative_cache_for: one negative cache per (Tenant, Branch Key ID), so a Tenant misconfigured with another's Branch Key ID cannot deny it to its owner. - keyring_for: a Tenant KMS Client bound to that negative cache, an AWS KMS Key Store, and a Hierarchical Keyring with a Cache Guard over the shared cache and the Tenant ID, as is, as its Partition ID. negative_caching.md: - Negative caches per Tenant and Branch Key ID; the factory owns the storm tracking cache; the Partition ID is the Tenant ID as is. - The Tenant ID is the authorization boundary of cached materials: vended keyrings need not be cached, and the application MUST pass the Tenant it authenticated and the Branch Key ID it assigned. Documents the alternative of a credentials Partition ID with cache deletion on Denial and on keyring cleanup. - Hierarchy version 2 was never shipped. Open Question 5: lock contention. Tests: - Denial recording, the factory offline (zero remote calls, isolation, thread safety, the grace period rule), and end to end on DynamoDB Local. - Stubbed DynamoDB calls are counted on provide-client-params: botocore's Stubber rejects a call before before-call, so the Cache Guard's and the factory's "no DynamoDB calls" asserts had held vacuously. Co-Authored-By: Claude Opus 5.5 --- examples/src/multi_tenant/negative_cache.py | 25 +- examples/src/multi_tenant/negative_caching.md | 121 +++++++- .../multi_tenant/tenant_keyring_factory.py | 270 +++++++++++++++++ .../src/multi_tenant/tenant_kms_client.py | 78 ++++- examples/test/multi_tenant/keyring_helpers.py | 70 +++++ .../test/multi_tenant/test_cache_guard.py | 70 +---- .../test_ddblocal_hierarchical.py | 65 ++++ .../test_tenant_keyring_factory.py | 282 ++++++++++++++++++ .../multi_tenant/test_tenant_kms_client.py | 104 +++++++ 9 files changed, 995 insertions(+), 90 deletions(-) create mode 100644 examples/src/multi_tenant/tenant_keyring_factory.py create mode 100644 examples/test/multi_tenant/keyring_helpers.py create mode 100644 examples/test/multi_tenant/test_tenant_keyring_factory.py diff --git a/examples/src/multi_tenant/negative_cache.py b/examples/src/multi_tenant/negative_cache.py index 48222e60b..cd82a35a9 100644 --- a/examples/src/multi_tenant/negative_cache.py +++ b/examples/src/multi_tenant/negative_cache.py @@ -88,6 +88,22 @@ def is_denial(error): return error_code(error) in DENIAL_ERROR_CODES +def validate_negative_ttl(negative_ttl): + """Rejects a Negative TTL that is not a number greater than zero. + + :param float negative_ttl: Seconds a Negative Cache Entry is honored + :raises ValueError: if ``negative_ttl`` is not a number greater than zero + """ + # NaN is rejected explicitly: every comparison with NaN is False, so `negative_ttl <= 0` would accept it. + if ( + isinstance(negative_ttl, bool) + or not isinstance(negative_ttl, (int, float)) + or math.isnan(negative_ttl) + or negative_ttl <= 0 + ): + raise ValueError("negative_ttl must be a number greater than zero") + + class BranchKeyNegativeCache(object): """The Negative Cache Entry, if any, of one Branch Key ID. @@ -103,14 +119,7 @@ def __init__(self, branch_key_id, negative_ttl=DEFAULT_NEGATIVE_TTL_SECONDS, clo """Validates the Branch Key ID and Negative TTL.""" if not isinstance(branch_key_id, str) or not branch_key_id: raise ValueError("branch_key_id must be a non-empty string") - # NaN is rejected explicitly: every comparison with NaN is False, so `negative_ttl <= 0` would accept it. - if ( - isinstance(negative_ttl, bool) - or not isinstance(negative_ttl, (int, float)) - or math.isnan(negative_ttl) - or negative_ttl <= 0 - ): - raise ValueError("negative_ttl must be a number greater than zero") + validate_negative_ttl(negative_ttl) self._branch_key_id = branch_key_id self._negative_ttl = negative_ttl self._clock = clock diff --git a/examples/src/multi_tenant/negative_caching.md b/examples/src/multi_tenant/negative_caching.md index 57e834679..104163809 100644 --- a/examples/src/multi_tenant/negative_caching.md +++ b/examples/src/multi_tenant/negative_caching.md @@ -120,7 +120,9 @@ only by implementing the public, extendable interfaces until the Negative TTL expires. 2. A Denial for a Branch Key ID MUST NOT affect any other Branch Key ID, - including Branch Key IDs protected by the same KMS Key. + including Branch Key IDs protected by the same KMS Key; + nor the same Branch Key ID for any other Tenant + ([7.2](#72-a-tenant-keyring-factory)). 3. While a Negative Cache Entry is honored, the Branch Key ID's cached materials MUST NOT be used, @@ -646,6 +648,15 @@ and the Tenant KMS Client records a Denial against that Branch Key ID. Where the request's encryption context includes `branch-key-id`, the Tenant KMS Client SHOULD check it matches. +UPDATE: Hierarchy version 2 was never shipped; +hierarchy version 1 is the only hierarchy version, +and the AWS KMS Key Store's `Decrypt` always includes `branch-key-id`. +The Branch Key ID is still bound rather than parsed: +the Cache Guard needs it before any request is made, +and the binding does not depend on the encryption context's format. +The Tenant KMS Client checks the `branch-key-id`, +and its source says it assumes hierarchy version 1. + As a Tenant KMS Client is a few references ([design.md, Tenant KMS Client](./design.md#tenant-kms-client)), one per Tenant Hierarchical Keyring, @@ -686,24 +697,54 @@ One per process. It holds: - the Shared Transport -- the shared cache +- the storm tracking cache shared by every Tenant, which it creates - the DynamoDB client - the AWS KMS Key Store configuration, other than the KMS client (table name, Logical Key Store Name, AWS KMS Configuration, grant tokens) +- the cache limit TTL of the keyrings it vends (default 10 minutes) - the Negative TTL (default 5 minutes) -Given a Branch Key ID, -the Tenant's Credentials, -and a stable Tenant identity ([8](#8-the-partition-id-revisited)), +**The factory creates the shared cache**: +It takes the storm tracking cache's settings +(default: those of [design.md 10.1.1](./design.md#1011-why-not-the-mpls-defaultcache)), +rather than a cache, +and creates the cache itself. +The specification requires the storm tracking cache's grace period +be less than the Hierarchical Keyring's cache limit TTL +(`framework/aws-kms/aws-kms-hierarchical-keyring.md`, Initialization); +only the factory knows both, +so only it can enforce this. +It also prevents a cache that is not a storm tracking cache, +or whose time units were not set, +from being shared by every Tenant. +The shared cache is exposed, read only, to the application and to tests. + +Given a Tenant's identifier, +a Branch Key ID, +and the Tenant's Credentials, it vends a Tenant Hierarchical Keyring, built with: -- a Tenant KMS Client, bound to the Branch Key ID +- a Tenant KMS Client, recording Denials in the negative cache of that Tenant and Branch Key ID - an AWS KMS Key Store, using that Tenant KMS Client -- a Cache Guard, bound to the Branch Key ID, delegating to the shared cache +- a Cache Guard, enforcing that negative cache, delegating to the shared cache - the Branch Key ID, statically -- a Partition ID derived from the stable Tenant identity +- the Tenant's identifier, as is, as the Partition ID ([8](#8-the-partition-id-revisited)) + +**Negative caches are per Tenant and Branch Key ID**: +The factory holds one negative cache per Tenant and Branch Key ID, +and every keyring it vends for that Tenant and Branch Key ID shares it; +so a Denial recorded through one is enforced by all. + +UPDATE: An earlier revision of this document keyed negative caches by Branch Key ID alone. +Then, +a Tenant misconfigured with another Tenant's Branch Key ID, +and so Denied by AWS KMS, +would deny that Branch Key ID to the Tenant that owns it; +one Tenant's misconfiguration MUST NOT deny another Tenant (Requirement 2). +The shared cache is already partitioned by Tenant ([8](#8-the-partition-id-revisited)); +the negative caches now are too. **This changes the guidance of [design.md 10](./design.md#10-the-hierarchical-keyring)**: which offers a Branch Key ID Supplier as the simpler multi-tenant design @@ -759,6 +800,9 @@ The Partition ID SHOULD be derived from a stable Tenant identity supplied to the Tenant Keyring Factory, rather than from the Tenant's Credentials. +UPDATE: The Partition ID is the Tenant's identifier, as is; +it is not a secret, so there is no reason to digest it. + As with any cache, a revoked KMS authorization is not noticed until a cached entry expires; once a fetch is Denied, @@ -767,6 +811,55 @@ the Cache Guard enforces it (Requirement 3). UPDATE: The fix to the existing example is deferred to a later change; [design.md 10.2](./design.md#102-the-partition-id) records this. +**The Tenant identifier is the authorization boundary of cached materials**: +The shared cache's entries are identified by +the Partition ID, +the Logical Key Store Name, +and the Branch Key ID +([Appendix A](https://github.com/awslabs/aws-encryption-sdk-specification/blob/master/framework/aws-kms/aws-kms-hierarchical-keyring.md#appendix-a-cache-entry-identifier-formulas)); +not by the credentials. +Thus, +a keyring vended for a Tenant identifier and Branch Key ID +is served the materials cached by any earlier keyring +vended for the same pair, +whatever credentials either was vended with, +until the entry expires or is Denied. + +Vended keyrings are a few references +around the MPL's formally verified Hierarchical Keyring; +applications need not cache them, +and MAY vend one per request or per tenant connection. +An application that does +relies on each new keyring, +built with fresh credentials, +being served the entries of the keyrings before it. + +This is a security decision of this design: +the consuming application's choice of Tenant identifier and Branch Key ID +is critical. +The application MUST pass the Tenant Keyring Factory +the identifier of the Tenant it authenticated, +never one taken from the request, +and the Branch Key ID it assigned to that Tenant. + +**An alternative: the credentials as the Partition ID**: +A Partition ID derived from the credentials, +as [design.md 10.2](./design.md#102-the-partition-id) does, +would make the credentials the boundary: +a keyring would be served only the entries +fetched with its own credentials. +To keep the cache from holding materials +for credentials no longer in use, +the Tenant Keyring Factory would additionally +delete a Branch Key's entries from the shared cache +when AWS KMS Denies it, +and when a vended keyring is cleaned up +(for example, on its tenant connection closing). +Its cost is +the cache miss per credential refresh described above, +and a vended keyring the application MUST clean up. +This design does not take it. + ## 9 The storm tracking cache The Cache Guard sits in front of the storm tracking cache, @@ -863,6 +956,18 @@ each Branch Key ID is Denied on its own. UPDATE: They are treated the same ([4.3](#43-a-cache-guard)). +5. **Contention on the Tenant Keyring Factory's lock**: + `negative_cache_for` takes one lock, per factory, + to look up or create a Tenant and Branch Key ID's negative cache; + the Key Store and keyring are built outside it. + At service startup, + a burst of keyrings vended at once contends on it. + If that matters, + a partitioned (striped) map, + like that behind Guava's cache, + would spread it. + A stress test on the factory SHOULD decide. + # Consequences - The fail fast behavior depends on the MPL returning a cache error, diff --git a/examples/src/multi_tenant/tenant_keyring_factory.py b/examples/src/multi_tenant/tenant_keyring_factory.py new file mode 100644 index 000000000..bc36dbf81 --- /dev/null +++ b/examples/src/multi_tenant/tenant_keyring_factory.py @@ -0,0 +1,270 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""The Tenant Keyring Factory: vends Tenant Hierarchical Keyrings with negative caching. + +See negative_caching.md in this directory, Design Question 7.2. + +One factory per process holds everything every Tenant Hierarchical Keyring shares: +the Shared Transport, the storm tracking cache shared by every Tenant (which it creates), +the DynamoDB client, the AWS KMS Key Store configuration (other than its KMS client), +the cache limit TTL, and the Negative TTL. + +Given a Tenant, a Branch Key ID, and the Tenant's Credentials, it vends a Tenant Hierarchical Keyring, built with: + +- a Tenant KMS Client, recording Denials in the negative cache of that Tenant and Branch Key ID +- an AWS KMS Key Store, using that Tenant KMS Client +- a Cache Guard, enforcing that negative cache, delegating to the shared cache +- the Branch Key ID, statically +- a Partition ID derived from the Tenant's identifier, which, unlike its credentials, is stable (negative_caching.md 8) + +It takes the Branch Key ID Supplier's place: +the application selects the Branch Key ID, and asks the factory for its keyring. + +The factory, and everything it vends, is thread safe (Requirement 8). + +This module imports the MPL. +""" +import threading +import time + +from aws_cryptographic_material_providers.keystore import KeyStore +from aws_cryptographic_material_providers.keystore.config import KeyStoreConfig +from aws_cryptographic_material_providers.mpl import AwsCryptographicMaterialProviders +from aws_cryptographic_material_providers.mpl.config import MaterialProvidersConfig +from aws_cryptographic_material_providers.mpl.models import ( + CacheTypeShared, + CacheTypeStormTracking, + CreateAwsKmsHierarchicalKeyringInput, + CreateCryptographicMaterialsCacheInput, + StormTrackingCache, + TimeUnits, +) + +from .cache_guard import CacheGuard +from .kms_transport import KmsTransport, validate_region_name +from .negative_cache import DEFAULT_NEGATIVE_TTL_SECONDS, BranchKeyNegativeCache, validate_negative_ttl +from .tenant_kms_client import TenantKmsClient + +#: The default cache limit TTL of the vended Hierarchical Keyrings, in seconds. +DEFAULT_TTL_SECONDS = 600 + + +def default_storm_tracking_cache(): + """Returns the default settings of the storm tracking cache a factory creates and shares. + + The values, and why, are design.md 10.1.1. + These are illustrative; applications SHOULD tune every one of them to their own usage pattern. + + :rtype: aws_cryptographic_material_providers.mpl.models.StormTrackingCache + """ + return StormTrackingCache( + # Maximum Branch Keys cached, across every Tenant. + entry_capacity=1000, + entry_pruning_tail_size=1, + grace_period=10_000, + grace_interval=1_000, + # Shared by every Tenant: the MPL default of 20 would serialize a burst of Tenants missing at once. + fan_out=100, + # A failed fetch holds its fan out slot this long; lowered from the MPL default of 10 seconds. + in_flight_ttl=5_000, + sleep_milli=20, + # MUST be set: a StormTrackingCache defaults to seconds. + time_units=TimeUnits.MILLISECONDS, + ) + + +def tenant_partition_id(tenant_id): + """Returns the Partition ID of a Tenant's Hierarchical Keyrings: the Tenant's identifier, as is. + + Derived from the Tenant's identifier, so it is constant across credential refreshes, + unlike a Partition ID derived from the credentials (negative_caching.md 8). + + :param str tenant_id: The Tenant's stable identifier + :rtype: str + :raises ValueError: if ``tenant_id`` is empty + """ + if not isinstance(tenant_id, str) or not tenant_id: + raise ValueError("tenant_id must be a non-empty string") + return tenant_id + + +def _validate_name(name, value): + if not isinstance(value, str) or not value: + raise ValueError("{} must be a non-empty string".format(name)) + + +def _grace_period_seconds(settings): + """The storm tracking cache's grace period, in seconds; a StormTrackingCache defaults to seconds.""" + if settings.time_units == TimeUnits.MILLISECONDS: + return settings.grace_period / 1000 + return settings.grace_period + + +class TenantKeyringFactory(object): # pylint: disable=too-many-instance-attributes + """Vends Tenant Hierarchical Keyrings that share one transport, cache, DynamoDB client, and Key Store configuration. + + The factory creates, and owns, the storm tracking cache every Tenant shares, + so it can require its grace period be less than the keyrings' cache limit TTL, + as the specification does (aws-kms-hierarchical-keyring.md, Initialization). + + :param transport: The Shared Transport + :type transport: KmsTransport + :param ddb_client: The DynamoDB client, shared by every Tenant's AWS KMS Key Store + :param str ddb_table_name: The AWS KMS Key Store's DynamoDB table + :param str logical_key_store_name: The AWS KMS Key Store's Logical Key Store Name + :param kms_configuration: The AWS KMS Key Store's AWS KMS Configuration, including Discovery and MRDiscovery + :param str region_name: The AWS Region of the Tenant KMS Clients + :param int ttl_seconds: The vended Hierarchical Keyrings' cache limit TTL (default 600) + :param float negative_ttl: Seconds a Negative Cache Entry is honored (default 300) + :param storm_tracking_cache: Settings of the storm tracking cache the factory creates and shares + (default ``default_storm_tracking_cache()``) + :type storm_tracking_cache: aws_cryptographic_material_providers.mpl.models.StormTrackingCache + :param clock: Returns the current time in seconds, for the negative caches; for tests (default ``time.monotonic``) + :raises TypeError: if ``transport`` is not a ``KmsTransport``, + or ``storm_tracking_cache`` is not a ``StormTrackingCache`` + :raises ValueError: if a name is empty, the region is invalid, a TTL is not greater than zero, + or the storm tracking cache's grace period is not less than ``ttl_seconds`` + """ + + def __init__( # pylint: disable=too-many-arguments + self, + transport, + ddb_client, + ddb_table_name, + logical_key_store_name, + kms_configuration, + region_name, + ttl_seconds=DEFAULT_TTL_SECONDS, + negative_ttl=DEFAULT_NEGATIVE_TTL_SECONDS, + storm_tracking_cache=None, + clock=time.monotonic, + ): + """Validates what every vended keyring shares, and creates the shared cache.""" + if not isinstance(transport, KmsTransport): + raise TypeError("transport must be a KmsTransport") + _validate_name("ddb_table_name", ddb_table_name) + _validate_name("logical_key_store_name", logical_key_store_name) + validate_region_name(region_name) + # An int: the Hierarchical Keyring's ttl_seconds is a whole number of seconds. + if isinstance(ttl_seconds, bool) or not isinstance(ttl_seconds, int) or ttl_seconds <= 0: + raise ValueError("ttl_seconds must be an integer greater than zero") + validate_negative_ttl(negative_ttl) + if storm_tracking_cache is None: + storm_tracking_cache = default_storm_tracking_cache() + if not isinstance(storm_tracking_cache, StormTrackingCache): + raise TypeError("storm_tracking_cache must be a StormTrackingCache") + # The specification: the grace period MUST be less than the cache limit TTL + # (aws-kms-hierarchical-keyring.md, Initialization). Only the factory knows both. + if _grace_period_seconds(storm_tracking_cache) >= ttl_seconds: + raise ValueError("the storm tracking cache's grace period must be less than ttl_seconds") + + self._transport = transport + self._ddb_client = ddb_client + self._ddb_table_name = ddb_table_name + self._logical_key_store_name = logical_key_store_name + self._kms_configuration = kms_configuration + self._region_name = region_name + self._ttl_seconds = ttl_seconds + self._negative_ttl = negative_ttl + self._clock = clock + self._mpl = AwsCryptographicMaterialProviders(config=MaterialProvidersConfig()) + # Created here, from the settings, so every vended keyring shares this one cache. + self._shared_cache = self._mpl.create_cryptographic_materials_cache( + input=CreateCryptographicMaterialsCacheInput(cache=CacheTypeStormTracking(value=storm_tracking_cache)) + ) + # Guards _negative_caches: concurrent requests for a Tenant and Branch Key ID get one negative cache. + # Needed even with the GIL, as the lookup and the insert are separate steps; and free threaded CPython + # has no GIL. It is held only for that lookup and insert: keyring_for builds the Key Store and keyring + # outside it. Whether one lock contends at service startup is an open question (negative_caching.md). + self._lock = threading.Lock() + self._negative_caches = {} + + @property + def shared_cache(self): + """The storm tracking cache shared by every Tenant, which this factory created. + + :rtype: aws_cryptographic_material_providers.mpl.references.ICryptographicMaterialsCache + """ + return self._shared_cache + + @property + def negative_ttl(self): + """Seconds a Negative Cache Entry is honored. + + :rtype: float + """ + return self._negative_ttl + + def negative_cache_for(self, tenant_id, branch_key_id): + """Returns the negative cache of a Tenant and Branch Key ID; the same object on every call. + + Keyed by Tenant as well as Branch Key ID: + a Tenant misconfigured with another Tenant's Branch Key ID, + and so Denied, MUST NOT deny that Branch Key ID to the Tenant that owns it. + + :param str tenant_id: The Tenant's stable identifier + :param str branch_key_id: The Branch Key ID + :rtype: BranchKeyNegativeCache + :raises ValueError: if ``tenant_id`` or ``branch_key_id`` is empty + """ + key = (tenant_partition_id(tenant_id), branch_key_id) + with self._lock: + negative_cache = self._negative_caches.get(key) + if negative_cache is None: + negative_cache = BranchKeyNegativeCache( + branch_key_id, negative_ttl=self._negative_ttl, clock=self._clock + ) + self._negative_caches[key] = negative_cache + return negative_cache + + def keyring_for(self, tenant_id, branch_key_id, credentials): + """Vends a Tenant Hierarchical Keyring for a Tenant's Branch Key ID. + + Every keyring vended for the same Tenant and Branch Key ID shares one negative cache, + so a Denial recorded through one is enforced by all. + + A vended keyring is a few references; applications MAY vend one per request or per tenant connection, + rather than caching them. + Every keyring vended for the same ``tenant_id`` and ``branch_key_id`` shares the same cache entries, + whatever its credentials: while an entry is cached, it is served without AWS KMS authorizing those + credentials. So the application MUST pass the ``tenant_id`` of the Tenant it authenticated, + never one taken from the request, and the ``branch_key_id`` it assigned that Tenant + (negative_caching.md 8). + + :param str tenant_id: The Tenant's stable identifier + :param str branch_key_id: The Branch Key ID + :param credentials: The Tenant's Credentials + :type credentials: botocore.credentials.Credentials + :rtype: aws_cryptographic_material_providers.mpl.references.IKeyring + :raises ValueError: if ``tenant_id`` or ``branch_key_id`` is empty + """ + negative_cache = self.negative_cache_for(tenant_id, branch_key_id) + # Records Denials of this Branch Key ID, for this Tenant, in its negative cache. + kms_client = TenantKmsClient( + transport=self._transport, + region_name=self._region_name, + credentials=credentials, + negative_cache=negative_cache, + ) + key_store = KeyStore( + config=KeyStoreConfig( + ddb_client=self._ddb_client, + ddb_table_name=self._ddb_table_name, + logical_key_store_name=self._logical_key_store_name, + kms_client=kms_client, + kms_configuration=self._kms_configuration, + ) + ) + return self._mpl.create_aws_kms_hierarchical_keyring( + input=CreateAwsKmsHierarchicalKeyringInput( + key_store=key_store, + branch_key_id=branch_key_id, + ttl_seconds=self._ttl_seconds, + # Enforces the negative cache before every lookup, delegating to the shared cache. + cache=CacheTypeShared(value=CacheGuard(self._shared_cache, negative_cache)), + partition_id=tenant_partition_id(tenant_id), + ) + ) + + +__all__ = ("TenantKeyringFactory", "tenant_partition_id", "default_storm_tracking_cache", "DEFAULT_TTL_SECONDS") diff --git a/examples/src/multi_tenant/tenant_kms_client.py b/examples/src/multi_tenant/tenant_kms_client.py index b16dfb778..b8e99622b 100644 --- a/examples/src/multi_tenant/tenant_kms_client.py +++ b/examples/src/multi_tenant/tenant_kms_client.py @@ -25,6 +25,7 @@ import botocore.exceptions from .kms_transport import KmsTransport, validate_region_name +from .negative_cache import BranchKeyNegativeCache, is_denial _Operation = collections.namedtuple("_Operation", ("api_name", "required", "optional", "input_blobs")) @@ -108,21 +109,45 @@ class TenantKmsClient(object): :type credentials: botocore.credentials.Credentials :param str endpoint_url: Endpoint override (optional) :param str user_agent_extra: Appended to the User-Agent (optional) + :param negative_cache: Where this client records Denials of its Branch Key ID (optional); + the Tenant Keyring Factory binds one per Tenant and Branch Key ID (negative_caching.md 3.3, 6) + :type negative_cache: BranchKeyNegativeCache """ def __init__( # pylint: disable=too-many-arguments - self, transport, region_name, credentials, endpoint_url=None, user_agent_extra=None + self, transport, region_name, credentials, endpoint_url=None, user_agent_extra=None, negative_cache=None ): - """Validates and stores the transport, region, and credentials.""" + """Validates and stores the transport, region, credentials, and negative cache.""" if not isinstance(transport, KmsTransport): raise TypeError("transport must be a KmsTransport") validate_region_name(region_name) if not callable(getattr(credentials, "get_frozen_credentials", None)): raise TypeError("credentials must be a botocore.credentials.Credentials") + if negative_cache is not None and not isinstance(negative_cache, BranchKeyNegativeCache): + raise TypeError("negative_cache must be a BranchKeyNegativeCache") self._transport = transport self._credentials = credentials + self._negative_cache = negative_cache self.meta = _ClientMeta(region_name, endpoint_url, user_agent_extra) + @property + def negative_cache(self): + """Where this client records Denials of its Branch Key ID, if anywhere. + + A Branch Key is a data key, generated by AWS KMS, that the Hierarchical Keyring reuses + to derive unique data keys for envelope encryption + (https://github.com/awslabs/aws-encryption-sdk-specification/blob/master/framework/structures.md#branch-key); + the AWS KMS Key Store persists each Branch Key in DynamoDB, encrypted under an AWS KMS Key. + + Only Denials of ``Decrypt`` are recorded: a Denial is a failure to decrypt a Branch Key + (negative_caching.md, Definitions). + Failures of every other operation, including the ``GenerateDataKeyWithoutPlaintext`` and ``ReEncrypt`` + that create and version Branch Keys, are re-raised without being recorded. + + :rtype: BranchKeyNegativeCache or None + """ + return self._negative_cache + def __repr__(self): """Describes the client's region and endpoint.""" # Never include the credentials; ESDK configuration objects include their client in their own repr. @@ -156,20 +181,53 @@ def re_encrypt(self, **kwargs): def _call(self, method_name, params): operation = _OPERATIONS[method_name] body = _serialize(operation, params) - response, metadata = self._transport.call( - operation_name=operation.api_name, - body=body, - region_name=self.meta.region_name, - credentials=self._credentials, - endpoint_url=self.meta.endpoint_url, - user_agent_extra=self.meta.config.user_agent_extra, - ) + try: + response, metadata = self._transport.call( + operation_name=operation.api_name, + body=body, + region_name=self.meta.region_name, + credentials=self._credentials, + endpoint_url=self.meta.endpoint_url, + user_agent_extra=self.meta.config.user_agent_extra, + ) + except botocore.exceptions.ClientError as error: + if self._negative_cache is not None: + self._record_denial(operation.api_name, params, error) + raise for member in _OUTPUT_BLOBS: if member in response: response[member] = base64.b64decode(response[member]) response["ResponseMetadata"] = metadata return response + def _record_denial(self, operation_name, params, error): + """Records ``error`` in the negative cache if it is a Denial of this client's Branch Key ID. + + Records only if all of: + + - ``operation_name`` is ``Decrypt``: a Denial is a failure to decrypt a Branch Key + (negative_caching.md, Definitions) + - ``error`` is a Denial (``is_denial``) + - the request's ``EncryptionContext`` has no ``branch-key-id``, + or its ``branch-key-id`` is the negative cache's Branch Key ID (negative_caching.md 6) + + :param str operation_name: The KMS operation (ex: Decrypt) + :param dict params: The request's boto3 style parameters + :param botocore.exceptions.ClientError error: The error AWS KMS returned; it is re-raised by the caller + """ + if operation_name != "Decrypt" or not is_denial(error): + return + # Assumes hierarchy version 1 Branch Keys, the only version the MPL has shipped: + # the AWS KMS Key Store's Decrypt always includes branch-key-id in the encryption context + # (branch-key-store.md, Encryption Context), so a mismatch is a Decrypt for another Branch Key. + # A Decrypt without it is not the Key Store's; it is recorded, as this client's Branch Key ID is bound. + encryption_context = params.get("EncryptionContext") or {} + branch_key_id = encryption_context.get("branch-key-id") + if branch_key_id is not None and branch_key_id != self._negative_cache.branch_key_id: + return + # The error itself, as is: the Cache Guard raises this object while the entry is honored. + self._negative_cache.record(error) + def _serialize(operation, params): """Validates boto3 style parameters and converts them to the KMS JSON protocol. diff --git a/examples/test/multi_tenant/keyring_helpers.py b/examples/test/multi_tenant/keyring_helpers.py new file mode 100644 index 000000000..333394612 --- /dev/null +++ b/examples/test/multi_tenant/keyring_helpers.py @@ -0,0 +1,70 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Test helpers for the negative caching tests that use the MPL. + +Branch Key materials, cache entries, the specification's cache identifiers, and finding the Cache Guard's error. +""" +import hashlib +import os +import time + +from aws_cryptographic_material_providers.keystore.models import BranchKeyMaterials +from aws_cryptographic_material_providers.mpl.errors import OpaqueWithTextError +from aws_cryptographic_material_providers.mpl.models import MaterialsBranchKey, PutCacheEntryInput + + +def branch_key_materials(branch_key_id, version): + return BranchKeyMaterials( + branch_key_identifier=branch_key_id, + branch_key_version=version, + encryption_context={}, + branch_key=os.urandom(32), + ) + + +def put_input(identifier, materials, now=None): + now = int(time.time()) if now is None else now + return PutCacheEntryInput( + identifier=identifier, + materials=MaterialsBranchKey(value=materials), + creation_time=now, + expiry_time=now + 600, + messages_used=0, + bytes_used=0, + ) + + +def denial_within(raised): + """Return the Cache Guard's OpaqueWithTextError within ``raised``, or None. + + On encrypt, the ESDK raises it; on decrypt, it is within the MPL's CollectionOfErrors, + within the ESDK's AWSEncryptionSDKClientError (negative_caching.md 4.3). + """ + pending, seen = [raised], set() + while pending: + error = pending.pop() + if id(error) in seen: + continue + seen.add(id(error)) + if isinstance(error, OpaqueWithTextError): + return error + for attribute in ("list", "args"): + value = getattr(error, attribute, None) or () + pending.extend(child for child in value if isinstance(child, BaseException)) + return None + + +def encrypt_cache_id(partition_id, logical_key_store_name, branch_key_id): + """aws-kms-hierarchical-keyring.md, Appendix A, Encryption Materials.""" + return hashlib.sha384( + b"\x02\x00\x01\x00" + partition_id.encode() + b"\x00" + logical_key_store_name.encode() + + b"\x00" + branch_key_id.encode() + ).digest() + + +def decrypt_cache_id(partition_id, logical_key_store_name, branch_key_id, version): + """aws-kms-hierarchical-keyring.md, Appendix A, Decryption Materials.""" + return hashlib.sha384( + b"\x02\x00\x02\x00" + partition_id.encode() + b"\x00" + logical_key_store_name.encode() + + b"\x00" + branch_key_id.encode() + b"\x00" + version.encode() + ).digest() diff --git a/examples/test/multi_tenant/test_cache_guard.py b/examples/test/multi_tenant/test_cache_guard.py index 832287324..3b0bf93b9 100644 --- a/examples/test/multi_tenant/test_cache_guard.py +++ b/examples/test/multi_tenant/test_cache_guard.py @@ -7,17 +7,14 @@ from cache hits; any call to Amazon DynamoDB or AWS KMS is counted, and asserted to be zero. """ # pylint: disable=redefined-outer-name -import hashlib -import os import threading -import time import uuid import boto3 import pytest from aws_cryptographic_material_providers.keystore import KeyStore from aws_cryptographic_material_providers.keystore.config import KeyStoreConfig -from aws_cryptographic_material_providers.keystore.models import BranchKeyMaterials, KMSConfigurationKmsKeyArn +from aws_cryptographic_material_providers.keystore.models import KMSConfigurationKmsKeyArn from aws_cryptographic_material_providers.mpl import AwsCryptographicMaterialProviders from aws_cryptographic_material_providers.mpl.config import MaterialProvidersConfig from aws_cryptographic_material_providers.mpl.errors import EntryDoesNotExist, OpaqueWithTextError @@ -29,8 +26,6 @@ DefaultCache, DeleteCacheEntryInput, GetCacheEntryInput, - MaterialsBranchKey, - PutCacheEntryInput, UpdateUsageMetadataInput, ) from aws_cryptographic_material_providers.mpl.references import ICryptographicMaterialsCache @@ -45,6 +40,7 @@ from ...src.multi_tenant.negative_cache import BranchKeyNegativeCache from . import fake_kms from .helpers import FakeClock, kms_error +from .keyring_helpers import branch_key_materials, decrypt_cache_id, denial_within, encrypt_cache_id, put_input # Offline: no network access, so these also run as local tests (tox: mplexampleslocal). pytestmark = [pytest.mark.examples, pytest.mark.local] @@ -82,47 +78,6 @@ def delete_cache_entry(self, param): self.calls.append(("delete", param)) -def branch_key_materials(branch_key_id, version): - return BranchKeyMaterials( - branch_key_identifier=branch_key_id, - branch_key_version=version, - encryption_context={}, - branch_key=os.urandom(32), - ) - - -def put_input(identifier, materials, now=None): - now = int(time.time()) if now is None else now - return PutCacheEntryInput( - identifier=identifier, - materials=MaterialsBranchKey(value=materials), - creation_time=now, - expiry_time=now + 600, - messages_used=0, - bytes_used=0, - ) - - -def denial_within(raised): - """Return the Cache Guard's OpaqueWithTextError within ``raised``, or None. - - On encrypt, the ESDK raises it; on decrypt, it is within the MPL's CollectionOfErrors, - within the ESDK's AWSEncryptionSDKClientError (negative_caching.md 4.3). - """ - pending, seen = [raised], set() - while pending: - error = pending.pop() - if id(error) in seen: - continue - seen.add(id(error)) - if isinstance(error, OpaqueWithTextError): - return error - for attribute in ("list", "args"): - value = getattr(error, attribute, None) or () - pending.extend(child for child in value if isinstance(child, BaseException)) - return None - - @pytest.fixture def mat_prov(): return AwsCryptographicMaterialProviders(config=MaterialProvidersConfig()) @@ -309,28 +264,15 @@ def toggles(): # End to end: a real Hierarchical Keyring, with the Cache Guard as its CacheTypeShared cache. -def encrypt_cache_id(partition_id, logical_key_store_name, branch_key_id): - """aws-kms-hierarchical-keyring.md, Appendix A, Encryption Materials.""" - return hashlib.sha384( - b"\x02\x00\x01\x00" + partition_id.encode() + b"\x00" + logical_key_store_name.encode() - + b"\x00" + branch_key_id.encode() - ).digest() - - -def decrypt_cache_id(partition_id, logical_key_store_name, branch_key_id, version): - """aws-kms-hierarchical-keyring.md, Appendix A, Decryption Materials.""" - return hashlib.sha384( - b"\x02\x00\x02\x00" + partition_id.encode() + b"\x00" + logical_key_store_name.encode() - + b"\x00" + branch_key_id.encode() + b"\x00" + version.encode() - ).digest() - - @pytest.fixture def remote_calls(): """Count every DynamoDB and KMS request the keyring makes; the tests assert there are none.""" ddb = boto3.client("dynamodb", region_name="us-west-2", aws_access_key_id="AKIAFAKE", aws_secret_access_key="x") ddb_calls = [] - ddb.meta.events.register("before-call.dynamodb", lambda **kwargs: ddb_calls.append(kwargs["model"].name)) + ddb.meta.events.register( + # provide-client-params: the first event, before the Stubber rejects an unstubbed call + "provide-client-params.dynamodb", lambda **kwargs: ddb_calls.append(kwargs["model"].name) + ) transport = KmsTransport() kms = fake_kms.install(transport) with Stubber(ddb): # no responses queued: a DynamoDB call fails, and is counted diff --git a/examples/test/multi_tenant/test_ddblocal_hierarchical.py b/examples/test/multi_tenant/test_ddblocal_hierarchical.py index f7a1b8f66..3370f6667 100644 --- a/examples/test/multi_tenant/test_ddblocal_hierarchical.py +++ b/examples/test/multi_tenant/test_ddblocal_hierarchical.py @@ -27,6 +27,7 @@ ) from aws_cryptographic_material_providers.mpl import AwsCryptographicMaterialProviders from aws_cryptographic_material_providers.mpl.config import MaterialProvidersConfig +from aws_cryptographic_material_providers.mpl.errors import OpaqueWithTextError from aws_cryptographic_material_providers.mpl.models import CreateAwsKmsHierarchicalKeyringInput from botocore.credentials import Credentials @@ -35,8 +36,10 @@ from ...src import multi_tenant_hierarchical_keyring_example as example from ...src.multi_tenant import KmsTransport, TenantKmsClient +from ...src.multi_tenant.tenant_keyring_factory import TenantKeyringFactory from ..branch_key_cleanup import cleanup_branch_keys, delete_branch_key from . import fake_kms +from .helpers import FakeClock pytestmark = [pytest.mark.ddblocal] @@ -206,3 +209,65 @@ def test_without_negative_caching_every_denied_attempt_calls_dynamodb_and_kms(dd assert [(r["operation"], r["access_key"]) for r in fake.requests] == [("Decrypt", "AKIATENANTB")] * 3 finally: assert delete_branch_key(client, table, branch_key_id) is True + + +def test_negative_caching_end_to_end(ddb, kms, table): # pylint: disable=too-many-locals + """Negative caching, end to end (negative_caching.md). + + Denied on a real cache miss, the Denial is recorded, enforced with no DynamoDB or KMS calls, + and retried once the Negative TTL expires. + """ + client, calls = ddb + transport, fake = kms + branch_key_id = key_store(client, table, transport, TENANT_A, KEY_A).create_key( + input=CreateKeyInput() + ).branch_key_identifier + clock = FakeClock() + factory = TenantKeyringFactory( + transport=transport, + ddb_client=client, + ddb_table_name=table, + logical_key_store_name=table, + kms_configuration=KMSConfigurationKmsKeyArn(value=KEY_A), + region_name=REGION, + negative_ttl=60, + clock=clock, + ) + esdk = aws_encryption_sdk.EncryptionSDKClient(commitment_policy=CommitmentPolicy.REQUIRE_ENCRYPT_REQUIRE_DECRYPT) + fake.requests.clear() + calls.clear() + try: + # Tenant B, misconfigured with tenant A's Branch Key: the fake KMS denies tenant B KEY_A. + denied = factory.keyring_for("tenant-b", branch_key_id, TENANT_B) + + # 1. The first request misses the cache: DynamoDB GetItem, then kms:Decrypt, which is Denied. + with pytest.raises(Exception, match="AccessDeniedException"): # pylint: disable=broad-except + esdk.encrypt(source=b"tenant data", keyring=denied) + assert calls["GetItem"] == 1 and [r["operation"] for r in fake.requests] == ["Decrypt"] + negative = factory.negative_cache_for("tenant-b", branch_key_id) + assert negative.get().error.response["Error"]["Code"] == "AccessDeniedException" + + # 2. Every later request, through any keyring for the Tenant and Branch Key ID, fails without a call. + for keyring in (denied, factory.keyring_for("tenant-b", branch_key_id, TENANT_B)): + with pytest.raises(OpaqueWithTextError) as excinfo: + esdk.encrypt(source=b"tenant data", keyring=keyring) + assert excinfo.value.obj is negative.get().error + assert calls["GetItem"] == 1 and len(fake.requests) == 1 + + # 3. Once the Negative TTL expires, one request is retried; still Denied, and recorded again. + clock.now += 60 + with pytest.raises(Exception, match="AccessDeniedException"): # pylint: disable=broad-except + esdk.encrypt(source=b"tenant data", keyring=denied) + assert calls["GetItem"] == 2 and len(fake.requests) == 2 + assert negative.get() is not None + + # 4. The Tenant that owns the Branch Key is unaffected (Requirement 2, and per Tenant). + owner = factory.keyring_for("tenant-a", branch_key_id, TENANT_A) + ciphertext, _ = esdk.encrypt(source=b"tenant data", keyring=owner) + assert esdk.decrypt(source=ciphertext, keyring=owner)[0] == b"tenant data" + assert [(r["operation"], r["access_key"]) for r in fake.requests[2:]] == [ + ("Decrypt", "AKIATENANTA"), + ("Decrypt", "AKIATENANTA"), + ] + finally: + assert delete_branch_key(client, table, branch_key_id) is True diff --git a/examples/test/multi_tenant/test_tenant_keyring_factory.py b/examples/test/multi_tenant/test_tenant_keyring_factory.py new file mode 100644 index 000000000..60e7dea21 --- /dev/null +++ b/examples/test/multi_tenant/test_tenant_keyring_factory.py @@ -0,0 +1,282 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Offline tests for the Tenant Keyring Factory (negative_caching.md, Design Question 7.2). + +The end-to-end tests seed the shared cache under the specification's cache identifiers, +with the Partition ID the factory derives from the Tenant, so vended keyrings encrypt and decrypt from cache hits; +any call to Amazon DynamoDB or AWS KMS is counted, and asserted to be zero. +""" +# pylint: disable=redefined-outer-name +import threading +import uuid + +import boto3 +import pytest +from aws_cryptographic_material_providers.keystore.models import KMSConfigurationKmsKeyArn +from aws_cryptographic_material_providers.mpl.errors import OpaqueWithTextError +from aws_cryptographic_material_providers.mpl.models import StormTrackingCache, TimeUnits +from aws_cryptographic_material_providers.mpl.references import ICryptographicMaterialsCache +from botocore.credentials import Credentials +from botocore.stub import Stubber + +import aws_encryption_sdk +from aws_encryption_sdk import CommitmentPolicy + +from ...src.multi_tenant import KmsTransport +from ...src.multi_tenant.negative_cache import BranchKeyNegativeCache +from ...src.multi_tenant.tenant_keyring_factory import ( + DEFAULT_TTL_SECONDS, + TenantKeyringFactory, + default_storm_tracking_cache, + tenant_partition_id, +) +from . import fake_kms +from .helpers import FakeClock, kms_error +from .keyring_helpers import branch_key_materials, decrypt_cache_id, encrypt_cache_id, put_input + +# Offline: no network access, so these also run as local tests (tox: mplexampleslocal). +pytestmark = [pytest.mark.examples, pytest.mark.local] + +KMS_KEY = "arn:aws:kms:us-west-2:111122223333:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" +TABLE = "KeyStoreTable" +TENANT_A = Credentials("AKIATENANTA", "secret-a") +TENANT_B = Credentials("AKIATENANTB", "secret-b") + + +# tenant_partition_id (negative_caching.md 8) + + +def test_partition_id_is_the_tenant_id_as_is(): + # Not a secret, so not digested (negative_caching.md 8); stable across credential refreshes. + assert tenant_partition_id("tenant-a") == "tenant-a" + assert tenant_partition_id("tenant-a") != tenant_partition_id("tenant-b") + + +def test_partition_id_rejects_an_empty_tenant(): + with pytest.raises(ValueError): + tenant_partition_id("") + + +# Construction + + +@pytest.fixture +def remote_calls(): + """Count every DynamoDB and KMS request; the end-to-end tests assert there are none.""" + ddb = boto3.client("dynamodb", region_name="us-west-2", aws_access_key_id="AKIAFAKE", aws_secret_access_key="x") + ddb_calls = [] + ddb.meta.events.register( + # provide-client-params: the first event, before the Stubber rejects an unstubbed call + "provide-client-params.dynamodb", lambda **kwargs: ddb_calls.append(kwargs["model"].name) + ) + transport = KmsTransport() + kms = fake_kms.install(transport) + with Stubber(ddb): # no responses queued: a DynamoDB call fails, and is counted + yield {"ddb": ddb, "ddb_calls": ddb_calls, "transport": transport, "kms": kms} + + +@pytest.fixture +def clock(): + return FakeClock() + + +def factory_arguments(remote_calls, clock, overrides=None): + arguments = { + "transport": remote_calls["transport"], + "ddb_client": remote_calls["ddb"], + "ddb_table_name": TABLE, + "logical_key_store_name": TABLE, + "kms_configuration": KMSConfigurationKmsKeyArn(value=KMS_KEY), + "region_name": "us-west-2", + "negative_ttl": 60, + "clock": clock, + } + # A dict, not **overrides: an override named like a parameter here (clock) + # would make this function raise TypeError itself, and pass a test expecting one. + arguments.update(overrides or {}) + return arguments + + +@pytest.fixture +def factory(remote_calls, clock): + return TenantKeyringFactory(**factory_arguments(remote_calls, clock)) + + +def test_defaults(remote_calls): + arguments = factory_arguments(remote_calls, None) + del arguments["negative_ttl"], arguments["clock"] + factory = TenantKeyringFactory(**arguments) + assert factory.negative_ttl == 300 + assert DEFAULT_TTL_SECONDS == 600 + + +def test_default_storm_tracking_cache_settings(): + # The values, and why, are design.md 10.1.1. + settings = default_storm_tracking_cache() + assert isinstance(settings, StormTrackingCache) + assert (settings.entry_capacity, settings.entry_pruning_tail_size) == (1000, 1) + assert (settings.grace_period, settings.grace_interval) == (10_000, 1_000) + assert (settings.fan_out, settings.in_flight_ttl, settings.sleep_milli) == (100, 5_000, 20) + assert settings.time_units == TimeUnits.MILLISECONDS + assert default_storm_tracking_cache() is not settings # a new object; callers may change theirs + + +def test_the_factory_creates_and_owns_one_shared_cache(factory): + shared_cache = factory.shared_cache + assert isinstance(shared_cache, ICryptographicMaterialsCache) + assert factory.shared_cache is shared_cache # one cache, created once + + +def test_custom_storm_tracking_cache_settings_are_accepted(remote_calls, clock): + settings = default_storm_tracking_cache() + settings.fan_out = 7 + factory = TenantKeyringFactory(**factory_arguments(remote_calls, clock, {"storm_tracking_cache": settings})) + assert isinstance(factory.shared_cache, ICryptographicMaterialsCache) + + +def test_grace_period_just_under_the_cache_limit_ttl_is_accepted(remote_calls, clock): + # The default grace period is 10,000 milliseconds: 10 seconds. + TenantKeyringFactory(**factory_arguments(remote_calls, clock, {"ttl_seconds": 11})) + + +def _seconds_storm(grace_period): + settings = default_storm_tracking_cache() + settings.time_units = TimeUnits.SECONDS + settings.grace_period, settings.grace_interval, settings.in_flight_ttl = grace_period, 1, 1 + return settings + + +@pytest.mark.parametrize("overrides, error", [ + ({"transport": object()}, TypeError), + ({"storm_tracking_cache": object()}, TypeError), + # The specification: the grace period MUST be less than the cache limit TTL. + ({"ttl_seconds": 10}, ValueError), # equal: 10,000 milliseconds is 10 seconds + ({"ttl_seconds": 600, "storm_tracking_cache": _seconds_storm(600)}, ValueError), # equal, in seconds + ({"ttl_seconds": 600, "storm_tracking_cache": _seconds_storm(700)}, ValueError), + ({"ddb_table_name": ""}, ValueError), + ({"logical_key_store_name": ""}, ValueError), + ({"region_name": "evil.example.com"}, ValueError), + ({"ttl_seconds": 0}, ValueError), + ({"negative_ttl": 0}, ValueError), +]) +def test_invalid_construction(remote_calls, clock, overrides, error): + with pytest.raises(error): + TenantKeyringFactory(**factory_arguments(remote_calls, clock, overrides)) + + +# negative_cache_for + + +def test_negative_cache_is_one_object_per_tenant_and_branch_key_id(factory): + negative = factory.negative_cache_for("tenant-a", "bk-a") + assert isinstance(negative, BranchKeyNegativeCache) + assert negative.branch_key_id == "bk-a" + assert negative.negative_ttl == 60 + assert factory.negative_cache_for("tenant-a", "bk-a") is negative + assert factory.negative_cache_for("tenant-a", "bk-b") is not negative + # Keyed by Tenant too: a misconfigured Tenant's Denial MUST NOT deny the Branch Key ID to its owner. + assert factory.negative_cache_for("tenant-b", "bk-a") is not negative + + +def test_negative_caches_use_the_factory_clock(factory, clock): + negative = factory.negative_cache_for("tenant-a", "bk-a") + negative.record(kms_error()) + clock.now += 60 + assert negative.get() is None + + +def test_negative_cache_for_is_thread_safe(factory): + # Requirement 8: concurrent requests for one Tenant and Branch Key ID MUST share one negative cache. + found, start = [], threading.Barrier(16) + + def lookup(): + start.wait() + found.append(factory.negative_cache_for("tenant-a", "bk-a")) + + threads = [threading.Thread(target=lookup) for _ in range(16)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + assert len({id(negative) for negative in found}) == 1 + + +# keyring_for, end to end: vended keyrings encrypt and decrypt from the shared cache. + + +def seed(shared_cache, tenant_id, branch_key_id): + """Put a Branch Key in the shared cache under the Tenant's Partition ID; returns its materials.""" + version = str(uuid.uuid4()) + materials = branch_key_materials(branch_key_id, version) + partition_id = tenant_partition_id(tenant_id) + shared_cache.put_cache_entry(put_input(encrypt_cache_id(partition_id, TABLE, branch_key_id), materials)) + shared_cache.put_cache_entry(put_input(decrypt_cache_id(partition_id, TABLE, branch_key_id, version), materials)) + return materials + + +@pytest.fixture +def esdk(): + return aws_encryption_sdk.EncryptionSDKClient(commitment_policy=CommitmentPolicy.REQUIRE_ENCRYPT_REQUIRE_DECRYPT) + + +def assert_no_remote_calls(remote_calls): + assert remote_calls["ddb_calls"] == [] + assert remote_calls["kms"].requests == [] + + +def test_vended_keyring_uses_the_shared_cache_under_the_tenant_partition(factory, remote_calls, esdk): + seed(factory.shared_cache, "tenant-a", "bk-a") + keyring = factory.keyring_for("tenant-a", "bk-a", TENANT_A) + ciphertext, _ = esdk.encrypt(source=b"tenant data", keyring=keyring) + plaintext, _ = esdk.decrypt(source=ciphertext, keyring=keyring) + assert plaintext == b"tenant data" + assert_no_remote_calls(remote_calls) + + +def test_vended_keyring_is_guarded_by_its_negative_cache(factory, remote_calls, esdk): + seed(factory.shared_cache, "tenant-a", "bk-a") + keyring = factory.keyring_for("tenant-a", "bk-a", TENANT_A) + error = kms_error("AccessDeniedException") + factory.negative_cache_for("tenant-a", "bk-a").record(error) + with pytest.raises(OpaqueWithTextError) as excinfo: + esdk.encrypt(source=b"tenant data", keyring=keyring) + assert excinfo.value.obj is error + assert_no_remote_calls(remote_calls) + + +def test_keyrings_vended_for_one_tenant_and_branch_key_share_a_denial(factory, remote_calls, esdk): + seed(factory.shared_cache, "tenant-a", "bk-a") + first = factory.keyring_for("tenant-a", "bk-a", TENANT_A) + second = factory.keyring_for("tenant-a", "bk-a", TENANT_A) + factory.negative_cache_for("tenant-a", "bk-a").record(kms_error()) + for keyring in (first, second): + with pytest.raises(OpaqueWithTextError): + esdk.encrypt(source=b"tenant data", keyring=keyring) + assert_no_remote_calls(remote_calls) + + +def test_a_denial_is_isolated_to_its_tenant_and_branch_key(factory, remote_calls, esdk): + # Requirement 2, per Branch Key ID; and per Tenant (see negative_cache_for). + seed(factory.shared_cache, "tenant-a", "bk-a") + seed(factory.shared_cache, "tenant-a", "bk-other") + seed(factory.shared_cache, "tenant-b", "bk-a") + factory.negative_cache_for("tenant-b", "bk-a").record(kms_error()) # tenant B, misconfigured with bk-a + for tenant_id, branch_key_id, credentials in ( + ("tenant-a", "bk-a", TENANT_A), + ("tenant-a", "bk-other", TENANT_A), + ): + keyring = factory.keyring_for(tenant_id, branch_key_id, credentials) + ciphertext, _ = esdk.encrypt(source=b"tenant data", keyring=keyring) + assert esdk.decrypt(source=ciphertext, keyring=keyring)[0] == b"tenant data" + with pytest.raises(OpaqueWithTextError): + esdk.encrypt(source=b"tenant data", keyring=factory.keyring_for("tenant-b", "bk-a", TENANT_B)) + assert_no_remote_calls(remote_calls) + + +def test_tenants_do_not_share_cache_entries(factory, remote_calls, esdk): + # tenant-b's keyring for bk-a misses tenant-a's entries (another Partition ID), so it calls DynamoDB. + seed(factory.shared_cache, "tenant-a", "bk-a") + with pytest.raises(Exception) as excinfo: # pylint: disable=broad-except + esdk.encrypt(source=b"tenant data", keyring=factory.keyring_for("tenant-b", "bk-a", TENANT_B)) + assert excinfo.value is not None # the stubbed DynamoDB client has no response for it + assert remote_calls["ddb_calls"] == ["GetItem"] diff --git a/examples/test/multi_tenant/test_tenant_kms_client.py b/examples/test/multi_tenant/test_tenant_kms_client.py index b614f44c2..6863ec1e1 100644 --- a/examples/test/multi_tenant/test_tenant_kms_client.py +++ b/examples/test/multi_tenant/test_tenant_kms_client.py @@ -7,6 +7,7 @@ from botocore.credentials import Credentials from ...src.multi_tenant import KmsTransport, TenantKmsClient +from ...src.multi_tenant.negative_cache import DENIAL_ERROR_CODES, BranchKeyNegativeCache from ...src.multi_tenant.tenant_kms_client import METHOD_TO_API_MAPPING from . import fake_kms @@ -136,3 +137,106 @@ def test_invalid_construction(transport, kwargs, error): kwargs.setdefault("transport", transport) with pytest.raises(error): TenantKmsClient(**kwargs) + + +# Recording Denials (negative_caching.md 3.3, 6). + + +@pytest.fixture +def negative(): + return BranchKeyNegativeCache("bk-a") + + +def _guarded_client(transport, credentials, negative): + return TenantKmsClient( + transport=transport, region_name="us-west-2", credentials=credentials, negative_cache=negative + ) + + +def _ciphertext_under(client_a, encryption_context): + response = client_a.encrypt(KeyId=KEY_A, Plaintext=b"branch key", EncryptionContext=encryption_context) + return response["CiphertextBlob"] + + +@pytest.mark.usefixtures("kms") # installs the fake KMS on the transport +def test_a_decrypt_denial_is_recorded_as_the_same_error(transport, client_a, negative): + ciphertext = _ciphertext_under(client_a, {"branch-key-id": "bk-a"}) + client_b = _guarded_client(transport, TENANT_B, negative) # tenant B's credentials cannot use KEY_A + with pytest.raises(botocore.exceptions.ClientError) as excinfo: + client_b.decrypt(CiphertextBlob=ciphertext, EncryptionContext={"branch-key-id": "bk-a"}) + assert excinfo.value.response["Error"]["Code"] == "AccessDeniedException" + assert negative.get() is not None + assert negative.get().error is excinfo.value # recorded, and re-raised, as is + + +@pytest.mark.parametrize("code", sorted(DENIAL_ERROR_CODES)) +def test_every_denial_code_on_decrypt_is_recorded(transport, kms, negative, code): + kms.scripted = [fake_kms.FakeResponse(400, {"__type": "com.amazonaws.kms#" + code, "message": "denied"})] + client = _guarded_client(transport, TENANT_A, negative) + with pytest.raises(botocore.exceptions.ClientError): + client.decrypt(CiphertextBlob=b"x", EncryptionContext={"branch-key-id": "bk-a"}) + assert negative.get().error.response["Error"]["Code"] == code + + +@pytest.mark.parametrize("failure", [ + fake_kms.FakeResponse(400, {"__type": "ThrottlingException", "message": "slow down"}), + fake_kms.FakeResponse(500, {"__type": "KMSInternalException", "message": "oops"}), +]) +def test_transient_failures_are_not_recorded(transport, kms, negative, failure): + kms.scripted = [failure] * 3 # every attempt + client = _guarded_client(transport, TENANT_A, negative) + with pytest.raises(botocore.exceptions.ClientError): + client.decrypt(CiphertextBlob=b"x", EncryptionContext={"branch-key-id": "bk-a"}) + assert negative.get() is None + + +@pytest.mark.parametrize("method, params", [ + ("generate_data_key", {"KeyId": KEY_A, "NumberOfBytes": 32}), + ("generate_data_key_without_plaintext", {"KeyId": KEY_A, "NumberOfBytes": 32}), + ("encrypt", {"KeyId": KEY_A, "Plaintext": b"x"}), + ("re_encrypt", {"CiphertextBlob": b"x", "DestinationKeyId": KEY_A}), +]) +def test_denials_of_other_operations_are_not_recorded(transport, kms, negative, method, params): + # A Denial is a failure to decrypt a Branch Key; creating or versioning one is not that. + kms.scripted = [fake_kms.FakeResponse(400, {"__type": "AccessDeniedException", "message": "denied"})] + client = _guarded_client(transport, TENANT_A, negative) + with pytest.raises(botocore.exceptions.ClientError): + getattr(client, method)(**params) + assert negative.get() is None + + +def test_a_denial_of_another_branch_key_id_is_not_recorded(transport, kms, negative): + kms.scripted = [fake_kms.FakeResponse(400, {"__type": "AccessDeniedException", "message": "denied"})] + client = _guarded_client(transport, TENANT_A, negative) + with pytest.raises(botocore.exceptions.ClientError): + client.decrypt(CiphertextBlob=b"x", EncryptionContext={"branch-key-id": "bk-other"}) + assert negative.get() is None + + +@pytest.mark.parametrize("encryption_context", [None, {}, {"aws-crypto-ec:department": "admin"}]) +def test_a_denial_without_a_branch_key_id_in_the_encryption_context_is_recorded( + transport, kms, negative, encryption_context +): + # The AWS KMS Key Store always includes branch-key-id (hierarchy version 1, the only one shipped); + # without it, the Denial is attributed to the Branch Key ID the client is bound to. + kms.scripted = [fake_kms.FakeResponse(400, {"__type": "AccessDeniedException", "message": "denied"})] + client = _guarded_client(transport, TENANT_A, negative) + params = {"CiphertextBlob": b"x"} + if encryption_context is not None: + params["EncryptionContext"] = encryption_context + with pytest.raises(botocore.exceptions.ClientError): + client.decrypt(**params) + assert negative.get() is not None + + +def test_without_a_negative_cache_nothing_is_recorded_and_the_error_is_unchanged(kms, client_a): + kms.scripted = [fake_kms.FakeResponse(400, {"__type": "AccessDeniedException", "message": "denied"})] + assert client_a.negative_cache is None + with pytest.raises(botocore.exceptions.ClientError) as excinfo: + client_a.decrypt(CiphertextBlob=b"x") + assert excinfo.value.response["Error"]["Code"] == "AccessDeniedException" + + +def test_negative_cache_must_be_a_branch_key_negative_cache(transport): + with pytest.raises(TypeError): + TenantKmsClient(transport=transport, region_name="us-west-2", credentials=TENANT_A, negative_cache=object())