Stop using CryptoJS for signing token requests #1325
Merged
Conversation
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
We already have BufferUtils methods that provide this functionality.
Move the platform-specific code to the BufferUtils classes (the Crypto classes might seem more appropriate, but we need HMAC functionality to be available even in the noencryption version of the library, which doesn’t have a Crypto class).
We’re going to use this to replace our current usage of CryptoJS, which we intend to remove in #1239. We considered using the implementation of HMAC from the Web Crypto API, but after weighing the implications of doing so (token signing becoming unavailable in non-secure contexts) against the mildly-increased library size (1.61 kB increase) from adding an HMAC implementation, decided to go with the latter [1]. I picked this implementation because it's a single file that contains precisely the functionality that we need and it claims to be “designed for efficient minification”. It doesn’t come with any tests, but since our SDK has quite a few tests that will fail if the result of token signing is incorrect, I believe we’re OK. The code added here is taken verbatim from the linked gist, and then I’ve added an attribution comment and run Prettier. [1] https://ably-real-time.slack.com/archives/C030C5YLY/p1686152214032779?thread_ts=1686096207.512069&cid=C030C5YLY
2eae3df to
1794a74
Compare
This message was added in the very first commit of this repo — perhaps it was relevant then, but it certainly no longer is, since the `hmac` function is always present.
Base automatically changed from
1296-document-generateRandomKey-return-type
to
integration/v2
June 12, 2023 12:04
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Note: This is based on top of #1320; please review that one first.
This introduces a lightweight implementation of HMAC-SHA-256 and uses that for signing token requests instead of CryptoJS. See commit messages for more details.
Resolves #1295. SDK-3605