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

From: [email-masked] (Henry S. Thompson)
Date: Tue, 14 Feb 2012 17:07:12 +0000

[Sigh, resending with corrected CC list]
Justin writes:

> [Henry wrote]:
>> 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 <a  rel="nofollow"
>> href="http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate">http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate</a>).
>> 
>> 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 <a rel="nofollow"
> href="http://localhost/">http://localhost/</a> 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 &quot;endpoint&quot; to differentiate it from the
> server-side pieces.

OK, sounds like I didn't fully understand the context here.  Providing
the examples you give as motivation would help keep others from making
the same mistake. . .

>> [3].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

Yes, sorry.

> 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.

Hmmm.  Either I'm misreading the text, or I don't understand your
argument. . .  The existing "MUST NOT" para applies to untrusted js.
The second para. is nearly identical in terms of what it wants done,
but covers _all_ js.  Writing MUST/SHOULD NOT and knowing people will
get it wrong just undermines the value of the spec.

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

If by that you mean, moving it to the security section, and observing
there that _failure_ to sanitise early is a risk, by effectively
changing MUST NOT to WILL RISK COMPROMISE and SHOULD NOT to MAY RISK
COMPROMISE, that would work for me.

>> 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 &quot;follow your nose&quot; 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.

No, I appreciate that you want to use registered short names in the
protocol, that's just fine.  My problem is that you have left users,
developers etc. with no way to discover what shortnames have been
registered short of a non-trivial and error-prone informal search
effort.

What I'm asking for is support for probe URI prefixes for each family
of shortnames.  So, just as today I use "text/n3" as the media type for
my Notation3 documents, I can check that this is a registered media
type by attempting to retrieve

 http://www.iana.org/assignments/media-types/text/n3

and I can see all the registered text types by retrieving from

 http://www.iana.org/assignments/media-types/text

likewise I ought to be able to check e.g. the "bearer" token type by
attempting retrieval from (something along the lines of)

 http://www.iana.org/assignments/access-token-types/bearer

and I should be able to see all the registered token types by retrieving from

 http://www.iana.org/assignments/access-token-types

Hope this clarifies what I meant.

>> 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 nfundamentally
> 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.

OK, thanks.  Even something like the above short statement would be
useful . . .

>> 4.1 Would it be helpful to indicate that steps D&amp;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.

OK, I just think as it is it invites misinterpretation.

>> 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?

Yes, in that what I had in mind was making 11.1 parallel to ll.2, 3
and 4 in this regard.

ht

[I'm sorry about the mailing list confusion.  But please don't punish
 me again by sending HTML change bar markup which I have to hand edit :-]
-- 
       Henry S. Thompson, School of Informatics, University of Edinburgh
      10 Crichton Street, Edinburgh EH8 9AB, SCOTLAND -- (44) 131 650-4440
                Fax: (44) 131 650-4587, e-mail: [email-masked]
                       URL: http://www.ltg.ed.ac.uk/~ht/
 [mail from me _always_ has a .sig like this -- mail without it is forged spam]