search

zaru / webpush

webpush, Encryption Utilities for Web Push protocol
MIT License
394 stars 73 forks source link

switch to aes128gcm encoding #84

Closed mohamedhafez closed 5 years ago

mohamedhafez commented 5 years ago

Me and @mplatov have been working out of this branch in the discussion of https://github.com/zaru/webpush/pull/75, so I'm making the pull request from here directly to avoid confusion as us and maybe others collaborate on the last remaining bit.

Everything is working, and has been tested with Chrome, Firefox, and Edge, and I've fixed up all but two of the tests: all that's needed is to be able to decrypt the payload correctly here and here in the tests.

All the values I've extracted from the payload header in those two tests are correct, its just that I don't think the ECE gem we're using to decode the payload supports aes128gcm encoding. It hasn't had a commit since 2016, we might be better off implementing our own decoding code instead just for this test, idk. Or maybe just testing against a hardcoded payload value we know is correct, like the one in the example from the rfc spec?

In any case I'm way out of my comfort zone on this and have been working on this on top of 50 hour work weeks, and would really appreciate some help fixing up this last issue so we can get https://github.com/zaru/webpush/issues/60 finally fixed up, before push services stop accepting the deprecated encoding we are using and we find out the hard way!

mplatov commented 5 years ago

I've implemented aes128gcm decoding and fixed the two remaining test in this pull request.

mohamedhafez commented 5 years ago

Ok all the tests are passing! @zaru or @rossta we're ready for you guys to review:)

A suggestion: After this gets merged, it would be nice if we could bump the version number up to 1.0.0 to communicate to the outside world that this is a stable project suitable for production use, and isn't just an experimental still-in-development project.

zaru commented 5 years ago

Thanks for everything!