Re: [OAUTH-WG] draft-ietf-oauth-revocation-01

Hi Hannes,

thanks for your review and feedback. Please find my comments inline.

Am 08.10.2012 13:52, schrieb Tschofenig, Hannes (NSN - FI/Espoo):
> Hi all,
>
> I went through draft-ietf-oauth-revocation-01 to what has changed
> between version -00 and -01.
>
> A few minor comments:
>
> Title: maybe you should change it from "Token Revocation" to "Revocation
> of OAuth Access and Refresh Tokens" to make it a bit more informative.

why not. What do other WG members think?

> The abstract is also a bit too short to explain what the document does.
> The RFC Editor will at least demand 3 lines of abstract. You may, for
> example say that the specification covers revocation of access and
> refresh tokens. Important would also to highlight that the revocation is
> initiated by the client. You may also want to highlight that this is
> mainly used as a logout mechanism.

ok.
>
> I you want to re-word the following sentence a bit:
> "
>     A revocation request may discard the actual token as well as other
>     tokens based on the same access grant and the access grant itself.
> "
> For example:
> "
> A revocation request by the client includes the token that shall be
> revoked. Revoking a refresh token will, however, also revoke any access
> token created earlier via the presented refresh token.

I see the following issues with your proposal:
- I assume not all authorization servers will support access token 
revocation (none of our servers does)
- Access tokens belonging to the same access grant might have been 
created based on different refresh tokens, since refresh tokens can be 
rotated (we do this for refresh tokens issued to installed 
applications). see also 
http://www.ietf.org/mail-archive/web/oauth/current/msg09244.html

That's why I suggested on the list to add the concept of an underlying 
access grant. This concept proved to be useful during discussions about 
the different token implementation philosophies at the last IIW.

It also allows an authorization server to delete access grants 
underlying an access token even if it does not issue refresh tokens at 
all - some authorization servers manage permanent access grants 
internally and issue access tokens automatically in implicite and code 
grant type (auto-approval).

I therefore would suggest the following wording:
"A revocation request by the client includes the token that shall be 
revoked. Revoking a token may, however, also revoke other tokens based 
on the same access grant and the acces grant itself."

> "
>
> Introduction: I would not include RFC 2119 language in the introduction.

And put the compliance statement into another section?

> I am also wondering why a specification shouldn't support the revocation
> of both tokens when requested.

The assumption is that not all authorization servers will support access 
token revocation because this comes at a cost. It either requires a 
lookup of the RS at the AS on every service request OR a push 
notification of the AS towards all relevant RS on revocation.

>
> You write that the specification supports the following use cases and
> you list two items. The issue, however, is that item #2 isn't what the
> specification deals with. Instead, the focus is on item #1 and the
> revocation feature is more a logout feature, as you describe it rather
> than a way to deal with stolen tokens.

What about the self-care portal (see below)?

>
>     o  The end-user triggers revocation from within the client that sends
>        the appropriate revocation request to the autorization server.
>        The request causes the removal of the client's access grant the
>        particular token refers to.  From the end-user's perspective, this
>        looks like a "logout" or "reset" function.  This use case makes it
>        even more comfortable to the end-user to revoke his access grant
>        immediately via the client.
>
>     o  In contrast to revocation by a client, the authorization server
>        (or a related entity) may offer its end-users a self-care portal
>        to delete access grants given to clients independent of any token
>        storing devices.  Such a portal offers the possibility to an end-
>        user to look at and revoke all access grants he once authorized.
>        In cases the token storing device is not available, e.g. it is
>        lost or stolen, revocation by a self-care portal is the only
>        possibility to limit or avoid abuse.
>
>
> Terminology: I would include a Terminology section with the following
> content:
> "
>     The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>     "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
>     document are to be interpreted as described in RFC 2119 [RFC2119].
> "

ok.

>
> You write:
>
>     In the end, security, usability, and ease of use are increased by
>     token revocation.
>
> Could you explain this a bit? The specification does not help with
> stolen tokens.

What about the self-care portal?

> It also does not help with clients who behave
> unsupportive.

I don't understand this statement. "unsupportive" with respective to 
token revocation?

>
>
> Example: I suggest to expand the example token a bit to avoid the
> impression that tokens are so short.
> s/token=45ghiukldjahdnhzdauz&/token=45ghiukldj....ahdnhzdauz&/

I'm wondering why the core spec shows example tokens of roughly the same 
size.

>
> I don't understand this note:
>
> "It is also conceivable to allow a dedicated user self-care portal to
> revoke all kinds of tokens."
> The self-care portal shouldn't have tokens and hence cannot really
> revoke them. As such, the self-care portal would be either at the
> authorization server or very closely related to it (for example with
> access to the same backend database).

Yes, that's true. Such a portal must have access to the token database. 
It may be part of the authorization server or a general purpose portal, 
which access the token database via well-defined (internal) interfaces. 
We implemented the later pattern.

>
>
> Regarding:
> "
> If the particular token is a refresh
>     token and the authorization server supports the revocation of access
>     tokens, then the authorization server SHOULD also invalidate all
>     access tokens based on the same access grant.
> "
>
> Why "SHOULD" and not MUST?

See above.

>
> Shouldn't this sentence say
>
> If the presented token is a refresh token  then the authorization server
> MUST invalidate all access tokens created by that refresh token."

See above. "that refresh token" might not be meaningful in conjunction 
with refresh token rotation.

>
> Note that I
>   - clarified what gets deleted, and
>   - removed unnecessary options.
>
>
> You write:
> "
>     Whether the revocation takes effect instantly or with some delay
>     depends on the architecture of the particular deployment.  The client
>     MUST NOT make any assumptions about the timing and MUST NOT use the
>     token again.
> "
>
> This is an interesting aspect since one would wonder why it isn't
> sufficient for the client to just delete the tokens it has locally.
> Nobody else should have the same set of tokens since they are supposed
> to be minted on the fly for the different clients.

Basically, you are right. I'm wondering whether there could be issues in 
clustered environment, potentielly different nodes could use the same 
access token?

>
> Then, wouldn't it be useful for a logout functionality that the
> revocation actually works instantly rather than at an unpredictable time
> later ?

Yes.

>
> Could you explain why the cross-origin support is needed?

I think this is the only way for a user-agent based application to 
revoke tokens due to the same-origin policy.

@Marius: Could you please shed some more light on this aspect?

>
> You write:
> "
> unsupported_token_type: the client
>             tried to revoke an access token on a server not supporting
>             this feature.
> "
> Does this mean that the server does not support the revocation
> specification at all or just not the ability to revoke access
> (refresh???) tokens?

Revocation of access tokens is an optional feature

>
> Security considerations: I would expect to see a discussion about what
> scenarios this revocation mechanism does not cover.

I thought such a section was intended to discuss the security of the 
proposed protocol?

Do you have specific scenarios in mind?

regards,
Torsten.

>
> Ciao
> Hannes
>
> _______________________________________________
> OAuth mailing list
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth