Subject: [OAUTH-WG] draft 16 review notes

From: Brian Eaton <[email-masked]>
Date: Sun, 22 May 2011 20:38:53 -0700

I just read over the whole of the draft for the first time in a while.
 I looked it over mostly for

a) places where spec and reality were going to have trouble intersecting
   and
b) places where security advice would be useful
   and
c) grammer and speling, because I notices things like that

Mostly noticed nits.  I'm really happy with the way some of the
trickier issues have gotten dealt with.  Sometimes those differences
get papered over, but they mostly seem to have been dealt with in
constructive ways.  I didn't agree with all of the decisions, but I
liked that the spec either made the decisions or gave clear guidance
on trade-offs.

So bear in mind that even though most of the comments in this e-mail
are criticism, that's not because there isn't a lot of good in the
spec.

I won't be able to make the interim meeting tomorrow, but I wanted to
send these out before hand.

1.  Introduction

nit: text mixes “resource-owner” and “resource owner”.  I think
“resource owner” is correct.

consider adding bullet point: Compromise of any third-party
application server results in compromise of the user’s password and
all of the data protected by that password.

nit: “web end-user” is an odd turn of phrase.  Maybe just “end user”

1.1 Roles

nit: “- access restricted resources which require authentication to
access:” seems awkward.  could eliminate the phrase entirely and make
the meaning of the paragraph just as obvious.

1.4.1 Authorization Code

maybe expand the section on important security benefits.  “The use of
authorization codes also improves the ability to recover in the event
of compromise of an application server or authorization server.”

1.4.2

nit: “intermediary authorization grant”.  I think “intermediate” is
the right term here, but might be wrong.

1.5 Refresh Token

“self-contain the authorization information in a verifiable manner.”

That is never secure, because it makes review of issued permissions
and revocation of those permissions impossible.  Refresh tokens are
different than access tokens; a refresh token is always going to be an
identifier that you use to look up authorization information.

The text should be: “The token denotes an identifier used to retrieve
the authorization information.”
(As another example of why you can’t have a self-verifying refresh
token, consider the role of the client id and client secret in the
protocol.  If refresh tokens were self-verifying, you would also need
bake the client secret into the refresh token, thus making client
secret rotation impractical.)

nit: “..., no longer valid, …” doesn’t make sense.  I think the text
should be “or is no longer valid.”  The parenthetical phrase throws
off this paragraph a bit, maybe rephrase as

“The refresh token can be used to obtain a new access token when the
current access token becomes invalid or expires, or to obtain
addtional access tokens with identical or narrower scope.  (Access
tokens may have a shorter lifetime and fewer permissions than
authorized by the resource owner.)

Should also add, maybe in the second paragraph:

“Unlike access tokens, refresh tokens are intended for use only with
authorization servers.  Refresh tokens are never sent to resource
servers.”

3.  Client Authentication

I like the compromise reached here on the language around client
secrets for installed app clients.  It doesn’t reflect reality, but
the language allows enough wiggle room to be practical.  The spirit of
the text is quite sensible, which is what I like best.

3.1 Client Password Authentication

Referring to a password as a shared symmetric secret is misleading.
“Symmetric secret” is used almost exclusively with secret key
cryptography, where both sides store copies of the secret.  That’s not
the right way to use client passwords; they should be treated like
*passwords*, not HMAC keys or encryption keys.

The reference to needing consensus on when to use basic auth and when
to use client_id and client_secret parameters seems quite reasonable.
The current language seems to reflect a poor compromise that will make
interoperable implementations less likely for no good reason.  I’d
rather that someone picked a winner and the spec was simple and
consistent.

3.2 Other Client Authentication Methods

Again, I like the compromise here.  The language is vague but useful.

4.1.1. Authorization Request

Please add “state” to the example.  “state” is an important part of a
secure client implementation, might as well encourage use by including
it in examples.

4.1.2 Authorization Response

“minimize the risk of leaks”: this should be “mitigate”, not “minimize”.

“SHOULD expire shortly”.  This should be a MUST.  The security
analysis of the protocol doesn’t hold unless authorization codes
expire.  Suggested language:

“The authorization code MUST expire shortly after it is issued to
mitigate the risk of leaks.  A maximum authorization code lifetime of
10 minutes is RECOMMENDED.”

Again, please include “state” in the example.

4.1.2.1 Error Response

“and MUST NOT redirect”... this is contrary to some industry practice.
 Various service providers show an interstitial to warn the user, but
will then redirect if the user clicks through the interstitial.  How
about “and MUST NOT automatically redirect” as alternative language.

The language around using HTTP status codes as values for the “error=”
parameter is surprising.  Who requested that?  Has anyone implemented
that?  Why is this a good idea?

Again, example including “state” parameter would be a good thing.

4.2 Implicit Grant

nit: The phrase “...maintaining their client credentials
confidential...”is a bit awkward.  This should probably be
“maintaining the confidentiality of their client credentials...”

The introductory paragraph says the flow is suitable for clients that
can’t keep their client credentials secret.  However, that’s not
sufficient.  In addition, the client must be able to maintain the
security of a redirect URI via something like the browser same-origin
policy.  Suggested language:

“The implicit grant type is suitable for clients that cannot maintain
the confidentiality of their client credentials, but which can be
known to operate on a particular redirect URI.”  These applications
are typically implemented in a browser using a scripting language such
as JavaScript.

nit in step (D): “(does not include the fragment).” should be “(which
does not include the fragment).”

4.2.1 Authorization Request

Please include “state” in example.

Validation of the redirect_uri parameter is more important for the
implicit grant type.  Section 2.1.1 leaves registration of the
redirect URI as “SHOULD”.  It’s a “MUST” for the implicit flow.
Suggested language:

The authorization server validates the request to ensure all required
parameters are present and valid.  The authorization server MUST
verify that the redirect URI to which it will redirect the
authorization code matches a redirect URI pre-registered by the
client.  The authorization server SHOULD verify the scheme, authority,
and path of the redirect URI.  The authorization server MAY verify the
query parameters as well.

4.2.2. Access Token Response

Please include “state” in example.

4.3 Resource Owner Password Credentials

nit: “converting the stored credentials with an access token” should
be “... to an access token.”

4.3.2 Access Token Request

Really need language in here about the risk of brute force attacks on passwords.

4.4 Client Credentials

The spec has this flow returning a refresh token.  This is a security
problem.  We agreed to remove it or at least recommend against it
almost a year ago.  Thread here:
http://www.ietf.org/mail-archive/web/oauth/current/msg03651.html

General comments on HTTP headers.

Should follow cache header recommendations from HTTP/1.1 spec for
backwards compatibility with HTTP/1.0 caching proxies.  This means
adding “Pragma: no-cache” to all responses.  See
http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.32.

Should include charset in all content-type examples, to prevent
security problems due to content sniffing.  See
http://code.google.com/p/browsersec/wiki/Part2#Survey_of_content_sniffing_behaviors.
 For example:

Content-Type: application/json; charset=UTF-8

6. Refreshing an Access Token

nit: should mention that when the authorization server returns a new
refresh token, they are going to revoke the old one as well.  How
about:

“The authorization server MAY issue a new refresh token, in which case
the client MUST discard the old refresh token and replace it with the
new refresh token.  The authorization server MAY revoke the old
refresh token after returning the new refresh token to the client.”

Section 8.2 Defining New Endpoint Parameters

Telling service providers to use vendor specific prefixes for query
parameters sent to their web page is just not going to get
implemented.  Every service provider has a collection of
long-established query parameters that they use on all or almost all
of their web pages.  That includes their authorization pages.

Section 10.  Security Considerations

I love the introduction here, this is an excellent break down of the
different client types, and I’ve never seen anyone else do it so
concisely.

One nit: I don’t think the statement “It is assumed that such an
application can protect dynamically issued credentials, such as
refresh tokens, from eavesdropping by other applications residing on
the same device” is actually essential to the security analysis, and
it rules out a large class of applications (e.g. everything on Windows
or MacOS.)  Maybe change the section on Native Applications to say

“It is assumed that any client authentication credentials included in
the application can be extracted, and furthermore that rotation of the
client authentication credentials is not practical.  Dynamically
issued credentials such as refresh tokens, on the other hand, can
receive an acceptable level of protection.  At a minimum those
credentials are protected from hostile servers which the client may
contact.  On some platform those credentials might be protected from
other applications residing on the same device.”

There are various security risks mentioned in the OAuth WRAP security
considerations that seem worth mentioning here:

http://trac.tools.ietf.org/wg/oauth/trac/raw-attachment/wiki/SecurityConsiderations/OAuthWRAP2.0SecurityConsiderations.pdf

Section 10.1 Client Authentication

nit: “MUST NOT issue client passwords to installed apps” is a dead
letter, it is not going to change standard industry practice in the
slightest.  The language from section 3 is more constructive.  I’d
suggest the following language for section 10.1 instead

“The authorization server MUST NOT assume that native or user-agent
based applications can maintain the confidentiality of client
secrets.”

That does conform with industry practice, so it’s more likely to be implemented.

“The authorization server is encouraged to consider using stronger
client authentication means than a client password.”

Could make that more specific by adding “, such as assertions as per
<OAuth SAML spec>”

Section 10.2 Client Impersonation

I like the content of this section, but it seems to mix several
different topics.

- general risks of native applications

- security considerations for “immediate mode”, where authorization
requests are processed without end-user interaction.

- validation rules for redirect URIs

- open redirectors

- access token design

Most of those merit separate discussion.

10.3  Access Token Credentials
“When using the implicit grant type, the access token credentials are
transmitted in the URI fragment, which can expose the credentials to
unauthorized parties.”

That’s basically FUD.  This should be made much more specific.  It
falls into the general category of security considerations for
redirectors and clients...

10.5 Request Confidentiality.

Why not specify TLS here, as is done elsewhere in the spec?

10.6 Endpoint Authenticity

nit: “server-side authentication” should be “server authentication”
(copying language from the TLS spec...)

nit: should refer to RFC 2818 for server authentication rules

10.9 Authorization Codes

“SHOULD be short lived and MUST be single use” is backwards.

These MUST be short-lived.  If they are long-lived, an attacker who
compromises an end-user account even temporarily can mint a very large
number of these tokens and replay them later as needed.

MUST be single use is a dead letter, because it requires atomic
operations at scale.  Even things like password changes are not atomic
in user databases of moderate size.  Authorization codes might be
generated quite frequently (e.g. when “immediate mode” flows are
used), so a MUST for an atomic operation is unrealistic.

nit: “Authorization codes operate as plaintext bearer credentials,
used to verify that the end-user who granted authorization at the
authorization server, is the same end-user returning to the client to
complete the process.”

That assumes that the user visited the client to trigger the
authorization.  That’s not always realistic, see the “unsolicited
authentication response” from the SAML 2 protocol for examples of
other protocols where this is common.

10.10 Session Fixation

I’m not sure about the attack being described here.  Some of the
mitigations make it sound like the attack described is session
hijacking, not session fixation.

If the mitigations mentioned the state parameter, then I would think
that the attack described was session fixation at the client, but that
doesn’t come up.

Can someone explain the attack in greater detail?

10.12 Resource Owner Password Credentials

Really like this section, nicely put. =)

After reading through the security considerations section a couple of
times, I think it could benefit from a different organization.
Specifically

- keep the introduction, it’s awesome.
- write new sections for each of the following
  1) Authorization Tokens
  2) Web Application Clients
  3) User-Agent Clients
  4) Installed Application Clients
  5) Authorization Servers
  6) Resource Servers
- merge the current recommendations (which are very sensible) into the
appropriate sections.

Just for kicks I started to write up security considerations for
authorization tokens.  I'll send that to the list separately.

Cheers,
Brian

---

From: Mike Jones <[email-masked]>
Date: Mon, 23 May 2011 05:45:41 +0000

It would be great if you could do a similarly detailed read of the bearer token spec as well!

-- Mike

Sent from my Windows Phone

-----Original Message-----
From: Brian Eaton
Sent: Sunday, May 22, 2011 8:39 PM
To: [email-masked]
Subject: [OAUTH-WG] draft 16 review notes

1.  Introduction

1.1 Roles

1.4.1 Authorization Code

1.4.2

1.5 Refresh Token

3.  Client Authentication

4.1.2.1 Error Response

4.2 Implicit Grant

4.4 Client Credentials

- open redirectors

- access token design

10.9 Authorization Codes

10.10 Session Fixation

Cheers,
Brian
_______________________________________________
OAuth mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/oauth