Re: [OAUTH-WG] Comments on -18

[Eran Hammer-Lahav <[e-mail]>]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf
> Of Torsten Lodderstedt
> Sent: Wednesday, July 20, 2011 2:49 PM
> To: OAuth WG
> Subject: [OAUTH-WG] Comments on -18
> 
> section 1.5
> 
> "(A)  The client requests an access token by authenticating with the
>          authorization server, and presenting an authorization grant."
> 
> This statement does not reflect that client authentication is not always
> required.
> 
> Proposal:
> 
> "The client requests an access token by presenting an authorization grant.
> The authorization server may require the client to authenticate as a pre-
> requisite."

The general design for fresh tokens does include authentication. Yes, the normative text allows you not to, but this is the general case and I like the simpler text for the introduction.

> section 4.1
> 
> "(D)  The client requests an access token from the authorization
>          server's token endpoint by including the authorization code
>          received in the previous step.  When making the request, the
>          client authenticates with the authorization server."
> 
> Same as above. Proposal:
> 
> ".... When making the request, the
>          client may be required to authenticate with the authorization server"

Same. Section 4.1.3 makes this clear.
 
> "(E)  The authorization server authenticates the client, validates the
>          authorization code, and ensures the redirection URI received
>          matches the URI used to redirect the client in step (C)."
> 
> Same as above.
> 
> Additionally, is the URI check also required if the client did not passed a
> redirect uri but relied on its pre-registered redirect_uri?

This is the overview. Section 4.1.3 provides the specific requirements.

> section 4.1.1
> 
> "state" parameter: Would it make sense to elaborate on the usage of this
> parameter and its importance for preventing CSRF attacks (incl. the
> nessessary entropy)? Alternatively, a reference to the security consideration
> section could be added.

I think we got this covered. I rather not start pointing to the security section from the sections above.

> section 4.1.3
> 
> "If the client type is private or was issued client credentials ..."
> Isn't the later the most important property of a "private" client? If so, "or was
> issued client credentials" is redundant.

All private clients must authenticate, but also public clients with issued credentials.

> section 4.4.3
> 
> "A refresh token SHOULD NOT be included." Why not "MUST NOT"?

This was debated a while back and the consensus was that there is no reason to restrict this to a MUST NOT. Someone might even had plans to issue refresh tokens using this flow.

> section 5.2
> 
> "The authorization server MAY return an HTTP 401
>                 (Unauthorized) status code to indicate which HTTP
>                 authentication schemes are supported."
> 
> Given the usage of HTTP authentication schemes is the way to authenticated
> client recommended by the spec, status code 401 should be the default
> status code for this kind of error. Usage of status code 400 should be the
> exception.
>
> "unauthorized_client"
> 
> So above - status code 403 seems to be a more appropriate default.

I think this is fine unchanged.

> section 10
> 
> "and furthermore that rotation of the client
>        authentication credentials is impractical."
> 
> What does this mean?

I'll take it out. I don't remember who suggested it (Brian?).

> Please update reference [I-D.lodderstedt-oauth-security] to [I-D.ietf-oauth-
> v2-threatmodel].

Done.

> section 10.2
> 
> last sentence: "... ensure the repeated request comes from an authentic
> client and not an
>     impersonator."
> 
> The authorization server must ensure that the request comes from the same
> client not just any authentic client. So I would propose to adjust the text as
> follows:
> 
> "... ensure the repeated request comes from the original client and not an
>     impersonator."

Ok.

> section 10.3
> 
> "Access token (as well as any type-specific attributes) MUST ..." does "type-
> specific" refer to access token type specific? If so, please add this
> information.

Ok.

> section 10.6
> 
> Please replace the first sentence with the following text:
> 
> "Such an attack leverages the authorization code ..."

That reads funny. How about 'An attacker can leverage...'

EHL