Page MenuHomePhabricator

[lib] Introduce PKCS#7 padding
ClosedPublic

Authored by bartek on Mar 13 2023, 7:40 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 4, 3:55 AM
Unknown Object (File)
Feb 27 2024, 11:10 AM
Unknown Object (File)
Feb 19 2024, 3:12 AM
Unknown Object (File)
Feb 19 2024, 2:14 AM
Unknown Object (File)
Feb 19 2024, 1:12 AM
Unknown Object (File)
Feb 19 2024, 12:09 AM
Unknown Object (File)
Feb 18 2024, 5:03 PM
Unknown Object (File)
Jan 30 2024, 4:11 AM

Details

Summary

Part of ENG-3099

This diff implements standard PKCS#7 padding using JS typed arrays.

Test Plan

Added unit tests. Also tested using my CLI tool and hex editor / viewer (e.g. xxd tool)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Mar 13 2023, 8:09 AM
bartek removed a reviewer: anunay.
bartek added a subscriber: anunay.
lib/utils/pkcs7-padding.js
12 ↗(On Diff #23691)

Nit: we don't use inline exports in the codebase except for Flow types and for files in lib/types (and other "types" files like that)

Can you move these to be a single export declaration at the bottom of the file?

Specifically adding @tomek as reviewer since he seemed to have some thoughts during the Encryption Sync (that might've been discussed offline?)

In D7059#210217, @atul wrote:

Specifically adding @tomek as reviewer since he seemed to have some thoughts during the Encryption Sync (that might've been discussed offline?)

Yes, I've met with @bartek and he clarified my concern. The thing which I was missing was the fact that the block padding is always added. Now I can't find any example that won't work.

Thanks for including unit tests!

This revision is now accepted and ready to land.Mar 17 2023, 1:01 PM
lib/utils/pkcs7-padding.js
13–14 ↗(On Diff #23691)

Would it maybe make more sense to throw exceptions instead of using invariant?

CC @ashoat

lib/utils/pkcs7-padding.js
13–14 ↗(On Diff #23691)

I think invariant is good for "programmer error" type scenarios like this but I don't feel strongly. Ultimately invariant just throws an exception so it doesn't really matter

lib/utils/pkcs7-padding.js
12 ↗(On Diff #23691)

Sure, will do

13–14 ↗(On Diff #23691)

I'd stick to invariant because it looks clean.

This revision was automatically updated to reflect the committed changes.