Subject: [COSE] Benjamin Kaduk's Yes on draft-ietf-cose-rfc8152bis-struct-14: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 06 Oct 2020 22:33:55 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-cose-rfc8152bis-struct-14: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-cose-rfc8152bis-struct/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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).

---

From: Carsten Bormann <[email-masked]>
Date: Wed, 7 Oct 2020 15:14:16 +0200

> incorporated still (and the report verified, presuming it is correct).

COSE_KDF_Context is defined in -algs, not -struct.

-algs has Section 9, which implements the addition intended by the errata.
(-struct has its own Section 9, covering the other structures).

So it seems the errata report is applied in -algs.
It could also be verified for RFC 8152.

Grüße, Carsten

---

From: Matthew Miller <[email-masked]>
Date: Wed, 7 Oct 2020 09:52:26 -0600

> On Oct 7, 2020, at 7:14 AM, Carsten Bormann <[email-masked]> wrote:
> 

Carsten's interpretation of this errata looks correct to me: -rfc1852bis-algs defines the COSE_KDF_Context struct and § 9 discusses the encoding restrictions for that structure.

This erratum should be marked verified.

- m&m
Matthew A. Miller

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 7 Oct 2020 09:10:14 -0700

> This erratum should be marked verified.

Thanks for confirming, and thanks Carsten for pointing out that it went to
-algs.

I will go ahead and mark it as verified.

I did take a look at the respective parts of -algs and -struct, which shows
a bit of skew:

-Encoding MUST be done using definite lengths and values MUST be the
minimum possible length. This means that the integer 1 is encoded as "0x01"
and not "0x1801".
+Encoding MUST be done using definite lengths and the length of the MUST be
the minimum possible length. This means that the integer 1 is encoded as
"0x01" and not "0x1801".

("values" vs "the length of the")
I think that -algs should be adjusted to match -struct.

-Ben

---

From: Carsten Bormann <[email-masked]>
Date: Mon, 19 Oct 2020 23:31:11 +0200

One terribly niggly nit:
The examples in C.3.3 and C.5.4 say “ECHD” in a comment where they need to say “ECDH”.

(This is also a trivial errata report on the same appendixes in RFC 8152 that I haven’t written up yet.)

Grüße, Carsten