feat: Add recommendation for detecting base64-encoded messages #154
Conversation
|
Realized I still need a change doc and version change etc. for this, coming soon. |
robin-aws
changed the title
robin-aws
changed the title
|
|
||
| ## Summary | ||
|
|
||
| When a decryption attempt is made on the Base64-encoding of a valid message, |
There was a problem hiding this comment.
a decryption attempt is made on the Base64-encoding of a valid message
Does this mean a user is trying to decrypt a Base64-string, the utf-8 encoding of a Base64-string, or both/either?
There was a problem hiding this comment.
The UTF-8 encoding of the string - decrypt accepts a message as a byte sequence after all.
FWIW, a Base64 string only uses ASCII characters so nearly all encodings will use the same byte values.
There was a problem hiding this comment.
decrypt accepts a message as a byte sequence after all
The spec must acknowledge dynamically typed languages though, no?
There was a problem hiding this comment.
Absolutely! But it MUST interpret whatever input it is given as a sequence of bytes somehow. If it's idiomatic to accept a string because it quacks like a byte sequence, the operation has to be clear about how it will interpret it as bytes: whether it will use a particular hard-coded encoding like UTF-8, or rely on an encoding attached to the string value, etc.
In a particular language implementation, I would expect the documentation to cover this if it's not obvious by context.
|
|
||
| | Specification | | ||
| | --------------------------------------- | | ||
| | [Decrypt](../../client-apis/decrypt.md) | |
There was a problem hiding this comment.
I was following the example here, which only includes the SHA in "Affected Features" but not in "Affected Specifications": https://raw.githubusercontent.com/awslabs/aws-encryption-sdk-specification/master/changes/2020-05-13_remove-keyring-trace/change.md
@mattsb42-aws Do you think we need to use permalinks in both places? Whatever we decide should be written down in #137.
There was a problem hiding this comment.
The pattern that I've been following is to use permalinks when referencing features that are being removed or referencing specific lines in files. I think that a standard relative link is appropriate here.
There was a problem hiding this comment.
As per my comment on #137, switching the existing permalink back to relative.
|
|
||
| | Specification | | ||
| | --------------------------------------- | | ||
| | [Decrypt](../../client-apis/decrypt.md) | |
There was a problem hiding this comment.
The pattern that I've been following is to use permalinks when referencing features that are being removed or referencing specific lines in files. I think that a standard relative link is appropriate here.
| This feature technically means the version value | ||
| cannot be incremented beyond the hex value `40`, | ||
| since `41` would then conflict with the Base64-encoded value for `1`. | ||
| This does not seem worth worrying about | ||
| given how rarely we intended to change the version, however. |
There was a problem hiding this comment.
I would consider this a reference-level detail. Implementations SHOULD detect base64-encodings of all supported version and type prefixes.
There was a problem hiding this comment.
To rephrase, it's not that the version cannot advance to 41, it's just that if it does this "SHOULD" becomes impossible to do for all versions, but that doesn't mean it shouldn't be done when possible. I'll improve this.
| Implementations SHOULD detect the first two bytes of the Base64 encoding of any supported message [versions](../data-format/message-header.md#version-1) | ||
| and [types](../data-format/message-header.md#type) | ||
| and fail with a more specific error message. | ||
| In particular, the hex values to detect for the current set of versions and types are: | ||
|
|
||
| | Version and type (hex) | Base64 encoding (ascii) | Base64 encoding (hex) | | ||
| | ---------------------- | ----------------------- | --------------------- | | ||
| | 01 80 | A Y ... | 41 59 ... | | ||
|
|
||
| Note that the bytes that follow the initial two in the Base64 encoding | ||
| partially depend on subsequent bytes in the binary message format | ||
| and hence are not as predictable. |
There was a problem hiding this comment.
Rather than stating a set of static values here, we should state how to find those values and provide an example of what they might be.
There was a problem hiding this comment.
I'd like to at least keep the table so that each implementation doesn't have to go through that process themselves.
Co-authored-by: Matt Bullock <[email protected]>
robin-aws
dismissed stale reviews from MatthewBennington, seebees, and acioc
via
73e6a36
Co-authored-by: Matt Bullock <[email protected]>
I can’t believe none of us caught this already :)
Fixes #42
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.