Re: [OAUTH-WG] FW: Appsdir review of draft-ietf-oauth-v2-23

[Justin Richer <[e-mail]>]

Responses inline. Don't know if you want to forward these to the 
appropriate other lists/authors as well.

On 02/07/2012 01:04 PM, Eran Hammer wrote:
>
> -----Original Message-----
> From: Henry S. Thompson [mailto:[e-mail]]
> Sent: Tuesday, February 07, 2012 9:31 AM
> To: [e-mail]; [e-mail]; [e-mail]; [e-mail]; Eran Hammer; [e-mail]; [e-mail]
> Cc: [e-mail]
> Subject: Appsdir review of draft-ietf-oauth-v2-23
>
> I have been selected as the Applications Area Directorate reviewer for this draft (for background on appsdir, please see http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate).
>
> Please resolve these comments along with any other Last Call comments you may receive. Please wait for direction from your document shepherd or AD before posting a new version of the draft.
>
> Document: draft-ietf-oauth-v2-23
>
> Title: The OAuth 2.0 Authorization Protocol
>
> Reviewer: Henry S. Thompson
>
> Review Date: 2012-02-07
>
> IETF Last Call Date: 2012-01-24
>
> Summary: This draft is almost ready for publication as an Proposed
>           Standard but has a few issues that should be fixed
>           before publication
>
> [Note - My expertise is in the areas of markup and architecture, with only the average geek's understanding of security and cryptographic technologies.  Any comments below on security issues are therefore of the nature of general audience concerns, rather than technical worries.]
>
> Major Issues:
>
> 3.1.2.1  The failure to require TLS here is worrying.  At the very least I would expect a requirement that clients which do _not_ require TLS MUST provide significant user feedback calling attention to this
> -- by analogy, absence of a padlock does _not_ suffice. . .

There are two bits going on here, and I think there's confusion again 
about this being a *client* controlled endpoint. First, not all callback 
redirects are remote HTTP URLs. A http://localhost/ callback or an 
app:// callback are going to be very common with installed clients, and 
in both of these cases TLS makes no sense to require. Second, and this 
is the reason spelled out in the text, you can't really expect all 
*clients* to use proper TLS on their redirects. If it's a remote HTTP 
call, then it's likely a very Bad Idea to go over a plaintext socket, 
yes. But there are enough other cases where mandating it doesn't make 
sense.

Suggestion: call out second non-HTTP use case here as well, perhaps stop 
calling it an "endpoint" to differentiate it from the server-side pieces.

> 2.1.2.5  In a somewhat parallel case, I would expect this section to at least explain why the SHOULD NOT is not a MUST NOT. Since in practice the distinction between trusted and untrusted 3rd-party scripting is difficult if not impossible to draw, as written the 2nd para is effectively overriden by the third.
Assuming this means 3.1.2.5

A MUST NOT is impossible to enforce here. You're telling people to not 
include jQuery in their JavaScript app. Truth is, any third party 
library that you include in your app could get access to the security 
bits whether it's on the callback page or not. I think it's appropriate 
to provide guidance for best practices here, and the MUST be first and 
SHOULD be only makes sense. People will get it wrong in spite of what 
the spec says here one way or another.

Suggestion: drop the requirements (and move them to the security doc), 
or otherwise no change.

> 11  It seems at best old-fashioned that the designer of a new access token type, parameter, endpoint response type or extension error has no better tool available than Google to help him/her discover whether the name they have in mind is in use already.  The same is true under the proposed approach for any developer trying to determine what they can use or must support.  Is there some reason that mandated URI templates, after the fashion of
>
>    http://www.iana.org/assignments/media-types/text/...
>
> are not mandated for the registries, e.g.
>
>    http://www.iana.org/assignments/access-token-types/bearer
>
> ?  If there is a good reason, please use it to at least explicitly acknowledge and justify the basis for failing to provide a way for users and developers to use the "follow your nose" strategy [1].  If there is no good reason, please include the appropriate language to require something along the lines suggested above.  If you need a model, see [2].

I'm confused - is this a request to use a full URI for all extension 
values? I thought the purpose of a registry was to deconflict the short 
names, and that URIs could be used for anything else.

> Minor Issues:
>
> A short summary of the changes from OAuth 1.0/RFC5849 would have been helpful, and it might still be a good idea to add one. . .

I don't think this is possible. OAuth2 is built fundamentally 
differently from OAuth1, so this paragraph might as well read "just 
about everything but the concept". Suggestion: don't add this, unless 
someone can come up with a succinct way to summarize the decision to 
build on a new foundation.

> 4.1  Would it be helpful to indicate that steps D&E may occur at any time after C, and may be repeated subsequently?

D&E may be delayed, but shouldn't be repeated. The Access Code is 
one-time-use, and in the best deployments will expire after a short 
period of time. Suggestion: leave as is.

> 11.1  It might be useful to have an 11.1.2, which repeats references to the bearer and mac access token type registration drafts?
Is this a request to pre-register the two "core" token types? I'm fine 
with that, if that's valid. I've long thought that the "core" document 
needs stronger pointers to the other two. Suggestion: add in the refs if 
it's editorially valid to do so.



(Nits mentioned are valid formatting errors, should be fixed.)



  -- Justin