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

[Roman Danyliw via Datatracker <[e-mail]>]

Roman Danyliw has entered the following ballot position for
draft-ietf-cellar-flac-12: 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-flac/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

** Section 8.4
   The application metadata block is for use by third-party
   applications.  The only mandatory field is a 32-bit identifier.  An
   ID registry is being maintained at https://xiph.org/flac/id.html
   (https://xiph.org/flac/id.html).

-- Did the WG discuss the consequences of having normative behavior maintained
on this external website?  I currently see < 30 entries.  Why can’t this be
maintained by IANA?

-- This third-party application registry URL needs to be a normative reference.

** Section 8.6.1
   For a more comprehensive list of possible field names, the list of
   tags used in the MusicBrainz project (http://picard-
   docs.musicbrainz.org/en/variables/variables.html) is recommended.

-- It is unclear how this guidance should be treated by implementers.  Is this
normative behavior to: (a) use the MusicBrainz URL to understand the semantics
of these field making this reference normative; or (b) this is a free-form
field outside the scope of this specification but there are community efforts
like MusicBrainz which is an _informative_ reference.  In either case this URL
needs to be some kind of formal reference.

-- Is there are reason why there isn’t an IANA registry for these field name?

** Section 8.7.1  Please make [ISRC-handbook] a normative reference since that
specification describes the track number value.

** Section 8.8.
  the media type and description
   are prepended with a 4-byte length field instead of being null
   delimited strings.

What is the format of “media type”?  Is it
https://www.iana.org/assignments/media-types/media-types.xhtml?  I ask because
“-->” is later mentioned as an escape sequence.

** Section 8.8.  The values of table 13 are underspecified:

-- per value = 1, “PNG file icon of 32x32 pixels”, please provide a reference
to the PNG format

-- per value = 17, “A bright colored fish”, what is that?

** Section 8.8.  The URI mechanism described in the Picture field of the
metadata block needs further security considerations.  The SECDIR review also
pointed this out and discussion on possible new language has started.  This
guidance needs to explicitly say that:

-- the Security Considerations of RFC3986 apply (to cover threats of
maliciously craft URLs)

-- following external URLs introduces a tracking risk from on-path observers
and the operator of the service hosting the URL.  Likewise, the choice of
scheme, if it isn’t protected like https, could also introduce integrity
attacks by an on-path observer.

-- a malicious operator of the service hosting the URL can return arbitrary
content that the parser will read

-- implementers must guard against directory traversal attacks (since relative
URIs are permitted) and be cognizant of same-origin issues (and localhost and
local network) even more broadly

-- (per SECDIR review) there could be a DoS attack against the operator of the
service when the URI from the FLAC file is retrieved

** Section 10.  Since this document is describing normative behavior on
embedded this work into another container, that other container needs to be
named normatively.  Specifically, please make [RFC3533] (Ogg) and
[I-D.ietf-cellar-matroska] (Matroska) normative references.

** Section 12
   FLAC files may contain executable code, although the FLAC format is
   not designed for it and it is uncommon.  One use case where FLAC is
   occasionally used to store executable code is when compressing images
   of mixed mode CDs, which contain both audio and non-audio data, of
   which the non-audio portion can contain executable code.

Thanks for calling this out.  From this cautionary text, it is not clear to me
where in the FLAC format this executable code would be insert.  What guidance
can be given to implementers about recognizing this executable code and
treating it with care?


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

Thank you to Robert Sparks for the SECDIR review.  Also, thank you to Martijn
van Beurden for engaging on the feedback.  Some of my DISCUSS feedback aligns
with this review.

** Section 7.
   The flac command-line tool,
   part of the FLAC reference implementation (see Section 11), generates
   streamable subset files by default unless the --lax command-line
   option is used.

-- This single reference to a particular command line option seems out of
place.  Why mention this one specifically?  Are there other command line
switches that matter?

-- Section 11 has text to remove upon publication.  Should this text also be
removed since it won’t make sense without that section?

** Section 8.2.

   The MD5 signature is made by performing an MD5 transformation on the
   samples of all channels interleaved, represented in signed, little-
   endian form. ...

This paragraph describes the need to construct a “MD5 signature”.  I had
trouble understanding on what a “MD5 transformation” is.  How is it computed?

** Section 8.8.  Please provide a reference for ID3v3 specification.

** Section 9.1.5
   When a frame number is encoded, the value MUST NOT be larger than
   what fits a value of 31 bits unencoded or 6 bytes encoded.  Please
   note that as most general purpose UTF-8 encoders and decoders follow
   [RFC3629], they will not be able to handle these extended codes.

I was confused on why a UTF-8 encoder or decoder would be used to process a raw
octet stream.  Coded numbers don’t seem to be related to text strings.

** Section 9.2.2.  Please provide informative references for “Meridian Lossless
Packing codec” and “lossyWAV”.

** Section 10.
   The FLAC format can be used without any container, as it already
   provides for a very thin transport layer.

It wasn’t clear to me what “thin transport layer” meant here.

** Section 10.3.  Recommend being clearer that the “full encapsulation
definition of FLAC audio in MP4 is outside the scope of this document”. 
Perhaps: OLD
   The full encapsulation definition of FLAC audio in MP4 files was
   deemed too extensive to include in this document.  A definition
   document can be found at [FLAC-in-MP4-specification]
NEW
The full encapsulation definition of FLAC audio in MP4 files was    deemed too
extensive and is out of scope for this document.  A possible approach is found
at [FLAC-in-MP4-specification]

** Section 12.
   Parsers MUST employ thorough checks on whether a found coded
   number or seekpoint is at all possible.

Is this check intended to verify that these seekpoints are in bounds?