Telechat Review of draft-ietf-ace-oscore-profile-17
review-ietf-ace-oscore-profile-17-genart-telechat-davies-2021-03-23-00

Request Review of draft-ietf-ace-oscore-profile
Requested rev. no specific revision (document currently at 19)
Type Telechat Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2021-03-23
Requested 2021-03-08
Authors Francesca Palombini, Ludwig Seitz, Göran Selander, Martin Gunnarsson
Draft last updated 2021-03-23
Completed reviews Genart Last Call review of -11 by Elwyn Davies (diff)
Secdir Last Call review of -13 by Kathleen Moriarty (diff)
Opsdir Last Call review of -11 by Linda Dunbar (diff)
Genart Telechat review of -17 by Elwyn Davies (diff)
Assignment Reviewer Elwyn Davies 
State Completed
Review review-ietf-ace-oscore-profile-17-genart-telechat-davies-2021-03-23
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/Es7PhQvSnCixYRfEYs0RLqcLYC0
Reviewed rev. 17 (document currently at 19)
Review result Ready with Nits
Review completed: 2021-03-23

Review
review-ietf-ace-oscore-profile-17-genart-telechat-davies-2021-03-23

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.

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.

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.

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

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.

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

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

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

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

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

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.

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.

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

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

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

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

s3.2.1, below Figure 9: Expand CDDL.

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

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

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

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

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/

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

s6. last para: s/observation/observations/

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

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

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.