Skip to content

Commit

Permalink
Decouple "aesgcm" and "aes128gcm" schemes, disable record chunking. (#59
Browse files Browse the repository at this point in the history
)

Decouple "aesgcm" and "aes128gcm" schemes, disable record chunking.

This is a significant refactor of the guts of the crypto code, but I think overall it
makes things easier to understand and to audit.

First, I've removed the `EceWebPush` trait that was previously used to share parts
of the encrypt/decrypt logic between the two schemes. The schemes are not that similar
in practice and on balance, I think the attempt to share code between them was
actually making both schemes harder to understand.

Second, I've cut all the record-chunking code out of "aesgcm". It now supports only
a single record on both encryption and decryption, in line with what the spec says
that a webpush client should support. We were already throwing errors when encountering
multiple records in "aesgcm"; this cleanup takes advantage of that fact to actually
remove the code without breaking the public API.

Finally, I've removed the record-chunking during encryption for "aes128gcm", instead
opting to support larger payloads by increasing the record size. I've also added
several layers of abstraction in the hope of making the code easier to understand -
for example there is a separate `Header` struct for reading/writing the header,
and a separate `PlaintextRecord` struct for reading/writing an individual record.

Of course "easier to understand" is subjective, but I think it's an improvement
(and I certainly understand things better as a result of having worked through it!).
Feedback and/or pushback on this is most welcome.

I'd like to try adding record chunking in back here, but as a separate PR building
atop these abstractions.

Connects to #55.
  • Loading branch information
rfk authored Mar 25, 2021
1 parent dc1c446 commit aaa12b1
Show file tree
Hide file tree
Showing 6 changed files with 621 additions and 515 deletions.
Loading

0 comments on commit aaa12b1

Please sign in to comment.