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

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

Simply added to the first paragraph:

           The authorization server MUST support the HTTP Basic authentication scheme
           for authenticating clients which were issued a client password.

EHL

> -----Original Message-----
> From: André DeMarre [mailto:[e-mail]]
> Sent: Tuesday, September 20, 2011 7:12 PM
> To: Eran Hammer-Lahav; OAuth WG
> Subject: Re: [OAUTH-WG] Fwd: secdir review of draft-ietf-oauth-v2
>
> > 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-ide
> >> a/] 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
> >