search

mozilla / rust-ece

Encrypted Content-Encoding for HTTP (RFC 8188) Rust implementation
Mozilla Public License 2.0
25 stars 11 forks source link

Add top-level access to AesGcm encoding scheme #47

Closed tiesselune closed 3 years ago

tiesselune commented 3 years ago

As discussed in #35, the current API does not allow access to encryption and decryption functionality for the AesGcm legacy encoding scheme without implementing a new back-end, even though it is fully implemented for OpenSSL internally.

This pull request provides an access to the AesGcm encoding scheme through top-level functions in a legacy module.

It tries to mimic the main Aes128Gcm top-level library structure and tests.

So far, 4 tests have been implemented, using the IETF example data from IETF Web Push Encryption Draft 4, with no padding and an rs of 4096.

For this to work, a minor fix has been implemented in the AesGcmEceWebPush::encrypt_with_keys function to allow the use of the salt provided in the params argument, which was previously ignored and made the tests fail.

To finalize this pull request, it would be better to provide more tests but for the moment I am unsure of where to get or generate relevant test data.

rfk commented 3 years ago

@jrconlin could you please gut-check the new API that's being proposed here, particularly that the "legacy" naming is conveying approximate the right thing? (It seems right to me, but you've deeper experience with the history of these crypto methods).

tiesselune commented 3 years ago

I have made a few fix commits to handle most of the issues addressed up there. I've removed the redundant test and added a disclaimer in the documentation for the AesGcm and Aes128Gcm encryption structs (since they appear higher in the documentation than the encrypt and decrypt functions, and tend to attract clicks) to redirect to the encrypt and decrypt functions of both schemes.

For the test part, I've taken a look at what's possible and the problem with the node web-push library or the http_ece one is that they don't expose an API that allows for different RS or padding length parameters... So I can't mimic the main library's tests without spending a lot more time taking it apart to generate test data. I've taken a look at the ecec library as well but it comes with the certainty that working with C is going to be even more cumbersome 😅

Anyway I'm going to get this Pull Request out of draft state, at least, and might open another issue to handle the test issue, which is something I should be able to tackle a bit later if you still need the workpower.

rfk commented 3 years ago

I've published v1.3.0 of the create with this change.

rfk commented 3 years ago

Whadya know, we were actually jumping through a few unnecessary hoops ourselves in order to work around the API design here, and now that the top-level legacy::* helpers are available, we're able to make a nice cleanup in some of the consumer code that previously had to couple itself to a specific crypto backend. Ref https://github.com/mozilla/application-services/pull/3941/files#r598391958, and thanks again @tiesselune!