Re: [Gen-art] Gen-ART review of draft-ietf-oauth-v2-bearer-15.txt

Alexey Melnikov <alexey.melnikov@isode.com> Tue, 31 January 2012 10:33 UTC

Return-Path: <alexey.melnikov@isode.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 C8DBF21F8598; Tue, 31 Jan 2012 02:33:46 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -102.599
X-Spam-Level:
X-Spam-Status: No, score=-102.599 tagged_above=-999 required=5 tests=[AWL=0.000, BAYES_00=-2.599, 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 MacxjZXKurAQ; Tue, 31 Jan 2012 02:33:45 -0800 (PST)
Received: from rufus.isode.com (cl-125.lon-03.gb.sixxs.net [IPv6:2a00:14f0:e000:7c::2]) by ietfa.amsl.com (Postfix) with ESMTP id 0566F21F8456; Tue, 31 Jan 2012 02:33:45 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1328006023; d=isode.com; s=selector; i=@isode.com; bh=wymAVhwEHKfcP6NkNNrsdP98i5LC6+9c9122TL72YGM=; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:Cc:MIME-Version: In-Reply-To:References:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description; b=YMiGNeogKBq/nO7j/gY9Qu22ttPm4GzouC+7uKFW3qgrQNwZLF69l4a//QvZUtcSErWacq iTT+MlssW0DkUCb4seE3T6+1jjEIgu84OWChvd4dtH8wYUfq/k1R3xcsA82axc2FkHoFAD X888fCT/IXJN8YCTsxr976pT2sW35HU=;
Received: from [188.29.102.51] (188.29.102.51.threembb.co.uk [188.29.102.51]) by rufus.isode.com (submission channel) via TCP with ESMTPSA id <TyfDfQAV521f@rufus.isode.com>; Tue, 31 Jan 2012 10:33:39 +0000
Message-ID: <4F27C37C.1090008@isode.com>
Date: Tue, 31 Jan 2012 10:33:32 +0000
From: Alexey Melnikov <alexey.melnikov@isode.com>
User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20111105 Thunderbird/8.0
To: Mike Jones <Michael.Jones@microsoft.com>
References: <4F2575CE.9040001@isode.com> <4E1F6AAD24975D4BA5B16804296739436638B7AD@TK5EX14MBXC284.redmond.corp.microsoft.com>
In-Reply-To: <4E1F6AAD24975D4BA5B16804296739436638B7AD@TK5EX14MBXC284.redmond.corp.microsoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: quoted-printable
Cc: General Area Review Team <gen-art@ietf.org>, "oauth@ietf.org" <oauth@ietf.org>, "draft-ietf-oauth-v2-bearer.all@tools.ietf.org" <draft-ietf-oauth-v2-bearer.all@tools.ietf.org>, IETF-Discussion Discussion <ietf@ietf.org>
Subject: Re: [Gen-art] Gen-ART review of draft-ietf-oauth-v2-bearer-15.txt
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, 31 Jan 2012 10:33:46 -0000

On 30/01/2012 05:20, Mike Jones wrote:
> Thanks for your useful feedback, Alexey.
Hi Mike,
>    Below, I'll respond to each of your comments.  I've also added the OAuth working group to the thread, so they are aware of them as well and can participate in the discussion.
>
> About your first issue with the WWW-Authenticate ABNF, I am already working with Julian, Mark Nottingham, and the chairs to resolve this issue.  Expect to see a proposal for review by the working group shortly.
Ok, I will have a look.
> About your comments on scope:  OAuth 2.0 (both the Core and Bearer specs) is designed to be deployed in diverse and non-interoperable application contexts, meeting a variety of application needs.  In those various settings, which are often distinct and potentially non-interoperable, parameters such as scope, realm, etc. may have very different meanings.  This is not a bug; it is a feature, because it allows the OAuth pattern to meet the needs of numerous, often distinct, application environments.  For that reason, a registry of scope (or realm) parameters would be ill-advised and counterproductive.  It's perfectly OK and expected for a scope value such as "email" to have one meaning in one application context and a different meaning in a different, but distinct application context.  Trying to impose a single meaning on particular scope values across distinct application contexts is both unnecessary and could break many existing deployments.  That being said, we fully expect
> interoperability profiles to emerge that define interoperable sets of scope values within particular application contexts.  (The OpenID Connect specifications are one such set of profiles.)  But these meanings will always be context-specific - not global in scope.
The way "scope" is currently defined in the document is completely 
useless. You don't have a single example in the document. You don't say 
how the semantics of the value differs from realm. You don't say that 
its values are deployment specific and can be profiled in future 
documents. Please say something about these issues in the document (your 
explanation above would work.)
> About your first minor issue, I'll reorder the bullets so the statement about the entity-body being single part is followed by the statement about it using application/x-www-form-urlencoded, so they will be read together.
Ok.
> About your second minor issue on error codes, the error codes registry already exists, but is in the OAuth Core spec.  Seehttp://tools.ietf.org/html/draft-ietf-oauth-v2-23#section-11.4.
Can you please make this clear in the document (by adding a reference)?
> About your third minor issue on RFC 6125 versus RFC 2818, you'll find that, per the history entries, a previous reference to RFC 2818 was changed to RFC 6125 in draft 14 at the request of Security Area Director Stephen Farrell.  If you'd like to see this reference reintroduced, I'd request that you work with Stephen on specific alternative proposed wording that is acceptable to both of you.
Ok, I can work with Stephen and Peter Saint-Andre (RFC 6125 co-author) on some text.

> Finally, I'll address both of your nits in the manner you suggested.
These are fixed in -16, thanks.
>
> 				Thanks again,
> 				-- Mike
>
> -----Original Message-----
> From:ietf-bounces@ietf.org  [mailto:ietf-bounces@ietf.org] On Behalf Of Alexey Melnikov
> Sent: Sunday, January 29, 2012 8:38 AM
> To: General Area Review Team; IETF-Discussion Discussion;draft-ietf-oauth-v2-bearer.all@tools.ietf.org
> Subject: Gen-ART review of draft-ietf-oauth-v2-bearer-15.txt
>
> 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>aq>.
>
> Please resolve these comments along with any other Last Call comments you may receive.
>
> Document: draft-ietf-oauth-v2-bearer-15.txt
> Reviewer: Alexey Melnikov
> Review Date: 29 Jan 2012
> IETF LC End Date: 7 Feb 2012
> IESG Telechat date: (if known) -
>
> Summary: Mostly ready, with a couple of things that should be addressed.
>
> Major Issues:
>
> I have 2 issues in section 3:
>
> 3.  The WWW-Authenticate Response Header Field
>
>      If the protected resource request does not include authentication
>      credentials or does not contain an access token that enables access
>      to the protected resource, the resource server MUST include the HTTP
>      "WWW-Authenticate" response header field; it MAY include it in
>      response to other conditions as well.  The "WWW-Authenticate" header
>      field uses the framework defined by HTTP/1.1, Part 7
>      [I-D.ietf-httpbis-p7-auth] as follows:
>
>      challenge       = "Bearer" [ 1*SP 1#param ]
>
>      param           = realm / scope /
>                        error / error-desc / error-uri /
>                        auth-param
>
>      scope           = "scope" "=" quoted-string
>      error           = "error" "=" quoted-string
>      error-desc      = "error_description" "=" quoted-string
>      error-uri       = "error_uri" "=" quoted-string
>
> 1). I am agreeing with Julian about redefinition of ABNF from HTTPBis documents. I believe there is a proposal to fix that but the new draft hasn't been posted yet.
>
> 2). My 2nd major issue is about the following paragraph:
>
>      The "scope" attribute is a space-delimited list of scope values
>      indicating the required scope of the access token for accessing the
>      requested resource.  In some cases, the "scope" value will be used
>      when requesting a new access token with sufficient scope of access to
>      utilize the protected resource.  The "scope" attribute MUST NOT
>      appear more than once.  The "scope" value is intended for
>      programmatic use and is not meant to be displayed to end users.
>
> I don't think this provide enough information about what this is, how it is to be used and which values are allowed. As this is not meant to be displayed to end users, then you need to say what values are allowed and which entity can allocate them. Is there a registry for these tokens, e.g. an IANA registry?
>
>
> Minor Issues:
>
> 1).
> 2.2.  Form-Encoded Body Parameter
>
>      When sending the access token in the HTTP request entity-body, the
>      client adds the access token to the request body using the
>      "access_token" parameter.  The client MUST NOT use this method unless
>      all of the following conditions are met:
>
>      o  The HTTP request entity-body is single-part.
>
>      o  The entity-body follows the encoding requirements of the
>         "application/x-www-form-urlencoded" content-type as defined by
>         HTML 4.01 [W3C.REC-html401-19991224].
>
>      o  The HTTP request entity-header includes the "Content-Type" header
>         field set to "application/x-www-form-urlencoded".
>
> I would combine the first and the third bullet into a single statement, because they seem to be a bit confusing while being read separately.
> (I.e., is it possible to have Content-Type of "application/x-www-form-urlencoded" with something which is multipart?)
>
> 2).
> Section "3.1.  Error Codes"
>
> I recommend creating an IANA registry for these or explain why one is not needed.
>
> 3).
> 4.2.  Threat Mitigation
>
>      To protect against token disclosure, confidentiality protection MUST
>      be applied using TLS [RFC5246] with a ciphersuite that provides
>      confidentiality and integrity protection.  This requires that the
>      communication interaction between the client and the authorization
>      server, as well as the interaction between the client and the
>      resource server, utilize confidentiality and integrity protection.
>      Since TLS is mandatory to implement and to use with this
>      specification, it is the preferred approach for preventing token
>      disclosure via the communication channel.  For those cases where the
>      client is prevented from observing the contents of the token, token
>      encryption MUST be applied in addition to the usage of TLS
>      protection.  As a further defense against token disclosure, the
>      client MUST validate the TLS certificate chain when making requests
>      to protected resources.
>
> and
>
>      To deal with token capture and replay, the following recommendations
>      are made: First, the lifetime of the token MUST be limited; one means
>      of achieving this is by putting a validity time field inside the
>      protected part of the token.  Note that using short-lived (one hour
>      or less) tokens reduces the impact of them being leaked.  Second,
>      confidentiality protection of the exchanges between the client and
>      the authorization server and between the client and the resource
>      server MUST be applied.  As a consequence, no eavesdropper along the
>      communication path is able to observe the token exchange.
>      Consequently, such an on-path adversary cannot replay the token.
>      Furthermore, when presenting the token to a resource server, the
>      client MUST verify the identity of that resource server, as per
>      Representation and Verification of Domain-Based Application Service
>      Identity within Internet Public Key Infrastructure Using X.509 (PKIX)
>      Certificates in the Context of Transport Layer Security (TLS)
>      [RFC6125].
>
> Firstly, I would move the RFC 6125 reference to the first paragraph quoted above (but see below). Secondly, you should either normatively reference RFC 2818 (HTTP over TLS) instead of RFC 6125, or you need to provide more information about how RFC 6125 is to be used, because it has several options which need to be described (use of SRV-IDs, URI-IDs, DNS-IDs, use of wildcards). I suspect you should just reference RFC 2818.
>
>
> Nits:
>
> 2.2.  Form-Encoded Body Parameter
>
>      o  The content to be encoded in the entity-body MUST consist entirely
>         of ASCII characters.
>
> ASCII needs a reference.
>
>
> ID-nits reports:
>
>     == The document seems to lack the recommended RFC 2119 boilerplate, even if
>        it appears to use RFC 2119 keywords -- however, there's a paragraph with
>        a matching beginning. Boilerplate error?
>
>        (The document does seem to have the reference to RFC 2119 which the
>        ID-Checklist requires).
>     == Using lowercase 'not' together with uppercase 'MUST', 'SHALL', 'SHOULD',
>        or 'RECOMMENDED' is not an accepted usage according to RFC 2119.
> Please
>        use uppercase 'NOT' together with RFC 2119 keywords (if that is what you
>        mean).
>
> and:
>
>        Found 'MUST not' in this paragraph:
>
>        o  Stated that bearer tokens MUST not be stored in cookies that can
>        be sent in the clear in the Threat Mitigation section.
> _______________________________________________
> Ietf mailing list
> Ietf@ietf.org
> https://www.ietf.org/mailman/listinfo/ietf
>
>