Subject: [Cellar] AD Evaluation of draft-ietf-cellar-ebml-08.

From: Ben Campbell <[email-masked]>
Date: Mon, 7 Jan 2019 15:14:58 -0600

Hi,

This is my AD Evaluation of draft-ietf-cellar-ebml-08.

Thanks for this work. It's generally on the right track, but I have a number of comments that I would like to discuss prior progressing.

Thanks!

Ben.

*** Substantive Comments ***

- The Shepherd Report says "There are minor nits to be resolved (adding one RFC 2119 keyword and some <element>schema  attributes which are mentioned but not defined."

Do that comment still apply? To be clear, those are more than nits. Adding a 2119 keyword is substantive change that needs to be agreed to but the working group. Undefined attributes suggest that the draft is incomplete. If the comment is still accurate, then these need to be fixed prior to progressing the draft. If it is not accurate, then I ask the shepherd to please update the shepherd report.

- I'm a little surprised to see the draft describe EBML as a general purpose markup language format, as opposed to one designed to be used by Matroska and similar technologies. I suspect others will also be surprised during the IETF LC and IESG review. The review bar is higher for the IETF to recommend EBML as a general purpose markup language than it is to recommend it for specific purposes. I also note that the CELLAR charter talks about EBML in terms of being used by Matroska and FFV1.

Would it be acceptable to add some scoping language to the effect of "EBML is used by Matroska[citation] and FFV1[citation]. It MAY be used for use cases similar to those. The applicability of EBML for other use cases is beyond the scope of this document".

- There are several defined elements/attributes related to versioning, but I don't find an explanation of how they all work together. Please add a section describing versioning in general.

§2: Please use the newer boilerplate from RFC 8174.

§3: Can you offer guidance on the specific impacts of the mentioned attacks, and how to mitigate them? (If there is no way to mitigate an attack, it's okay to say that.)

§5: Please elaborate on how this is similar to UTF8. I assume this refers to using prefix bits to indicate the field length. UTF8 does that to maintain backwards compatibility with Ascii. What is the design motivation for using that approach for EBML, which I presume doesn't have such a requirement?

§5.3: "If the number of bits required for "VINT_DATA"
are less than the bit size of "VINT_DATA", then "VINT_DATA" SHOULD be
zero-padded to the left to a size that fits."

Why not MUST? What happens if this is violated?

§6:
- "The "VINT_DATA" component of the "Element ID" MUST
NOT be either defined or written as either all zero values or all one
values."
Why?

§8.7: "In this case, the "EBML Reader" should skip data
until a valid..."
Should that be a normative statement? If so, why should and not MUST?

§8.8: If the EBML reader does not interpret Binary Elements, why add them at all? What does interpret them? is the point that the EBML reader treats Binary Elements as opaque, and just hands them to the application?

§10.1.3: "NOT RECOMMENDED" seems overly strong, especially in light of the MAY in the first paragraph. Is the point to say "MUST NOT... except when the implementation needs to update an element without rewriting the entire document"?

§11: "An "EBML Document" consists of "EBML Elements" and MUST
NOT contain any data that is not part of an "EBML Element"."

That contradicts text in §8.7 that says you can do this in streaming use cases.

§13.1: "The "DocType"
value for an "EBML Document Type" SHOULD be unique and persistent."

What is the scope of uniqueness? How should it be achieved? Is there an expectation to register Document Types?  (Also, why not MUST)?

§13.1.4.1: Are there uniqueness requirements for name?

§13.1.4.2:
- The idea behind "path" needs elaboration. I'd like to see some high-level discussion of how you indicate where an element is allowed, and how you encode the structure. An example (local to the section) would be helpful.

§13.1.4.4:
- "The "minOccurs" value MUST be
equal to the "EBMLMinOccurrence" value of the "path"."

Why have both if they have to be the same? (Same question for §13.1.4.5)

§13.1.4.12:
- "If the "recurring" attribute
is not present then the "EBML Element" MUST be considered to NOT be
an "Identically Recurring Element"."

"be considered to" is vague for a MUST. What concrete action must implementations take in this case?

§13.1.6.2:
- It seems likely readers will confuse "type" with "data type". Would it be reasonable to call this "document-type" or something similar?

§13.1.10:
- Please state (and cite) which XML schema format is used here.

- Has the schema been mechanically verified?

§13.3.1:
- "The CRC value MUST be computed on a little
endian bitstream and MUST use little endian storage."

Please elaborate. Most elements have been big-endian so far. Do there have to be converted to calculate the CRC?

§15.1:
- "The numbers 0x3FFF and 0x4000 are RESERVED.", "The numbers 0x1FFFFF and 0x200000 are RESERVED.", and "The numbers 0xFFFFFFF and 0x1000000 are RESERVED."
What is the purpose of reserving them?

- "Four octet Element IDs whose lower three octets (as
encoded) would make printable 7-bit ASCII values may be allocated
only Specification Required."

Can you state a range or a list? Don't make IANA judge which values are printable.

§15.2:
- "The strings may be allocated according to
First Come First Served"

Is there a requirement to register them, or is this optional?

*** Editorial Comments and Nits ***

- General: The draft puts double quotes around every mention of terms that it defines. That is unconventional for IETF documents. I personally find that it makes the draft harder to read. I suggest quoting them on the first mention, then just capitalizing them in subsequent mentions.

- General (style comment): Please consider removing most instances of the word "considered". When the text says "X is considered to be equal to Y", that sounds like a weaker (and harder to read) statement than "X is equal to Y". Along the same lines, "considered" is rarely appropriate to use in a normative statement. For example, instead of saying "Implementations MUST consider X to be an error", it is better to state what concrete actions you expect the implementation to take when X occurs.

§3:

- The Security Considerations are required to be one of the last two sections in the body of the document (that is, before the references). More precisely, Security Considerations and IANA considerations should be the last two sections in the documents.

- first paragraph: Please expand CRC on first mention. (Which may or may not be this instance once the Security Considerations are moved to the proper place.)

§5.2: "The "VINT_MARKER" MUST be one bit in length and
contain a bit with a value of one."
That's really more of a definition or statement of fact than a normative requirement. Please consider replacing MUST with "is".

§5.3:
- "The bits required for the "VINT_WIDTH" and the
"VINT_MARKER" combined use one out of eight bits"
Consider "use one out of every eight bits"

§5.4, paragraph after first table: "Data encoded as a "Variable Size Integer" MAY be rendered at octet
lengths larger than needed to store the data."
Is that intended as permission, or a statement of fact? If the latter, then the normative keyword is not appropriate.

§6:
- "The "Element ID" MUST be encoded as a "Variable Size Integer"."
That seems like a definition. Consider s/MUST/is
- "MUST NOT be considered an error in the "EBML Document"."
"be considered" is vague for a normative requirement. What concrete action is being prohibited (or required)?

§7:
- "The "Element Data Size" itself MUST be encoded as a "Variable
Size Integer"."
Consider s/MUST/is

- 'Only "Master Elements" SHALL be "Unknown-Sized Elements".'
The use of "only" in normative statements can be ambiguous. In this case it can be read that non-master elements MUST NOT be of unknown size, or that the SHALL only applies to master elements, and there is no rule for non-master elements. Please consider formulating this as "MUST NOT".

- "For example, an "EBML Element" with an octet length of 127
MUST NOT be encoded in an "Element Data Size" encoding with a one
octet length."

Examples are not normative, therefore they should not include normative keywords.

§8.1: "Signed Integers MUST be stored with two’s
complement notation"
Please consider s/MUST/are

§10.1.2: "This method is only RECOMMENDED for reducing "Element Data" by a
single octet..."
Please avoid using "only" in normative requirements (see previous comment for explanation).

§11.1
- 'that uses an"EBMLVersion" of "1" MUST only contain "EBML Elements"'
Please avoid "only" on normative statements.

- last paragraph: Please consider dropping "All" from the beginning of each sentence.

§11.1:
- Is the concept of a file concrete (in the sense of a unit of persistent storage) or abstract (as in any stream of data)? I had assumed the latter, but with the different rules about data outside of EBML elements for "files" and "streaming applications", I am not so sure. If you mean "file" concretely, does there need to be additional text in this section describing the structure for streaming applications?

- "The "EBML Body" MUST consist
only of "EBML Elements""
Please avoid "only" on normative statements.
- 'what "EBML Elements"' (several occurances). s/what/which

§13.1:
- "one or many" (several times throughout draft): Should this be "one or more"? Some people do not tend to think of numbers like 2 or 3 as counting as "many".

- "An "EBML Schema" must be
expressed as well-formed XML."
Normative?

- "An "EBML Schema" MUST declare exactly one "EBML Element" at "Root
Level" (referred to as the "Root Element") that MUST occur exactly
once within an "EBML Document"."

The second "MUST" seems like a statement of fact.

- If an "EBML Schema"
adopts the "EBML Header Element" as-is, then it is not REQUIRED to
document that Element within the "EBML Schema"."

"REQUIRED" is not used normatively, and therefore should not be capitalized.

§13.1.1.2:
- "The "version" lists an incremental non-negative integer..."

I'm not sure what it means for a single integer to be "incremental". (But see substantive comment concerning versioning.)

§13.4.1.2:
- The ""*"", ""("" and "")"" symbols MUST be interpreted as they are
defined in the ABNF."
This _is_ the ABNF. Do you mean "as defined in RFC 5234?  Also, the normative MUST is not appropriate here; it's a matter of definition, not a normative requirement.

- "The "EBMLElementOccurrence" part is interpreted as an ABNF Variable
Repetition.

I don't understand what that means. There won't be ABNF in an actual schema, will there? (Same for "VariableParentOccurrence")

- ""EBML Elements" with "EBMLMinOccurrence" set to "1" that also have a
"default" value (see Section 13.1.4.8) declared are not REQUIRED to
be stored but are REQUIRED to be interpreted,"

Statement of fact?

§13.1.4.7
- The attribute is called "size" but defined to be "length". Why not call it length? (Or describe it as "size")?

§13.1.4.10:
- "A boolean to express if an "EBML Element" MAY be used as an "Unknown-
Sized Element""
Statement of fact.

§13.1.11: It would be helpful to have the example closer to the element descriptions. I suggesting putting this section before the schema section.

§13.1.13: Why is the material is this section separate from the range element description?

§13.1.14:
- "When a float value is represented textually in an "EBML Schema", such
as within a "default" or "range" value,..."

The examples only talk about ranges. Can there be an example for a default value?

§13.1.15:
- "In this case, the default value of the "Mandatory EBML
Element" MUST be interpreted by the "EBML Reader" although the "EBML
Element" is not present within its "Parent Element"."

I'm not sure the intent here. Is "interpreted" the correct word choice?

§13.3.2:
- "Used to void damaged data, to avoid unexpected behaviors
when using damaged data."

Comma splice.

§14
- "If a "Master Element" contains a "CRC-32 Element" that doesn’t
validate, then the "EBML Reader" MAY ignore all contained data except
for "Descendant Elements" which contain their own valid "CRC-32
Element"."

s/ ", which" / " that"

§15.1:
- "Values from 1 to 126 are to
be allocated according to RFC Required."

I suggest '... allocated according to the "RFC Required" policy.' (Note: This pattern occurs several times in the RFC considerations section.) Also, please cite 8126 on the first mention of each policy (or state in advance that all registration policies mentioned herein are defined in 8126).

- "Numbers MAY be allocated within this range according to
Specification Required."
s/MAY/are

---

From: Ben Campbell <[email-masked]>
Date: Mon, 14 Jan 2019 17:43:02 -0600

Hi,

Any thoughts on my AD evaluation review?

I know there’s a lot of comments. Please don’t take that as critical; it’s not at all unusual for the Area Director review to generate a lot of comments :-)

Thanks!

Ben.

> On Jan 7, 2019, at 3:14 PM, Ben Campbell <[email-masked]> wrote:
> 
> 
> Hi,
>

---

From: Dave Rice <[email-masked]>
Date: Mon, 14 Jan 2019 19:13:01 -0500

Hi Ben,
I did start dissecting the comments into issues for discussions and edits for what is straightforward so do have a draft response in the works. I’ll get that out tomorrow even though it will be a partial response. Thanks for your work here.
Dave

> On Jan 14, 2019, at 18:43, Ben Campbell <[email-masked]> wrote:
> 
> Hi,
> 
> _______________________________________________
> Cellar mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/cellar

---

From: Ben Campbell <[email-masked]>
Date: Mon, 14 Jan 2019 18:23:46 -0600

Excellent, thanks!

Ben.

> On Jan 14, 2019, at 6:13 PM, Dave Rice <[email-masked]> wrote:
> 
> Hi Ben,

---

From: Moritz Bunkus <[email-masked]>
Date: Tue, 15 Jan 2019 09:13:47 +0100

Hey,

thanks for your work, Ben. I intended to work on the points & a thorough
reply but was a bit overwhelmed by the amount of them (not so much by what
you wrote as most points you brought up seemed uncontroversial and easy to
fix for me) and short on time. It's still on my TODO list.

Kind regards,
mosu

---

From: Steve Lhomme <[email-masked]>
Date: Thu, 17 Jan 2019 16:05:47 +0100

Same for me. I'll try to get some time during the weekend.

---

From: Steve Lhomme <[email-masked]>
Date: Sat, 19 Jan 2019 10:30:05 +0100

Hi Ben,

On 07/01/2019 22:14, Ben Campbell wrote:
> Do that comment still apply? To be clear, those are more than nits. Adding a 2119 keyword is substantive change that needs to be agreed to but the working group.

I don't understand this. RFC2119 has been mentioned in the header and a 
link is generated at the bottom of the document since 2016. I don't 
think we should include the whole document, so what is missing ?

>   Undefined attributes suggest that the draft is incomplete. If the comment is still accurate, then these need to be fixed prior to progressing the draft. If it is not accurate, then I ask the shepherd to please update the shepherd report.

Which attributes are mentioned but not defined ? All the "attribute" 
mentions I see in the text are all attributes described further in their 
own section.

> - I'm a little surprised to see the draft describe EBML as a general purpose markup language format, as opposed to one designed to be used by Matroska and similar technologies. I suspect others wil…

It's actually not used in FFV1, only Matroska. But we are aware of other 
people using EBML for other purposes/formats and some considering to use it.

> Would it be acceptable to add some scoping language to the effect of "EBML is used by Matroska[citation] and FFV1[citation]. It MAY be used for use cases similar to those. The applicability of EBML…

Yes, (apart from the FFV1 part).
Done in this PR https://github.com/Matroska-Org/ebml-specification/pull/199

> - There are several defined elements/attributes related to versioning, but I don't find an explanation of how they all work together. Please add a section describing versioning in general.

Done in PR https://github.com/Matroska-Org/ebml-specification/pull/200

> §2: Please use the newer boilerplate from RFC 8174.

Done in PR https://github.com/Matroska-Org/ebml-specification/pull/201

> §3: Can you offer guidance on the specific impacts of the mentioned attacks, and how to mitigate them? (If there is no way to mitigate an attack, it's okay to say that.)

Partially covered by 
https://github.com/Matroska-Org/ebml-specification/pull/202
I'm not sure how to mitigate side channels errors, or rather if we have 
to go in deep details for each (it could go very deep on all kinds of 
security things).
> §5: Please elaborate on how this is similar to UTF8. I assume this refers to using prefix bits to indicate the field length. UTF8 does that to maintain backwards compatibility with Ascii. What is t…

Done in https://github.com/Matroska-Org/ebml-specification/pull/203

> Why not MUST? What happens if this is violated?

Done in https://github.com/Matroska-Org/ebml-specification/pull/204

I'll try to continue with the rest ASAP

Thanks a lot for all your comments.

---

From: Steve Lhomme <[email-masked]>
Date: Sun, 27 Jan 2019 16:31:17 +0100

Hi,Some more comments on the comments:

> NOT be either defined or written as either all zero values or all one

There is no technical reason why these values can't be used. For
example VINT_DATA all set to 0 is valid for the Element Size (and all
ones has a special meaning). These are rather reserved values that may
help expand EBML one day if necessary. Should we call them reserved
rather than invalid ?

> Should that be a normative statement? If so, why should and not MUST?

It can't be a MUST if we tolerate values to be there, someone might
want to make use of these data. It could also be data that have been
damaged and don't appear as EBML Elements anymore. In that case the
reader should do its best to skip over these data. In any case the
behaviour of the reader when such data is found not normative.

> §8.8: If the EBML reader does not interpret Binary Elements, why add them at all? What does interpret them? is the point that the EBML reader treats Binary Elements as opaque, and just hands them t…

Yes, it's exactly that. Binary is a blob that is unknown to the EBML
Reader. Only an app that knows the particular Element in the DocType
can interpret what is in this blob.

> §10.1.3: "NOT RECOMMENDED" seems overly strong, especially in light of the MAY in the first paragraph. Is the point to say "MUST NOT... except when the implementation needs to update an element wit…

The MAY means zeros can be added after a string, but one shouldn't do
that.The NOT RECOMMENDED also says that it shouldn't be used unless
you know what you're doing (the definition of NOT RECOMMENDED).Should
we rephrase the paragraph ? Saying that use Null terminated should not
be used in general but must be handled by a Reader because it's a
possibility ?

>> That contradicts text in §8.7 that says you can do this in streaming use cases.

Correct, it should be SHOULD NOT rather than MUST NOT. See Pull
Request #212https://github.com/Matroska-Org/ebml-specification/pull/212

>> What is the scope of uniqueness? How should it be achieved? Is there an expectation to register Document Types?  (Also, why not MUST)?

There is a IANA registry and it MUST be unique (see 15.2 CELLAR EBML
DocType Registry), although if people use the same separately string
separately without going through IANA registration, it's possible they
won't be unique. That being said I think it should be a MUST.

See Pull Request #213https://github.com/Matroska-Org/ebml-specification/pull/213

> §13.1.4.1: Are there uniqueness requirements for name?

No, it's a human readable name that's only there for readability. I
never ends up in the actual EBML data. The same name could be found in
different path for example.

> §13.1.4.2:
> - The idea behind "path" needs elaboration. I'd like to see some high-level discussion of how you indicate where an element is allowed, and how you encode the structure.

I don't understand what you mean.

> An example (local to the section) would be helpful.

Added in Pull Request #214
https://github.com/Matroska-Org/ebml-specification/pull/214

EBMLMinOccurrence is just a coded name in the path definition. It's
not found in the path itself. Whereas minOccurs is found in the XML
Schema describing the format.The former has a long name to make it
clearer what it does in this context. The latter has a short name to
be more readable and is always found in its context anyway.

Fixed by https://github.com/Matroska-Org/ebml-specification/commit/45497f5ed9d9d4f0fe206c69431fd0f39be3dbd4

I renamed it to "purpose" in Pull Request #215
https://github.com/Matroska-Org/ebml-specification/pull/215

>> - Has the schema been mechanically verified?

I'm not competent on that one (I suppose Dave or Jerome have used it ?)

>> Please elaborate. Most elements have been big-endian so far. Do there have to be converted to calculate the CRC?

The EBML headers and integers are in Big Endian. But the Binary data
can be anything. For convenience with modern CPUs the CRC-32 is done
on little endian reading of the EBML header+data and stored as such.I
can't access the ISO spec, but the ITU one doesn't mention anything
about endianess.

> What is the purpose of reserving them?

The answer can be found here
https://github.com/Matroska-Org/ebml-specification/pull/179#discussion_r196481366
There are invalid values, but that's not a word used for IANA
registration. Should we mention that they are invalid anyway ?

> Can you state a range or a list? Don't make IANA judge which values are printable.

Done in Pull Request 216
https://github.com/Matroska-Org/ebml-specification/pull/216

> Is there a requirement to register them, or is this optional?

We can't force people to do it, but it's encouraged.

---

From: Steve Lhomme <[email-masked]>
Date: Sun, 27 Jan 2019 16:48:50 +0100

More:

> - General: The draft puts double quotes around every mention of terms that it defines. That is unconventional for IETF documents. I personally find that it makes the draft harder to read. I suggest…

I suppose you don't mean full capital letters ? The idea is that if
you read a particular section and not the whole document you can still
understand what is internal terms or generic names.

> - Is the concept of a file concrete (in the sense of a unit of persistent storage) or abstract (as in any stream of data)? I had assumed the latter, but with the different rules about data outside…

It's more of an abstract thing. When we mention streaming it's some
kind of reading where you can't seek forward (and only backward is
there's a cache). But in general the storage itself doesn't matter.

The path is in the Schema and is interpreted as ABNF

> be stored but are REQUIRED to be interpreted,"

It is normative, if that's what you mean. Even if such element is not
found in the data, it must be considered to be there and used with the
default value.

> - The attribute is called "size" but defined to be "length". Why not call it length? (Or describe it as "size")?

That must be for historical reasons. I'm fine with renaming it to
length since that's the term we use everywhere else.
See Pull Request #217
https://github.com/Matroska-Org/ebml-specification/pull/217