Subject: [COSE] AD review of draft-ietf-cose-rfc8152bis-struct-08 — THE REST OF THE REVIEW

From: Barry Leiba <[email-masked]>
Date: Thu, 14 May 2020 01:18:14 -0400

This review mostly includes comments about text that was NOT changed in
8152bis, and is all minor editorial things.

— Section 1 —

   CBOR was designed specifically to be both small in terms
   of messages transport and implementation size and be a schema-free
   decoder.

A couple of nits fixed here, “messages transport” and “be both ... and be”:

NEW
CBOR was designed specifically both to be small in terms of messages
transported and implementation size, and to be a schema-free decoder.
END

   The JOSE working group produced a set of documents [RFC7515]
   [RFC7516] [RFC7517] [RFC7518] using JSON that specified how to
   process encryption, signatures, and Message Authentication Code (MAC)
   operations and how to encode keys using JSON.

There’s a redundancy with “using JSON” twice in that sentence.  I would
remove the first instance and change the second to “, all using JSON.”

— Section 1.3 —

   *  Use binary encodings for binary data rather than base64url
      encodings.

As written, that says that the document uses binary encodings for binary
data, rather than using binary encodings for base64url encodings.

NEW
   *  Use binary encodings, rather than base64url encodings,
      to encode binary data.
END

— Section 1.5 —

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

I think you mean this:

NEW
   The presence in a CBOR map of a label that is not a text string or an
   integer is an error.
END

— Section 1.6 —

   context' header parameter defined in [RFC8613], or identified by a
   protocol specific identifier.

Nit: hyphenate “protocol-specific”.

   Context should generally be included
   in the cryptographic configuration, for more details see Section 4.3.

Comma splice: change the comma to a semicolon.

— Section 4.1 —

   An example of
   header a parameter about the content is the content type.  Examples
   of a header parameters about the signature would be

Nits: “of a header parameter” and “of header parameters”

— Section 6.1 —

   The same techniques and nearly the same structure is used for
   encrypting both the plaintext and the keys.

Nit: the subject is plural, so “are used”.  Or to avoid the awkward
“structure are used”, recast the sentence: “Both the plaintext and they
keys are encrypted using the same techniques and nearly the same structure.”

— Section 10 —
Same comment as in the algs doc:

   There has been an attempt to limit the number of places where the
   document needs to impose restrictions on how the CBOR Encoder needs
   to work.

Both instances of “needs to” seem odd.  For that matter, so does
“there has been an attempt”.  Maybe this, or something like it?:

NEW
This document limits the restrictions it imposes on how the CBOR
Encoder can work.
END

— Appendix A —

   The first set of
   recommendations apply to having an implicit algorithm identified for
   a single layer of a COSE object.  The second set of recommendations
   apply to having multiple implicit algorithms identified for multiple
   layers of a COSE object.  The third set of recommendations apply to
   having implicit algorithms for multiple COSE object constructs.

Nits: For all three sentences, “set of recommendations” is a singular
subject (“set” is singular), so it needs “applies”, not “apply”.  (And it’s
correct two paragraphs later.)

—
Barry

---

From: Jim Schaad <[email-masked]>
Date: Thu, 14 May 2020 16:47:19 -0700

I’ll post both at the same time

From: Barry Leiba <[email-masked]> 
Sent: Wednesday, May 13, 2020 10:18 PM
To: [email-masked]
Cc: [email-masked]
Subject: AD review of draft-ietf-cose-rfc8152bis-struct-08 — THE REST OF THE REVIEW

This review mostly includes comments about text that was NOT changed in 8152bis, and is all minor editorial things.

— Section 1 —

   decoder.

NEW

CBOR was designed specifically both to be small in terms of messages transported and implementation size, and to be a schema-free decoder.

END

[JLS] looks fine - done

There’s a redundancy with “using JSON” twice in that sentence.  I would remove the first instance and change the second to “, all using JSON.”

[JLS] Looks fine – the first one is now gone.

— Section 1.3 —

      encodings.

As written, that says that the document uses binary encodings for binary data, rather than using binary encodings for base64url encodings.

NEW

      to encode binary data.

END

[JLS] Reasonable - done

— Section 1.5 —

   integer is an error.

I think you mean this:

NEW

   integer is an error.

END

[JLS] Looks fine – done.

— Section 1.6 —

[JLS] I guess I need to learn rules on this - done

[JLS] done

— Section 4.1 —

   An example of

[JLS] done

— Section 6.1 —

Nit: the subject is plural, so “are used”.  Or to avoid the awkward “structure are used”, recast the sentence: “Both the plaintext and they keys are encrypted using the same techniques and nearly the same structure.”

[JLS] Just changed to plural.  I think it stresses what I want to stress better.

— Section 10 —

   to work.

NEW

Encoder can work.

END

[JLS] Yes that reads better - done

— Appendix A —

   The first set of

Nits: For all three sentences, “set of recommendations” is a singular subject (“set” is singular), so it needs “applies”, not “apply”.  (And it’s correct two paragraphs later.)

[JLS] that looks right – done.

— 

Barry