search

andyblarblar / rust-web-push

A Web Push library for Rust
Apache License 2.0
0 stars 0 forks source link

aesgcm fails to send notifications #3

Closed andyblarblar closed 3 years ago

andyblarblar commented 3 years ago

headers appear to be correct, perhaps the firebase implementation no longer works?

andyblarblar commented 3 years ago

when testing with autopush I get: [ERROR] When sending webpush to client: Please provide valid credentials to send the notification. Looks like perhaps VAPID is bad after all?

andyblarblar commented 3 years ago

both services build correct payload tests fail. Considering both of these only cover aesgcm, this is where we should focus.

andyblarblar commented 3 years ago

those test failures are unrelated and due to removing padding. The real issue per firebase is 'VAPID public key must be on the P-256 curve'. This only effects the aesgcm varient so it looks like were messing up the header.

andyblarblar commented 3 years ago

it looks like this could be the ecdh Cypto-key header or the JWT. Leaning towards the former as its litterally just the key.

andyblarblar commented 3 years ago

The Crypto-key header is incorrect while using aesGCM. It should match the public key exactly.

andyblarblar commented 3 years ago

it looks like the issue is that the key is getting base64 encoded twice. I believe the server is already base64 encoding, so this may be a bug with that and not the lib. On the other hand, this should be documented if this is the case. Most likely, we can remove base64 encoding on the pub key on certain or all paths.

andyblarblar commented 3 years ago

The ece crate was already base64 encoding the pubkey, so a double encoding was applied. With this fixed, aesgcm now sends to the server, but fails to decrypt on the client.

andyblarblar commented 3 years ago

Need to take a look at the body, as it doesnt have the \x delims as in the aes128gcm thats working. The token is correct because 128 is working. Perhaps testing with master thats working could help.

andyblarblar commented 3 years ago

Just going to make a branch that removes aesgcm code as its ultimately quite unneeded and is an easy migration for consumers as it constitutes a single enum change. This may prevent this fork from being accepted by the main repo, but we could always make this repo into its own crate.

andyblarblar commented 3 years ago

Pretty sure the changes in e2d0505cc27e3356afc97c58aa02e29c913347b3 would have fixed the issue with aesgcm. I stand by removing the protocol however, as it has no advantages and will be deprecated anyways. Closing as aesgcm is fully removed now.