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

From: Stephen Farrell <[email-masked]>
Date: Mon, 12 Sep 2011 22:02:21 +0100

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 <[email-masked]>
To: [email-masked], [email-masked], [email-masked]

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

---

From: Greg Brail <[email-masked]>
Date: Tue, 13 Sep 2011 16:52:43 -0700

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
<[email-masked]> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> -----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
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth

---

From: André DeMarre <[email-masked]>
Date: Wed, 14 Sep 2011 17:25:03 -0700

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

---

From: Greg Brail <[email-masked]>
Date: Thu, 15 Sep 2011 13:08:47 -0700

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.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] 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

Regards,
Andre DeMarre

---

From: Leif Johansson <[email-masked]>
Date: Thu, 15 Sep 2011 23:39:29 +0200

> prevent its usage by some applications.

Thats why I said that whenever you have negotiation you must think
about downgrade attacks. There are ways to mitigate those - for instance
by re-examining the available methods/mechanisms once a secure channel
has been established.

	Cheers Leif

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Tue, 20 Sep 2011 15:12:45 -0700

(+OAuth WG, -everyone else)

Thanks Leif.

See comments inline. Whatever issues are still open will be addressed along with the rest of the IETF LC feedback.

EHL

> From: Leif Johansson [mailto:[email-masked]]
> Sent: Monday, September 12, 2011 11:31 AM

> 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
> 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
>
> there are some error responses that are triggered by unsupported
> request parameters. This seems like an inconsistency.
>
> ** Specifics
>
> 1.1 Roles
>

What sections? That's the only place these roles are defined.

> As an illustration of an alternative model for the authorization
> server maybe an informative reference to UMA would be illustrative here.

A reference to UMA would be confusing in this document.

> 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).

Changed to:

              (A) The client requests authorization from the resource owner. The authorization request
              can be made directly to the resource owner (as shown), or preferably indirectly via
              the authorization server as an intermediary.

> and password)" that seems to indicate that other credentials could be supported.
>
> Owner Credentials with no reference to passwords other than as as an
> example of one possible credential type.

Changed to:

        (i.e. username and password)

This is explicitly just for username and password. Other types require an extension.

> guidance on generating those tokens (which has been brought up on the
> list) possibly as part of an implementation-guide.

There is not requirement for the string to be opaque, but the general architecture assumes it is. Otherwise, please suggest language.

> 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.

Simpler architecture but probably more confusing to many developers.

> language here - there are security implications of allowing a refresh
> token to get more permissions
> (say) than the original access token.

This was discussed before and we could not reach consensus.

> cloud service is 'confidential'...hmm...

These are the best terms we could come up with. Not intuitive but well defined.

> 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.

Please suggest text.

> 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.

We could not reach consensus beyond this. This language was carefully crafted to work around the disagreements about what is required and what is not.

> 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.

When we say password we mean 'plain old password'. Any other types of credentials are not covered by design.

> 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.
> client_password for anything and MUST follow normal HTTP
> authentication method negotiation.
>

Downgrade attacks security section sounds useful. Text?

> document and I don't understand why it isn't a MUST - after all
> doesn't extensibility depend on this?

I don't have an objection, but extensibility isn't currently required.

> 3.1.1 Response Type
>
> The response_type parameter is REQURED but its absence SHOULD result
> in an error. Why not MUST?

This is an OAuth 1.0 legacy of not mandating a particular error behavior. MUST seems like the right language - any objections?

> 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.

This has been discussed by the WG and no consensus was reached. We can reconsider if someone submits proposed text.

> "(the client MAY use the state request parameter to achieve
> per-request customization)". Doesn't this overload its use for CSRF-protection?

Yep. That's intentional.

> "The authorization server SHOULD NOT redirect...". Why isn't this a

I'm ok with MUST NOT - any objections?

> since in this case it could refer to both server-side code aswell as

This is only about the HTML response sent back to the user-agent. This is about including something like 'Share on Twitter/Facebook' widget on a page with an access token in the URI fragment (which has caused tokens to leak to Twitter in some cases).

> which case I argue for something easier to understand ;-)

Ok.

> The second bullet: Using client credentials more often also exposes
> them more which might be worth mentioning aswell.

Proposed text?

> 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.

Password is intentional. This is a special case for passwords.

> client in client registration"

Changed to:

              Assuming the resource owner grants access, the authorization server redirects the
              user-agent back to the client using the redirection URI provided earlier (in the
              request or during client registration). The redirection URI includes an authorization
              code and any local state provided by the client earlier.

> redirect_uri is either resent or part of client registration but that needs to be made explicit in that case.

3.1.2 describe how to handle this parameter.

> 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

Changed to server MUST not allow reuse based on previous feedback. Not sure treating it as an attack is the right language, but the normative language is the same either way (about revoking tokens).

> Also note that this is potentially a DOS attack should a single authz code leak.

Explain?

> 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"

Not sure what you mean.

> how unsupported parameters can generate an error since endpoints
> should ignore unsupported parameters (in order to support extensibility).

Good catch. Dropped 'unsupported parameter' part.

> client issued client credentials (or with other authentication requirements)"
>
> 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).

I felt it was important to call out confidential clients explicitly, but I agree that if we require issuing credentials to such clients, this can be made simpler. What do other think? The same end result, but different emphasis.

> 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?

Here yes. String comparison.

> extend "fragment-transport" to include a refresh-token, so it isn't a
> technically motivated constraint, right?

Instead I added this to 4.2.2:

            The authorization server MUST NOT issue a refresh token.

> security dependency for this grant type.

That's RFC2616. Seems odd to reference it here.

> 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.

Worse how? You can't use this with resource owner password.

> "MUST NOT include any scope not originally granted by the resource owner".

Ok.

> 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?

None suggested.

> 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.

Ok.

> this needs to be defined (cf comments on 3.1.2 above).

Changed to identical.

> 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.

Public clients rely on the registration to provide a minimal level of client identification while confidential clients are later authenticated which can mitigate redirection URI manipulation.

> 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.

Need someone to suggest text.

EHL

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Tue, 20 Sep 2011 15:20:21 -0700

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

> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of André DeMarre
> 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.
> 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.
>
> Clients in possession of client credentials MAY use any form of authentication
> scheme supported by the authorization server.
> ---
> >> 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.
> >>
> >> 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.
> >>
> >> 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
> >>
> >> 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.
> >>
> >> token to get more permissions (say) than the original access token.
> >>
> >> 2.1 Client types
> >>
> >> 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.
> >>
> >> request parameters". This formulation returns in several places in
> >> the document and I don't understand why it isn't a MUST - after all
> >> this comparison has security implications it should be a priority for
> >> the specification to be air-tight.
> >>
> >> 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.
> >>
> >> 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 ;-)
> >>
> >> 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
> >> (C) - "using the redirection URI provided earlier" should perhaps
> >> read "using the redirection URI provided earlier or associated with
> >> the client in client registration"
> >>
> >>
> >> 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.
> >>
> >> the session cookie..." feels like it is implementation advice
> >> yearning to come out of the closet.
> >>
> >>
> >> Thats it.
> >>
> >>        Cheers Leif

---

From: André DeMarre <[email-masked]>
Date: Tue, 20 Sep 2011 19:11:46 -0700

> 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 i…

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

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Tue, 20 Sep 2011 20:52:31 -0700

This will go into the IETF LC feedback loop.

EHL

> From: André DeMarre [mailto:[email-masked]]
> Sent: Tuesday, September 20, 2011 7:12 PM
> To: Eran Hammer-Lahav; OAuth WG
> > 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:
> ----
> This helps to communicate that authorization servers are only required to
> support HTTP Basic if they issue client passwords.
>
> Andre
>
> > 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.
> >
> >> From: [email-masked] [mailto:[email-masked]] On
> >> Behalf Of André DeMarre
> >> 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 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.
> >> >
> >> > 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.
> >> >
> >> > client identifier is used as the username, and the client password
> >> > is used as the password.
> >> >
> >> > Alternatively, the authorization server MUST also allow the client
> >> > to include the client credentials in the request body using the
> >> > following
> >> > parameters:
> >> >
> >> >   client_id
> >> > Clients in possession of a client password MAY use either mechanism
> >> > in order to authenticate with the authorization server. However,
> >> > 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).
> >> >
> >> >> 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.
> >> >>
> >> >> 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.
> >> >>
> >> >> 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.
> >> >> _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
> >> >>
> >> >> 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.
> >> >>
> >> >> (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).
> >> >>
> >> >> 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.
> >> >>
> >> >> 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
> >> >>
> >> >> should be language defining the behavior of public and
> >> >> confidential client aswell as the three profiles.
> >> >>
> >> >> 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
> >> >> 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
> >> >> 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?
> >> >>
> >> >> 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.
> >> >>
> >> >> 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.
> >> >>
> >> >> ambiguous since in this case it could refer to both server-side
> >> >> code aswell as in-browser code.
> >> >>
> >> >> The phrase "clients issued client credentials" could be rephrased
> >> >> to make less violence on English - perhaps "clients that have been
> >> >> The second bullet: Using client credentials more often also
> >> >> exposes them more which might be worth mentioning aswell.
> >> >>
> >> >> 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
> >> >>
> >> >> read "using the redirection URI provided earlier or associated
> >> >> with the client in client registration"
> >> >>
> >> >>
> >> >> 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 either resent or part of client registration but
> >> >> that needs to be made explicit in that case.
> >> >>
> >> >> 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"
> >> >> 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).
> >> >>
> >> >> This text seems unnecessarily convoluted. Isn't enough to say that
> >> >> if you have issued credentials to a client you MUST require
> >> >> 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
> >> >>
> >> >> 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.
> >> >>
> >> >> scope - The term "lesser than" is intuitive but imprecise. I
> >> >> suggest "MUST NOT include any scope not originally granted by the
> >> >> resource
> >> owner".
> >> >>
> >> >> 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?
> >> >>
> >> >> In the third paragraph "the same as" wrt redirection URIs occur
> >> >> and this needs to be defined (cf comments on 3.1.2 above).
> >> >>
> >> >> 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