Thank you all for the follow-up.

Yours,
Daniel
________________________________
From: Roman Danyliw <[email protected]>
Sent: Wednesday, May 12, 2021 9:34 AM
To: Stefanie Gerdes <[email protected]>; The IESG <[email protected]>
Cc: [email protected] <[email protected]>; 
[email protected] 
<[email protected]>; [email protected] <[email protected]>
Subject: RE: Roman Danyliw's Discuss on draft-ietf-ace-dtls-authorize-16: (with 
DISCUSS and COMMENT)

Hi Steffi!

Thank you for the explanations below and edits made in -17 in response to my 
review.  All of my feedback is addressed and I've cleared my ballot.

Thanks,
Roman

> -----Original Message-----
> From: iesg <[email protected]> On Behalf Of Stefanie Gerdes
> Sent: Tuesday, May 11, 2021 8:42 AM
> To: Roman Danyliw <[email protected]>; The IESG <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]
> Subject: Re: Roman Danyliw's Discuss on draft-ietf-ace-dtls-authorize-16: 
> (with
> DISCUSS and COMMENT)
>
> 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://protect2.fireeye.com/v1/url?k=6154f2cc-3ecfca1e-6154b257-866132fe445e-003c0148b9c30169&q=1&e=2e35afc9-ad97-4505-ac7e-3c595783ec1b&u=https%3A%2F%2Fgithub.com%2Face-wg%2Face-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

_______________________________________________
Ace mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/ace

Reply via email to