Subject: [Ace] Genart telechat review of draft-ietf-ace-oscore-profile-17

From: Elwyn Davies via Datatracker <[email-masked]>
Date: Tue, 23 Mar 2021 15:29:40 -0700

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.

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.

---

From: Lars Eggert <[email-masked]>
Date: Thu, 25 Mar 2021 13:29:53 +0200

Elwyn, thank you for your review. I have entered a No Objection ballot for this document.

Lars

On 2021-3-24, at 0:29, Elwyn Davies via Datatracker <[email-masked]> wrote:
> --
> last-call mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/last-call