Subject: [Cbor] AD review of draft-ietf-cbor-file-magic-10

From: Francesca Palombini <[email-masked]>
Date: Fri, 1 Apr 2022 15:56:45 +0000

Thank you for this document. Here is my AD review.

The draft has a number of minor issues that can be fixed during IETF Last Call (however I suggest you do fix them asap so that reviewers won't point them out again). I also have a couple bigger questions re reasoning around motivation, context and doc structure (see 9. 15. 18. 19.), but they can be addressed with any additional Last Call comment, which I will start soon.

Thanks,
Francesca

1. -----

   servers derive the MIME-type information from file extensions.

FP: I'd suggest using the terminology "media type" rather than "MIME type" as this is not supposed to use the MacOS terminology. (see https://trac.ietf.org/trac/art/wiki/TypicalARTAreaIssues#MediaTypes)

2. -----

   It is noted that in the media type registration, that a magic number
   is asked for, if available, as is a file extension.

FP: the sentence does not parse (too many "that"?).

3. -----

   into the beginning of a CBOR format file.  Two possible methods of
   enveloping data are presented: a CBOR Protocol author will specify
   one.  (A CBOR Protocol is a specification which uses CBOR as its

FP: Suggestion to add that two methods are defined based on the data being an unique CBOR data item or a sequence of CBOR items (this becomes clear later in the text later on, but might be good to clarify for the reader at this point).

4. ----

   Examples of CBOR Protocols currently under development include CoSWID
   [I-D.ietf-sacm-coswid] and EAT [I-D.ietf-rats-eat].  COSE itself

FP: Please expand acronyms on first use (here and later - see https://www.rfc-editor.org/materials/abbrev.expansion.txt for the list of acronyms that don't need to be expanded, such as ASN.1). Also for later in the text, it would be good to add references when possible (for example, where DER or PKCS1 are mentioned).

5. -----

   A major inspiration for this document is observing the mess in
   certain ASN.1 based systems where most files are PEM encoded; these

FP: Can we replace "mess" with a less colloquial term such as "confusion"?

6. -----

   data item defined in [STD94].

   The term "diagnostic notation" refers to the human-readable notation
   for CBOR data items defined in Section 8 of [RFC8949] and Appendix G

FP: (Here and later on in the doc) Please reference consistently either STD94 or RFC8949 - just use one (and fix the reference accordingly).

7. -----

   16777216) to 0xFFFFFFFF (decimal 4294967295).  It is further
   suggested to avoid values that have an embedded zero byte in the four
   bytes of their binary representation (such as 0x12003456).

FP: Why is it suggested to avoid embedded zero byte?

8. -----

Section 2.1

FP: In this section I would also add a sentence saying that protocols registering this type of tag should also reference this specification (additionally to the registration template or any other spec they might have as reference).

9. -----

   Whether these two tags should be removed for specific further
   processing is a decision made by the CBOR Protocol specification.

FP: It is not very clear to me what the sentence adds: this does not help the implementer make that decision, since it does not give some guidance about what could be a reason for removing the tags for processing. It is a "you may do that if you so wish" without any additional information.

10. -----

Section 2.2.1.

FP: (Weak) suggestion to add an informative reference to senml+cbor Content Format IANA registration (so just to the IANA registry).

11. -----

      0x42_4F_52 ('BOR' in diagnostic notation).

FP: Could you avoid the underscores? I don't think they are necessary and they are not hex valid syntax (and same comments for other occurrences throughout the doc)

12. -----

3.  Advice to Protocol Developers

FP: Really nitpicking here :) But could we use "Protocol Authors", to be consistent with the terminology used elsewhere in the doc?

13. -----

   As an example of where there was a problem with previous security
   systems, "PEM" format certificate files grew to be able to contain
   multiple certificates by simple concatenation.  The PKCS1 format

FP: Can we add an informative reference if readers want to dive deeper into this?

14. -----

   then it may be easier to explain if the Labeled CBOR Sequence format
   is used.

FP: I am not sure I understand the meaning of the sentence.

15. -----

Section 3

FP: This is a very useful section, however I think I haven't understood from it the main advantage from using the tag wrapped vs the labeled CBOR sequence, except the fact that "programs need to be able to skip the first item of the sequence" in order to use the labeled CBOR sequence, which to me does not seem like such a huge requirement. Is that another way to say "programs that do not support CBOR sequences"? Then it would be good to explicitely state that, IMO.

16. -----

Section 5.1 and 5.2

FP: Please add that this is the CBOR Tags registry. Also please add a note to IANA that the existing registration should be updated to reference this document and update the fields as defined.

Additional comment: I think it is better to described the Data Item as "byte string" as for the current registration, rather than update to "tagged byte string" - that is not a CBOR Data Item per se.

17. -----

   Semantics:
      for each tag number NNNNNNNN, the representation of content-format
      (RFC7252) NNNNNNNN-1668546560

FP: I think this should get a reference to https://www.iana.org/assignments/core-parameters/core-parameters.xhtml#content-formats rather than RFC7252. I also wonder how this is going to look like in the registry, but I guess IANA will tell us.

Also it is a bit confusing to use "NNNNNNNN" and not just "N" (as I read it as a decimal, and 8xN doesn't really give me any more information about the number).

18. -----

Appendix A

FP: I have some vague memory of this being discussed during interims - so please remind me if I have forgotten, but why is this an appendix rather than in the document itself?

19. -----

Appendix C

FP: I think a bit more text around the motivation and need for such a tag would be useful.

20. -----

FP: Please update the reference: draft-ietf-core-new-block has been published as RFC 9177

---

From: Michael Richardson <[email-masked]>
Date: Sat, 02 Apr 2022 13:01:03 -0400

Carsten, I've assembled my changes into many small commits at:
         https://github.com/cbor-wg/cbor-magic-number/pull/19

So that everyone can see the changes.
I could add commit ids the email if that's helpful.

    > The draft has a number of minor issues that can be fixed during IETF
    > Last Call (however I suggest you do fix them asap so that reviewers
    > won't point them out again). I also have a couple bigger questions re
    > reasoning around motivation, context and doc structure (see
    > 9. 15. 18. 19.), but they can be addressed with any additional Last
    > Call comment, which I will start soon.

Working on this now.

    > FP: I'd suggest using the terminology "media type" rather than "MIME
    > type" as this is not supposed to use the MacOS terminology. (see
    > https://trac.ietf.org/trac/art/wiki/TypicalARTAreaIssues#MediaTypes)

okay.

    > FP: the sentence does not parse (too many "that"?).

fixed.

    > FP: Suggestion to add that two methods are defined based on the data
    > being an unique CBOR data item or a sequence of CBOR items (this
    > becomes clear later in the text later on, but might be good to clarify
    > for the reader at this point).

I've changed "unique" to "single"

    > FP: Please expand acronyms on first use (here and later - see
    > https://www.rfc-editor.org/materials/abbrev.expansion.txt for the list
    > of acronyms that don't need to be expanded, such as ASN.1). Also for
    > later in the text, it would be good to add references when possible
    > (for example, where DER or PKCS1 are mentioned).

I've expanded DER, but it's really awkward.
Expanding PKCS1 would actually hamper understanding, I think.

    > FP: Can we replace "mess" with a less colloquial term such as

okay.
(cf: https://en.wikipedia.org/wiki/Poutine
"ça va faire une maudite poutine!" (English: "It will make a damn mess!") )
[take what you will on the current Ukrainian situation...]

    > FP: (Here and later on in the doc) Please reference consistently either
    > STD94 or RFC8949 - just use one (and fix the reference accordingly).

fixed.

    > FP: Why is it suggested to avoid embedded zero byte?

added:
   This suggestion avoids potential issues with null terminated strings in C.

There ought not be any issues, but the tags could be processed by non-CBOR
aware systems.

    > FP: In this section I would also add a sentence saying that protocols
    > registering this type of tag should also reference this specification
    > (additionally to the registration template or any other spec they might
    > have as reference).

added: "When making the IANA request, a reference to this specification will
       help the reviewer to understand why the specific tag value and size is being
       requested. "

Is this what you had in mind?

    > FP: It is not very clear to me what the sentence adds: this does not
    > help the implementer make that decision, since it does not give some
    > guidance about what could be a reason for removing the tags for
    > processing. It is a "you may do that if you so wish" without any
    > additional information.

"Whenever bytes transmitted over the wire are at a premium, the tags should
be removed."

(Not SHOULD, this document is not normative)

    > FP: (Weak) suggestion to add an informative reference to senml+cbor
    > Content Format IANA registration (so just to the IANA registry).

I don't think the reference will be useful.  It's an example.

    > FP: Could you avoid the underscores? I don't think they are necessary
    > and they are not hex valid syntax (and same comments for other
    > occurrences throughout the doc)

There are a number of places where underscore is allowed in hex literals, it
acts like a comma to aid in readability.    It appears to originate in
Verilog.
Carsten?

    > FP: Really nitpicking here :) But could we use "Protocol Authors", to
    > be consistent with the terminology used elsewhere in the doc?

Okay, but, "Author" does not appear any other place.

    > FP: Can we add an informative reference if readers want to dive deeper into this?

Well, not really.  It would be contained in dozens of emails and
stackoverflow discussions where people try to do some with DER encoded
things, fail and then try with PEM encoded, and succeed.
But, none of the specifications say either is allowed, it's just that
software was made to be more tolerant in one case and not another.
It would probably take me a take to find a useful, consise thing to reference.

    > FP: I am not sure I understand the meaning of the sentence.

How about:

## Are there tags at the start?

-If the Protocol expects to use other tags values at the top-level, then it may be easier to explain if the Labeled CBOR Sequence format is used.
+If the Protocol expects to use other tags values at the top-level, then the use
+of the tag wrapped format may be easier to explain in the protocol description.

    > FP: This is a very useful section, however I think I haven't understood
    > from it the main advantage from using the tag wrapped vs the labeled
    > CBOR sequence, except the fact that "programs need to be able to skip
    > the first item of the sequence" in order to use the labeled CBOR
    > sequence, which to me does not seem like such a huge requirement. Is
    > that another way to say "programs that do not support CBOR sequences"?
    > Then it would be good to explicitely state that, IMO.

I'm rewording that section.

    > FP: Please add that this is the CBOR Tags registry. Also please add a
    > note to IANA that the existing registration should be updated to
    > reference this document and update the fields as defined.

done.

    > Additional comment: I think it is better to described the Data Item as
    > "byte string" as for the current registration, rather than update to
    > "tagged byte string" - that is not a CBOR Data Item per se.

Carsten was very specific about this.

    > FP: I think this should get a reference to
    > https://www.iana.org/assignments/core-parameters/core-parameters.xhtml#content-formats
    > rather than RFC7252. I also wonder how this is going to look like in
    > the registry, but I guess IANA will tell us.

okay.

    > Also it is a bit confusing to use "NNNNNNNN" and not just "N" (as I
    > read it as a decimal, and 8xN doesn't really give me any more
    > information about the number).

I agree, the many Ns are intended to imply it's a big number, I think.

    > FP: I have some vague memory of this being discussed during interims -
    > so please remind me if I have forgotten, but why is this an appendix
    > rather than in the document itself?

Because the document is about tagging CBOR protocols, and this secttion is
about tagging Non-CBOR Protocols.

    > FP: Please update the reference: draft-ietf-core-new-block has been published as RFC 9177

It happened so fast, it seems.

--
Michael Richardson <[email-masked]>   . o O ( IPv6 IøT consulting )
           Sandelman Software Works Inc, Ottawa and Worldwide

---

From: Carsten Bormann <[email-masked]>
Date: Sat, 2 Apr 2022 19:45:45 +0200

Hi Francesca,

thank you for this detailed review.

I have created a pull request with changes for the items that I think we could resolve:

https://github.com/cbor-wg/cbor-magic-number/pull/20

A few responses below, with a few questions embedded.

I haven’t done 14 to 19 yet.

Next up will be coordinating with Michael, who apparently didn’t get my messages.

Grüße, Carsten

> FP: Please expand acronyms on first use (here and later - see https://www.rfc-editor.org/materials/abbrev.expansion.txt for the list of acronyms that don't need to be expanded, such as ASN.1). Also…

One interesting case is PEM, where I cannot find a useful reference (PEM is really the industry term for “multi-line base64 in an envelope that has BEGIN/END and a fuzzy format/purpose indicator”).  It is also an abbreviation that according to the RFC editor needs to be expanded, but it does not mean “Privacy-enhanced mail” in the context of base64-encoded certificates any more.

> FP: Can we replace "mess" with a less colloquial term such as "confusion"?

My dictionary suggested “disarray” :-)
 
> FP: (Here and later on in the doc) Please reference consistently either STD94 or RFC8949 - just use one (and fix the reference accordingly).

There is some brokenness about referencing Internet Standards in xml2rfc; I hope the workaround I found survives the RFC editor.
 
> FP: Why is it suggested to avoid embedded zero byte?

Some broken implementations use (zero-byte terminated) C strings for magic numbers, so avoiding zero bytes can avoid some pain.

➔
 It is further suggested to avoid values that have an embedded zero byte in
-the four bytes of their binary representation (such as 0x12003456).
+the four bytes of their binary representation (such as 0x12003456), as
+these may confuse implementations that treat the magic number as a C string.

> FP: It is not very clear to me what the sentence adds: this does not help the implementer make that decision, since it does not give some guidance about what could be a reason for removing the tags…

I came up with this replacement:

A CBOR Protocol specification may specify the use of two tags only for
specific cases.  For instance, it might use them when storing the
representation in a local file or for Web access, but not when using them
in protocol messages that already provide the necessary context.
 
> FP: (Weak) suggestion to add an informative reference to senml+cbor Content Format IANA registration (so just to the IANA registry).

Right now we don’t have a way to point to specific registrations (I have suggested that to IANA earlier, but it will take some time until procedures are adjusted and it can be decided whether and how to enable that).
So this reference now points to the entire (sub-)registry.
 
> FP: Could you avoid the underscores? I don't think they are necessary and they are not hex valid syntax (and same comments for other occurrences throughout the doc)

They are certainly not necessary.
Since Ada started this convention in 1979, in many languages (e.g, Perl, Ruby, Rust, Swift, recently Java 7+, Python 3.6, but notably not C++), underscores are part of the valid syntax for numbers:
>> 0x42_4F_52
=> 4345682
>> 0x42_4F_52.to_s(16)
=> "424f52"
I think this is more of a matter of taste.

(So, weak pushback, but if others agree with the comment, the underscores are of course removed quickly.  If only we had thin spaces in RFCs...)

> FP: Really nitpicking here :) But could we use "Protocol Authors", to be consistent with the terminology used elsewhere in the doc?

Actually, "protocol designers" is the most used term in the current text.
We normalized to that now.
 
> FP: Can we add an informative reference if readers want to dive deeper into this?

If you mean informative reference to PEM, see above.
If you mean an informative reference to the, er, disarray reported here, there are lots of stackoverflow entries with people confused by this (and by the openssl conversion routines that silently drop all but the first cert).
We certainly can go searching for a better citation than those, but I don’t know whether there actually is a specification we could point to.

TODO section follows.

---

From: Michael Richardson <[email-masked]>
Date: Sun, 03 Apr 2022 15:41:47 -0400

> I haven’t done 14 to 19 yet.

I perhaps finished all the points on top of Carsten's branch.

    > Next up will be coordinating with Michael, who apparently didn’t get my messages.

Yeah, my desktop Telegram disconnected, and don't walk around the house with
my phone.
Interesting to think about the comments independantly of each other and then merge.

---

From: Carsten Bormann <[email-masked]>
Date: Sun, 3 Apr 2022 22:01:39 +0200

> I perhaps finished all the points on top of Carsten's branch.

Thank you!

I see two comments from Francesca we haven’t replied to and two more we haven’t addressed yet:

> […]

Well, 55800 and 55801 are meant to have a tagged byte string as their content:
The tag is the 32-bit tag that actually defines the format.

So it’s 55800(1330664270(h'424F52’)) (diagnostic notation in Appendix B) or
#6.55801(#6.1668546992('BOR’)) (Appendix C.1, this time in CDDL form).

> […]

I think NNNNNNNN is a reasonable representation of a larger number, but I’m also not opposed to changing this to N (twice).

> FP: I have some vague memory of this being discussed during interims - so please remind me if I have forgotten, but why is this an appendix rather than in the document itself?

We might provide a little more text about moving this into an appendix.

> FP: I think a bit more text around the motivation and need for such a tag would be useful.

Yes, that also could use a bit more work.

Despite 18 and 19 not yet being addressed, I would prefer to submit a -11 with what we have now so people actually review that and not -10.

Grüße, Carsten

---

From: Michael Richardson <[email-masked]>
Date: Mon, 04 Apr 2022 10:03:34 -0400

> I think NNNNNNNN is a reasonable representation of a larger number, but
    > I’m also not opposed to changing this to N (twice).

I replied in the email that I didn't think we needed to change here.

    > We might provide a little more text about moving this into an appendix.

I will look again, I thought we already did that.

    > Despite 18 and 19 not yet being addressed, I would prefer to submit a
    > -11 with what we have now so people actually review that and not -10.

published.

---

From: Francesca Palombini <[email-masked]>
Date: Tue, 12 Apr 2022 14:09:33 +0000

Michael, Carsten,

Thanks for the changes, they look good. I will schedule the document for the next telechat (April 21st) – additional text for the appendices would be welcome, you can maybe see if any can be added together with any changes from Last Call reviews that should be arriving this week.

Francesca

On 04/04/2022, 16:04, "Michael Richardson" <[email-masked]> wrote:

published.

--