Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autopush-rs Endpoint fails to read AUTHENTICATION header #282

Closed
jrconlin opened this issue Jul 26, 2021 · 6 comments · Fixed by #284
Closed

Autopush-rs Endpoint fails to read AUTHENTICATION header #282

jrconlin opened this issue Jul 26, 2021 · 6 comments · Fixed by #284
Labels
bug Something isn't working p1

Comments

@jrconlin
Copy link
Member

We're seeing https://sentry.prod.mozaws.net/operations/autopush-prod/issues/12011604/events/latest/ on test deploy of the autopush-rs endpoint.

The error being returned indicates that get_token_from_auth_header is failing, indicating that either the AUTHORIZATION header changed format or we're not reading it correctly. We need to test against Mozilla/5.0 (Android 10; Mobile; rv:89.0) Gecko/89.0 Firefox/89.0 or later to verify that the header is correct and readable.

@pjenvey
Copy link
Member

pjenvey commented Aug 3, 2021

Despite the registration docs only showing Bearer tokens for the Auth header, autopush python allows 3 auth schemes here:

# Older versions used "bearer", newer specification requires "webpush"
AUTH_SCHEMES = ["bearer", "webpush", "vapid"]

The clients affected appear to be Fenix (Firefox android) -- which looks to be sending the webpush scheme instead (see the push rust component)

@pjenvey pjenvey added the bug Something isn't working label Aug 3, 2021
@pjenvey
Copy link
Member

pjenvey commented Aug 3, 2021

python's AUTH_SCHEMES is also used by the webpush handler -- so I'm not entirely sure if the registration handler really needs to support the vapid scheme, but it certainly needs at least webpush added.

This should be a quick fix as the auth header's parsed the same as the bearer scheme is.

@jrconlin
Copy link
Member Author

jrconlin commented Aug 3, 2021

Yeah, fun with VAPID.

IIRC, "Bearer" was for VAPID draft 00, "WebPush" was for Vapid draft 01, and "vapid" was for Vapid draft 02 - RFC. "Bearer" and "WebPush" both used Vapid 01 format.

@jrconlin
Copy link
Member Author

Sigh. The problem is that this is the internal Authentication header (used to auth operational calls, and NOT to be used by the subscription provider endpoint.) For instance, if a mobile client wants to mange it's own list of subscriptions, it would use this bearer token based Authentication.
so
https://updates.push.services.mozilla.com/v1/fcm/{key}/registration/{uaid} would use this authorization, and
https://updates.push.services.mozilla.com/wpush/v1/... would use VAPID.

I don't know why this auth is being called, but I suspect that it's being triggered by the middleware and therefore being brought in on all calls.

@pjenvey
Copy link
Member

pjenvey commented Aug 13, 2021

Sigh. The problem is that this is the internal Authentication header (used to auth operational calls, and NOT to be used by the subscription provider endpoint.)

Are you saying webpush is the internal auth header?

I'm not sure I understand -- webpush style auth is being sent by the push rust component (see my link to the impl above).

@jrconlin
Copy link
Member Author

jrconlin commented Aug 14, 2021

No, I'm saying that there's a separate Authentication header for the completely different routes that handle subscription management from the mobile UA. That's what's failing to authenticate.

I'm trying to work out a few tests to validate that things are working but here are a few horror cases for why this might be failing:

  1. The configuration file is using a different value for auth_keys between the old python version and the rust version. (annoying but not terrible)
  2. The sha256 hash that is generated by rust doesn't match the same sha256 hash that was originally generated by the python version. (terrible).
  3. Or we screwed up the key on either the python or rust and didn't convert from base64 (because awesome...)

As it stands, I'm trying to recover a running version of autopush_endpoint (currently building it in a python2.7 docker, so fingers crossed) to see if I can generate a "legacy" authentication header and compare it to one being generated by rust.

jrconlin added a commit that referenced this issue Aug 14, 2021
So the problem isn't with Vapid. It's with how we were generating the
other authorization token, specifically differences with how python
and rust were handling things.

Python was converting the hex UAID string into a set of bytes, rust
was converting the 128bit UUID UAID into bytes. Combine that with some
fun regarding the base auth key string, and.. yeah.

I added a test to this to compare the auth key generated by python with
rust, broke the generate and validate functions into the
AuthorizationCheck impl and added a bunch of comments.

Closes #282
@jrconlin jrconlin added the p1 label Aug 16, 2021
jrconlin added a commit that referenced this issue Sep 28, 2021
* bug: fix local Authorization header

So the problem isn't with Vapid. It's with how we were generating the
other authorization token, specifically differences with how python
and rust were handling things.

Python was converting the hex UAID string into a set of bytes, rust
was converting the 128bit UUID UAID into bytes. Combine that with some
fun regarding the base auth key string, and.. yeah.

I added a test to this to compare the auth key generated by python with
rust, broke the generate and validate functions into the
AuthorizationCheck impl and added a bunch of comments.

Closes #282
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants