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.

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

Incorrect sort order for encryption context keys in AES-GCM keyrings #428

Open
bdonlan opened this issue Sep 4, 2020 · 0 comments
Open
Labels
bug Something isn't working

Comments

@bdonlan
Copy link

bdonlan commented Sep 4, 2020

The encryption context serialization code incorrectly sorts context keys using a locale-dependent comparator:

.sort(([aKey], [bKey]) => aKey.localeCompare(bKey))

While unsorted context keys are generally tolerated in the ESDK header itself, when using the AES-GCM keyring this is used to canonicalize the encryption context before entering it into the AAD for the AES-GCM keyring. As such, when certain unicode characters (eg Cryllic characters) are used in encryption context keys, the sort order can disagree both with other ESDK languages, and even within the JS ESDK (depending on locale selection). @seebees has more specific examples of keys that can cause this failure mode.

The correct sort order is a binary comparison of the UTF-8 encodings of the encryption context keys (this is what the C ESDK does, at least).

Security implications: Even with the locale sort (which may not be a total ordering), the AAD encoding is unambiguous (keys and values are unambiguously delineated) and therefore it's hard to see a way it could lead to incorrectly accepting ciphertexts that shouldn't. As such I would consider this to be purely a compatibility issue unless proven otherwise.

@seebees seebees added the bug Something isn't working label Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants