Re: [Gen-art] Gen-ART Last Call review of draft-ietf-ace-dtls-authorize-12

Olaf Bergmann <bergmann@tzi.org> Tue, 08 September 2020 11:14 UTC

Return-Path: <bergmann@tzi.org>
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 005973A0F0E; Tue, 8 Sep 2020 04:14:48 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.897
X-Spam-Level:
X-Spam-Status: No, score=-1.897 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, 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 MWEKLDBqSos2; Tue, 8 Sep 2020 04:14:44 -0700 (PDT)
Received: from gabriel-vm-2.zfn.uni-bremen.de (gabriel-vm-2.zfn.uni-bremen.de [134.102.50.17]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 698EF3A0DE4; Tue, 8 Sep 2020 04:14:41 -0700 (PDT)
Received: from wangari.tzi.org (p508a4e5d.dip0.t-ipconnect.de [80.138.78.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gabriel-vm-2.zfn.uni-bremen.de (Postfix) with ESMTPSA id 4Bm2bz3b8szyX1; Tue, 8 Sep 2020 13:14:39 +0200 (CEST)
From: Olaf Bergmann <bergmann@tzi.org>
To: Paul Kyzivat <pkyzivat@alum.mit.edu>
Cc: draft-ietf-ace-dtls-authorize.all@ietf.org, General Area Review Team <gen-art@ietf.org>
References: <8c2725a3-f89f-7ea1-dda9-681edd463a32@alum.mit.edu> <87y2muo6ix.fsf@wangari>
Date: Tue, 08 Sep 2020 13:14:39 +0200
In-Reply-To: <87y2muo6ix.fsf@wangari> (Olaf Bergmann's message of "Tue, 04 Aug 2020 16:00:22 +0200")
Message-ID: <87v9gomsf4.fsf@wangari>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/bUPOhr2QkRLfyYfckeCn274UnK8>
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: Tue, 08 Sep 2020 11:14:48 -0000

Hi Paul,

I hope you are doing well. On request from Jim Schaad who acts as the
document shepherd, we have submitted draft-ietf-ace-dtls-authorize-13
including the changes proposed below. Please feel free to contact us if
you do not agree with these proposals.

Grüße
Olaf

Olaf Bergmann <bergmann@tzi.org> writes:

> Thank you very much for your review and the very useful suggestions
> therein. Please find our comments inline. All issues that have not been
> resolved in the prior discussion threads with Ben and Jim have been
> addressed in the editor's copy of the draft.
>
> Paul Kyzivat <pkyzivat@alum.mit.edu> writes:
>
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> 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.
>
> There is no normative requirement to keep tokens until their expiry. A
> resource server could dispose of a stored token at any time, forcing a
> client to retrieve another access token for subsequent requests directed
> to the respective resource server. The only requirement imposed on the
> resource server is that it must not send data to the client if there is
> no valid access token that authorizes this action (this is described in
> Section 3.4).
>
> Usually, a resource server implementation would keep the received access
> tokens as long as it deems useful for the intended interaction with the
> client, with the expiration time being an upper bound for the storage
> time (see Section 3.4).
>
>> 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.
>
> The ACE framework already states in Section 5.8.1 that the "RS
> MUST be prepared to store at least one access token for future use"
> and "RECOMMENDS that an RS stores only one token per
> proof-of-possession key". This is now re-iterated in this profile
> as suggested:
>
> NEW:
>
>   A resource server MUST have the capacity to store one access token
>   for every proof-of-possession key of every authorized client.
>
> Although a reasonable implementation
> of this profile would try to provide storage for every PoP key of
> every authorized client, this could easily get out of bounds with the
> separate upload mechanism to the authz-info resource as pointed out in
> the cited paragraph from the security considerations. This now has
> been elaborated as follows:
>
> OLD:
>
>   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.
>
> NEW:
>
>   A similar issue exists with the unprotected authorization
>   information endpoint when the resource server needs to keep valid
>   access tokens for a long time. Adversaries could fill up the
>   constrained resource server's internal storage for a very long time
>   with interjected or otherwise retrieved valid access tokens.  To
>   mitigate against this, the resource server should set a time
>   boundary until an access token that has not been used until then
>   will be deleted.
>
>> 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.)
>
> We have changed "needs to be unique" to "MUST be unique" in the first
> paragraph as suggested, and "the use of a key derivation IKM that is
> unique" to "the use of a key derivation IKM that MUST be unique" in the
> second paragraph you have mentioned.
>
>> * 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.
>
> I take it that you and Ben have agreed that the example description does
> not necessarily need normative language as the description of this key
> derivation procedure is meant as an example how the authorization server
> and the resource server can securely agree on a shared secret to be used
> between the client and the resource server.
>
>> * 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".
>
>>From [1] I take it that you have been ok with the current wording to
> indicate that this means a MUST implement while there may be cases where
> a client knows that it can offer something else.
>
>    [1] https://mailarchive.ietf.org/arch/msg/ace/V8WXg8dhE3UkDxbIbOK-D0VmD9U/
>
>> * 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".
>
> We are happy to mandate PoP tokens. In the beginning when
> the protocol was intended to be similar to OAuth, people wanted to be
> have some loophole for other token types. The text now has been
> changed as proposed in [2] to
>
>   [2] https://mailarchive.ietf.org/arch/msg/ace/ddtl8a4fmvEdXhEWc3HQtv6RQgo/
>
> OLD: 
>
>   This specification assumes that the access token is a PoP token as
>   described in [I-D.ietf-ace-oauth-authz] unless specifically stated
>   otherwise.
>
> NEW:
>
>   This specification implements access tokens as proof-of-possession
>   tokens.
>  
>> * 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"?
>
> The ACE profiles would continue to work if there were multiple
> access tokens for a single resource server/client association in
> effect at a particular point in time. But neither OAuth nor ACE have
> defined algorithms for merging authorization rules, and therefore, the
> ACE WG has decided to keep things simple and assume that a newer
> access token always replaces the old one.
>
> As a consequence, the framework document and the profiles are written
> as if a newer access token always replaces older access tokens for a
> respective association between resource server and client.  To make
> this assumption more clear, we have removed the "are supposed to". The
> text now reads
>
> OLD:
>
>   New access tokens issued by the authorization server are supposed to
>   replace previously issued access tokens for the respective client.
>
> NEW:
>
>   New access tokens issued by the authorization server replace
>   previously issued access tokens for the respective client.
>
>> 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.
>
> See above. Basically, we now require PoP tokens. (An alternative type of
> access token would have been indicated with the token_type claim as
> defined in the framework document.)
>
>> * 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.
>
> The reason for this seemingly mysterious statement is that it is
> very difficult to define a strict order on access tokens that works
> for all intended use cases the protocol has initially been designed
> for. This was discussed in [3].
>
>   [3] https://mailarchive.ietf.org/arch/msg/ace/x9JFbn0a6CRdsIwmQdKReGu5q1E/
>
> The only resolution the working group had agreed on by then was to treat
> use the last token that was received as the newest. There is some text
> in the framework that tells how the resource server might be able to
> order access tokens. The most general would be to use the cti claim,
> therefore the following text has been added to clarify this:
>
> NEW:
>
>   The authorization server may, e.g., specify a "cti" claim for the
>   access token (see Section 5.8.3 of [I-D.ietf-ace-oauth-authz] to
>   employ a strict order.
>
>
>> 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.
>
> Good point, this is indeed very odd. The structure now has been changed
> to include a subsection "Access Token Retrieval from the Authorization
> Server" for each of the two modes where the interaction between client
> and authorization server is described.
>
> Grüße
> Olaf