webpush-wg / webpush-vapid

Voluntary application server self-identification for web push
3 stars 2 forks source link

"aud" URL needs to be clarified #23

Closed jrconlin closed 8 years ago

jrconlin commented 8 years ago

https://tools.ietf.org/html/draft-thomson-webpush-vapid-02#section-2

notes that:

An "aud" (Audience) claim in the token MUST include the unicode
      serialization of the origin (Section 6.1 of [RFC6454]) of the push
      resource URL.  This binds the token to a specific push service.
      This ensures that the token is reusable for all push resource URLs
      that share the same origin.

I presume that this means that the "aud" would always be the same value for a given push service (e.g. "push.example.com")?

This is not really useful. Would it not make more sense for the "aud" to be some value that is most meaningful to the subscription data provider?

I would presume that the "aud" would contain information that would allow the two parties to quickly identify a given feed (particularly important or useful in the cases where a Subscription Provider is a proxy, and may wish to encode information about the true origin into the "aud" in order to rapidly identify the source.

martinthomson commented 8 years ago

The "aud" claim is intended to limit the portability of tokens by including something that the push service knows and can validate. Originally, it was the full push resource URL, but we got some complaints that it made it too hard to reuse the tokens, so this is what we have.

"aud" (Audience) means "who consumes the token", so this is entirely appropriate.

The purpose of the "sub" claim is to include information about the application server.

I see @gauntface say:

Everyone I've spoken to regarding this matter thought it was the origin of the application server and after reading the spec wasnt sure what it should or could be. It's definitely one of those moments where the spec is phrased awkwardly, but FCM will reject and the final sentence in the spec regarding audience does suggest it should be the push service

Which is odd because "push resource URL" is very well defined. I'm happy to take a PR that somehow clarifies this, but reading web-push-libs/web-push#183 doesn't help me understand where the confusion came from, or how I might fix it.

jrconlin commented 8 years ago

Hmm, as it stands "aud" is effectively useless. True, a given provider could reuse the VAPID header for various providers, but I'm not really sure why that's a problem. In essence, the VAPID provider would be identified by their public key (verified using a ECDH token or like mechanism). There's nothing preventing that public key from being reused for multiple sites (except, perhaps the mournful sobs of their OpSec). VAPID is data that's provided to the Push Service for assistance, providing bogus information would be annoying, but again, little prevents that and there's little value to be gained by it.

In a different case, let's suppose that you are a push republisher (such as onesignal, amazon, or similar). In this case, you may be handling or proxying numerous back end feeds. If a push service noted a problem with a feed, it would have an email address, and an endpoint. The Subscription service would then get a list of endpoints that are problematic and then have to decipher who those endpoints belonged to using whatever heavy lifting they had available.

This would not make for happy ops folk.

Another scenario could be a potentially problematic publishing machine, one of a swarm of publishing machines that may be acting up.

Honestly, having some value that allowed such a service to quickly determine the source of a problem would make for very happy ops folk.

gauntface commented 8 years ago

To be honest I don't find "push resource URL" to be particilarly clear (chances are it is clearly defined else where in the spec and I've missed it). In my head it could be the url that requested the subscription push or it could be the url of the push service, neither clearly fit under label "resource" .

I agree with @jrcolin that if the url was the applications origin, it would serve a purpose should the push service need information on the application. I see little use for the audience being the origin of the push service since the push service would be receiving the api request on that origin and has to verify the jwt BUT at least its clear and easy to generate the audience for any push service.

I'm somewhat concerned with respect to the audience being any value as that would negate part of the goal of web push to be agnostic to user agent and push service. How would a developer know the correct audience for a push service they've never encountered before?

Just wanted to put my two cents out there

martinthomson commented 8 years ago

Useless to whom? The point is to make it less useful to an attacker that manages to steal it. The same reason it has a "notAfter" claim. As I said, the original design included the complete push resource URL, which would have been much better at that.

As for identifying where the message came from, no one prohibited you from including extra information in the token. That could happen in a few ways.

Imagine that the "sub" field contained "mailto:push@example.com?subject=node%20aws-69126" Or better yet: "mailto:push+aws-69126@example.com".

You could also include other proprietary claims. Though it's probably not a good idea to rely on the push service keeping that information. Or maybe you are OK with that because you don't want them saving that stuff until you actually need them to use it.

martinthomson commented 8 years ago

For the definition, see: https://webpush-wg.github.io/webpush-protocol/#resources

I'm somewhat concerned with respect to the audience being any value as that would negate part of the goal of web push to be agnostic to user agent and push service. How would a developer know the correct audience for a push service they've never encountered before?

I don't get this concern, you are sending a PUT request to https://push.example.net/blah, the audience is https://push.example.net. It's a pretty straightforward transformation.

jrconlin commented 8 years ago

True, the specification is simply a guideline that parties can and will modify to their preferred ends, I'm not contesting that. I'll also agree that since VAPID information is most likely going to be logged with the incoming requests, that information is going to be readily present to the Push Service Admins who will most likely be using the claim information.

So a Push Service Admin would have, the subscription endpoint URL, the claimed update provider contact email, and the host associated with the subscription endpoint URL. That final bit of information being effectively redundant. So, in essence, there is one, well-known, useful field in the VAPID header.

Presuming that a restricted endpoint had already been issued, and that the expected checks have passed (key validity, exp tolerance, etc), there's some issue with the subscription. Push service Ops sends a message to subscription update ops with that endpoint. You're suggesting that each service provider offer a "best practices" guide in addition to this specification that outlines how to add additional information into that one field, which will most likely appear in a log that the Push Service Provider Operator must try and suss out, manipulate into an unknown mail system so that it's usable to the receiving subscription provider operator, who may then have to determine which process used the subscription endpoint in order to send out the data in order to determine what may have gone wrong, or shut down the mis-behaving feed.

My apologies if I don't expect that to go well.

On the other side is the concern that this unchecked, voluntarily provided data could be abused.

If I understand, the following are the major attack vectors:

1) Evil party uses intercepted VAPID header for replay. This can be mitigated by having a very low "exp" (as in seconds) to cover the transmission time of the update. Although, for the replay to be successful, the attacker would either need to replay the entire update or have already broken the encryption key, otherwise the push service would reject the data (probably with an increase in operational metric error reporting which could trigger a channel reset)

2) Evil uses counterfeit information for VAPID Since the claim information is not validated, any information could be used. This means that Evil uses bogus information in attempt to cause mischief. Since the VAPID key would be different, the bogus record would not be able to use the correct restricted update endpoints, and since the VAPID information is never sent to the client, it means that the long suffering Push Service NOC sends messages to "ipfreely@ebay.com" or whoever before throttling or blocking that key. (Honestly, a more effective griefing attack would be to skip using VAPID and just hammer with bogus subscription endpoints with massive payloads.) This would be the same as if no VAPID information were present.

Yes, it's certainly possible that an attacker could use a VAPID header designated for a different push system. They could still by simply stripping the optional "aud" field.

To be dead honest, I'd prefer that the "aud" (or even a "dat"/"etc"/"foo" field) be optional to allow subscription update providers the ability to embed whatever data they need to make their lives easier. Mostly because I can absolutely see the need and desire.

VAPID exists to help the Push Service and Subscription Update Provider. We should strive to not make it harder to do that.

jrconlin commented 8 years ago

Having thought about this more and discussed this with others, I believe that I may be arguing a valid point against an invalid assertion.

I will instead argue the "moron in a hurry" point that we may want to not require that people sift through other specs where they need not. I will suggest that we add an example of the "aud" that points to the destination machine and note that this is useful for cryptographic reasons.

martinthomson commented 8 years ago

I still don't understand why you don't use "sub" for this. Most of this screed would make perfect sense if you just replaced all instances of "aud" with "sub".

Yes, it's certainly possible that an attacker could use a VAPID header designated for a different push system. They could still by simply stripping the optional "aud" field.

It has a signature to prevent that sort of tampering.

jrconlin commented 8 years ago

As I understand the JWT specification, there are a limited set of defined fields available, most are associated with the crypto. I had incorrectly presumed that "aud" was an open field and not associated with the crypto elements. That leaves "sub" as a field. VAPID notes that this field is to be used for the contact email. It may be overloaded (as you've noted) in any number of ways as users attempt to work around the restrictions we are putting on them.

https://tools.ietf.org/html/rfc7519#section-4.3 notes that private claim names could be used if safeguards are present and caution is exercised.

My concern is that if we overload the "sub" field, we are introducing needless complexity. I am fine with keeping the "sub" a simple email field. This will keep things simple for the critical task of locating a party to report an anomaly to. Adding complexity to this field is prone to error for many reasons.

This is why I'm fine with keeping "aud" and "sub" as defined in the specification and proposing a well-known, optional, private field containing additional information.

Closing in favor of #24