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

From: Roman Danyliw via Datatracker <[email-masked]>
Date: Mon, 22 Mar 2021 20:16:21 -0700

Roman Danyliw has entered the following ballot position for
draft-ietf-ace-dtls-authorize-16: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ace-dtls-authorize/

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

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thank you to Russ Mundy for the SECDIR review, and thank you to the authors for
responding to it.

** What is the motivation for specifying this profile around DTLS 1.2 instead
of 1.3??

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

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

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

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

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

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

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

** 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_*)”.

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

** 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?).

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

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

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

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