[Gen-art] Gen-ART Telechat review of draft-ietf-oauth-v2-25

Richard L. Barnes <rbarnes@bbn.com> Tue, 10 April 2012 13:10 UTC

Return-Path: <rbarnes@bbn.com>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 1126521F85F6; Tue, 10 Apr 2012 06:10:54 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -106.599
X-Spam-Level:
X-Spam-Status: No, score=-106.599 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, RCVD_IN_DNSWL_MED=-4, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id j2ejoQvcdaWn; Tue, 10 Apr 2012 06:10:53 -0700 (PDT)
Received: from smtp.bbn.com (smtp.bbn.com [128.33.1.81]) by ietfa.amsl.com (Postfix) with ESMTP id 5D02121F85D0; Tue, 10 Apr 2012 06:10:53 -0700 (PDT)
Received: from [128.89.253.10] (port=55028) by smtp.bbn.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.77 (FreeBSD)) (envelope-from <rbarnes@bbn.com>) id 1SHaq4-000FpD-0l; Tue, 10 Apr 2012 09:10:28 -0400
From: "Richard L. Barnes" <rbarnes@bbn.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Date: Tue, 10 Apr 2012 09:11:19 -0400
Message-Id: <22E22875-8112-42A3-974D-770167AB9536@bbn.com>
To: General Area Review Team <gen-art@ietf.org>, draft-ietf-oauth-v2.all@tools.ietf.org, IETF-Discussion list <ietf@ietf.org>
Mime-Version: 1.0 (Apple Message framework v1257)
X-Mailer: Apple Mail (2.1257)
Subject: [Gen-art] Gen-ART Telechat review of draft-ietf-oauth-v2-25
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/gen-art>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 10 Apr 2012 13:10:54 -0000

I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-oauth-v2.txt
Reviewer:  Richard Barnes 
Review Date:  10 Apr 2012
IETF LC End Date: 
IESG Telechat Date: 12 Apr 2012

Summary: Almost ready, but with some gaps that need to be addressed


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.

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?  

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.

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.

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.


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.

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

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.

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.

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?


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.

3.2: It's not clear why only POST is allowed here.  A couple of words of explanation would help.

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.