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

Updating previous subscriptions that have a hub.secret #47

Closed
aaronpk opened this issue Nov 6, 2016 · 11 comments
Closed

Updating previous subscriptions that have a hub.secret #47

aaronpk opened this issue Nov 6, 2016 · 11 comments

Comments

@aaronpk
Copy link
Member

aaronpk commented Nov 6, 2016

If a subscription is currently active for a subscriber that used a hub.secret when creating the initial subscription, is it okay if a new subscription request comes in without a hub.secret? It looks like just the topic and callback are used to identify a previous subscription:

... must override the previous subscription state for a specific topic URL and callback URL combination ...

I believe there is no risk in allowing a future request that doesn't have a secret, because:

Any failures to confirm the subscription action must leave the subscription state unchanged.

In particular, I'm thinking of a case where an attacker tries to make a subscription request for a subscriber that previously used a secret. When the subscriber gets a subscription request, it must only acknowledge the verification if it specifically requested it, otherwise it's possible for someone to turn a signed subscription into an unsigned subscription.

There are two issues I would like to see some clarification on:

  1. Should a hub allow the secret of a subscription to be changed or removed or added? I think the answer is yes, but I feel like this should be made explicit in the spec since I had this question while implementing a hub.
  2. We should add some text for subscribers to clarify the security requirements of their verification endpoint, to ensure an attacker can't accidentally cause them to confirm a malicious subscription.
@julien51
Copy link
Collaborator

julien51 commented Nov 8, 2016

I think the issue raised on #36 could be helpful here.

@julien51
Copy link
Collaborator

julien51 commented Nov 18, 2016

^ I am wrong!

@julien51
Copy link
Collaborator

  1. Should a hub allow the secret of a subscription to be changed or removed or added? I think the answer is yes, but I feel like this should be made explicit in the spec since I had this question while implementing a hub.

The key is in

When the subscriber gets a subscription request, it must only acknowledge the verification if it specifically requested it.

We should make it that when the subscribers gets a 2nd verification request, it ignores it completely.

@tonyg
Copy link
Contributor

tonyg commented Nov 19, 2016

We should make it that when the subscribers gets a 2nd verification request, it ignores it completely.

Doesn't this make subscription verification non-idempotent?

@julien51
Copy link
Collaborator

I guess the right answer is "it depends". Basically the subscriber should response the same but ignore them. So from a protocol standpoint they're idempotent.

@tonyg
Copy link
Contributor

tonyg commented Nov 19, 2016

The subscriber has no way to distinguish a (second or subsequent) verification-of-intent resulting from a request it made from a verification-of-intent made by some other party. So it has to acknowledge verification success consistently in all cases, it seems to me.

... unless hub.challenge could be required to be the same for second-and-subsequent request for a given verification-of-intent instance. Then the subscriber could tell that a second-or-subsequent request was arriving because of some failure, and that it should acknowledge the change as verified.

Another interesting idea might be to allow an un/subscriber to supply hub.verify_token in the un/subscription request, that is to be passed through unmodified to the verification-of-intent request.

@julien51
Copy link
Collaborator

julien51 commented Nov 29, 2016

So it has to acknowledge verification success consistently in all cases, it seems to me.

Like I said:

the subscriber should response the same but ignore them.

;p

I see no use in hub.verify_token which is not already fulfilled by a "custom" hub.callback url.

@julien51
Copy link
Collaborator

julien51 commented Feb 7, 2017

@aaronpk Are you satisfied wth the answers? Can you clarify anything we should discuss further?

@aaronpk
Copy link
Member Author

aaronpk commented Mar 28, 2017

This kind of got off track from my original questions.

  1. Should a hub allow the secret of a subscription to be changed or removed or added? I think the answer is yes, but I feel like this should be made explicit in the spec since I had this question while implementing a hub.

I've sent a PR with a sentence that clarifies the above: #92

  1. We should add some text for subscribers to clarify the security requirements of their verification endpoint, to ensure an attacker can't accidentally cause them to confirm a malicious subscription.

We kind of lost the discussion thread here. It seems like the subscriber's verification endpoint should not just blindly echo the challenge for every request it gets, is that true?

@julien51
Copy link
Collaborator

Yes of course. I think the subscriber should always check on their end whether they want to accept a verification. This may involve checking things against business logic, but also things like whether a subscription has already been confirmed, or whether it is expired...etc

@aaronpk
Copy link
Member Author

aaronpk commented Apr 4, 2017

I guess this is already covered by "The subscriber must confirm that the hub.topic corresponds to a pending subscription or unsubscription that it wishes to carry out." so I don't see any further language that needs to be added.

@aaronpk aaronpk closed this as completed Apr 4, 2017
@aaronpk aaronpk removed their assignment Apr 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants