Skip to content

docs: specify how caching CMM should handle max plaintext length in Get Encryption Materials #159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

docs: specify how caching CMM should handle max plaintext length in Get Encryption Materials #159

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7c824aa
doc: specify how caching CMM should handle max plaintext length in Ge…
alex-chew Jul 15, 2020
524f7a1
Apply copy edits from code review
alex-chew Jul 16, 2020
89024b3
Clarify unsafe max plaintext length example
alex-chew Jul 16, 2020
82ee1e9
Merge branch 'master' into caching-cmm-max-plaintext-length
alex-chew Jul 20, 2020
860226a
Merge branch 'master' into caching-cmm-max-plaintext-length
alex-chew Jul 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions changes/2020-07-13_caching-cmm-max-plaintext-length/change.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
[//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved."
[//]: # "SPDX-License-Identifier: CC-BY-SA-4.0"

# Enforce Safe Handling of Max Plaintext Length in Caching Cryptographic Materials Manager

## Affected Features

| Feature |
| ------------------------------------------------------------------------- |
| [Caching Cryptographic Materials Manager](../../framework/caching-cmm.md) |

## Affected Specifications

| Specification |
| ------------------------------------------------------------------------- |
| [Caching Cryptographic Materials Manager](../../framework/caching-cmm.md) |

## Affected Implementations

| Language | Repository |
| ---------- | ------------------------------------------------------------------------------------- |
| C | [aws-encryption-sdk-c](https://github.com/aws/aws-encryption-sdk-c) |
| Java | [aws-encryption-sdk-java](https://github.com/aws/aws-encryption-sdk-java) |
| JavaScript | [aws-encryption-sdk-javascript](https://github.com/aws/aws-encryption-sdk-javascript) |
| Python | [aws-encryption-sdk-python](https://github.com/aws/aws-encryption-sdk-python) |

## 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).

## Summary

The Get Encryption Materials operation accepts an optional `max plaintext length` parameter,
but the specification does not state how the caching CMM should behave
when the caller does not provide the parameter value.
This change specifies that the caching CMM should bypass the cache in this case.

Also, when the caching CMM is performing a Get Encryption Materials operation
for which no materials are cached,
it MUST call its underlying CMM's Get Encryption Materials operation.
The specification does not state what value of `max plaintext length` (if any)
the caching CMM should pass to its underlying CMM.
This change specifies that the caching CMM should pass its `byte limit` value
as the `max plaintext length` parameter of the call to the underlying CMM.

## Out of Scope

- Changing the shape of any CMM APIs is out of scope.

## Motivation

### Fulfulling Encryption Materials Requests Without `max plaintext length`

The Get Encryption Materials operation of the Cryptographic Materials Manager (CMM)
accepts an Encryption Materials Request that MAY specify a `max plaintext length` field,
indicating the maximum length of plaintext
that the caller intends to encrypt using the returned materials.
The caching Cryptographic Materials Manager (caching CMM)
aims to prevent its users from using encryption materials
to encrypt more bytes than its configured `byte limit`,
and existing implementations do so by bypassing the cache
whenever it does not know their users' `max plaintext length`.
This behavior is safe but previously unspecified,
and this change mandates that implementations implement it.

### Setting `max plaintext length` for Underlying CMM's Get Encryption Materials

The CMM interface specification implicitly allows an underlying CMM
to produce encryption materials using logic
that depends on the `max plaintext length` of its Encryption Materials Request.
Suppose the caching CMM passes a `max plaintext length` value of `N`
to its underlying CMM's Get Encryption Materials operation,
and caches the materials that it receives.
If a later Get Encryption Materials operation fetches those materials from the cache,
but has a `max plaintext length` value that exceeds `N`,
then the caching CMM returns those materials unsafely.
Copy link
Contributor

@lavaleri lavaleri Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we mean by "unsafely"? We should say exactly what we mean here. By passing N to the underlying CMM, this CMM is signaling to the underlying CMM that it intends to only use the returned materials to encrypt up to N bytes. By then reusing those materials such that when it reuses the materials it encrypts plaintext with length exceeding N, this CMM is violating it's previous intent. This violation is why this supposed approach is bad.

Copy link
Contributor

@seebees seebees Jul 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an edge case here is 2 CMMs share a CMC (and entries via partitions) but have different max bytes?
If I'm right, is this something to worry about? I don't think that the byte limit passed to the CMM is include in the entry, perhaps it should?

Say:

CMM A max bytes = 100K
CMM B max bytes = 200K

CMM A puts in an entry but then will never touch it again.
CMM B will use the entry to exhaustion.

Copy link
Contributor Author

@alex-chew alex-chew Jul 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lavaleri thanks, I've incorporated your wording to clarify the explanation.

After a discussion with @lavaleri, @seebees, and @mattsb42-aws offline, we think that the edge case that @seebees described requires more thought to resolve properly, and doing so may require changes to other parts of this PR. So we are moving the PR to a later milestone.


It is therefore important that the caching CMM
carefully selects the `max plaintext length` parameter to pass to its underlying CMM.
This change mandates that the caching CMM passes its configured `byte limit` value
as the `max plaintext length` parameter to the underlying CMM,
whereas previously, the caching CMM could choose any value.
With this change in place,
even if the underlying CMM produces materials using plaintext-length-dependent logic,
the caching CMM receives and caches materials
that are safe to use for up to `byte limit` bytes of plaintext.
It follows that if a later Get Encryption Materials operation's `max plaintext length`
does not exceed the caching CMM's `byte limit`,
then it is also safe for the caching CMM to return corresponding cached materials.

## Drawbacks

Bypassing the cache in the caching CMM on Get Encryption Materials operations
that do not specify `max plaintext length`
could be confusing to the user.
Warning the user when this happens could help prevent users from running into
unexpected (and potentially expensive) calls to the underlying CMM,
but we leave this as an implementation choice.

## Security Implications

This change SHOULD NOT have any security implications.
Copy link
Member

@mattsb42-aws mattsb42-aws Jul 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because our current implementations do not pass a max plaintext length value to the underlying CMM, and this is changing to now pass the byte limit value instead, the security implications for the value received from the underlying CMM are changing. This is both a security and operational change, as the underlying CMM could be making security decisions based on the max plaintext length value (or absence); for example if it is another caching CMM.


## Operational Implications

This change will break any customer use case
that depends on the Caching CMM passing no `max plaintext length` parameter
to the underlying CMM on Get Encryption Materials calls.
Comment on lines +118 to +120
Copy link
Member

@mattsb42-aws mattsb42-aws Jul 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good call-out, but you don't actually mention anywhere that this is what implementations currently do.


## Guide-level Explanation

When you make a Get Encryption Materials call to the caching CMM,
you may optionally specify a value for the `max plaintext length` parameter,
which indicates the maximum number of bytes you will encrypt
with the materials you receive from the call.
If you do not specify a `max plaintext length` value,
then the caching CMM will neither use cached materials
nor cache materials from its underlying CMM.
This helps to ensure that you do not exceed the `byte limit`
that you configure on the caching CMM.
So in order to maximize the benefits of caching,
you should specify the `max plaintext length` value
whenever you know an appropriate upper bound for bytes you will encrypt.

If you customize the underlying CMM of a caching CMM,
then whenever the caching CMM makes a Get Encryption Materials call
to your customized underlying CMM,
the Encryption Materials Request will have a `max plaintext length` value
equal to the `byte limit` value configured on the caching CMM.
This helps to ensure that the caching CMM can safely cache materials
that it receives from the customized underlying CMM.

## Reference-level Explanation

When the caching CMM is performing the Get Encryption Materials operation
for an Encryption Materials Request that does not specify a `max plaintext length`,

- The caching CMM MUST NOT return materials from its CMC.
- If the caching CMM calls its underlying CMM's Get Encryption Materials operation
in order to obtain encryption materials,
it MUST NOT cache the encryption materials in its CMC.

When the caching CMM calls its underlying CMM's Get Encryption Materials operation
in order to obtain encryption materials,
it MUST use its configured `byte limit` value
as the `max plaintext length` field of the Encryption Materials Request
that it sends to the underlying CMM.
28 changes: 22 additions & 6 deletions framework/caching-cmm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,24 @@

## Version

0.1.0-preview
0.2.0

### Changelog

- 0.2.0
- [Enforce Safe Handling of Max Plaintext Length in Caching Cryptographic Materials Manager](../changes/2020-07-13_caching-cmm-max-plaintext-length/change.md)
- 0.1.0-preview
- Initial record

## Implementations

- [C](https://github.com/aws/aws-encryption-sdk-c/blob/master/include/aws/cryptosdk/cache.h)
- [Python](https://github.com/aws/aws-encryption-sdk-python/blob/master/src/aws_encryption_sdk/materials_managers/caching.py)
- [Java](https://github.com/aws/aws-encryption-sdk-java/blob/master/src/main/java/com/amazonaws/encryptionsdk/caching/CachingCryptoMaterialsManager.java)
- [NodeJS](https://github.com/awslabs/aws-encryption-sdk-javascript/blob/master/modules/caching-materials-manager-node/src/caching_materials_manager_node.ts)
- [Browser JS](https://github.com/awslabs/aws-encryption-sdk-javascript/blob/master/modules/caching-materials-manager-browser/src/caching_materials_manager_browser.ts)
| Language | Confirmed Compatible with Spec Version | Minimum Version Confirmed | Implementation |
| ---------- | -------------------------------------- | ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| C | 0.1.0-preview | 0.1.0 | [cache.h](https://github.com/aws/aws-encryption-sdk-c/blob/master/include/aws/cryptosdk/cache.h) |
| NodeJS | 0.1.0-preview | 0.1.0 | [caching_materials_manager_node.ts](https://github.com/awslabs/aws-encryption-sdk-javascript/blob/master/modules/caching-materials-manager-node/src/caching_materials_manager_node.ts) |
| Browser JS | 0.1.0-preview | 0.1.0 | [caching_materials_manager_browser.ts](https://github.com/awslabs/aws-encryption-sdk-javascript/blob/master/modules/caching-materials-manager-browser/src/caching_materials_manager_browser.ts) |
| Python | 0.1.0-preview | 1.3.0 | [materials_managers/caching.py](https://github.com/aws/aws-encryption-sdk-python/blob/master/src/aws_encryption_sdk/materials_managers/caching.py) |
| Java | 0.1.0-preview | 1.3.0 | [CachingCryptoMaterialsManager.java](https://github.com/aws/aws-encryption-sdk-java/blob/master/src/main/java/com/amazonaws/encryptionsdk/caching/CachingCryptoMaterialsManager.java) |
Copy link
Contributor

@acioc acioc Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure I understand this, is 1.3.0 the version of the ESDK? So Python and Java are on 1.3.0 while C and JS are both on 0.1.0?

Copy link
Contributor Author

@alex-chew alex-chew Jul 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, these lines state that the Python ESDK version 1.3.0 is the minimum version that is confirmed to be compatible with spec version 0.1.0-preview, and likewise for the Java ESDK. Similarly, the C ESDK version 0.1.0 is the minimum version that is confirmed to be compatible with spec 0.1.0-preview, and likewise for the JS ESDKs.


## Overview

Expand Down Expand Up @@ -113,6 +122,7 @@ The number of bytes encrypted by the [encryption](structures.md#encryption-mater
### Get Encryption Materials

If the [algorithm suite](algorithm-suites.md) requested contains a [Identity KDF](algorithm-suites.md#identity-kdf),
or if the request does not include a [Max Plaintext Length](cmm-interface.md#encryption-materials-request) value,
the caching CMM MUST obtain the encryption materials by making a call to the underlying CMM's [Get Encryption Materials](cmm-interface.md#get-encryption-materials) function.

Otherwise, the caching CMM MUST attempt to find the [encryption materials](structures.md#encryption-materials)
Expand All @@ -122,10 +132,16 @@ If a cache entry is found, the caching CMM MUST return the encryption materials
If a cache entry is not found, the caching CMM MUST then attempt to obtain the encryption materials
by making a call to the underlying CMM's [Get Encryption Materials](cmm-interface.md#get-encryption-materials).

If the caching CMM makes a call to the underlying CMM's [Get Encryption Materials](cmm-interface.md#get-encryption-materials) operation,
then it MUST include a [Max Plaintext Length](cmm-interface.md#encryption-materials-request) value,
which MUST be equal to its [Limit Bytes](#limit-bytes) value.

If the [algorithm suite](algorithm-suites.md) requested does not contain an [Identity KDF](algorithm-suites.md#identity-kdf),
and if the request includes a [Max Plaintext Length](cmm-interface.md#encryption-materials-request) value,
the caching CMM MUST add the encryption materials obtained from the underlying CMM into the underlying CMC.

If the [algorithm suite](algorithm-suites.md) requested contains an Identity KDF,
or if the request does not include a [Max Plaintext Length](cmm-interface.md#encryption-materials-request) value,
the caching CMM MUST NOT store the encryption materials in the underlying CMC.

### Decrypt Materials
Expand Down