Re: [Last-Call] Genart telechat review of draft-ietf-ace-oscore-profile-17

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Elwyn,

Thank you very much for the in depth review! We have incorporated your changes in the newly submitted v-18 https://datatracker.ietf.org/doc/html/draft-ietf-ace-oscore-profile-18 , but you can also see the specific changes in the github: https://github.com/ace-wg/ace-oscore-profile/commit/c98ea9c53994aaa85add5c4a3436a14935fa0471 

Answers inline.

Thanks again,
Francesca

On 23/03/2021, 23:29, "Elwyn Davies via Datatracker" <noreply@xxxxxxxx> wrote:

    Reviewer: Elwyn Davies
    Review result: Ready with Nits

    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 wait for direction from your
    document shepherd or AD before posting a new version of the draft.

    For more information, please see the FAQ at

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

    Document: draft-ietf-ace-oscore-profile-17
    Reviewer: Elwyn Davies
    Review Date: 2021-03-23
    IETF LC End Date: 2020-07-20
    IESG Telechat date: 2021-03-25

    Summary:
    Ready with nits.  A very great improvement on the previously reviewed version. 
    Thanks.

FP: Thank you!

    Major issues:
    None

    Minor issues:

    Would it be useful to provide some advice on the length of salts and IDs to go
    with the advice on length of nonces?  There is some in s3.3 of RFC 8613 but
    some other reference might be helpful, maybe placed in s3.2.1. and/or s4.

FP: This draft does not have any other requirements on salts and IDs than RFC 8613. The IDs need not be long, but unique. The salt need not even be unique. According to RFC 5869 Section 3.1 “HKDF is defined to operate with or without salt”. However, the use of salt adds to the strength of HKDF. “Ideally, the salt value is a random (or pseudorandom) string of the length HashLen.” But the salt going into HKDF is composed of the master salt and the nonces. It therefore suffices with nonces of length HashLen/2 generated by the peers as described in this draft, independent of salt.

    Nits/editorial comments:

    General: The RFC Editor conforms rigorously to American practice and allows
    only the use of double quote marks (") in the text when marking strings as
    quotations and such like.  The document makes extensive but not totally
    consistent, use of single quotes to flag up field names and such like (e.g.,
    'nonce1').  In practice these are unnecessary, but may be replaced by the RFC
    Editor if left in place.  Personally. I think most of them can be removed. NB
    this does not affect CBOR items such as h'1645.

FP: We have now removed all single quotes "'" and replaced them with the XML2RFC v3 supported <tt></tt>. We will make sure to follow RFC Editor recommendations about this.

    General: There are lots of usages of 'CBOR diagnostic notation without the tag
    and value abbreviations'.  An abbreviation would reduce the verbiage.

FP: Fair point, we have added this to the terminology.

    General: It is slightly confusing to have Nonce 1/N1/nonce1 and Nonce
    2/N2/nonce2 used in the document.  Am I right in thinking Nonce 1 and N1 are
    the same with nonce1  being the name of the JSON/CBOR parameter used to carry
    the value?  A few words of clarrification would help.

FP: There is a difference between the three: Nonce 1 is the name of the field, N1 is the value, nonce1 is the parameter's label. If you have suggestion on where to add this, that would be helpful.

    Abstract/s1:  It would be useful to introduce the name of the profile
    (coap_oscore) up front.  It rather sneaks out in s3.

FP: Good point, added.

    s1, para 2: Need to expand CBOR on first use.

FP: Usually yes, but because here it is used as the expansion of COSE, we used the RFC Editor abbreviation expansion, which keeps CBOR non-expanded.

    s2, end of para 3: s/as well/instead/? or s/as well/alternatively/.

FP: Ok, replaced with alternatively.

    s2, para 7 and s6, bullet 2: s/e.g. expiration./for example, expiration./

FP: Ok, fixed.

    s3.1, para 3 and last para: s/reported/shown/

FP: Ok, fixed.

    s3.1, Figure 2 and Figure 3: Appendix F.3 of draft-ietf-ace-oauth-authz reports
    that req_aud was replaced by audence at version 19 of the document.

FP: Good catch! Fixed.

    s3.2, second set of bullets:  Need to expand HMAC and HKDF on first use (not
    well-known in RFC Editor list).  It would also be useful to put a pointer to
    section 11.1 of RFC 8152 here to indicate the allowed HKDF algorithms.

FP: Agreed. We have added it to the terminology section (to expand it) and added a reference to it. Also added the pointer to COSE.

    s3.2, 2nd para after 2nd set of bullets: s/The applications needs/The
    application needs/

FP: Ok, fixed.

    s3.2, 3rd para after 2nd set of bullets: s/parameeter/parameter/

FP: Fixed.

    s3.2, 4th para after 2nd set of bullets: s/the use of CBOR web token/the use of
    a CBOR web token/

FP: Fixed

    s3.2.1:
    OLD:
    IANA "OSCORE Security Context Parameters" registry (Section 9.4), defined for
    extensibility, and is specified below. NEW: IANA "OSCORE Security Context
    Parameters" registry (Section 9.4), defined for extensibility, and the initial
    set of parameters defined in this document is specified below. END

FP: Ok, fixed.

    s3.2.1, below Figure 9: Expand CDDL.

FP: Why? This is not the first time CDDL appears.

    s4.1, para 1 and s4.2, para 2: s/RECOMMENDS to use/RECOMMENDS using/

FP: Ok, fixed.

    s4.1, para 1 and s4.2. para 2: s/as nonce's value/as the nonce's value/

FP: Fixed.

    s4.1, para 7: s/renew/update/  [renew implies the same identifiers are used -
    which is already specified!]

FP: Right, fixed.

    s4.1, last para and s4.3, last para: Does /authz-info have some special meaning?

FP: The same as in draft-ietf-ace-oauth-authz. I haven't changed anything here because we use the same phrasing as that document (which we also reference in the terminology).

    s4.3, para 1: s/ Once receiving the 2.01 (Created) response from the RS/ Once
    the 2.01 (Created) response is received from the RS/

FP: Fixed.

    s4.3, Figure 12:  I assume the Master Salt is supposed to be a CBOR indefinite
    length string encoding (it doesn't say so) as it it consists of the
    concatenated CBOR strings of its component byte strings.  It would be strictly
    correct to start it with 0x5f and end with (0x)ff I would have thought. 

FP: No, this is to prevent attacks based on confusing the delimitation between the two nonces.

Be that
    as it may, I do not understand why the document is concerned with either CBOR
    or JSON/base64 encodings of the master salt.  It may be that I am missing
    something, but I didn't think that the master salt was ever put in a protocol
    message as such (deliberately), but only as one or two of its components such
    that it could be privately constructed at both endpoints once the three
    components had been shared, and was just the concatenation of the data bytes of
    the 3 components rather than involving their lengths.

FP: The OSCORE master salt is constructed from two parts: one part agreed beforehand (if present) and the other part from the nonces sent in the message. Since both parties need to agree on this parameter to derive the same keys, the method of encoding used in the key derivation needs to be defined.

    s6. last para: s/observation/observations/

FP: Fixed.

    s7, para 3: s/RS pass/RS passes/

FP: Fixed.

    s9: It is now usual to give the URLs for the various existing registries as
    normative references.

FP: Added (to the existing one).

    s9.4: I am not aware that a single registry can have different
    review/specfication requirements for portions of its parameter space.  Is it
    seriously expected that there will be significant numbers of requests for
    values in this registry?  My instinct would be to go for specification required
    and advise allocation according to the orign and type of the specification.

FP: Yes, that is done, for example in the COSE registries.


-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux