Skip to content
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

Jump to bottom

Add proposal for adding Keyring CMM and updating Default CMM definition #220

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

Add proposal for adding Keyring CMM and updating Default CMM definition #220

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
92777bb
Add proposal for adding Keyring CMM and updating Default CMM definition
lavaleri Jul 29, 2020
b4b170a
minor edits
lavaleri Jul 29, 2020
35c4b7e
PR feedback
lavaleri Jul 30, 2020
02ecf22
Apply suggestions from code review
lavaleri Jul 31, 2020
4b41095
Minor fix
lavaleri Jul 31, 2020
682ca06
Add new CMM impl to set alg suite
lavaleri Aug 7, 2020
9864c96
Merge branch 'master' of github.com:awslabs/aws-encryption-sdk-specif…
lavaleri Aug 7, 2020
e40cf03
Update proposals/2020-07-28_keyring-cmm/proposal.md
lavaleri Aug 7, 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
198 changes: 198 additions & 0 deletions proposals/2020-07-28_keyring-cmm/proposal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
[//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved."
[//]: # "SPDX-License-Identifier: CC-BY-SA-4.0"

# Base CMM

## Affected Features

This serves as a reference of all features that this change affects.

| Feature |
| --------------------------------------------- |
| [Default CMM](../../framework/default-cmm.md) |

## Affected Specifications

This serves as a reference of all specification documents that this change affects.

| Specification |
| --------------------------------------------- |
| [Default CMM](../../framework/default-cmm.md) |

## Affected Implementations

This serves as a reference for all implementations that this change affects.

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

## 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 Default CMM is an implementation of the CMM interface that serves a majority of use cases,
lavaleri marked this conversation as resolved.
Show resolved Hide resolved
using [keyrings](../../framework/keyring-interface.md) or
[master key providers](../../framework/master-key-provider-interface.md) to get and
decrypt materials.

This change proposes that we instead define that implementation under new names,
Keyring CMM and Master Key Provider CMM,
and define the Default CMM to be the specific composition of one or more CMMs
that customers SHOULD use.

## Out of Scope

- The behavior of any existing CMMs implementations.

## Motivation

Prior to this change, the Default CMM serves two purposes.
lavaleri marked this conversation as resolved.
Show resolved Hide resolved
First, it provides a base implementation of a CMM that uses a
keyring or master key provider to ensure valid materials.
Second, it serves as a safe default that can be used for most use cases.

While there is currently no conflict in these purposes,
this might not always be true.
In the future we might want to update the behavior customers get when they choose some "default"
CMM option.

One way to address this would be to directly add the new desired behavior
to the current Default CMM implementation,
however this makes our provided CMMs less composable.
lavaleri marked this conversation as resolved.
Show resolved Hide resolved
Keeping the CMM composable is important because it allows users to assert desired properties
[by construction](../../tenets.md#correct-by-construction).
Because it is important to retain this base implementation,
we should define a CMM whose sole purpose is to be this base implementation.
We will call this CMM implementation the Keyring CMM.
For implementations which need to support master key provider, we will also define a
Master Key Provider CMM.

Similar to how we define a set of algorithm suites and define a default algorithm suite,
we should similarly have a set of provided CMM implementations
and a Default CMM which represents a particular configuration of one of those CMMs
(possibly composed with other CMMs).
We will define that the Default CMM is "the Keyring CMM".
Just as an example, if we wanted to provide caching as a default behavior,
the Default CMM could be defined as "the Caching CMM configured with a Keyring CMM."
By defining the default in this way, we have greater flexibility to update the default
CMM that customers use in the future without losing the composability of these implementations.

Such a default MUST NOT allow any options other than the keyring to be configured in order
to allow us to more easily maintain backwards compatability with possible Default CMM updates.

## Drawbacks

This change SHOULD NOT have any drawbacks.

## Security Implications

This change SHOULD NOT have any security implications.

## Operational Implications

This change SHOULD NOT have any operational implications.

This change MUST maintain behavior for customers who are using a Default CMM.

## Guide-level Explanation

This change defines the [Keyring CMM](#keyring-cmm) and
the [Master Key Provider CMM](#master-key-provider-cmm)
as base CMM implementations that can be used "as is" or composed with other CMM.
lavaleri marked this conversation as resolved.
Show resolved Hide resolved

This change also defines a [Default CMM](#default-cmm) as a specific configuration of a CMM
that the AWS Encryption SDK provides as a safe default.

Customers SHOULD either use the [Default CMM](#default-cmm)
or compose a CMM using the AWS Encryption SDK's provided CMM implementations
(including the Keyring CMM).
Comment on lines +149 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

This implies that customers SHOULD NOT implement their own CMM, which isn't what we want. Perhaps we should say that most customers should use one of these two options, but implementing their own is also possible, and if they do they will probably want to compose it with other provided CMM implementations.


### Default CMM

The Default CMM is a specific configuration of a
CMM implementation provided by the AWS Encryption SDK.

The specific CMM configuration describes a safe default that serves most use cases.
lavaleri marked this conversation as resolved.
Show resolved Hide resolved

The CMM configuration defined by the Default CMM is the [Keyring CMM](#keyring-cmm) as is
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the Keyring CMM take an option for the default algorithm
and then the Default CMM defines this option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand. If you are suggesting that we be able to initialize the KeyringCMM with a "default alg" option that overrides the default defined in the alg suite, then that would be changing the current behavior of the Default CMM and is out of scope.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was suggesting that the Keyring CMM takes an option for "default alg",
and then when the Keyring CMM is created in the Default CMM,
then the option would be what we define as a default.

So the ESDK default algorithm is defined in the Default CMM and not in the Keyring CMM.

Copy link
Member

Choose a reason for hiding this comment

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

What if the keyring CMM just requires that the request identify the algorithm suite?

I'm loathe to squeeze opinions about algorithm suite into a CMM that SHOULD only care about wrangling keyrings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand what you're proposing now. This would mean that the Default CMM is no longer just a configuration. Unless we also want to move to the "default to the default alg CMM" logic out of the Keyring CMM and into a new CMM.

If we were to do something like this, I agree with Matt's approach, the KeyringCMM should just fail if it doesn't get an alg suite. I'm not sure if splitting up the logic this way is useful or not though.

Copy link
Contributor

Choose a reason for hiding this comment

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

This makes sense, but the keyring CMM should be easy to use on its own.
If customers chose to use only the keyring CMM and inject that into the ESDK, it would not work in the default case.
Maybe that is a good thing though.

What I would like is that the default algorithm suite for the ESDK is defined in the Default CMM, not the keyring CMM.

Copy link
Member

Choose a reason for hiding this comment

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

What I would like is that the default algorithm suite for the ESDK is defined in the Default CMM, not the keyring CMM.

Agreed. If you're opting out of the default then you don't get any of the defaults. I think this is reasonable.

Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity, we do not want to have the Keyring CMM take a REQUIRED option "default alg"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I might be misunderstanding this conversation, but near the end of the doc, we say "This CMM MUST NOT offer any additional features beyond the composed CMM created above." To me, this means the default CMM MAY offer the same configuration options as the thing it is configuring. However, it MAY additionally support sane defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After offline discussion:

  • It is important that the Default CMM MUST represent an expression of default values without accompanying logic
    • This defines all defaults for encryption and decryption in one place, and additionally makes these defaults more extendable for any future code that may use CMMs but is not necessarily our current Encrypt/Decrypt client APIs.
    • By ensuring this is a strict configuration, we ensure that each behavior is encapsulated in a composable CMM which can be reused by customers, making it easy to compose CMMs which opt out of certain default behaviors or reuse default behaviors within another configuration.
    • This also makes updating the default behaviors easy, and able to be done in a way to preserves composability of our behaviors.
  • As such, the default algorithm suite is defined by the Default CMM
    • Customers MUST be able to get this default algorithm suite statically
  • One more behavior of the KeyringCMM in this version of the spec is that it defaults to the default algorithm. This is not desired and this behavior should be encapsulated outside the Keyring CMM implementation.
    • The Keyring CMM MUST require an algorithm suite
  • We should add a new CMM implementation DefaultRequestValues (name TBD) that is solely responsible for setting default values in the CMM's materials requests, and then passes that request to an underlying CMM.
    • For this proposal, this should ONLY be setting some default algorithm suite for the get encryption materials request.
  • The Default CMM should thus represent the composition of this new DefaultRequestValues CMM and a Keyring CMM.
  • We need to add examples for composing CMMs together.

I will update the doc with this motivation and behavior.

(The Keyring CMM provides no additional options other than specifying an underlying Keyring).
lavaleri marked this conversation as resolved.
Show resolved Hide resolved

As the AWS Encryption SDK provides more CMM implementations that would benefit default use cases,
we expect to update the Default CMM to a configuration which composes such CMMs together provide
lavaleri marked this conversation as resolved.
Show resolved Hide resolved
useful properties.
Any update to the composition defined by Default CMM SHOULD be backwards compatable.

To ensure that its use is correct by construction,
the instantiation of the Default CMM MUST NOT take any options
lavaleri marked this conversation as resolved.
Show resolved Hide resolved
except for the keyring that should be composed with the Default CMM.

### Keyring CMM

The Keyring CMM is a base CMM implementation that
[gets encryption materials](../../framework/cmm-interface.md#get-encryption-materials)
and [decrypts materials](../../framework/cmm-interface.md#decrypt-materials)
through use of a [keyring](../../framework/keyring-interface.md) configured on instantiation.

Any desired CMM implementation that intends to use keyrings to ensure valid materials SHOULD
compose with the Keyring CMM.

### Master Key Provider CMM

Any implementation that previously supported
[master key providers](../../framework/master-key-provider-interface.md)
MUST have a Master Key Provider CMM.
Any implementation that does not support master key providers MUST NOT
provide a Master Key Provider CMM.

This CMM SHOULD NOT be used except for backwards compatability reasons.

The Master Key Provider CMM is a base CMM implementation that
[gets encryption materials](../../framework/cmm-interface.md#get-encryption-materials)
and [decrypts materials](../../framework/cmm-interface.md#decrypt-materials)
through use of a [master key provider](../../framework/master-key-provider-interface.md)
configured on instantiation.

Any desired CMM implementation that intends to use master key providers to ensure valid materials
SHOULD compose with the Master Key Provider CMM.

## Reference-level Explanation

### Default CMM

On initialization it MUST accept:

- an underlying [Keyring](../../framework/keyring-interface.md)
lavaleri marked this conversation as resolved.
Show resolved Hide resolved

It MUST NOT take any additional configuration.

It MUST construct a CMM in the following manner:

- Initialize a [Keyring CMM](#keyring-cmm) with the configured underlying Keyring

This CMM MUST NOT offer any additional features beyond the composed CMM created
above.

Functionality exposed by the AWS Encryption SDK MUST NOT require the Default CMM.

### Keyring CMM

The specification of the Keyring CMM MUST be the specification of the
Default CMM prior to this change.

### Master Key Provider CMM

The specification of the Master Key Provider CMM MUST be the specification of
the Default CMM, except describing use of a master key provider instead of a keyring
lavaleri marked this conversation as resolved.
Show resolved Hide resolved
where appropriate.