Re: [Ace] Roman Danyliw's Discuss on draft-ietf-ace-dtls-authorize-16: (with DISCUSS and COMMENT)

Stefanie Gerdes <gerdes@tzi.de> Tue, 11 May 2021 12:42 UTC

Return-Path: <gerdes@tzi.de>
X-Original-To: ace@ietfa.amsl.com
Delivered-To: ace@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 1F77C3A163C; Tue, 11 May 2021 05:42:05 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.898
X-Spam-Level:
X-Spam-Status: No, score=-1.898 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_BLOCKED=0.001, SPF_FAIL=0.001, SPF_HELO_NONE=0.001] autolearn=no 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 HlZHPKSzWBbb; Tue, 11 May 2021 05:42:02 -0700 (PDT)
Received: from gabriel-2.zfn.uni-bremen.de (gabriel-2.zfn.uni-bremen.de [IPv6:2001:638:708:32::19]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A8E163A166B; Tue, 11 May 2021 05:42:02 -0700 (PDT)
Received: from [192.168.0.48] (p5b36f986.dip0.t-ipconnect.de [91.54.249.134]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by gabriel-2.zfn.uni-bremen.de (Postfix) with ESMTPSA id 4Ffcxg2hmMz315b; Tue, 11 May 2021 14:41:59 +0200 (CEST)
To: Roman Danyliw <rdd@cert.org>, The IESG <iesg@ietf.org>
Cc: draft-ietf-ace-dtls-authorize@ietf.org, ace-chairs@ietf.org, ace@ietf.org
References: <161647032007.11307.14702169079766002256@ietfa.amsl.com>
From: Stefanie Gerdes <gerdes@tzi.de>
Message-ID: <ec91a0c7-cf80-3d29-145d-feb91862b7f9@tzi.de>
Date: Tue, 11 May 2021 14:41:58 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <161647032007.11307.14702169079766002256@ietfa.amsl.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 8bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/ace/V6Hl97htBTzpH3P3ooHYnjPzXYQ>
Subject: Re: [Ace] Roman Danyliw's Discuss on draft-ietf-ace-dtls-authorize-16: (with DISCUSS and COMMENT)
X-BeenThere: ace@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Authentication and Authorization for Constrained Environments \(ace\)" <ace.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ace>, <mailto:ace-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ace/>
List-Post: <mailto:ace@ietf.org>
List-Help: <mailto:ace-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ace>, <mailto:ace-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 11 May 2021 12:42:09 -0000

Hi Roman,

Thank you for your detailed comments. We addressed most of your comments
in the latest version. Please find my comments inline.

On 3/23/21 4:32 AM, Roman Danyliw via Datatracker wrote:
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> (A simple editorial fix) Per Section 5.8.2 of [I-D.ietf-ace-oauth-authz], the
> name of the parameter in the C-to-AS communication is “ace_profile” (not
> “profile”).  The “ace_profile” parameter is mistakenly referenced as “profile”
> in the following places:
> 
> -- Section 3.2.1:
>    The response MAY contain a "profile" parameter with the value
>    "coap_dtls" to indicate that this profile MUST be used for
>    communication between the client and the resource server.
> 
> -- Section 3.3.1:
>    If the
>    profile parameter is present, it is set to "coap_dtls".

Yes, you are correct. The name of the parameter changed in
ace-oauth-authz-25 and this occurrence must have slipped through.

> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thank you to Russ Mundy for the SECDIR review, and thank you to the authors for
> responding to it.
> 
> ** Does this profile only cover part of the oauth-authz framework?  Section 3.3
> explicitly says “the use of introspection is out of scope for this
> specification.”  It might be helpful to note in the introduction that this
> profile only covers C-to-AS and C-to-RS communication.

We added to the introduction that introspection is out of scope for this
specification.

> 
> ** Section 3.2.1 Figure 3, uses the “req_aud” parameter, but this was renamed
> to “audience” in -20 of draft-ietf-ace-oauth-authz

Yes, fixed.

> 
> ** Section 3.2.1.  Per ‘The response MAY contain a "profile" parameter with the
> value "coap_dtls" to indicate that this profile MUST be used for communication
> between the client and the resource server’, this is true (see the DISCUSS
> above though).  However, it might be worthwhile to point out that per Section
> 5.8.2 of draft-ietf-ace-oauth-authz-38, this “MAY” is actually a MUST if the
> request has an empty “ace_profile” parameter.

Okay, fixed.

> 
> ** Section 3.2.2.  Per “This specification therefore mandates implementation
> support for curve25519 ...”, perhaps RFC2119 language should be used here

Okay, changed to MUST.

> 
> ** Section 3.3.1.  Per all of the text after “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”, consider removing all of the RFC2119 language is this text as its an
> example.

The Gen-ART review from Paul Kyzivat of 19 Jul 2020 suggested to include
the normative language to avoid ending up with unclear specifications.
(The normative language has been added in
https://github.com/ace-wg/ace-dtls-profile/commit/9ab383c0e08f8d4bff5335cbfadb1c6b48289472)

> 
> ** Section 3.3.2.  Per “When the resource server receives an access token, it
> MUST check if the access token is still valid ...”, a reference to Section
> 5.10.1.1 of [I-D.ietf-ace-oauth-authz] for additional verification procedures
> might be helpful

Okay, done.

> 
> ** Section 3.2.2. and 7:
> 
> (a) Section 3.2.2.
>    To be consistent with [RFC7252] which allows for shortened MAC tags
>    in constrained environments, an implementation that supports the RPK
>    mode of this profile MUST at least support the ciphersuite
>    TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8 [RFC7251].
> 
> (b) As this specification aims at constrained devices and
>    uses CoAP [RFC7252] as transfer protocol, at least the ciphersuite
>    TLS_PSK_WITH_AES_128_CCM_8 [RFC6655] should be supported
> 
> The text in (b) is weaker on the mandatory required of the ciphersuite.  In
> (b), likely s/should be supported/must be supported/.

Okay, changed b to MUST support.

> 
> ** Section 7.  Per “For longer-lived access tokens, DHE ciphersuites should be
> used”, perhaps add a parenthetical at the end of this sentence of “(i.e.,
> ciphersuites of the form TLS_DHE_PSK_*)”.

Fixed as suggested.

> 
> ** Section 7.1.  Session resumption is noted to be NOT RECOMMENDED.  Is there a
> reason this can’t be stronger (MUST NOT)?

Session resumption can be very useful for very constrained clients. We
therefore changed it as follows:

OLD:

   Therefore, the use of session resumption is NOT RECOMMENDED for
   resource servers.

NEW:

   Therefore, session resumption should be used only in combination with
   reasonably short-lived PoP keys.

> 
> ** Section 7.2.  No issues with the guidance here.  Is there anything DTLS
> specific that suggests that developers "SHOULD" avoid multiple access tokens
> per client?  That guidance isn’t in the core framework.  I made the comment on
> the core framework that perhaps this text should be there (too?).

We moved the respective paragraph to the framework document.

> 
> ** Please reviews all of the reference numbers to [I-D.ietf-ace-oauth-authz] as
> a number of them seem to be incorrect (likely due to renumbering).  For example:

Okay, checked and fixed.

> 
> -- Section 2.  Per “the client MUST upload the access token to the authz-info
> resource, i.e. the authz-info endpoint, on the resource server before starting
> the DTLS handshake, as described in Section 5.8.1 of   
> [I-D.ietf-ace-oauth-authz]”, Section 5.8.1 is not the right reference.  It’s
> likely 5.10.1.
> 
> -- Section 3.4.  Per “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”, Section 5.8.3 is the wrong section in
> [I-D.ietf-ace-oauth-authz].
> 
> -- Section 3.4.  Per “The response SHOULD include AS Request Creation Hints as
> described in Section 5.1.1 of [I-D.ietf-ace-oauth-authz].”, there is no Section
> 5.1.1. The appropriate section is either 5.2 to reference this behavior or 5.3
> for the details of the hints.
> 
> -- Section 3.4. Per “Incoming CoAP requests received on a secure DTLS channel
> that are not thus authorized MUST be rejected according to Section 5.8.2 of
> [I-D.ietf-ace-oauth-authz]”, Section 5.8.2 is not the right reference here.
> 
> ** idnits returned the following:
> 
>   == Unused Reference: 'RFC8152' is defined on line 1148, but no explicit
>      reference was found in the text

Fixed (added reference in the draft)

> 
>   == Unused Reference: 'RFC8613' is defined on line 1212, but no explicit
>      reference was found in the text

Fixed (removed)

> 
> ** Nits
> -- Section 7.1.  Typo. s/renogiation/renegotiation/

Fixed.

Thank you for your time,
Steffi