> -----Original Message-----
> From: oauth-boun...@ietf.org [mailto:oauth-boun...@ietf.org] On Behalf
> Of Stephen Farrell
> Sent: Thursday, October 13, 2011 10:13 AM
> Suggested non-trivial clarifications:
> -------------------------------------
>
> (1) 1.3.4 - "previously arranged" might trigger discusses on the document
> since it implies that this spec might not be suited for broad use. I think
> that
> making it clear that the normal mode for client developers is to work against
> an existing service (AS and resource server) would help to clarify that such
> arrangements are ok here.
Added new 'Interoperability' section to the introduction:
OAuth 2.0 provides a rich authorization framework with well-defined
security properties.
However, as a rich and highly extensible framework with many optional
components, this
specification is likely to produce a wide range of non-interoperable
implementations.
In addition, this specification leaves a few required components
partially or fully
undefined (e.g. client registration, authorization server
capabilities, endpoint
discovery).
This protocol was design with the clear expectation that future work
will define
prescriptive profiles and extensions necessary to achieve full
web-scale
interoperability.
There is no way to sugar coat reality and hopefully by being blunt about it
upfront, we will avoid a prolonged debate about the protocol's failings in that
regard.
> (2) p11, in step (F) is there a way to distinguish between an access token
> that
> is invalid due to expiry vs. e.g. data corruption? Section 6 refers to 5.2
> for the
> error codes but its not clear to me which one is returned for this case. I
> think
> clarifying that in section 6 or 5.2 is needed.
That depends on the token specification. Steps C-F are outside the scope of
this document. I'll note that.
> (3) p13, 2.2 and 2.3 - these things happens at registration time right? I
> think
> making that clear is needed since we don't specify a registration protocol
> here.
The entire section 2 is 'Client Registration' which is described as out of
scope for implementation details.
> (4) 2.3.1 uses the term "token endpoint" without definition (its defined in
> section 3) and in particular without making it clear if both access and
> refresh
> token issuance is covered (I guess it is).
Changed to 'when sending requests using password authentication'.
> (5) The same text about x-www-form-urlencoded is repeated various times,
> it'd be better to do that once and refer to it where necessary.
Not enough to be worth the change.
> (6) 3.1.2.2 states the rules for when AS'es are to require registration of
> redirection URIs. I think you need to clarify that some. First, you use the
> term
> "redirection_uri" for both a "complete" URI and for a scheme/authority/path
> triple that can be added to via a query component which is confusing.
> Second, its overall a very complex rule with a MUST, two MAYs and 3
> SHOULDs. I do think it could be made clearer by putting the MUST up front
> and separating issues related to complete URI and triples separately from the
> when something MUST be registered.
New text:
The authorization server MUST require the following clients to
register their redirection endpoint:
o Public clients.
o Confidential clients utilizing the implicit grant type.
The authorization server SHOULD require all clients to register their
redirection endpoint prior to utilizing the authorization endpoint
The authorization server SHOULD require the client to provide the
complete redirection URI (the client MAY use the "state" request
parameter to achieve per-request customization). If requiring the
registration of the complete redirection URI is not possible, the
authorization server SHOULD require the registration of the URI
scheme, authority, and path (allowing the client to dynamically vary
only the query component of the redirection URI when requesting
authorization).
The authorization server MAY allow the client to register multiple
redirection endpoints.
Lack of a redirection URI registration requirement can enable an
attacker to use the authorization endpoint as open redirector as
described in Section 10.15.
> (7) 4.2.1 and elsewhere - refers back to 3.1.2 for the way in which the
> redirection-uri is OPTIONAL - I'm not sure that's sufficiently clear, 3.1.2 is
> quite long and discusses a bunch of things - couldn't it be made clearer when
> the messages are defined?
The reference is not for the OPTIONAL definition. I changed the coma to a
period.
> More generally, is there no way to avoid the
> extensive cross-referencing in the message field definitions? E.g. 4.2.2 has
> references to 7.1 and 3.3, and others are similar. Organizing the text for the
> benefit of the reader is a good thing so would it be worthwhile to do an
> editing pass for just this purpose - to reduce the number of forward
> references and minimize the number of pointers in general?
We've gone back and forth on this for 22 drafts and this is the best balance
between readability and consistency we found. I'm inclined to avoid another
reshuffle.
> On a similar note, 4.1 & 4.2 call out specific errors, but 4.3 defers to 5.2,
> why?
> Be good if that could be made more consistent at the TOC level.
Because 4.3 doesn't use the authorization endpoint.
> (8) How can the AS protect against brute force attacks in 4.3.2? I think you
> could give a bit more guidance, e.g. say to rate-limit or generate alerts or
> whatever, but not as normative text, just good hints.
Ok.
> (9) In 10.12 you say how a client can protect against CSRF via the state field
> but you don't say how the AS can do the same in order to satisfy the MUST in
> the last para of 10.12. Can you not add a hint or reference here?
The WG sentiment was that server developers do not require as much hand holding
as the client developers. If the authors of the original text want to propose
text, I'm happy to include it.
>
> Some nits: (Stuff that seemed more serious at first:-)
> ---------
>
> (1) In 2.3.1 I think you're ruling out putting the client_id and
> client_secret in
> the request URL - is that right? If so, that's good, but I think it needs
> calling
> out since people do that all the time and it'd be good to say why its bad to
> do
> that.
Added:
The parameters can only be transmitted in the request body and MUST
NOT be included in the
request URI
> (2) The redirection endpoint is introduced in 3.2.1 but is not listed at the
> start
> of section 3 which only lists two endpoints.
I think you mean 3.1.2. Added.
> (3) In 4.1.2 what does it mean to "attempt" to revoke tokens? Why can't the
> AS just revoke them? (Where revoke == not accept them when they are
> next presented, right?)
Can't revoke self-encoded tokens (e.g. stateless on the server). Changed
'attempt' to 'when possible'
> (4) I think this is just language but wanna check. 4.1.2.1 and
> 4.2.2.1 errors have a state field. Text says: "If a valid "state"
> parameter was present..." which would imply that state variables are either
> valid or invalid according to the AS. I don't think that's the case, and nor
> should it be. (If it were, I'd have a security concern that I could use
> otherwise
> crap requests to probe for good state values.) s/valid// I think?
Yep.
> (5) I don't get the benefit of saying the client SHOULD ignore unknown fields
> in the response in 4.2.2 - what's effectively the difference between that and
> "MUST ignore" - I don't get it, and hence don't see why you don't say MUST
> ignore.
Changed all 'ignore' to MUST.
> (6) Why say "MUST NOT issue a refresh token" in 4.2.2? Are you making an
> assumption that access & refresh tokens are distinguishable to anyone other
> than the issuer? If not, then is this just saying "don't make a mistake"?
No. It's a security concern that developer might decide to be flexible and
provider a refresh token in addition to an access token using this flow.
> (7) 5.1 says that the client SHOULD ignore "unrecognized response
> parameters" - does that mean unrecognized parameters in the JSON entity
> body? Is it clear enough as-is that those are "response parameters"?
Changed 'parameters' to 'value names in the response'.
> (8) How does the use of TLS on endpoints used for end-user interaction
> "reduce the risk of phishing attacks"? I don't get that. Maybe you mean that
> TLS+users actually checking server identity reduces the risk of successful
> phishes? I think that's a bit different. (I do like the MUST though.)
Dropped 'phishing'. If the authors of the original text would like to explain,
I'll put it back in.
(To be continued...)
EHL
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth
