chore(decrypt): specify behavior on decrypt with encryption context #261
+224
−0
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| [//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved." | ||
| [//]: # "SPDX-License-Identifier: CC-BY-SA-4.0" | ||
|
|
||
| # Which Encryption Context should be used in Decrypt Operations | ||
|
|
||
| ## 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). | ||
|
|
||
| ## Issues and Alternatives | ||
|
|
||
| ### What kind of Encryption Context should Decrypt return? | ||
|
|
||
| With the addition of the [required encryption context cmm](../../framework/required-encryption-context-cmm.md), | ||
| the `encrypt` API is able to filter out encryption context key-value pairs that | ||
| are not stored on the message. | ||
|
|
||
| `decrypt` returns Encryption Context. | ||
| We were able to satisfy this requirement by returning the parsed message header. | ||
| This is no longer adequate as the header may not contain all Encryption Context values used to | ||
| authenticate the message. | ||
|
|
||
| Currently, `decrypt` takes in optional encryption context. | ||
| This optional encryption context MAY not be stored in the message header, but is still authenticated. | ||
|
|
||
| If `decrypt` uses [encryption context to only authenticate](../../client-apis/decrypt.md#encryption-context-to-only-authenticate) | ||
| to successfully decrypt a message, then the encryption context output | ||
| MUST be the union of the encryption context serialized into the message header and | ||
| the [encryption context for authentication only](#encryption-context-to-only-authenticate), if available. | ||
|
|
||
| ### Can you construct message that stores different encryption context than the encryption context returned by a CMM? | ||
|
|
||
| None of the built-in implementations of the [CMM interface](../../framework/cmm-interface.md) | ||
| modify the encryption context on DecryptMaterials. | ||
|
|
||
| However, the CMMs DecryptMaterials is allowed to modify the encryption context on decrypt. | ||
| A poorly designed CMM could theoretically return materials with an "incorrect" encryption context | ||
| that contradicts the encryption context in the header of the message. | ||
|
|
||
| The `decrypt` API SHOULD not check if the encryption context that was stored on the message matches | ||
| what it receives from the CMMs DecryptMaterials because the current implementation | ||
| of the `encrypt` API used with the built-in CMMs is not able to write a message | ||
|
There was a problem hiding this comment. Stronger than built-in. |
||
| that on `decrypt` the CMM returns "incorrect" encryption context that contradicts | ||
| the encryption context stored in the header of the message. | ||
|
|
||
| ### What changes to the existing Required Encryption Context CMM are required? | ||
|
There was a problem hiding this comment. Are there any changes to the CMM interface? |
||
|
|
||
| None | ||
|
|
||
| ### What changes to the existing Cryptographic Materials Manager Interface are required? | ||
|
|
||
| None | ||
|
|
||
| ## Testing | ||
|
There was a problem hiding this comment. Thinking more about this, I think it would be better to more clearly define what the matrix is we care about, as opposed to listing out the different tests and code for them. I would prefer we cut the example code, but make it extremely clear what every case we need to check is in a way that makes it easy to reason that it is complete. For example, maybe frame it like this: First, suppose we have a message where {A:B,C:D} is in the header, and {E:F} is not stored, but is included in the authentication. For this message, go through the following test cases:
For every one of the above cases, attempt to decrypt using both a Default CMM, and a REC CMM with E configured as a "required key". Next, suppose we have a message where {A:B,C:D,E:F} is in the header, and there is no additional non-stored key-values authenticated with the message. For this message, go through the following test cases:
For each of these use a Default CMM. This is more test cases than we currently have, and we may be able to convince ourselves that some of those test cases are not actually useful. However, I would prefer if this document focused on listing the parameters for the test cases in a way that is easy for readers to see whether we are missing any (And if we decide any are not useful, say why). If we do this, I don't think we need to give example code for each test case. |
||
|
|
||
| In order to test that decrypt behaves as expected according to the specification, testing should include the following | ||
| scenarios to accurately reason about the behavior of using the Required Encryption Context CMM. | ||
|
|
||
seebees marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| To test the new encryption context (EC) features | ||
| the following dimensions exists: | ||
|
|
||
| - Stored EC -- EC stored in the header | ||
| - Not stored EC -- EC authenticated to the header but not stored | ||
| - CMM Material EC -- EC on the decryption material | ||
| - Required keys -- Keys for the EC that MAY be only authenticated | ||
|
There was a problem hiding this comment. As described by the CMM in the decryption materials? Otherwise this is "the same" as "Not stored EC" |
||
| - Reproduced EC -- EC passed on decrypt | ||
|
|
||
| Given the EC `{ a: a, b: b }` | ||
| we break this into the following interesting options: | ||
|
|
||
| Stored EC/Not Stored EC | ||
|
|
||
| - `{a: a, b: b}` / `{}` | ||
| - `{a: a}` / `{b: b}` | ||
| - `{}` / `{a: a, b: b}` | ||
|
|
||
| CMM Material/Required Keys | ||
|
There was a problem hiding this comment. This is missing:
|
||
|
|
||
| - `{a: a, b: b}` / `{}` | ||
| - `{a: a, b: b}` / `{a}` | ||
| - `{a: a, b: b}` / `{a,b}` | ||
| - `{a: a, b: c}` / `{a}` | ||
| - `{a: a, b: b}` / `{c}` | ||
|
There was a problem hiding this comment. The specification for decryption materials doesn't allow this case. |
||
|
|
||
| Reproduced EC | ||
|
|
||
| - `{}` | ||
| - `{ a: a }` | ||
| - `{ b: b }` | ||
| - `{ a: a, b: b }` | ||
| - `{ a: c }` | ||
| - `{ b: c }` | ||
| - `{ a: c, b: b }` | ||
| - `{ a: c, b: c }` | ||
| - `{ c: c }` | ||
| - `{ a: a, c: c }` | ||
| - `{ b: b, c: c}` | ||
| - `{ a: a, b: b, c: c }` | ||
|
|
||
| ### Message: `{a: a, b: b}` / `{}` | ||
|
There was a problem hiding this comment. I think we can further wordsmith these tables to simplify. |
||
|
|
||
| | CMM Material/Required Keys → <br/>Reproduced EC ↓ | `{a: a, b: b}` / `{}` | `{a: a, b: b}` / `{a}` | `{a: a, b: b}` / `{a,b}` | `{a: a, b: c}` / `{a}` | `{a: a, b: b}` / `{c}` | | ||
| | ----------------------------------------------------------- | --------------------- | ---------------------- | ------------------------ | ---------------------- | ---------------------- | | ||
| | `{}` | pass | fail | fail | fail | fail | | ||
| | `{ a: a }` | pass | pass | fail | fail | fail | | ||
|
There was a problem hiding this comment. We should expect every column except for the first to always fail during the header authentication step, regardless of the inputted reproduced encryption context. ('a' or 'b' would be incorrectly appended to the header authentication). |
||
| | `{ b: b }` | pass | pass | fail | fail | fail | | ||
| | `{ a: a, b: b }` | pass | pass | pass | fail | fail | | ||
| | `{ a: c }` | fail | fail | fail | fail | fail | | ||
| | `{ b: c }` | fail | fail | fail | fail | fail | | ||
| | `{ a: c, b: b }` | fail | fail | fail | fail | fail | | ||
| | `{ a: c, b: c }` | fail | fail | fail | fail | fail | | ||
| | `{ c: c }` | fail | fail | fail | fail | fail | | ||
| | `{ a: a, c: c }` | fail | fail | fail | fail | fail | | ||
| | `{ b: b, c: c}` | fail | fail | fail | fail | fail | | ||
| | `{ a: a, b: b, c: c }` | fail | fail | fail | fail | fail | | ||
|
|
||
| ### Message: `{a: a}` / `{b: b}` | ||
|
|
||
| | CMM Material/Required Keys → <br/>Reproduced EC ↓ | `{a: a, b: b}` / `{}` | `{a: a, b: b}` / `{a}` | `{a: a, b: b}` / `{a,b}` | `{a: a, b: c}` / `{a}` | `{a: a, b: b}` / `{c}` | | ||
|
There was a problem hiding this comment. I think you actually want the message to be |
||
| | ----------------------------------------------------------- | --------------------- | ---------------------- | ------------------------ | ---------------------- | ---------------------- | | ||
| | `{}` | fail | fail | fail | fail | fail | | ||
|
There was a problem hiding this comment. We may want to represent some of these as N/As. For example, if the reproduced context does not contain It may be easier to reason about this matrix if we represent the obviously wrong values with something like |
||
| | `{ a: a }` | fail | fail | fail | fail | fail | | ||
| | `{ b: b }` | pass | fail | fail | fail | fail | | ||
| | `{ a: a, b: b }` | pass | pass | pass | pass | fail | | ||
| | `{ a: c }` | fail | fail | fail | fail | fail | | ||
| | `{ b: c }` | fail | fail | fail | fail | fail | | ||
| | `{ a: c, b: b }` | fail | fail | fail | fail | fail | | ||
| | `{ a: c, b: c }` | fail | fail | fail | fail | fail | | ||
| | `{ c: c }` | fail | fail | fail | fail | fail | | ||
| | `{ a: a, c: c }` | fail | fail | fail | fail | fail | | ||
| | `{ b: b, c: c}` | fail | fail | fail | fail | fail | | ||
| | `{ a: a, b: b, c: c }` | fail | fail | fail | fail | fail | | ||
|
|
||
| ### Message:`{}` / `{a: a, b: b}` | ||
|
|
||
| | CMM Material/Required Keys → <br/>Reproduced EC ↓ | `{a: a, b: b}` / `{}` | `{a: a, b: b}` / `{a}` | `{a: a, b: b}` / `{a,b}` | `{a: a, b: c}` / `{a}` | `{a: a, b: b}` / `{c}` | | ||
|
There was a problem hiding this comment. Similar to the other tables, I think that {a: a, b: b} / {} is the only column that could ever succeed. |
||
| | ----------------------------------------------------------- | --------------------- | ---------------------- | ------------------------ | ---------------------- | ---------------------- | | ||
| | `{}` | fail | fail | fail | fail | fail | | ||
| | `{ a: a }` | fail | pass | fail | fail | fail | | ||
| | `{ b: b }` | fail | fail | fail | fail | fail | | ||
| | `{ a: a, b: b }` | pass | fail | pass | fail | fail | | ||
| | `{ a: c }` | fail | fail | fail | fail | fail | | ||
| | `{ b: c }` | fail | fail | fail | fail | fail | | ||
| | `{ a: c, b: b }` | fail | fail | fail | fail | fail | | ||
| | `{ a: c, b: c }` | fail | fail | fail | fail | fail | | ||
| | `{ c: c }` | fail | fail | fail | fail | fail | | ||
| | `{ a: a, c: c }` | fail | fail | fail | fail | fail | | ||
| | `{ b: b, c: c}` | fail | fail | fail | fail | fail | | ||
| | `{ a: a, b: b, c: c }` | fail | fail | fail | fail | fail | | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| [//]: # "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved." | ||
| [//]: # "SPDX-License-Identifier: CC-BY-SA-4.0" | ||
|
|
||
| # Check Required Encryption Context Keys exist in Encryption Context in Decrypt and the values match in the stored Encryption Context in the message header | ||
|
|
||
| ## Affected Implementations | ||
|
|
||
| This serves as a reference for all implementations that this change affects. | ||
|
|
||
| | Language | Repository | | ||
| | ---------- | -------------------------------------------------------------------------------------------------------------- | | ||
| | Java | [aws-encryption-sdk-java](https://github.com/aws/aws-encryption-sdk-java) | | ||
| | .NET | [aws-encryption-sdk-net](https://github.com/aws/aws-encryption-sdk-dafny/tree/mainline/aws-encryption-sdk-net) | | ||
| | Python | [aws-encryption-sdk-python](https://github.com/aws/aws-encryption-sdk-python) | | ||
| | CLI | [aws-encryption-sdk-cli](https://github.com/aws/aws-encryption-sdk-cli) | | ||
| | 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 [Decrypt](../../client-apis/decrypt.md) only requires | ||
| that the [encryption context](../../framework/structures.md#encryption-context) that is used as | ||
| additional authenticated data during the decryption of the input [encrypted message](#encrypted-message) | ||
| is returned. | ||
|
|
||
| Additionally it should also return the | ||
| [encryption context used for authentication only](../../client-apis/decrypt.md#encryption-context-to-only-authenticate) | ||
| if it used any during the operation. | ||
|
|
||
| ### `decrypt` API returns encryption context used as AAD and encryption context used for authentication only | ||
|
|
||
| On encrypt the [required encryption context cmm](../../framework/required-encryption-context-cmm.md), | ||
| is able to filter out encryption context key-value pairs that are not stored on the message. | ||
|
|
||
| If the required encryption context CMM filters out encryption context keys from the Additional Authenticated | ||
| Data stored on the header, Decrypt MUST use the | ||
| [encryption context to only authenticate](../../client-apis/decrypt.md#encryption-context-to-only-authenticate) | ||
| to verify the header auth tag. | ||
|
|
||
| The encryption context to only authenticate MUST be the [encryption context](../framework/structures.md#encryption-context) | ||
| in the [decryption materials](../framework/structures.md#decryption-materials) | ||
| filtered to only contain key value pairs listed in | ||
| the [decryption material's](../framework/structures.md#decryption-materials) | ||
| [required encryption context keys](../framework/structures.md#required-encryption-context-keys-1) | ||
| serialized according to the [encryption context serialization specification](../framework/structures.md#serialization). | ||
|
|
||
| `decrypt` MUST return the union of the encryption context serialized into the message header and | ||
| the [encryption context for authentication only](#encryption-context-to-only-authenticate), if available. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.