Subject: [OAUTH-WG] WGLC on draft-ietf-oauth-v2-bearer-03.txt

From: Hannes Tschofenig <[email-masked]>
Date: Wed, 2 Mar 2011 10:31:33 +0200

This is a Last Call for comments on 

http://www.ietf.org/id/draft-ietf-oauth-v2-bearer-03.txt

Please have your comments in no later than March 25 (extended deadline because of the ongoing OAuth base specification WGLC).

Do remember to send a note in if you have read the document and have no 
other comments other than "its ready to go" - we need those as much as we need "I found a problem".

Thanks!
Hannes & Blaine

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Wed, 2 Mar 2011 09:33:40 -0700

Last call comments on draft-ietf-oauth-v2-bearer-03:

Section 2.1:

- ABNF includes '1#auth-param', but no prose is provided about how new parameters may be defined. There is no reason for this scheme to be extensible, given its designed simplicity and limitations. If more attributes are needed in the future, a new scheme must be defined.

- RWS should be replaces with SP since the header has no other attributes (and should not have any). This will make parsing easier by mandating a single space (unless HTTPbis has other guidelines).

Section 2.4:

- Realm is a required attribute per RFC 2617. While discussions about this are on-going in HTTPbis (open issue), this document must comply with the current standard. If the WG feels strongly about it, it should engage the HTTPbis working group and provide feedback.

- ABNF includes '( token "=" ( token / quoted-string ) )', but no prose is provided about how new parameters may be defined. Retained this extensibility point must be justified with actual use cases.

Section 2.4.1:

- Why are the HTTP error code suggested and not required?

Section 4.1.1:

- There is no 'Additional Resource Request Parameters' in the template - remove.
- Missing 'Additional Token Endpoint Response Parameters: None'.

Section 4.2:

- The entire registry is ridiculous. Protected resource endpoints are completely within the authority of the resource provider. The fact that this document defines the 'oauth_token' parameter which infringes on the provider's namespace is bad enough. To suggest that this document has any authority over other parameters defined and used by the resource server is overreaching. No single use cases has been suggested for this registry in over three years of OAuth work.

Section 4.2.2.2:

- There is no 'error' parameter for resource responses. It is not defined anywhere in this specification or elsewhere. There is an 'error' attribute defined for the WWW-Authenticate header, but so is 'error_description' and 'error_uri'.  How exactly is this parameter added to the resource response if not in the header? If it is in the header, there is no such location in the registry (nor is header extensibility discussed see 2.4).

Section 4.3:

- This registry is unnecessary and adds no value here (namespace collision is unlikely in general, and unlikely to cause problems). No use cases where suggested to justify it, and no additional error codes were proposed in over a year of discussions. Error codes were intentionally left non-extensible to increase interoperability. If addition color is needed for existing error codes, additional response parameters may be registered. Otherwise, if new error codes are needed, a new RFC must be published updating draft-ietf-oauth-v2.
- The only way to define such a registry is to bring it up as a comment for draft-ietf-oauth-v2. Otherwise, it is limited to the Bearer token header only (and since this document is not extensible, not needed even here).

This document is not ready for publication.

EHL

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of Hannes Tschofenig
> Sent: Wednesday, March 02, 2011 12:32 AM
> To: OAuth WG
> Subject: [OAUTH-WG] WGLC on draft-ietf-oauth-v2-bearer-03.txt
> 
> Please have your comments in no later than March 25 (extended deadline
> because of the ongoing OAuth base specification WGLC).
> 
> _______________________________________________
> OAuth mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth

---

From: Mike Jones <[email-masked]>
Date: Tue, 15 Mar 2011 14:53:53 +0000

This specification is ready to publish.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Hannes Tschofenig
Sent: Wednesday, March 02, 2011 12:32 AM
To: OAuth WG
Subject: [OAUTH-WG] WGLC on draft-ietf-oauth-v2-bearer-03.txt

Do remember to send a note in if you have read the document and have no other comments other than "its ready to go" - we need those as much as we need "I found a problem".

Thanks!
Hannes & Blaine

---

From: Justin Richer <[email-masked]>
Date: Wed, 16 Mar 2011 17:19:42 -0400

Overall this is in good shape. Some points:

Mutual cross-reference between the MAC and Bearer specs might help 
people trying to decide what kind of OAuth token to use.

2.3
The parameter name "oauth_token" is already taken by OAuth 1.0 and 
should NOT be re-used here. Suggest changing it to "oauth2_token". An 
alternative is to go back to the original non-namespaced "access_token", 
which I don't recommend.

4.2
The protected resource parameter registry doesn't make much sense as 
specified, and seems to imply that all non-oauth PR parmeters would also 
need to be registered here. Suggest removal of this.

4.3
The error registry needs to be removed from this draft and either placed 
into the core spec or another extension capability should be defined. 
Either way, this spec isn't the place for it.

  -- Justin

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Wed, 16 Mar 2011 15:20:20 -0700

> Of Justin Richer

> people trying to decide what kind of OAuth token to use.

Speaking for MAC - over my dead body :-)

EHL

---

From: Justin Richer <[email-masked]>
Date: Wed, 16 Mar 2011 18:41:58 -0400

> Speaking for MAC - over my dead body :-)

Now now, that's just not nice. :)

---

From: Peter Saint-Andre <[email-masked]>
Date: Wed, 23 Mar 2011 12:11:09 -0600

<hat type='AD'/>

> other comments other than "its ready to go" - we need those as much as we need "I found a problem".

I agree with much of the last call feedback that other folks have
provided. Here are some additional comments.

1. It would be helpful to define "bearer token" in the introduction
because this term is not defined in RFC 4949 or the base OAuth spec.

2. Please cite RFC 2617 when mentioning the "Authorization" request
header in Section 2.

3. Some of the conformance language is confusing, e.g. the construction
"SHOULD only X if Y" is better as "SHOULD NOT X unless Y".

For example (Section 2):

   Clients SHOULD only use the request body or URI when the
   "Authorization" request header field is not available, and MUST NOT
   use more than one method to transport the token in each request.
   Because of the Security Considerations (Section 3) associated with
   the URI method, it SHOULD only be used if no other method is
   feasible.

That is better as:

   Clients SHOULD NOT use the request body or URI unless the
   "Authorization" request header field is not available, and MUST NOT
   use more than one method to transport the token in each request.
   Because of the Security Considerations (Section 3) associated with
   the URI method, it SHOULD NOT be used unless no other method is
   feasible.

In Section 2.2:

   The client can use this method only if the
   following REQUIRED conditions are met:

That is better as:

   The client MUST NOT use this method unless the
   following conditions are met:

In Section 2.2, I think you mean "MUST NOT" instead of "MAY NOT" here:

   o  The HTTP request method is one for which a body is permitted to be
      present in the request.  In particular, this means that the "GET"
      method MAY NOT be used.

In Section 2.2, I think you mean "SHOULD NOT ... unless" here:

   The "application/x-www-form-urlencoded" method should typically only
   be used in application contexts where participating browsers do not
   have access to the "Authorization" request header field.

In Section 2.3, I think you mean "SHOULD NOT ... unless" here:

   Because of the Security Considerations (Section 3) associated with
   the URI method, it SHOULD only be used if no other method is
   feasible.

In Section 2.4.1, change "SHOULD not" to "SHOULD NOT".

4. For the ABNF in Section 2.1, please reference the "auth-param" rule
from RFC 2617. (However, I tend to agree with Eran that extensibility is
not particularly necessary here, and that some justification and
description of the extensibility mechanism is needed.)

5. I'll post separately about the error registry mentioned in Section 2.4.1.

6. It's nice to see the security considerations about attacks on tokens.
Perhaps we need a similar section in the SAML-bearer spec.

7. Please provide a reference to RFC 5246 for TLS.

8. What is the basis for defining "short-lived" a lifetime less than one
hour? That's plenty of time in which to launch an attack.

9. Regarding identity verification of resource servers, please reference
either RFC 2818 or draft-saintandre-tls-server-id-check.

10. In Section 4.1.1, I would agree with other WG participants that use
of "oauth_token" here is problematic.

11. Why is Section 4.2 on the OAuth Parameters Registry in this
document? It's already defined in the base spec.

12. Regarding Section 4.3, I'll post separately about an OAuth Errors
Registry, but if it's defined it would belong in the base spec, not here.

Peter

-- 
Peter Saint-Andre
https://stpeter.im/

---

From: Torsten Lodderstedt <[email-masked]>
Date: Wed, 23 Mar 2011 21:51:55 +0100

Here are my comments on this draft:

I repeat my proposal to name the URI parameter for passing the token 
"bearer_token" instead of "oauth_token". The same applies to the 
respective body parameter. This is more inline with the I-D's terminology.

section 2.4

"If the protected resource request does not include authentication
    credentials, contains an invalid access token, or is malformed, the
    resource server MUST include the HTTP "WWW-Authenticate" response
    header field. "

If the request is malformed, I would expect the resource server to 
respond with status code 400 (w/o WWW-Authenticate header).

What is the meaning of the component "( token "=" ( token / 
quoted-string ) )" in the definiton of the rule "param"?

"The "scope" attribute MUST NOT appear more than once."

Is the scope optional?

section 2.4.1

I don't see the benefit of those error codes as they replicate the 
meaning of HTTP status codes 400, 401, and 403.

section 3.1

"Token redirect" - does this also cover the threat of a counterfeit 
server phishing and abusing tokens 
(https://tools.ietf.org/html/draft-lodderstedt-oauth-security-01#section-4.6.4)? 

"Second,
    confidentiality protection of the exchanges between the client and
    the authorization server and between the client and the resource
    server MUST be applied"

I would suggest to add ", e.g. by using TLS".

I would furthermore suggest to add the following paragraph to this section:

Token abuse by counterfeit server: Clients SHOULD not make authenticated 
requests with an access
       token to unfamiliar resource servers, regardless of the presence
       of a secure channel.  If the resource server address is well-known
       to the client, it MUST authenticate the resource servers by using 
HTTPS server authentication prior sending the request.

section 3.3

"Don't store bearer tokens in cookies As cookies are generally sent in 
the clear, implementations MUST NOT store bearer tokens within
       them."

Every cookie can be declared to be send over encrypted channels only. 
Moreover, it is subject to a same origin policy. So what is the threat 
here? Is it about the storage in the clear?

"Instead, pass browser tokens in message bodies for which confidentiality
       measures are taken."

I suggest to add "authorization headers or" before message bodies.

regards,
Torsten.

Am 02.03.2011 09:31, schrieb Hannes Tschofenig:
> Hannes&  Blaine

---

From: Mike Jones <[email-masked]>
Date: Thu, 31 Mar 2011 18:18:00 +0000

Responses to suggestions not adopted on draft 04 are inline below.  Thanks for your input.

                                                            -- Mike

From: [email-masked] [mailto:[email-masked]] On Behalf Of Torsten Lodderstedt
Sent: Wednesday, March 23, 2011 1:52 PM
To: Hannes Tschofenig
Cc: OAuth WG
Subject: Re: [OAUTH-WG] WGLC on draft-ietf-oauth-v2-bearer-03.txt

> section 2.4
>
> What is the meaning of the component "( token "=" ( token / quoted-string ) )" in the definiton of the rule "param"?

Can any of the other working group members who participated in writing this particular text answer this question?  I agree that this should be defined in the specification.

> "The "scope" attribute MUST NOT appear more than once."

Yes.  Is there a particular change to the text that you believe is necessary?

> section 2.4.1
>
> I don't see the benefit of those error codes as they replicate the meaning of HTTP status codes 400, 401, and 403.

I disagree with this comment, as these error codes are more specific than the corresponding HTTP status codes.  Furthermore, before making any change to the error codes, I would want to see a demonstration of working group consensus for a specific change before making it.

> section 3.1
>
> "Token redirect" - does this also cover the threat of a counterfeit server phishing and abusing tokens (https://tools.ietf.org/html/draft-lodderstedt-oauth-security-01#section-4.6.4)?

No, it does not, as I see it, but because the list of threats comes from NIST800-63, I'm not prone to expand it here.  I believe that this was text originally authored by you.  What specific change would you suggest?

> Token abuse by counterfeit server: Clients SHOULD not make authenticated requests with an access
>       to the client, it MUST authenticate the resource servers by using HTTPS server authentication prior sending the request.

I believe that the suggested addition is largely redundant with this existing text:  "Presenting the token to an unauthenticated and unauthorized resource server or failing to validate the certificate chain will allow adversaries to steal the token and gain unauthorized access to protected resources".  Is there are specific change you'd like to propose that would consolidate both statements of this security principle?

> section 3.3
>
> "Don't store bearer tokens in cookies As cookies are generally sent in the clear, implementations MUST NOT store bearer tokens within
>       them."
>
> Every cookie can be declared to be send over encrypted channels only. Moreover, it is subject to a same origin policy. So what is the threat here? Is it about the storage in the clear?

The threat is about transmission in the clear.  This point has already been reworked in response to other comments.  Your comments on the new text would be welcome.

---

From: Mike Jones <[email-masked]>
Date: Thu, 31 Mar 2011 18:18:50 +0000

Responses to suggestions not adopted are inline below.  Thanks for your input.

				-- Mike

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Eran Hammer-Lahav
Sent: Wednesday, March 02, 2011 8:34 AM
To: Hannes Tschofenig; OAuth WG
Subject: Re: [OAUTH-WG] WGLC on draft-ietf-oauth-v2-bearer-03.txt

> - RWS should be replaces with SP since the header has no other attributes (and should not have any). This will make parsing easier by mandating a single space (unless HTTPbis has other guidelines).

This is insufficient rationale to introduce a potentially breaking change.  I would want to see a demonstration of working group consensus for this change before making it.

> - ABNF includes '( token "=" ( token / quoted-string ) )', but no prose is provided about how new parameters may be defined. Retained this extensibility point must be justified with actual use case…

This is insufficient rationale to introduce a breaking change.  I would want to see a demonstration of working group consensus for this change before making it.

> - Why are the HTTP error code suggested and not required?

Because the spec should not presume that only the example HTTP error codes might occur.

> - The only way to define such a registry is to bring it up as a comment for draft-ietf-oauth-v2. Otherwise, it is limited to the Bearer token header only (and since this document is not extensible,…

Your "Error extensibility proposal" note sent on 3/29/2011 documents the need for error extensibility.  This registry meets this need, while being simpler than the 3/29 proposal and in line with standard IETF extensibility practices.

---

From: Mike Jones <[email-masked]>
Date: Thu, 31 Mar 2011 18:20:09 +0000

-- Mike

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Peter Saint-Andre
Sent: Wednesday, March 23, 2011 11:11 AM
To: Hannes Tschofenig
Cc: OAuth WG
Subject: Re: [OAUTH-WG] WGLC on draft-ietf-oauth-v2-bearer-03.txt

<hat type='AD'/>

> 8. What is the basis for defining "short-lived" a lifetime less than one hour? That's plenty of time in which to launch an attack.

Torsten or other working group members - can you comment on this question by Peter?  Is there a specific change that any of you would like to propose to this text?

> 12. Regarding Section 4.3, I'll post separately about an OAuth Errors Registry, but if it's defined it would belong in the base spec, not here.

I've left this registry definition in the spec for the time being, since it has not yet been incorporated into the framework specification.

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Thu, 31 Mar 2011 11:25:13 -0700

> From: Mike Jones [mailto:[email-masked]]
> Sent: Thursday, March 31, 2011 11:19 AM

> > Section 2.4:
> >
> > - ABNF includes '( token "=" ( token / quoted-string ) )', but no prose is
> provided about how new parameters may be defined. Retained this
> extensibility point must be justified with actual use cases.
> 
> This is insufficient rationale to introduce a breaking change.  I would want to
> see a demonstration of working group consensus for this change before
> making it.

Adding prose is a breaking change? Really??

> > Section 4.3:
> >
> > - This registry is unnecessary and adds no value here (namespace collision is
> unlikely in general, and unlikely to cause problems). No use cases where
> suggested to justify it, and no additional error codes were proposed in over a
> year of discussions. Error codes were intentionally left non-extensible to
> increase interoperability. If addition color is needed for existing error codes,
> additional response parameters may be registered. Otherwise, if new error
> codes are needed, a new RFC must be published updating draft-ietf-oauth-
> v2.
> > - The only way to define such a registry is to bring it up as a comment for
> draft-ietf-oauth-v2. Otherwise, it is limited to the Bearer token header only
> (and since this document is not extensible, not needed even here).
> 
> Your "Error extensibility proposal" note sent on 3/29/2011 documents the
> need for error extensibility.  This registry meets this need, while being
> simpler than the 3/29 proposal and in line with standard IETF extensibility
> practices.

Whatever this registry defines has no impact on v2. I think it's silly but I don't care about this spec enough to waste any more time with it. My proposal for V2 is currently being reviewed and unless sufficient objections are raised, it will be added to -14 over the weekend.

EHL