Based on SubtleCrypto. Functions we will need for encryption database.
Depends on D6994
Details
Details
- Reviewers
michal tomek bartek - Commits
- rCOMM7096eb583d29: [web-db] implement crypto functions
Encryption:
- Create data ArrayBuffer
- Encrypt
- Decrypt
- Compare
Crypto Key:
- Check if it's properly generated
- Verify that can not be extracted using js
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/database/utils/worker-crypto-utils.js | ||
|---|---|---|
| 10–11 ↗ | (On Diff #23513) | Why aren't these read-only? |
Comment Actions
rebase, make type read-only
| web/database/utils/worker-crypto-utils.js | ||
|---|---|---|
| 10–11 ↗ | (On Diff #23513) | oversight, thanks for pointing out |
Comment Actions
Generally, the code itself looks good to me. But I have a general note regarding naming conventions.
I haven't seen all call sites yet (only the key generation diff is up) so I can't really tell but maybe it's worth revisiting the types and function arguments to have this as clean and consistent as possible.
In my diffs regarding AES encryption, I defined a few naming rules which I stick to (you can find them in this Notion doc). You can take inspiration from them.
If you think there's no point in doing this, just let me know ;)
| web/database/utils/worker-crypto-utils.js | ||
|---|---|---|
| 5 ↗ | (On Diff #23697) | What about calling it EncryptedData? See my comment in decryptDatabaseFile |
| 7 ↗ | (On Diff #23697) | I'd call it ciphertext to be consistent, but up to you |
| 21 ↗ | (On Diff #23697) | Nit |
| 45–47 ↗ | (On Diff #23697) | How about connecting cipher and iv by using the EncryptionResult type you defined above (and perhaps renaming the type)? In GCM mode, they always need to be provided together anyways |
Comment Actions
I haven't seen all call sites yet (only the key generation diff is up) so I can't really tell but maybe it's worth revisiting the types and function arguments to have this as clean and consistent as possible.
I updated the types to match those added in D7101.
Also, I added unit tests so you can check possible callsites: D7104
In my diffs regarding AES encryption, I defined a few naming rules which I stick to (you can find them in this Notion doc). You can take inspiration from them.
Oh, I didn't know those rules exist (btw. good job on those).
If you think there's no point in doing this, just let me know ;)
I think your suggestions make sense, thanks!