Repository navigation
fix(notify): make Web Push payloads decryptable (RFC 8291 aes128gcm) - #156
Conversation
cc5ac99 to
ec4af54
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Browsers dropped every onWatch push with "AES-GCM decryption failed" (visible in chrome://gcm-internals): the push service accepted the request (201) but the user agent could not decrypt the payload. Two derivation inputs did not follow RFC 8291 / RFC 8188: - key_info prefixed both public keys with a 2-byte length, which is the legacy aesgcm draft format; aes128gcm concatenates them directly. - the CEK and nonce info strings ended in 0x00 0x01. The 0x01 is the HKDF-Expand counter that the hkdf package already appends, so it was added twice. Adds an independent RFC 8291 decoder in tests that decrypts the output of encryptPayload the way Chrome/Firefox/Edge do; the previous test only checked the header length. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ec4af54 to
410b99a
Compare
prakersh
left a comment
prakersh
left a comment
There was a problem hiding this comment.
Reviewed the RFC 8291 derivation changes: public keys are concatenated without length prefixes and the HKDF info labels drop the expansion counter, which matches Appendix A. Verified the vector locally (IKM, CEK, nonce). CI is green.
|
Thanks for tracking this down and for the focused fix. Both derivation changes are correct: RFC 8291 concatenates the public keys directly, and the HKDF info labels must not carry the expansion counter. I checked against the RFC 8291 Appendix A vector independently, and the corrected formulas reproduce the published IKM, CEK and nonce. The previous code was a broken aes128gcm implementation rather than a working legacy aesgcm one, so your diagnosis was right. Merged, and it ships in v2.14.16. The new test decrypts with its own decoder, which is what matters here: a round trip through our own encrypt and decrypt would have passed even with both sides wrong. We will follow up with a known-answer test using Appendix A and a payload size limit, since plaintext over about 4KB exceeds what push services accept. Thanks again! |
Summary
Web Push notifications never showed up in the browser. The push service accepted every request (
201 Created), but Chrome/Edge dropped the message;chrome://gcm-internalsshows:Two key-derivation inputs in
internal/notify/push.godid not follow RFC 8291 / RFC 8188 (aes128gcm):key_infoprefixed both public keys with a 2-byte length. That is the legacyaesgcmdraft format; RFC 8291 §3.4 concatenates them directly:"WebPush: info" || 0x00 || ua_public || as_public.0x00 0x01. The0x01is the HKDF-Expand block counter, whichgolang.org/x/crypto/hkdfalready appends, so it was applied twice. RFC 8188 §2.2/2.3 uses"Content-Encoding: aes128gcm" || 0x00and"Content-Encoding: nonce" || 0x00.Tests
The existing
TestEncryptPayloadonly checked the header length, so it could not catch this. This PR addspush_rfc8291_test.gowith an independent RFC 8291 decoder (written the way user agents decrypt) and checks thatencryptPayloadoutput round-trips.main(cipher: message authentication failed) and passes with the fixgo vet ./internal/notify/,go test -race ./internal/notify/