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

From: Stefanie Gerdes <[email-masked]>
Date: Fri, 23 Apr 2021 18:08:21 +0200

Hi Roman,

Thank you very much for your detailed comments!

We addressed your comments with two exceptions as explained in detail
below. You can see most of the changes at
https://github.com/ace-wg/ace-dtls-profile/commit/a9ee40dff5093f1cf43e1b734eb6e02745f43f4d

On 3/23/21 4:32 AM, Roman Danyliw via Datatracker wrote:
> Roman Danyliw has entered the following ballot position for
> draft-ietf-ace-dtls-authorize-16: Discuss
>
> ----------------------------------------------------------------------
> 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".

Okay, done.

>
>
> ----------------------------------------------------------------------
> 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.

Fixed in introduction.

>
> ** 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

Good catch! 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 implement.

>
> ** 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

Fixed.

>
> ** 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/.

We 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)?

Prohibiting session resumption would be too restrictive as this can be
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 are working on that with the other authors.

>
> ** 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:

We fixed all framework references (including the ones below).

>
> -- 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.

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

removed.

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

Fixed.

Best regards
Steffi