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

Alexey Melnikov <alexey.melnikov@isode.com> Sun, 29 January 2012 16:38 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 8BB1B21F84DF; Sun, 29 Jan 2012 08:38:04 -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 xplDmnZa7p+t; Sun, 29 Jan 2012 08:38:03 -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 1677C21F84E6; Sun, 29 Jan 2012 08:38:03 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1327855081; d=isode.com; s=selector; i=@isode.com; bh=2hS2GFyX98c6SaJMNY1/b74IIXoTxIZDxSbheb8Xvz4=; 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=waJ//w4ideY1RfhPuGb8vQihJfe7LgXKql5Oaei6k7s9QHOcZR2oQN+OPZivBUSnDL11eI OjTv+atLhdFchcbdcNQcoLd1byBPIXH88c/6QEyNg9y1V2s/6dvZ0EJOa523FYuMvu71KN gazNeZUwL53p9EZaVHQTBLKx03JS/SY=;
Received: from [188.29.114.61] (188.29.114.61.threembb.co.uk [188.29.114.61]) by rufus.isode.com (submission channel) via TCP with ESMTPSA id <TyV14QAV52EP@rufus.isode.com>; Sun, 29 Jan 2012 16:37:55 +0000
Message-ID: <4F2575CE.9040001@isode.com>
Date: Sun, 29 Jan 2012 16:37:34 +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: General Area Review Team <gen-art@ietf.org>, IETF-Discussion Discussion <ietf@ietf.org>, draft-ietf-oauth-v2-bearer.all@tools.ietf.org
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: [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: Sun, 29 Jan 2012 16:38:04 -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>.

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.