Skip to main content

CBOR Object Signing and Encryption (COSE): Structures and Process
draft-ietf-cose-rfc8152bis-struct-15

Note: This ballot was opened for revision 10 and is now closed.

Erik Kline
No Objection
Comment (2020-06-10 for -10) Sent
[[ nits ]]

[ section 1.6 ]

* "authentication check of the contents algorithm" ->
  "authentication check of the contents" ?

  How should I read this sentence about AE?  It seems to be copied from 8152,
  but I kinda feel like the 3rd (last) use of the word "algorithm" could be
  removed?

[ section 2 ]

* Table 1, CBOR tag 17: "COSE Mac w/o" -> "COSE MAC w/o"

[ section 5 ]

* "This is same" -> "This is the same"

* "That is the counter signature..." -> "That is, the counter signature..."

[ section 5.1 ]

* Isn't #6.98 for COSE_Sign_Tagged?  I think the CDDL fragment needs to fixed
  temporarily to:

    COSE_CounterSignature_Tagged = #TBD0(COSE_CounterSignature)

[ section 7 ]

* "of other than direct" -> "if other than direct"?

[ section 7.3 ]

* #2: "COSE_MAC" -> "COSE_Mac"?

[ section 9 ]

* "this new algorithms" -> "these new algorithms"

[ section 9.5.2 ]

* "the key from next layer down" -> "the key from the next layer down"
Murray Kucherawy
No Objection
Comment (2020-06-15 for -10) Sent
In the interests of brevity, my review focused on the diff between this document and RFC 8152.

Major points:

I agree with Benjamin's concern that we seem to be leaving the Designated Experts for the IANA registries described here with advice in RFC 8152, which this document renders obsolete.   I think that prose should be copied to this document so that it lives someplace current.  I was originally tempted to DISCUSS this but I'm hesitating, so I'll just say I hope this is resolved appropriately.

Some nits:

Section 1:
* "... structure for the CBOR objects which are ..." -- s/which/that/
* "... or offline protocols, different solutions ..." -- "different" should start a new sentence
* "Any application which uses COSE for security ..." -- s/which/that/

Section 5:
* The last sentence in the first paragraph:
OLD:
   It needs to be noted that the counter
   signature needs to be treated as a separate operation from the
   initial operation even if it is applied by the same user as is done
   in [I-D.ietf-core-groupcomm-bis].
NEW:
   It needs to be noted that the counter
   signature is to be treated as a separate operation from the
   initial operation, even if it is applied by the same user, as is done
   in [I-D.ietf-core-groupcomm-bis].
* "This is same structure ..." -- s/same/the same/
* "That is the counter signature ..." -- comma after "is"
* "... existence of the encrypted data is attested to." -- maybe "asserted" instead of "attested to"?

Section 9:
* "New algorithms will be created which will not fit ..." -- s/which/that/

Section 9.5.5:
* In the final bullet, why aren't these MUSTs?  Or in the alternative, when would one legitimately deviate from those SHOULDs?
Roman Danyliw
(was Discuss) No Objection
Comment (2020-09-08 for -13) Sent for earlier
Thanks for making an easy to read and compare bis document.

Thank you for addressing my DISCUSS and COMMENT items around Counter Signatures.
Éric Vyncke
No Objection
Comment (2020-06-11 for -10) Not sent
Thank you for the work put into this document. I really appreciate the section 1 with the detailed and clear introduction.

My current workload prevented me to do a deep review, I will then rely mainly on the other Area Directors.

-éric
Barry Leiba Former IESG member
Yes
Yes (for -10) Unknown

                            
Benjamin Kaduk Former IESG member
(was Discuss) Yes
Yes (2020-10-06 for -14) Sent
Mostly just nits left, though the lingering errata report and the IANA
updates seem somewhat significant.  (It would probably be fine to leave
the nits for the RFC Editor.)

It seems that https://www.rfc-editor.org/errata/eid5066 needs to be
incorporated still (and the report verified, presuming it is correct).

Section 1

   providing security services.  The use of COSE, like JOSE and CMS, is
   only for use in store and forward or offline protocols.  The use of

nit: we probably can just start the sentence with "COSE", since the "use in"
has moved to later in the sentence.

   additional data fields as part of the structure.  A one such
   application-specific element would be to include a URI or other

nit: s/A one/One/.

Section 1.5

   The presence a label that is neither a a text string or an integer,

nit: s/a a/a/

Section 2

   layer.  The content layer contains the plaintext is encrypted and
   information about the encrypted message.  The recipient layer contins

nit: s/plaintext is encrypted/encrypted plaintext/ (or similar?)

   the content encryption key (CEK) is encrypted and information about
   how it is encrypted for each recipient.  A single layer version of

nit: s/content encryption key (CEK) is encrypted/encrypted content encryption
key (CEK)/.

Section 4.1

   parameter.  An examples of a header parameter about the signature

nit: s/An examples/An example/

      Note: When a signature with a message recovery algorithm is used
      (Section 8.1), the maximum number of bytes that can be recovered
      is the length of the original payload.  The size of the encoded
      payload is reduced by the number of bytes that will be recovered.
      If all of the bytes of the original payload are consumed, then the
      transmitted payload is encoded as a zero-length byte string rather
      than as being absent.

I guess this is where the difficulty that Jim mentioned comes in -- the
signature recovery scheme gives you the intended recovered content but also
some of the other framing and such that was added as input to the signature
algorithm, and you have to properly figure out where the end of the actual
payload is.  (I'm not sure whether it would be useful to try to have text
about the issue in this location, though.)

Section 8.1

   consume the same bytes of message content.  This means that the
   mixing of the signature with message recovery and signature with
   appendix schemes in a single message is not supported, and if a
   recovery signature scheme is used.

nit: this sentence just trails off ....  I think it would be okay to just end
the sentence after "is not supported", since we covered the "must recover the
same [number of] bytes" previously.

   integrating message recovery signature algorithms.  The first
   algorithm defined is going to need to make decisions about these
   issues and those decisions re likely to be binding on any further
   algorithms defined.

nit: s/ re / are /

   Some of the issue2 that have already been identified are:

nit: s/issue2/issues/

Section 8.5.2, 8.5.3, 8.5.4, 8.5.5

I see that a bunch of things were changed from COSE_Encrypt to COSE_Recipient,
apparently prompted by my earlier review.  I would feel more comfortable if
someone confirmed that each change is correct, as I have lost some of the
context here.

Section 10

      comparable for the desired security functionality.  Additional
      guidence can be found in [BCP201].

nit: s/guidence/guidance/

Section 11

We lost the section about the "CBOR Tags" registry (where we were previously
allocating a codepoint for COSE_Signature).  I think we need to bring back a
stub section with the directive to update the references from 8152 to this
document.

[Noting here for convenience that as of the -12 of -algs, the Header Algorithm
Parameters registry has not been added as a target for reference updates; it
still needs to be listed there.]

Acknowledgments

nit: Göran's name doesn't seem to have made it through some step of the
process (eve in the native HTML output).
Alvaro Retana Former IESG member
No Objection
No Objection (2020-06-09 for -10) Sent
The references to the COSE registries should not be Normative.
Deborah Brungard Former IESG member
No Objection
No Objection (for -10) Not sent

                            
Magnus Westerlund Former IESG member
(was Discuss) No Objection
No Objection (2020-07-29 for -11) Sent
Thanks for addressing my issue with the IANA registration rules.
Robert Wilton Former IESG member
No Objection
No Objection (2020-06-10 for -10) Sent
I've only reviewed the diffs, not the historical approved text.  Everything looks okay to me.  A few minor comments/nits:

Comment:

1.4.  CBOR Grammar

  The CDDL grammar is informational; the prose description is normative.

I'm not familiar with the CDDL grammar, and specifically whether there is any tooling that can use the grammar to generate structures/etc.  If there is, then I think that it would be helpful if the CDDL grammar was also normative, in the sense that readers of the spec should be able to assume that the CDDL is correct.  I would still be okay with a statement that says that if there is any ambiguity between the two then the prose description should be taken as being definitive.
   
Nits:

1.5.  CBOR-Related Terminology
   
  The presence in a CBOR map of a label that is not a text string or an integer is an error.

This sentence was changed from the original formulation, but I find it slightly clunky.  Perhaps:

The presence of a label, that is neither a text string nor an integer, in a CBOR map, is an error.

9.  Taxonomy of Algorithms used by COSE

   In this section, a taxonomy of the different algorithm types that can
   be used in COSE is laid out.  This taxonomy should not be considered
   to be exhaustive.  New algorithms will be created which will not fit
   into this taxonomy.  If this occurs, then new documents addressing
   this new algorithms are going to be needed.
   
Nit, this -> these

Regards,
Rob