Theresa,

Here is my response for your review.  

I had to really think about a couple of your comments at least in part because 
I am so immersed in this that it is hard for me to see things with new eyes.  
There are comments below prefixed with [JLS] and I have merged the comments 
into the repository so you can look at the text that I inserted (or deleted) 
but did not copy into this message

Jim



Pull request with updated text: 
https://github.com/cose-wg/cose-rfc8152bis/pull/22/commits/a9862ec70fa74339ce4a01a4ee743728f815952c
Full text with last review: https://github.com/cose-wg/cose-rfc8152bis

-----Original Message-----
From: Theresa Enghardt via Datatracker <nore...@ietf.org> 
Sent: Monday, May 25, 2020 2:15 PM
To: gen-art@ietf.org
Cc: c...@ietf.org; draft-ietf-cose-rfc8152bis-struct....@ietf.org; 
last-c...@ietf.org
Subject: Genart last call review of draft-ietf-cose-rfc8152bis-struct-09

Reviewer: Theresa Enghardt
Review result: Ready with Issues

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?

[JLS]  This document lists the three most complete implementations of the 
document that I know about.  I am aware of at least nine different 
implementations of the specification.  The rest of them implement a subset of 
the structures in the document and are not as complete.  I am aware of at least 
two different places where one of my implementations has been adapted to work 
in a different location.  I have had several different jolts where I have 
learned that COSE is either being used or is planned to be used in different 
locations.  So yes, I believe that there are a number of implementations and 
the usage of parts of COSE are wide spread.  

[JLS] The only complexity reduction in the document that has been made is to 
remove the algorithm descriptions from the document and thus shortening the 
structure document from 120 pages to around 80 pages.   The problem with 
removing any additional items is that some security feature will no longer be 
available for applications to use.  For that reason no features have been 
removed.

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 de
 velopers to understand how to use COSE.

[JLS]  I have added such a paragraph which you can view in the github version 
of the document.  I did not reference 7165 as you suggested but went directly 
to the CoRE solutions using COSE instead.

"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?

[JLS] modified to "It is better for the structure to be defined in each 
protocol as different protocols will want to include a different set of fields 
as part of the structure."  I also added a reference to 
"I-D.ietf-cose-hash-algs" which contains additional information and algorithms 
for solving this problem.

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

[JLS] I do not believe that this needs to be introduced in the text.  The 
section discusses only a part of CDDL that is used in the document and points 
to CDDL so that some additional things can be found.  The syntax "label=>value" 
is equivalent to "label: value" but either in a yet to be defined sense or for 
something which is not a string or number as the label.  Given that the text is 
normative it does not worry me that a complete description of CDDL is not 
presented here.

"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?

[JLS] There are a number of reasons why CDDL is not the normative version of 
the grammar.  Firstly, the RFC for it did not exist when RFC 8152 was written.  
This meant that it was either going to be gated on getting CDDL published or it 
needed to just not use CDDL in a normative manner.  Given that CDDL was not 
published until June of last year I am just as happy that we did not wait for 
it.  This means that it would require a fairly major re-write of the document 
to make CDDL the normative version now.  While it is important that there is no 
ambiguity in the layout, I believe that the prose version does not suffer from 
that fault.  Additionally, using CDDL normatively now would require a 
discussion on the downwards reference from this document as a Full Standard to 
CDDL as a Proposed Standard.

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?

[JLS] I think that you mean section 3 if that is not right please let me know.  
The wrapping applies to all encoding of protected buckets.  For a zero length 
bucket this is less of an issue as there is only one way to encode it per the 
rules in section 10.  The issue is that an intermediate entity could decode the 
message, including the map, and encode it using a slightly different encoding.  
This normally has to do with the order of items in the map but could be any 
number of things as the rules for deterministic encoding would need to be 
expanded to cover all possible CBOR objects rather than just the few that are 
given in section 10.  If one decodes all the way to the data model of CBOR then 
things such as dates become difficult to encode canonically as it could be 
encoded either as a floating point number or an integer number.  By doing the 
wrapping all parties doe NOT need to be able to do a common canonical encoding. 
 Experience with ASN.1 and the XML signature security have sh
 own that the number of times that at least one party is going to get things 
wrong is much higher than one would like to see for a security protocol that 
wants to be simple and small. Would starting a new paragraph at this point make 
it easier to understand that it is nothing to deal with zero-length maps?

In Section 4.1:
"An example of
   header a parameter about the content is the content type. "
This sentence appears to be broken.
[JLS] Yes, appending "header attribute" to the end of the sentence makes it 
clearer.

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.

[JLS] That makes sense and I have done so.

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.

[JLS] I have eliminated the sentence as it does not really add to anything.    
I don't think that there is a real benefit to adding the explanation for each 
of the restrictions.  The first one has to do with a deterministic encoding.  
The second one has to do with the possibility of getting two different answers. 
 The CBOR WG has been spending lots of cycles recently on how to make this 
statement an the justifications for it in the update CBOR document.  I would 
rather not get into that type of argument.

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

[JLS] No specific threat models or security analysis has been done for COSE.  
In general this is done either at the protocol layer or at the algorithm layer. 
 Details like what is sent and how identity and keys are managed are at the 
protocol layer.  Details on how much a symmetric algorithm can be used are at 
the cryptographic layer.  Some of the types of things you might be looking for 
are going to be found in the algorithms draft rather than here.

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?

[JLS]  This is a change in the document that I missed and should have caught.  
Originally only reference updates were done and then the WG decided that there 
should be a tag assignment made for countersignatures.  I have changed the 
sentence from "only known" to "majority of", with some additional changes for 
readability.

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?

[JLS] The registration template for a media-type is not placed into an IANA 
registry.  Rather the name of the media-type and a pointer to the document is 
placed in the registry (see 
https://www.iana.org/assignments/media-types/media-types.xhtml).  Copying the 
registration template forward is therefore needed in order to have something in 
this document to point to.  


_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to