Skip to content

Move out crypto/aes#4431

Merged
florianduros merged 19 commits intodevelopfrom
florianduros/rip-out-legacy-crypto/aes
Oct 1, 2024
Merged

Move out crypto/aes#4431
florianduros merged 19 commits intodevelopfrom
florianduros/rip-out-legacy-crypto/aes

Conversation

@florianduros
Copy link
Contributor

@florianduros florianduros commented Sep 25, 2024

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

Task element-hq/element-web#26922

The goal of the new src/utils folder is to share utility functions without to have to import all the js-sdk content (ie: src/matrix.ts). See #4339

Changes:

  • decryptAES and encryptAES are moved into src/utils.
  • deriveKeys is used only by decryptAES and encryptAES. Moved into src/to keep it internal.
  • Rename IEncryptedPayload to AESEncryptedSecretStoragePayload. Move it into src/@types/

The PR can be reviewed by commit.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like the right direction! A few suggestions for improvements.

// string of zeroes, for calculating the key check
const ZERO_STR = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0";

/** Calculate the MAC for checking the key.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/** Calculate the MAC for checking the key.
/**
* Calculate the MAC for checking a secret storage key.
*
* See https://spec.matrix.org/v1.11/client-server-api/#msecret_storagev1aes-hmac-sha2, steps 3 and 4.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4431 (comment) seems to have got lost.

LGTM otherwise

@florianduros florianduros added this pull request to the merge queue Sep 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 30, 2024
@florianduros florianduros enabled auto-merge October 1, 2024 07:45
@florianduros florianduros added this pull request to the merge queue Oct 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 1, 2024
@florianduros florianduros enabled auto-merge October 1, 2024 08:17
@florianduros florianduros added this pull request to the merge queue Oct 1, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 1, 2024
* Move `SecretEncryptedPayload` in `src/utils/@types`

* Move `encryptAES` to a dedicated file. Moved in a utils folder.

* Move `deriveKeys` to a dedicated file in order to share it

* Move `decryptAES` to a dedicated file. Moved in a utils folder.

* Move `calculateKeyCheck` to a dedicated file. Moved in a utils folder.

* Remove AES functions in `aes.ts` and export new ones for backward compatibility

* Update import to use new functions

* Add `src/utils` entrypoint in `README.md`

* - Rename `SecretEncryptedPayload` to `AESEncryptedSecretStoragePayload`.
- Move into `src/@types`

* Move `calculateKeyCheck` into `secret-storage.ts`.

* Move `deriveKeys` into `src/utils/internal` folder.

* - Rename `encryptAES` on `encryptAESSecretStorageItem`
- Change named export by default export

* - Rename `decryptAES` on `decryptAESSecretStorageItem`
- Change named export by default export

* Update documentation

* Update `decryptAESSecretStorageItem` doc

* Add lnk to spec for `calculateKeyCheck`

* Fix downstream tests
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 1, 2024
@florianduros florianduros added this pull request to the merge queue Oct 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 1, 2024
@florianduros florianduros force-pushed the florianduros/rip-out-legacy-crypto/aes branch from 182a802 to 25a34c1 Compare October 1, 2024 13:48
@florianduros florianduros enabled auto-merge October 1, 2024 13:48
@florianduros florianduros added this pull request to the merge queue Oct 1, 2024
Merged via the queue into develop with commit 5f3b899 Oct 1, 2024
@florianduros florianduros deleted the florianduros/rip-out-legacy-crypto/aes branch October 1, 2024 14:08
@florianduros florianduros mentioned this pull request Oct 2, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Task Tasks for the team like planning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants