Last Call Review of draft-ietf-cose-rfc8152bis-struct-09
review-ietf-cose-rfc8152bis-struct-09-genart-lc-enghardt-2020-05-25-00

Request Review of draft-ietf-cose-rfc8152bis-struct
Requested rev. no specific revision (document currently at 14)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2020-05-29
Requested 2020-05-15
Authors Jim Schaad
Draft last updated 2020-05-25
Completed reviews Opsdir Last Call review of -10 by Shwetha Bhandari (diff)
Secdir Last Call review of -09 by Derek Atkins (diff)
Genart Last Call review of -09 by Theresa Enghardt (diff)
Genart Telechat review of -10 by Theresa Enghardt (diff)
Assignment Reviewer Theresa Enghardt 
State Completed
Review review-ietf-cose-rfc8152bis-struct-09-genart-lc-enghardt-2020-05-25
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/GtDnPNEJ-jTu4_tbhuy7yh3iuC4
Reviewed rev. 09 (document currently at 14)
Review result Ready with Issues
Review completed: 2020-05-25

Review
review-ietf-cose-rfc8152bis-struct-09-genart-lc-enghardt-2020-05-25

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-cose-rfc8152bis-struct-09
Reviewer: Theresa Enghardt
Review Date: 2020-05-25
IETF LC End Date: 2020-05-29
IESG Telechat date: Not scheduled for a telechat

Summary: This document describes data structures and processes in detail, with minor updates. In combination with a companion document on algorithms, it obsoletes RFC 8152. 
The document has some issues and editorial nits, which should be addressed prior to publication.

Major issues: 

The intended RFC status is Internet Standard, while RFC 8152 is a Proposed Standard. 
While this document points to three different implementations, how widespread is their deployment? Were there other lessons learned from implementation and deployment experience? Has complexity been reduced by removing unused features, as RFC 6410 suggests?

Minor issues:

In the introduction, it would be great to add some more context on how COSE is intended to be used. 
Is this solely for adding security to objects within CoAP, e.g., by signing and encrypting messages exchanged as CoAP payload? Or can it be used with other protocols or in other contexts? 
In RFC 7165, which describes JOSE use cases, Section 5.8.2 mentions Object Security for CoAP. Is COSE addressing this use case? If so, please add a link to Section 5.8.2 of RFC 7165.
Section 9.5.4 mentions that COSE "is designed for a store and forward environment rather than an online environment". Perhaps it's worth mentioning such design goals and context in the introduction already.
When adding one or two paragraphs on how COSE is intended to be used to the introduction, it's also worth stating explicitly that each application is expected to select the COSE objects and processes that it needs, and that Section 11 provides more guidance on how to do this. This will make it easier for application developers to understand how to use COSE.

"One feature that is present in CMS [RFC5652] that is not present in
   this standard is a digest structure.  This omission is deliberate.
   It is better for the structure to be defined in each document as
   different protocols will want to include a different set of fields as
   part of the structure."
Does "in each document" refer to documents for different encryption algorithms or protocols? Please clarify.
Is draft-ietf-cose-rfc8152bis-algs an example of one of these documents, as it describes algorithms? Or is another type of documents meant here?

When introducing the CDDL syntaxes: 
Is "* label => values" missing here? It appears to be used in some examples below.

"The CDDL grammar is informational; the prose
   description is normative."
Why? Isn't it important for interoperability that the objects are structured in a certain way and that there is no ambiguity?

In Section 2:
"[...] The wrapping allows for the encoding of the
  protected map to be transported with a greater chance that it will
  not be altered in transit.  (Badly behaved intermediates could
  decode and re-encode, but this will result in a failure to verify
  unless the re-encoded byte string is identical to the decoded byte
  string.)  This avoids the problem of all parties needing to be
  able to do a common canonical encoding."
I'm not sure I understand this scenario. Does this statement just apply to zero-length maps or to all kinds of protected buckets? 
What does "to be transported with a greater chance" mean here? That intermediates are less likely to block traffic containing such a COSE object? That intermediates are less likely to accidentally (or deliberately) modify it in such a way that it gets accepted by the receiver? Is this an effect of some cryptographic protection, is it related to different encoding techniques being used in different implementations, or both? Why is it a problem if all parties need to be able to do a common canonical encoding? In fact, what kind of encoding does this refer to: Binary encoding vs. base64 encoding? Or specific ways of encoding certain information as binary data?

In Section 4.1:
"An example of
   header a parameter about the content is the content type. "
This sentence appears to be broken.

In Section 4 or 5, maybe it's worth adding a brief description or reference of what a countersignature is. In contrast to the concepts of signing, encryption, and Message Authentication Codes, I would not expect every application developer to be familiar with the concept of a countersignature and what it's needed for.

In Section 10, the document presents restrictions on the CBOR encoder.
"We have managed to narrow it down to the following restrictions" sounds like these might have been narrowed down from RFC 8152, and maybe could be rephrased to sound more neutral.
Why are these restrictions necessary? Maybe it's worth adding a brief explanation for each restriction.

In the Security Considerations:
What threat models have been considered for COSE?

Nits/editorial comments:

Section 12:
"The only known action at this time
   is to update the references."
Right after this statement, the document describes another IANA action. So maybe updating the references is not "the only known action" anymore?

For most IANA registries, this document simply asks to update the references from RFC 8152 to this document.
However, for the Media Type Registrations, Section 12.5 appears to repeat the registry information. Does this document introduce any changes here? If not, why is the information repeated here, but not in the other sections?