Re: [Cellar] Roman Danyliw's Discuss on draft-ietf-cellar-matroska-16: (with DISCUSS and COMMENT)

[Steve Lhomme <[e-mail]>]

Hi Roman,

Thanks for the lengthy review. Comments below:

> On 23 May 2023, at 03:39, Roman Danyliw via Datatracker <[e-mail]> wrote:
> 
> Roman Danyliw has entered the following ballot position for
> draft-ietf-cellar-matroska-16: Discuss
> 
> 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/about/groups/iesg/statements/handling-ballot-positions/ 
> for more information about how to handle DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-cellar-matroska/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> ** Section 5.1.4.1.31.9.  Table 26.  ContentEncAlgo Element.  Several insecure
> algorithms are suggested as options (e.g., DES, 3DES).  Please add guidance
> that:
> 
> -- Mastroska Writers MUST NOT use insecure ciphers (e.g., DES, 3DES) to create
> "new content" (not sure of the appropriate word) either in this section or in
> the Security Considerations
> 
> -- Add cautionary text that these algorithms are not secure
> 
> ** Section 14
> 
> ==[ snip ]==
> This document does not specify any kind of standard for encrypting elements.
> The issue of key scheduling, authorisation, and authentication are out of
> scope. External entities have used these elements in proprietary ways. ==[ snip
> ]==
> 
> Based on this text, it isn’t clear if Mastroska security properties are
> supposed be interoperable solution for encrypted content across applications
> (and how much is out of scope/left to proprietary implementations).  If
> interoperability is not expected, please add language to that effect.  Some of
> the feedback below might not be applicable if interoperability isn’t expected.
> 
> ** Section 5.1.4.1.31.*.  AES and associated modes (CTR and CBC) are named. 
> What is the key size?  Is it AES-128 (?) or -256. [WebM-Enc] references in
> Section 14 suggests it is 128.  Please be explicit.
> 
> ** Section 5.1.4.1.31.10.  ContentEncKeyID Element.
>     definition:  For public key algorithms this is the ID of the public
>      key the the data was encrypted with.
> 
> The descriptive text is suggesting that the ID to a public key if public key
> algorithms are described.  However, Table 26 only lists symmetric algorithms. 
> How is this key ID used?
> 
> ** Section 5.1.4.1.31.12.  In AES CTR and CBC modes typically, an IV is also
> passed in the encryption and description algorithm.  How is that handled for
> description in an interoperable way?  Section 14 references [WebM-Enc] which
> suggest that the IV is stored in the block.
> 
> ** Section 14.
>   Encryption in Matroska is designed in a very generic style to allow
>   people to implement whatever form of encryption is best for them.  It
>   is possible to use the encryption framework in Matroska as a type of
>   DRM (Digital Rights Management).
> 
> Enabling flexibility makes sense.  How is algorithm agility enabled?  How would
> a new algorithm be added to Table 26?  In a protocol context, this might be
> handled by an IANA registry enabling extensibility.
> 
> ** Section 14.  This section seems to be missing basic caution on using AES-CTR
> and AES-CBC such as:
> 
> -- For AES-CTR, IVs cannot be reused for a given key (see Section 7 of
> [RFC3686] describing it in the IPsec context)
> 
> -- It is best to use AES-CTR and CBC with an integrity mechanism, and the
> consequences if it is not.

Replying, instead of the discussion with Spencer and Michael and also mentioned to John…
The encryption layer was likely never used until WebM added AES and used it. The WebM specification is more complete than we provide: https://www.webmproject.org/docs/webm-encryption/
They even added elements to support AES.

The elements are defined in our document for compatibility and because the Matroska Elements existed before that. Technically it might be a separate document. But the elements still need a definition in this document.

We already mention the WebM document in https://www.ietf.org/archive/id/draft-ietf-cellar-matroska-15.html#section-14-8

> 
> ** Please add cautionary language in the Security Considerations about what
> explicitly would NOT be covered even if encryption was used.  For example,
> Section 16 of RFC8794 notes that:
> 
> ==[ snip ]==
> Even if the semantic layer offers any kind of encryption, EBML itself could
> leak information at both the semantic layer (as declared via the DocType
> Element) and within the EBML structure (the presence of EBML Elements can be
> derived even with an unknown semantic layer using a heuristic approach -- not
> without errors, of course, but with a certain degree of confidence). ==[ snip
> ]==
> 
> What are the specifics here on what “leaks”?
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thank you for documenting this important format in the IETF.
> 
> ** Section 2.
>   No
>   new elements are expected in files with version numbers 1, 2, or 3.
> 
> Is this the same thing as saying “no elements will appear in version 1 – 3 that
> is not documented here?”  If so, please make it clearer.

The document defines version 1, 2, 3 and 4 of Matroska. 1 to 3 are frozen.

> 
> ** Values from ISO/IEC 23001-8:2016 or ITU-T H.273 are cited in Tables 14, 18,
> and 19.  This (H.273) needs to be a normative reference.

Good catch! There’s even a free version.
Addressed here: https://github.com/ietf-wg-cellar/matroska-specification/pull/784

> 
> ** Section 5.1.4.1.31.3. ContentEncodingScope Element.  What does it mean for
> the scope to be private (value = 2). What is “the track’s private data”?

It should read "track’s CodecPrivate data”.
Addressed here: https://github.com/ietf-wg-cellar/matroska-specification/pull/785

> 
> ** Section 21.1.  Please a normative reference for JPEG and PNG.

Fixed in https://github.com/ietf-wg-cellar/matroska-specification/pull/776

> ** Section 23.1.
>   File access can simply be reading a file located on your computer,
>   but also includes accessing a file from an HTTP (web) server or CIFS
>   (Windows share) server.  These protocols are usually safe from
>   reading errors and seeking in the stream is possible.  However, when
>   a file is stored far away or on a slow server, seeking can be an
>   expensive operation and should be avoided.
> 
> Can the assurance that HTTP server is “usual safe … to seek” given that it
> could be deployed “far away” or on a “slow server”?

It depends if the server supports range request. The next sentence says it can be slow anyway. Both are true and have to be taken in consideration.

> 
> ** Section 26.
> -- Borrowing from the Security Considerations in various codec documents
> (RFC9043 and RFC6386).  Consider adding:
> 
> NEW
> Matroska Reader implementations need to be robust against malicious payloads. 
> Those related to denial of service are outlined in Section 2.1 of [RFC4732].  
> Although rarer, the same may apply to a Matroska Writer.  Malicious stream data
> must not cause the Writer to misbehave, as this might allow an attacker access
> to transcoding gateways
> 
> -- Also adding a reminder that there will be additional formats inside. 
> Consider adding:
> 
> NEW
> As an audio and visual container format, a Matroska file or stream will
> potentially encapsulated numerous byte streams created with a variety of
> codecs.  Implementers will need to consider the security considerations of
> these encapsulated formats.

I added both suggestions in https://github.com/ietf-wg-cellar/matroska-specification/pull/786
Thanks a lot!

> 
> 
> _______________________________________________
> Cellar mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/cellar