Re: [OAUTH-WG] Fwd: secdir review of draft-ietf-oauth-v2

I understand and thanks for clarifying. I agree that there may be services
that do not want to support HTTP Basic at all for their authorization
flows and that requiring it would weaken the security of OAuth 2.0 and
prevent its usage by some applications.

Still, the spec, to me, implies that authorization servers MUST support
Basic because it says that the client MAY use it.

So instead of my proposal, I think it's important to clarify the spec as
Andre suggests, and change 2.3.1 as he proposes below.

Gregory Brail   |   Technology   |   Apigee   |   +1-650-937-9302

-----Original Message-----
From: oauth-bounces@ietf.org [mailto:oauth-bounces@ietf.org] On Behalf Of
André DeMarre
Sent: Wednesday, September 14, 2011 5:25 PM
To: OAuth WG
Subject: Re: [OAUTH-WG] Fwd: secdir review of draft-ietf-oauth-v2

I agree that stating "Clients in possession of a client password MAY
use the HTTP Basic authentication scheme" [Section 2.3.1 paragraph 1]
implies that authorization servers MUST support HTTP basic
authentication, but such is never asserted. Instead, it says "The
authorization server MAY accept any form of client authentication
meeting its security requirements." [Section 2.3 paragraph 1] This is
somewhat contradictory.

I can understand that requiring a specific method of client
authentication is desirable for maximum interoperability, but this
would be problematic for authorization server implementations that
wish to enforce stronger security than HTTP Basic. Such
implementations would be forced to deviate from the specification. In
particular, implementations which choose MAC access tokens instead of
Bearer tokens may wish to add a layer of security to defend against
improperly configured TLS connections, or to protect clients who
connect to the wrong server.
[http://hueniverse.com/2010/09/oauth-bearer-tokens-are-a-terrible-idea/]
Such implementations will also find HTTP Basic undesirable for client
authentication. To require a form of client authentication that isn't
universally sufficient could become a source of criticism and deter
adoption of OAuth 2.0.

I think the best solution is to clarify section 2.3.1 as follows:
---
Clients in possession of client credentials MAY use any form of
authentication scheme supported by the authorization server.
---
And then follow with the existing example that demonstrates HTTP Basic.

Regards,
Andre DeMarre

On Tue, Sep 13, 2011 at 4:52 PM, Greg Brail <greg@apigee.com> wrote:
> I would like to add my support to the comments below on section 2.3,
> specifically 2.3.1.
>
> It is clear to me from reading section 2.3 that clients MAY use HTTP
> basic, or they MAY include client_id and client_secret in the request
> body -- however, the latter is not recommended.
>
> It is not clear what the authorization server MUST support. IMHO, that
> leads us to a situation in which there is no universally-agreed set of
> authentication technology that all programmers can assume is going to
> work, which means that interoperability will be difficult as some
> authorization servers will support Basic, others will support the
> request body, and others will do neither in favor of something else.
>
> I would prefer that we make both HTTP basic AND the request body
> mechanisms in this section both required on the server side, thus
> giving the client the option of choosing one or the other. That would
> mean re-writing the beginning of section 2.3.1 as shown below.
>
> If I have missed other discussion on this topic I apologize. If there
> is already consensus to make the "message body" authentication
> optional rather than required for the authorization SERVER then I
> would still recommend that we make HTTP Basic a MUST in order to allow
> easier interop.
>
> Proposed change to 2.3.1:
>
> ----
>
> The authorization server MUST support the HTTP Basic authentication
> scheme as defined in [RFC2617] as a way to identify clients. The
> client identifier is used as the username, and the client password is
> used as the password.
>
> For example (extra line breaks are for display purposes only):
>
>   Authorization: Basic czZCaGRSa3F0MzpnWDFmQmF0M2JW
>
> Alternatively, the authorization server MUST also allow the client to
> include the client credentials in the request body using the following
> parameters:
>
>   client_id
>         REQUIRED.  The client identifier issued to the client during
>         the registration process described by Section 2.2.
>   client_secret
>         REQUIRED.  The client secret.  The client MAY omit the
>         parameter if the client secret is an empty string.
>
> Clients in possession of a client password MAY use either mechanism in
> order to authenticate with the authorization server. However,
> including the client credentials in the request body using the two
> parameters is NOT RECOMMENDED, and should be limited to clients unable
> to directly utilize the HTTP Basic authentication scheme (or other
> password-based HTTP authentication schemes).
>
>  (Rest of section remains as-is with the paragraph beginning "For
example...")
>
> Gregory Brail   |   Technology   |   Apigee   |   +1-650-937-9302
>
> On Mon, Sep 12, 2011 at 2:02 PM, Stephen Farrell
> <stephen.farrell@cs.tcd.ie> wrote:
>>
>> FYI, probably best for the WG to see/process these secdir
>> comments as appropriate. I've not read 'em in detail myself
>> yet, so as Leif says, feel free to react as appropriate.
>>
>> S.
>>
>> PS: Thanks Leif for reviewing this.
>>
>> -------- Original Message --------
>> Subject: secdir review of draft-ietf-oauth-v2
>> Date: Mon, 12 Sep 2011 20:31:06 +0200
>> From: Leif Johansson <leifj@sunet.se>
>> To: draft-ietf-oauth-v2@tools.ietf.org, secdir@ietf.org, iesg@ietf.org
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>
>> Security review of OAUTH 2.0 core: draft-ietf-oauth-v2-21
>>
>> Do not be alarmed.  I have reviewed this document as part of the
>> security directorate's ongoing effort to review all IETF documents
being
>> processed by the IESG.  These comments were written primarily
>> for the benefit of the security area directors.  Document editors
>> and WG chairs should treat these comments just like any other last
>> call comments.
>>
>> This review is rather lengthy. This should not be interpreted as
>> anything beyond a desire to do a thorough review.
>>
>> It may well be that I have stumbled on things already covered on the
>> list. If so I apologize and ask that you silently ignore such bits.
>> Also I have included things that are not directly security related
>> but that I found problematic for other reasons.
>>
>> The notes are presented in the order I wrote them down.
>>
>> ** General observations:
>>
>> POST and/or GET
>>
>> Examples are sometimes POST and sometimes GET. In many cases it is not
>> clear to me from the surrounding text if both POST and GET are allowed
>> or if only one is mandated. Illustrating with both a GET _and_ POST
>> example in the cases where both are supported would help or make the
>> method explicit in the text before the example.
>>
>> The P-word
>>
>> The term 'password' is sprinkled throughout the document, sometimes
>> as in "client password" or "resource owner password credentials" and
>> I suspect that sometimes it is password as in 'an example of a
>> credential type' and in other cases it is password as in 'plain old
>> password'. This needs to be cleared up throughout (I've included some
>> examples below).
>>
>> Normative Language
>>
>> I've often found myself wanting more normative language often to
replace
>> existing but less precise text. I've called out some important cases
>> below.
>>
>> Unknown parameters
>>
>> The sentence "The client SHOULD ignore unrecognized response
parameters"
>> occurs in several places. First of all I would argue that this has to
>> be a MUST if you want to be able to guarantee extensibility. Secondly
>> there are some error responses that are triggered by unsupported
request
>> parameters. This seems like an inconsistency.
>>
>> ** Specifics
>>
>> 1.1 Roles
>>
>> Forward references to the sections where the roles are defined would
>> improve readability.
>>
>> As an illustration of an alternative model for the authorization server
>> maybe an informative reference to UMA would be illustrative here.
>>
>> 1.2 Protocol Flow
>>
>> (A) talks about 'an intermediary such as an authorization server.' it
>> isn't clear it me if this refers to a separate authorization server
>> or if it is the same authorization server that is referenced in (B).
>>
>> 1.3.3 Resource Owner Password Credentials
>>
>> When I first read this I thought - why not talk about Resource Owner
>> Credentials - in fact there is a parenthesis there "(e.g a username
>> and password)" that seems to indicate that other credentials could
>> be supported.
>>
>> This needs to be cleared up. Either its username and password and
>> nothing else or there is a way to support things like Negotiate or
>> X.509-based credentials in which case it should really be Resource
>> Owner Credentials with no reference to passwords other than as as
>> an example of one possible credential type.
>>
>> 1.4 Access Token
>>
>> first paragraph: "The string is usually opaque". This should be
>> reformulated as normative language. In fact I would have liked
>> guidance on generating those tokens (which has been brought up
>> on the list) possibly as part of an implementation-guide.
>>
>> 1.5 Refresh Token
>>
>> Why is the refresh token not simply treated as an access token
>> for the access token resource in the authorization server? This
>> would seem to simplify the model a bit by removing a type of
>> token whose semantic (at least to this reviewer) is a duplication
>> of a normal access token for a particular type of resource.
>>
>> Also in the first paragraph: "(access tokens may have a shorter
>> lifetime and fewer permissions". Why not try to write normative
>> language here - there are security implications of allowing
>> a refresh token to get more permissions (say) than the original
>> access token.
>>
>> 2.1 Client types
>>
>> I find the terminology public/confidential somewhat counter-intuitive.
>> An app you have on your personal device is 'public' while a shared
>> cloud service is 'confidential'...hmm...
>>
>> This section also lacks normative language which isn't good. There
>> should be language defining the behavior of public and confidential
>> client aswell as the three profiles.
>>
>> For instance under native application we have "It is assumed that
>> any client authentication credentials included in the application
>> can be extracted". This should really be normative language. Some
>> of what I am looking for can be found in section 2.3 but it isn't
>> clear to me that it covers the definition of the profiles.
>>
>> 2.3.1 Client Password
>>
>> There is also some confusion here about what the client must implement
>> and what the server must implement wrt the use of client_id. I read the
>> text as trying to say that Servers SHOULD support both and clients
SHOULD
>> only do Basic.
>>
>> This section also seems to have been the product of a partial
>> s/password/credential/g operation. Either OAUTH is only meant to use
>> Basic and passwords or we want to cover all/most HTTP authentication
>> methods and credentials and then section 2.3.2 (which feels like an
>> afterthought) needs to get folded into 2.3.1 and the normative language
>> (and examples!) need some work to cover at least X.509s and perhaps
>> even Negotiate.
>>
>> Personally I would try to fold 2.3.1 and 2.3.2 into one section and say
>> that servers MUST support Basic and client_id+client_password. Servers
>> MAY support any HTTP authentication method with a mapping to client_id.
>> If a client supports username+passwords it SHOULD do Basic and if it
>> supports other HTTP authentication methods it MUST NOT use
>> client_password for anything and MUST follow normal HTTP authentication
>> method negotiation.
>>
>> Finally, everywhere there is negotiation there must imho be some
>> mention/discussion/protection of downgrade attacks.
>>
>> 3.1 Authorization Endpoints
>>
>> 6th paragraph: "The authorization server SHOULD ignore unrecognized
>> request parameters". This formulation returns in several places in the
>> document and I don't understand why it isn't a MUST - after all doesn't
>> extensibility depend on this?
>>
>> 3.1.1 Response Type
>>
>> The response_type parameter is REQURED but its absence SHOULD result in
>> an error. Why not MUST?
>>
>> 3.1.2 Redirection Endpoint
>>
>> There should be a clear normative specification for how to  match
>> endpoints. There are traces of this in various parts of the document
>> when matching is discussed. I recommend making a concise definition
>> in one place (namely here) and referencing this throughout. Since
>> this comparison has security implications it should be a priority
>> for the specification to be air-tight.
>>
>> 3.1.2.2 Registration Requirements
>>
>> "(the client MAY use the state request parameter to achieve per-request
>> customization)". Doesn't this overload its use for CSRF-protection?
>>
>> 3.1.2.4 Invalid Endpoint
>>
>> "The authorization server SHOULD NOT redirect...". Why isn't this a
>> MUST NOT?
>>
>> 3.1.2.5 Endpoint Content
>>
>> This section basically seems to say "Serve with server-side code not
>> with html or client-side code". If this is the case then I suggest
>> reformulate to say precisely that using normative language.
>>
>> The use of the word "script" could perhaps also be made less ambiguous
>> since in this case it could refer to both server-side code aswell as
>> in-browser code.
>>
>> 3.2.1 Client Authentication
>>
>> The phrase "clients issued client credentials" could be rephrased to
>> make less violence on English - perhaps "clients that have been issued
>> with client credentials", unless that is not the intended meaning in
>> which case I argue for something easier to understand ;-)
>>
>> The second bullet: Using client credentials more often also exposes
them
>> more which might be worth mentioning aswell.
>>
>> 4. Obtaining Authorization
>>
>> Perhaps not critical but the term 'password credentials' occurs in the
>> first paragraph and this doesn't seem compatible with resource owner
>> authentication being out of scope. It could just be that it should read
>> 'resource owner credentials' but it could also signal an underlying
>> assumption about username and password being used for resource owner
>> authentication. In either case I thing its best to avoid the use of the
>> word 'password' to avoid any confusion.
>>
>> 4.1 Authorization Code
>>
>> (C) - "using the redirection URI provided earlier" should perhaps read
>> "using the redirection URI provided earlier or associated with the
>> client in client registration"
>>
>>
>> 4.1.1 and 4.1.2 Authorization Request/Authorization Response
>>
>> The redirect_uri is listed as OPTIONAL with a reference to 3.1.2 but
>> there is no mention in 4.1.2 how to handle the case when the
>> redirect_uri is not present. I believe the assumption is that the
>> redirect_uri is either resent or part of client registration but that
>> needs to be made explicit in that case.
>>
>> This may apply to other uses of the redirect_uri parameter eg in 4.2.1.
>>
>> Furthermore in 4.2.2 "code" I suggest the following re-formulatation of
>> the last sentence: "The client MUST NOT use an authorization code for
>> more than one request. If an authorization code is re-used, the
>> authorization server should treat that as a replay attack and SHOULD
>> revoke all tokens associated with the client." (i.e loose the "attempt"
>> bit which carries no real meaning)
>>
>> Also note that this is potentially a DOS attack should a single authz
>> code leak.
>>
>> 4.1.2.1 Error Response
>>
>> First paragraph, last sentence "and MUST NOT automatically redirect".
In
>> the light of how willing users normally are to click on links presented
>> to them I would strengthen this to "MUST prevent the user from
following
>> the redirect URI"
>>
>> In the definition of the invalid_request parameter I don't understand
>> how unsupported parameters can generate an error since endpoints should
>> ignore unsupported parameters (in order to support extensibility).
>>
>> 4.1.3 Access Token Request
>>
>> "require client authentication for confidential clients or for any
>> client issued client credentials (or with other authentication
>> requirements)"
>>
>> This text seems unnecessarily convoluted. Isn't enough to say that if
>> you have issued credentials to a client you MUST require authentication
>> from that client? This applies equally to the other sections where
>> client authentication is an issue (eg 4.3.2).
>>
>> Also cf my comment on 3.1.2 for the discussion of matching redirect_uri
>> in the last bullet: ".. and that their values are identical". Is this
>> really meant to mean identical?
>>
>> 4.2 Implicit Grant
>>
>> The parenthesis "(it does not support the issuance of refresh tokens)"
>> sounds like it should really be normative language, "refresh tokens
>> MUST NOT be issues for implicit grant" because afaiu you could easily
>> extend "fragment-transport" to include a refresh-token, so it isn't
>> a technically motivated constraint, right?
>>
>> In (D) I would like to have a normative reference to a document that
>> specifies browser behavior for URL fragments since this is a critical
>> security dependency for this grant type.
>>
>> 4.4 Client Credentials
>>
>> I think the text should note that this model effectively implies
>> that the client is able to impersonate all users which has the
potential
>> for worse security problems than if the client has access to individual
>> user passwords.
>>
>> 6 Refreshing an Access Token
>>
>> scope - The term "lesser than" is intuitive but imprecise. I suggest
>> "MUST NOT include any scope not originally granted by the resource
owner".
>>
>> 7.1 and 8.1 Access Token Types
>>
>> The section 7.1 lists two definitions of access token types and
provides
>> a couple of examples but doesn't provide any constraints on future
>> development of access tokens except in 8.1 where it is implied that not
>> all access token types would be associated with HTTP authentication
>> mechanisms. Are there really no constraints on access token types
>> that should be captured?
>>
>> 10.6 Authorization Code Redirection URI Manipulation
>>
>> Suggest replace the word 'familiar' with 'trusted' in the first
sentence
>> of the second paragraph. The notion of trust opens up several boat
loads
>> of worm but it is the correct term here I think.
>>
>> In the third paragraph "the same as" wrt redirection URIs occur and
>> this needs to be defined (cf comments on 3.1.2 above).
>>
>> Finally "The authorization server MUST require public clients and
SHOULD
>> require confidential clients to register their redirection URI". I am
>> missing a discussion of why the two types of client differ wrt this
>> attack vector.
>>
>> 10.10 Credentials Guessing Attack
>>
>> I found myself wanting implementation advice for how to generate good
>> tokens at this point. This has been raised on the list too. The same
>> goes for how to generate good CSRF cookies where the "(eg a hash of the
>> session cookie..." feels like it is implementation advice yearning to
>> come out of the closet.
>>
>>
>> Thats it.
>>
>>        Cheers Leif
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.11 (GNU/Linux)
>> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>>
>> iEYEARECAAYFAk5uT+YACgkQ8Jx8FtbMZncXQgCfZmTlzuESq0plfpkceQN1ontE
>> a1QAoIEcg06GYK+6Fn4y40cTL1jQ+KmS
>> =ox42
>> -----END PGP SIGNATURE-----
>>
>> _______________________________________________
>> OAuth mailing list
>> OAuth@ietf.org
>> https://www.ietf.org/mailman/listinfo/oauth
> _______________________________________________
> OAuth mailing list
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth
>
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth