Hiya, Thanks - I'm fine with all that and especially the new security considerations text which seems much better,
Cheers, S. On 17/02/16 02:11, Timothy B. Terriberry wrote: > Thanks for the detailed comments. Responses are in-line below. > > Stephen Farrell wrote: >> - general, (but a nit): there are some odd unexplained numbers >> here, which is fine ... but odd. (E.g. 125,829,120) It might be >> nicer to explain to the reader why these values were chosen. I > > The number quoted is just 120 MB (120*1024*1024). I added a > parenthetical to clarify this. The 61,440 number in the same paragraph > is explained later in the text. Happy to clarify any others you think > should be explained better. > >> - An example or diagram in the intro or section 3 would maybe >> have helped. > > The appropriate figure for the introduction is probably the one from > Section 5 of RFC 3533. We could copy it here, but I think the reference > is enough (it would probably require more text to explain it, text which > is already in RFC 3533). > > Section 3 is a bit more specific to this draft. I added the following > figure: > > Page 1 Pages 2 ... n Pages (n+1) ... > +------------+ +---+ +---+ ... +---+ +-----------+ +---------+ +-- > | | | | | | | | | | | | | > |+----------+| |+-----------------+| |+-------------------+ +----- > |||ID Header|| || Comment Header || ||Audio Data Packet 1| | ... > |+----------+| |+-----------------+| |+-------------------+ +----- > | | | | | | | | | | | | | > +------------+ +---+ +---+ ... +---+ +-----------+ +---------+ +-- > ^ ^ ^ > | | | > | | Mandatory page break > | | > | ID header is contained on a single page > | > 'Beggining Of Stream' > > Figure 1: Example packet organization for a logical Ogg Opus stream > > >> - section 5: Q7.8 is a new one to me. Maybe add a reference? > > Added a reference to > <https://en.wikipedia.org/w/index.php?title=Q_%28number_format%29&oldid=697252615>. > > >> - 5.1.1.5: Could figures 3-8 do with some body text to explain >> them? That's (having body text that refers to the figures), >> general good practice and I'm not sure if they're sufficiently >> clear for an implementer to get right. (But as I'm not coding >> this, I don't know, so just checking:-) > > I think the meaning is relatively unambiguous, but certainly adding a > reference to the figures instead of just saying "the following matrices" > would be clearer, so I have done that. > >> - 5.2: You don't say that the user comment strings must also be >> UTF8, but you do for the vendor string. Why not? I think it'd be >> good to call that out. > > The text doesn't but the header says: > > "User Comment #i String (variable length, UTF-8 vector):" > > I've normalized the text to match that of the vendor string for > consistency, however. > >> - 5.2.1: [vorbis-comment] is, correctly, a normative reference >> here but I wonder if a xiph.org URL is a good enough reference >> for that. Is there anything better, or are we confident enough >> in that URL? > > According to archive.org, that URL has been stable since at least 2005 > (since the year 2000, according to our svn logs, but the final version > for Vorbis 1.0 wasn't posted until July 11th 2002). I don't know of a > better authority. > >> - 5.2.1: From what namespace are the -573 and 111 values here >> selected? How is that managed? (Just wondering.) > > Those values are just examples. I've added a sentence saying that, since > it must not have been clear. The syntax for them is described in the > paragraph below. If people feel that description is imprecise we could > probably add some EBNF. > >> - section 9: While I like the MUST NOT here, it's really only >> wishful thinking and isn't a strictly valid use of 2119 terms. >> I'm ok with that though. The SHOULD NOT statement is also kind >> of bogus. Generally this section would be better if it had >> guidance on where implementers are likely to go wrong in ways >> that cause security issues. Reference such as are provided in > > I agree. The current text was adapted from RFC 6716, but I think we have > enough implementation experience at this point to write something > better. I propose the following additional text: > > Header parsing code contains the most likely area for potential > overruns. It is important for implementations to ensure their > buffers contain enough data for all of the required fields before > attempting to read it (for example, for all of the channel map data > in the ID header). Implementations would do well to validate the > indices of the channel map, also, to ensure they meet all of the > restrictions outlined in Section 5.1.1, in order to avoid attempting > to read data from channels that do not exist. > > To avoid excessive resource usage, we advise implementations to be > especially wary of streams that might cause them to process far more > data than was actually transmitted. For example, a relatively small > comment header may contain values for the string lengths or user > comment list length that imply that it is many gigabytes in size. > Even computing the size of the required buffer could overflow a > 32-bit integer, and actually attempting to allocate such a buffer > before verifying it would be a reasonable size is a bad idea. After > reading the user comment list length, implementations might wish to > verify that the header contains at least the minimum amount of data > for that many comments (4 additional octets per comment, to indicate > each has a length of zero) before proceeding any further, again > taking care to avoid overflow in these calculations. If allocating > an array of pointers to point at these strings, the size of the > pointers may be larger than 4 octets, potentially requiring a > separate overflow check. > > Another bug in this class we have observed more than once involves > the handling of invalid data at the end of a stream. Often, > implementations will seek to the end of a stream to locate the last > timestamp in order to compute its total duration. If they do not > find a valid capture pattern and Ogg page from the desired logical > stream, they will back up and try again. If care is not taken to > avoid re-scanning data that was already scanned, this search can > quickly devolve into something with a complexity that is quadratic in > the amount of invalid data. > > In general when seeking, implementations will wish to be cautious > about the effects of invalid granule position values, and ensure all > algorithms will continue to make progress and eventually terminate, > even if these are missing or out-of-order. > > >> RFC 6562 about the dangers of VBR might also be useful here, not >> sure. > > In most of the expected uses of Ogg Opus on the internet (e.g., https > streaming), the encryption and network packetization would be separate > from the container's packetization/pagination, so I don't think the > dangers referred to by RFC 6562 apply (if you can see the Ogg packet > sizes, it's because you already have access to the decrypted plaintext). > In other contexts, RFC 6716 already includes a reference to 6562. > > I've attached a diff of the relevant changes to the XML.
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ codec mailing list [email protected] https://www.ietf.org/mailman/listinfo/codec
