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

Remove crypto-js library in next major release #1239

Closed
lawrence-forooghian opened this issue May 4, 2023 · 3 comments
Closed

Remove crypto-js library in next major release #1239

lawrence-forooghian opened this issue May 4, 2023 · 3 comments
Assignees
Labels
breaking Backwards incompatible changes made to the public API.

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented May 4, 2023

As suggested by @SimonWoolf here as a way of reducing bundle size. This would mean removing the crypto-js library and instead using the browser’s Crypto interface.

Split into:

@lawrence-forooghian lawrence-forooghian added the breaking Backwards incompatible changes made to the public API. label May 4, 2023
@lawrence-forooghian lawrence-forooghian changed the title Remove "`WordArray' stuff" in next major release Remove "WordArray stuff" in next major release May 4, 2023
@lawrence-forooghian lawrence-forooghian self-assigned this May 4, 2023
@sync-by-unito
Copy link

sync-by-unito bot commented May 4, 2023

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3570

@lawrence-forooghian
Copy link
Collaborator Author

Putting this on hold whilst I work on #1252, which I've split from this task.

@lawrence-forooghian lawrence-forooghian changed the title Remove "WordArray stuff" in next major release Remove crypto-js library in next major release May 10, 2023
lawrence-forooghian added a commit that referenced this issue May 22, 2023
Preparation for converting platforms’ crypto.js files to TypeScript
(#1252).

We’ll be removing these types shortly, though, when we stop using the
CryptoJS library in #1239.
lawrence-forooghian added a commit that referenced this issue May 22, 2023
Preparation for converting platforms’ crypto.js files to TypeScript
(#1252).

We’ll be removing these types shortly, though, when we stop using the
CryptoJS library in #1239.
lawrence-forooghian added a commit that referenced this issue May 25, 2023
Preparation for converting platforms’ crypto.js files to TypeScript
(#1252).

We’ll be removing these types shortly, though, when we stop using the
CryptoJS library in #1239.
lawrence-forooghian added a commit that referenced this issue May 25, 2023
Preparation for converting platforms’ crypto.js files to TypeScript
(#1252).

We’ll be removing these types shortly, though, when we stop using the
CryptoJS library in #1239.
lawrence-forooghian added a commit that referenced this issue Jun 1, 2023
This removes our use of the CryptoJS library for performing encryption
and decryption operations, instead using the browser’s built-in crypto
APIs. We’re doing this as part of our work to remove the CryptoJS
library (#1239) to reduce the size of our SDK.

Resolves #1292.
lawrence-forooghian added a commit that referenced this issue Jun 1, 2023
TODO also look for CryptoJS

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 5, 2023
Preparation for converting platforms’ crypto.js files to TypeScript
(#1252).

We’ll be removing these types shortly, though, when we stop using the
CryptoJS library in #1239.
lawrence-forooghian added a commit that referenced this issue Jun 5, 2023
This removes our use of the CryptoJS library for performing encryption
and decryption operations, instead using the browser’s built-in crypto
APIs. We’re doing this as part of our work to remove the CryptoJS
library (#1239) to reduce the size of our SDK.

Resolves #1292.
lawrence-forooghian added a commit that referenced this issue Jun 6, 2023
TODO also look for CryptoJS

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 6, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 6, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 6, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 6, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 6, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 7, 2023
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, 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
lawrence-forooghian added a commit that referenced this issue Jun 7, 2023
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
lawrence-forooghian added a commit that referenced this issue Jun 7, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 7, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 12, 2023
Preparation for #1239 (removing CryptoJS dependency).
lawrence-forooghian added a commit that referenced this issue Jun 12, 2023
Preparation for #1239 (removing CryptoJS dependency).
lawrence-forooghian added a commit that referenced this issue Jun 12, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 13, 2023
Preparation for #1239 (removing CryptoJS dependency).
lawrence-forooghian added a commit that referenced this issue Jun 13, 2023
Preparation for #1239 (removing CryptoJS dependency).
lawrence-forooghian added a commit that referenced this issue Jun 13, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
lawrence-forooghian added a commit that referenced this issue Jun 13, 2023
Preparation for #1239 (removing CryptoJS dependency).
lawrence-forooghian added a commit that referenced this issue Jun 13, 2023
Preparation for #1239 (removing CryptoJS dependency).
lawrence-forooghian added a commit that referenced this issue Jun 13, 2023
We’re no longer making use of this library (in order to reduce bundle
size, given that modern browsers provide all of the necessary crypto
functionality).

Resolves #1239.
@lawrence-forooghian
Copy link
Collaborator Author

Closed by #1333.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Backwards incompatible changes made to the public API.
Development

No branches or pull requests

1 participant