bug: fix local Authorization header #284
Conversation
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
There was a problem hiding this comment.
This looks good, but isn't it also an issue that get_token_from_auth_header only allows the Bearer scheme whereas mobile sends webpush?
| use uuid::Uuid; | ||
|
|
||
| /// Verifies the request authorization via the authorization header. | ||
| /// | ||
| /// The expected token is the HMAC-SHA256 hash of the UAID, signed with one of | ||
| /// the available keys (allows for key rotation). | ||
| /// NOTE: This is *ONLY* for internal calls that require authorization and should | ||
| /// NOT be used by calls that are using VAPID authentication (e.g. | ||
| /// subscription provider endpoints) |
There was a problem hiding this comment.
what do we mean by "local auth" exactly?
All the registration endpoints use this auth check.
There was a problem hiding this comment.
There are two types of authentication: VAPID, which is meant for external, 3rd party subscription providers, and "local" User Agent authorization, which uses a bearer token provided as part of the registration process.
The complication here is that both share the "Authentication" header. This call should only be used to check the latter Authentication form.
|
Sigh. you're right. The fennec Android component does send the local, non-vapid Auth token |
| sign_with_key(key.as_bytes(), uaid.to_simple().to_string().as_bytes()) | ||
| .map_err(ApiErrorKind::RegistrationSecretHash)?; | ||
|
|
||
| dbg!(&expected_token, &token); |
There was a problem hiding this comment.
Temporary debugging code? This logs secrets.
There was a problem hiding this comment.
Yep, this needs to go. It's not super critical since the local log is discarded and the token would already be compromised at that point, but it's needless chatter.
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