Thanks Richard,
IMO, no changes are required as result of this review. I did mention one change
that's missing from the current draft -25 which was decided during the IETF
meeting period and was not push out yet (my appologized) - it is not
significant.
EH
-----Original Message-----
From: Richard L.Barnes [mailto:rbarnes(_at_)bbn(_dot_)com]
Sent: Tuesday, April 10, 2012 6:11 AM
MAJOR:
General: At several points, the document seems to deal in plain-text
usernames and passwords, ranging from the requirement for HTTP Basic
authentication (2.3.1) to the explicit username and password fields one of
the grant types (4.3.2, 10.7). In these cases, it seems like it would
encourage
better security practices to use some sort of digest or other secure scheme
for proving ownership of passwords. Otherwise, there's a risk that proxies,
caches, etc. will have access to users' plaintext credentials.
The use of HTTP Basic is limited to client credentials, not end-user
credentials. In both cases, the credentials are sent over a secure channel via
TLS. The use of HTTP Digest was discussed and ruled out by WG consensus due to
its complexity and limited adoption. As a general rule, the WG has opted to
rely on TLS when transmitting plaintext credentials (as well as not to transmit
them in the URI query).
I believe section 10.7 Resource Owner Password Credentials sufficiently
addresses these concerns when it comes to end-user credentials.
General: The document requires the Authorization Server to distinguish
between confidential and public clients at several points. It's not clear,
however, how the server is supposed to tell which is which. Perhaps Section
2.1 could elaborate a little more on this?
Addressed in section 2: the client type is provided by the client developer
during registration.
BTW, there is one change approved by the WG which has not yet been applied to
-25 (due to timing issues with the IESG review and IETF meeting feeze). The
text in section 2.1 beginning with "A client application consisting of" is to
be replaced with:
---
A clients may be implemented as a distributed set of components, each with a
different client type and security context (e.g. a distributed client with both
a confidential server-based component and a public browser-based component). If
the authorization server does not provide support for such clients, or does
not provide guidance with regard to their registration, the client SHOULD
register each component as a separate client.
---
4: Throughout the document, the assumption is that the client is moved from
one URI to another via HTTP redirects (302 responses). Could the protocol
also accommodate other techniques of moving clients around, e.g., in
JavaScript (setting window.location). It seems like this could make
deployment much easier in some cases.
302 is only used in two examples but the normative text only discussed
"redirects" and "directs" which are open to any implementation alternatives.
10.3: This section seems to ignore the risk of client impersonation at the
resource server. Just as with client credentials in Section 10.2, if an
access
token is compromised, then it could be presented by another party in order
to access the protected resource. So the Resource Server needs to
authenticate the client and validate that the token goes with the client, in
addition to just checking the validity of the token.
Client authentication is intentionally not included in protected resources
requests. This is a change made from 1.0 because of significant deployment
issues.
I can add a warning but there is nothing the resource server can do given the
protocol design.
10: Throughout this section, there are mentions of which parameters require
secure transmission/storage and which do not. It would be helpful to
summarize these requirements, say in a table at the end of the section.
I find the current organization with each item featured in a separate section
easier to implement. Also, while some require secure transmission, it is still
optional for everything else. Adding such a table might give the wrong
impression that this list is normative and restrictive.
MINOR:
2.3.1: When you say "request body" in this section, do you actually mean
"query"? I don't see a prohibition on GET here, in which case the parameters
would be in the URI query section.
This is only used with the Token Endpoint (3.2) which is limited to POST only.
3.1.2.2: It seems like an explicit requirement would be helpful here, e.g.:
"
The Authorization Server MUST verify that either no redirect_uri is provided
(in which case it uses the registered URI), or else that the provided
redirect_uri matches the registered redirection URI for the authenticated
client_id (where the match can be based on a full or partial registered URI).
"
We did not have WG consensus to mandate redirection URI registration for all
clients.
4.1.3: Why does this request use redirect_uri and not client_id to identify
the
client? (Or both?) It seems like involving the client_id would better
support
the goal of client authentication, in order to mitigate the risk of client
impersonation.
It does include the client identifier via the client authentication
requirements specified for the Token endpoint. The example shows that.
Notice the text:
If the client type is confidential or the client was issued client
credentials (or assigned other authentication requirements), the
client MUST authenticate with the authorization server as described
in Section 3.2.1.
The reason for including the redirect_uri is explained here:
http://hueniverse.com/2011/06/oauth-2-0-redirection-uri-validation/
4.3.2: Shouldn't there be a client authentication here? (In particular, a
client_id.) Otherwise, it seems like this is essentially the same as the
flow in
4.4.
Same as above.
7. It seems like it would be helpful to have a way to pass access tokens in
the
request URI / query in some deployment cases (e.g., a JavaScript based
client). Is there any particular reason this was excluded?
It's not. The examples show the header but there is nothing to prevent using
other means. In fact, the bearer draft offers such mechanism (IIRC).
EDITORIAL:
3. Would be helpful to note here that these endpoints are in addition to the
resource endpoint (the one started out to protect), which is handled in
Section 7.
The resource endpoint isn't really part of the authorization protocol.
3.2: It's not clear why only POST is allowed here. A couple of words of
explanation would help.
To prevent credentials from leaking via the URI query. I don't think we need to
explain the reason as we do not explain any other similar choices made.
5.1: Why bother with a case-insensitive token_type here? It doesn't look
like anything else in the whole document has this property.
Because it is linked to the HTTP authentication scheme name which is case
insensitive.
EH