Re: [Ace] Genart last call review of draft-ietf-ace-oscore-profile-11

[Francesca Palombini <[e-mail]>]

Hi Elwyn,

Thank you so much for your review. We have now worked on all your comments in a pull request, and will soon submit an update to the document. All the nits are adressed in two commits: https://github.com/ace-wg/ace-oscore-profile/commit/a7f9483e96107a678b80217ba0b2d3dcfb488192  and https://github.com/ace-wg/ace-oscore-profile/commit/855c34865120a1f09c28ebe6dce93acedb1f3e04 . Detailed comments inline, prefaced with [FP].

Thanks again for the good comments,
Francesca

On 22/07/2020, 00:56, "Elwyn Davies via Datatracker" <[e-mail]> wrote:

    Reviewer: Elwyn Davies
    Review result: Almost Ready

    I am the assigned Gen-ART reviewer for this draft. The General Area
    Review Team (Gen-ART) reviews all IETF documents being processed
    by the IESG for the IETF Chair.  Please treat these comments just
    like any other last call comments.

    For more information, please see the FAQ at

    <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

    Document: draft-ietf-ace-oscore-profile-11
    Reviewer: Elwyn Davies
    Review Date: 2020-07-21
    IETF LC End Date: 2020-07-20
    IESG Telechat date: Not scheduled for a telechat

    Summary:  Almost ready.  There is one minor issue that needs sorting out and a
    fair number of nits.  Overall I have to say that I found it difficult to keep
    clear in my mind what messages were fully encrypted and which ones were sent en
    clair and which are in some intermediate class.  The authors might wish to go
    back over the document from the point of a naive reader to ensure that it is
    clear for implementers.

    Major issues:
    None

    Minor issues:
    s2, para 5:  Where does the 'input salt' come from?  The term is not used
    anywhere else in this document and  isn't defined or mentioned in either
    dreft-ace-oauth-authz or RFC 8613.

[FP]: Right, as Ben mentioned, this was the result of an update to the name of the term. The input salt is used as one of the inputs to the OSCORE Master Salt. I have now rephrased to clarify that "salt" contains in fact an input to the OSCORE Master Salt. (https://github.com/ace-wg/ace-oscore-profile/commit/07ced6a4f908491d7d70c8c2d6fca7596e3801d4 )

    Nits/editorial comments:
    s1:  Need to expand CoAP on first use.

[FP]: Ok.    

    s1: Need to expand CBOR on first use.

[FP]: Actually, because CBOR appears on first use as the first term of COSE, I have not expanded it in this location. I have added a normative reference to CBOR in the terminology and expanded it there.

    s1.2, CDDL:  It would useful to mention that the predefined type names from
    CDDL, especially bstr for byte strings and tstr for text strings,  are used
    extensively in the document.

[FP]: Thanks for the suggestion, now added.

    s2, para 1: s/overview on how/overview of how/

[FP]: Ok.

    s2, para 1: s/as well as OSCORE setup/as well as the OSCORE setup/

[FP]: Ok.

    s2, para 2: s/that's/that is/

[FP]: Ok.

    s2, para 8: Need to expand AEAD on first use.

[FP]: Ok.

    s2 and Figure 1:  It would be helpful to the reader if Figure 1 and its
    descriptive paragraph was placed closer to the beginning of s2.  Otherwise
    things like Client C' need more explanation to point the reader at the figure.

[FP]: I have kept Figure 1 at the end of the section, but I have now removed all instances of "Client C", since they don't make sense before seeing the picture, as you rightly noted.

    s2, para 3:

    This says:
    To determine the AS in charge of a resource hosted at the RS, the client C MAY
    send an initial Unauthorized Resource Request message to the RS. The RS then
    denies the request and sends the address of its AS back to the client C as
    specified in section 5.1 of [I-D.ietf-ace-oauth-authz]. The access token
    request and response MUST be confidentiality-protected and ensure authenticity

    I found the combination of the Unauthorized Requst and the
    confidentiality-protected etc confusing.  If the last sentence does apply to
    the Unuthorized Request it would be helpful to make it clear that this is not
    just a generic statement but does apply to the Unauthorized Request as well.

[FP]: Ok, thank you for pointing it out. I have now clarified in the beginning of the paragraph that the access token request is different from the Unauthorized Request.

    Figure1:  For consistency the first line should say Unauthorized Rsource
    Request.  I would also suggest explaining the mapping between what is said in
    the text and the terms 'Ceation Hints' and 'Access Information' used in the
    figure.

[FP]: Ok about the Unauthorized Resource Request. I have not explained further about the mapping between the overview text and the figure, as I do not want to go into too much detail there, but I have clarified that the names of messages come from the framework.

    s3.1, para after Figure 2:  The term 'audience' appears in this paragraph
    without any context indicating what it means .  Later in s3.2 it appears that
    audience is associated with CBOR web tokens (RFC 8392).  But it may also might
    also be realted to draft-oauth-token exchange.  The appropriate reference
    ahould be added and should be explained in s3.1.

[FP]: Ok, added a reference to the right section in the framework.

    Figure 3:  Should IdContext be ContextId?  ContextId is used evrywhere else.

[FP]: Good catch!

    s3.2: Expand HKDF on first use ( in second set of bullets).

[FP]: Ok.

    s3.2, para after 2nd set of bullets:  I think the four instances of 'may' 
    ought to be 'MAY'.

[FP]: These may were not normative on purpose, as the normative MAY is the one above the bullet list. I have now rephrased to remove "may" from this paragraph, to avoid confusion.

    s3.2.1:  It would be helpful to provide references to the online versions of
    the  IANA registries (3 places).

[FP]: Ok.

    s4.2, para 1:   A foward reference to s5 where the comunication mechanisms
    needed for introspection are described.

[FP]: I added a reference to the section of the framework where introspection is described.

    s4.1, para 2: s/from what described/from what is described/

[FP]: Ok.

    s4.2, para 5: s/that's/that is/

[FP]: Ok.

    s4.2, last para; s/This simplifies for the RS track/This simplies the process
    needed by the RS to keep track/

[FP]: Ok

    s8, para 6: s/tasked of/tasked with/

[FP]: Ok

    s9.3:  I don't think the Value Type for nonce is 'IESG'! lol

[FP]: Indeed! Thanks.