Re: [Gen-art] Gen-ART Last Call review of draft-ietf-ace-dtls-authorize-12
Benjamin Kaduk <kaduk@mit.edu> Mon, 27 July 2020 19:11 UTC
Return-Path: <kaduk@mit.edu>
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 7CEC83A0B91; Mon, 27 Jul 2020 12:11:03 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.919
X-Spam-Level:
X-Spam-Status: No, score=-1.919 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_MSPIKE_H4=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id irKFAOjwVtD6; Mon, 27 Jul 2020 12:11:01 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B898D3A0B35; Mon, 27 Jul 2020 12:10:58 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 06RJAqRh002537 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 27 Jul 2020 15:10:54 -0400
Date: Mon, 27 Jul 2020 12:10:52 -0700
From: Benjamin Kaduk <kaduk@mit.edu>
To: Paul Kyzivat <pkyzivat@alum.mit.edu>
Cc: draft-ietf-ace-dtls-authorize.all@ietf.org, General Area Review Team <gen-art@ietf.org>
Message-ID: <20200727191052.GI41010@kduck.mit.edu>
References: <8c2725a3-f89f-7ea1-dda9-681edd463a32@alum.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <8c2725a3-f89f-7ea1-dda9-681edd463a32@alum.mit.edu>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/59ouWy6beg8DpBLRjCzCTkWxIHE>
Subject: Re: [Gen-art] Gen-ART Last Call review of draft-ietf-ace-dtls-authorize-12
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.29
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: <https://mailarchive.ietf.org/arch/browse/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: Mon, 27 Jul 2020 19:11:10 -0000
Hi Paul, Just a couple notes in addition to what Jim already mentioned. On Sun, Jul 19, 2020 at 04:23:46PM -0400, Paul Kyzivat wrote: > I am the assigned Gen-ART reviewer for this draft. The General Area > Review Team (Gen-ART) reviews all IETF documents being processed > by the IESG for the IETF Chair. Please treat these comments just > like any other last call comments. > > For more information, please see the FAQ at > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-ace-dtls-authorize-12 > Reviewer: Paul Kyzivat > Review Date: 2020-07-19 > IETF LC End Date: 2020-07-20 > IESG Telechat date: ? > > Summary: > > This draft is on the right track but has open issues, described in the > review. > > General: > > TBD > > Issues: > > Major: 2 > Minor: 1 > Nits: 1 > > 1) MAJOR: Management of token storage in RS > > There seems to be an expectation that when the client uploads an access > token that the RS will retain it until the client attempts to establish > a DTLS association. This seems to require some sort of management of > token lifetime in the RS. The only discussion I can find of this issue > is the following in section 7: > > ... A similar issue exists with the > unprotected authorization information endpoint where the resource > server needs to keep valid access tokens until their expiry. > Adversaries can fill up the constrained resource server's internal > storage for a very long time with interjected or otherwise retrieved > valid access tokens. > > This seems to imply a normative requirement to keep tokens until their > expiry. But I find no supporting normative requirements about this. And, > this section only presents it as a DoS attack, rather than a potential > problem with valid usage. > > ISTM that there is an implied requirement that the RS have the capacity > to store one access token for every PoP key of every authorized client. > If so, that ought to be stated. If not, then some other way of bounding > storage ought to be discussed. > > 2) MAJOR: Missing normative language > > I found several places where the text seems to suggest required behavior > but fails to do so using normative language: > > * In section 3.3: > > ... Instead of > providing the keying material in the access token, the authorization > server includes the key identifier in the "kid" parameter, see > Figure 7. This key identifier enables the resource server to > calculate the symmetric key used for the communication with the > client using the key derivation key and a KDF to be defined by the > application, for example HKDF-SHA-256. The key identifier picked by > the authorization server needs to be unique for each access token > where a unique symmetric key is required. > ... > Use of a unique (per resource server) "kid" and the use of a key > derivation IKM that is unique per authorization server/resource > server pair as specified above will ensure that the derived key is > not shared across multiple clients. > > The uniqueness seems to be a requirement. Perhaps "needs to be unique" > should be "MUST be unique". (And something similar for the IKM.) > > * Also in section 3.3: > > All CBOR data types are encoded in CBOR using preferred serialization > and deterministic encoding as specified in Section 4 of > [I-D.ietf-cbor-7049bis]. This implies in particular that the "type" > and "L" components use the minimum length encoding. The content of > the "access_token" field is treated as opaque data for the purpose of > key derivation. > > IIUC the type of serialization and encoding is a requirement. Will need > some rewording to make it so. Both this and the previous item are scoped by the following text: The method for how the resource server determines the symmetric key from an access token containing only a key identifier is application- specific; the remainder of this section provides one example. So it's not entirely clear that normative language is appropriate in the discussion of the example behavior. > * In section 3.3.1: > > ... To > be consistent with the recommendations in [RFC7252] a client is > expected to offer at least the ciphersuite TLS_PSK_WITH_AES_128_CCM_8 > [RFC6655] to the resource server. > > I think "is expected" should be "MUST". > > * Also in section 3.3.1: > > ... This > specification assumes that the access token is a PoP token as > described in [I-D.ietf-ace-oauth-authz] unless specifically stated > otherwise. > > I think "assumes ... unless" should be "MUST ... unless". My understanding is that this is just talking about the text in the document itself. But as far as I remember we always require PoP tokens, so this could just be removed. > * Also in section 3.3.1: > > ... New access tokens issued by the > authorization server are supposed to replace previously issued access > tokens for the respective client. > > Is this normative? Should "are supposed to" be "MUST"? I don't think this is a "MUST"; it refers to some behavior in the core framework that is merely suggested and not mandatory. Thanks for the review; it brought up some good points! -Ben > 3) MINOR: Insufficient specification > > (I'm waffling whether this is minor or major.) > > There are a couple of places where what seem to be requirements are > stated too vaguely to be implemented consistently: > > * In the previously mentioned paragraph in 3.3.1: > > ... This > specification assumes that the access token is a PoP token as > described in [I-D.ietf-ace-oauth-authz] unless specifically stated > otherwise. > > The "unless specifically stated otherwise" is too vague to be normative. > How would the alternative be indicated? Is this an escape hatch for > future extensions? If so, it needs more work to make that clear and to > open a path for that future work. > > * Also in section 3.3.1: > > ... The resource server therefore must > have a common understanding with the authorization server how access > tokens are ordered. > > The last statement ("must have a common understanding") is mysterious. > IIUC section 4 is covering the same topic in a less mysterious way. > > 4) NIT: Subsection organization > > Both 3.2 and 3.3 share a common structure: > > * The section begins with discussion of the interaction between the > client and the AS. > > * it is followed by a subsection discussing the interaction between the > client and the RS. > > It is odd to have a section with a single subsection. And the structure > isn't easily discerned from the TOC. > > I suggest it would be clearer if each of these sections had *two* > subsections, one covering the AS interactions and the other the RS > interactions. IOW, put the material covering the AS interactions into a > new subsection.
- [Gen-art] Gen-ART Last Call review of draft-ietf-… Paul Kyzivat
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Jim Schaad
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Benjamin Kaduk
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Paul Kyzivat
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Paul Kyzivat
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Benjamin Kaduk
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Seitz Ludwig
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Stefanie Gerdes
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Seitz Ludwig
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Benjamin Kaduk
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Olaf Bergmann
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Olaf Bergmann
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Paul Kyzivat
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Olaf Bergmann
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Paul Kyzivat
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Göran Selander
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Göran Selander
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Benjamin Kaduk
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Paul Kyzivat
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Olaf Bergmann
- Re: [Gen-art] Gen-ART Last Call review of draft-i… Paul Kyzivat