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

[André DeMarre <[e-mail]>]

> If the server issues clients with password credentials it MUST support HTTP Basic (this is the flip side of 'the client MAY use HTTP Basic') and should only support the client_secret parameter if it has clients incapable of using HTTP Basic.

Very well. Without changing the meaning, I think the community would
be well served by appending paragraph 2 of Section 2.3 as follows:
----
   Confidential clients are typically issued (or establish) a set of
   client credentials used for authenticating with the authorization
   server (e.g. password, public/private key pair).  If clients are
   issued passwords, the authorization server MUST support the HTTP
   Basic authentication scheme as defined in [RFC2617] and described
   by Section 2.3.1.
----
This helps to communicate that authorization servers are only required
to support HTTP Basic if they issue client passwords.

Andre

On Tue, Sep 20, 2011 at 3:20 PM, Eran Hammer-Lahav <[e-mail]> wrote:
> If the server issues clients with password credentials it MUST support HTTP Basic (this is the flip side of 'the client MAY use HTTP Basic') and should only support the client_secret parameter if it has clients incapable of using HTTP Basic.
>
> This language has been tweaked to reach a delicate balance. I'm not inclined to touch it.
>
> EHL
>
>> -----Original Message-----
>> From: [e-mail] [mailto:[e-mail]] 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 <[e-mail]> 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   |   [phone]
>> >
>> > On Mon, Sep 12, 2011 at 2:02 PM, Stephen Farrell
>> > <[e-mail]> 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 <[e-mail]>
>> >> To: [e-mail], [e-mail],
>> >> [e-mail]
>> >>
>> >> -----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+YACgkQ8Jx8FtbMZncXQgCfZmTlzuESq0plfpkceQN1ont
>> E
>> >> a1QAoIEcg06GYK+6Fn4y40cTL1jQ+KmS
>> >> =ox42
>> >> -----END PGP SIGNATURE-----
>> >>
>> >> _______________________________________________
>> >> OAuth mailing list
>> >> [e-mail]
>> >> https://www.ietf.org/mailman/listinfo/oauth
>> > _______________________________________________
>> > OAuth mailing list
>> > [e-mail]
>> > https://www.ietf.org/mailman/listinfo/oauth
>> >
>> _______________________________________________
>> OAuth mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/oauth
>