Re: [payload] Stephen Farrell's Discuss on draft-ietf-payload-rtp-h265-14: (with DISCUSS and COMMENT)

[Stephen Farrell <[e-mail]>]

Hiya,

Sorry for the slow response ...

On 04/09/15 22:07, Stephan Wenger wrote:
> Hi Stephen, all, With all respect due, this appears to us authors as
> one or the more sloppy reviews of the security consideration section
> and security features of the document.  

Hmm. I can see how a "please show your work, because I'm not sure it's
been done" discuss may be irritating at this point in what was probably
a very long and already irritating process, but I don't agree that
sloppy is at all a good descriptor for that.

Anyway, some more below, but maybe we should think about how best to
resolve the discuss and not get too wrapped around blow-by-blow
responses which could easily happen here and not get us too far. Could
be that a call is the way to do that. If so, I'm happy to do one.
(And do feel free to not respond to everything below.)

> Please see our comments
> inline. Regards, Stephan
> 
> 
> 
> 
> 
> On 9/1/15, 05:49, "Stephen Farrell" <[e-mail]>
> wrote:
> 
>> Stephen Farrell has entered the following ballot position for 
>> draft-ietf-payload-rtp-h265-14: Discuss
>> 
>> [...]
>> 
>> ----------------------------------------------------------------------
>>
>> 
DISCUSS:
>> ----------------------------------------------------------------------
>>
>>
>>
>> 
(1) For a 99 page detailed specification, the security
>> considerations are tremendously brief.  There are two 
>> non-boilerplate paragraphs - one says developers "MUST exercise
>> caution" (meaning what precisely?)
> 
> The third paragraph says developers MUST use caution with respect to
> a very specific codepoint in the codec spec, namely the user data SEI
> messages.  But also please see below.

"MUST use caution" isn't really a sensible use of a 2119 MUST. It
needs to be something a developer can interpret, and it's not clear
to me that that is. (And also more below too.)

> 
> 
>> and the other extolls the virtues of not encrypting.
> 
> The fourth paragraph simply states that the middlebox called MANE as
> used in this doc (and as used in many current video conferencing
> systems) is incompatible with end-to-end encryption.  The IETF tries
> to solve that vulnerability in the perc WG.  Until such solutions are
> widely deployed, it’s probably appropriate to warn people of certain
> features of MANEs.
> 
> 
> 
>> For a 99 page specification of a highly non-trivial encoding
>> scheme, I would have expected to see the results of a better 
>> analysis. Was that analysis done?
> 
> To the extent an analysis is required for the RTP payload
> specification, the answer is yes, it was done; and no, no
> vulnerabilities beyond those mentioned were found.  

Great. Can you send me a pointer to where that was described on
the wg list or in a meeting?

> We neither
> analyzed the security aspects of RTP itself, nor in much detail the
> security aspects of H.265, 

That's the norm in the payload WG. However, this draft does not fit
that normal pattern since it has such a large amount of new text.

For example, I searched for the first substantive " must " in the
document. That's on page 9, para 2 and says "must contain an end of
bitstream NAL unit and..." (various other conditions) but that
doesn't say what an implementer should do if those conditions are
not met. And I could see crafted packets not meeting those conditions
and perhaps leading to remote code execution.

The subsequent paragraphs also generate security concerns, dealing
as they do with random access - messing with whatever pointers are
used for that could cause lots of implementation errors/crashes.

Now, are either of those things new (in this draft) and not described
in the ITU documents? I don't know is the answer, but presumably you
do. I'd guess they probably are in the ITU specs in this case, but
equally I might guess that the ITU specs also don't contain guidance
as to how to handle failure cases. That lack of guidance does I think
increase the liklihood of implementation error.

And you're adding some guidance here, so I don't get why that guidance
doesn't extend to security considerations.

> although we point out the one
> vulnerability that is perhaps not commonly known and well understood
> in the third paragraph (the one with the “MUST exercise caution”).
> 
> 
>> That should at least include consideration of the CVEs already
>> published relating to H.264 [1] which include 34 CVEs relating to 
>> various kinds of remote-code-execution and DoS.
> 
> We were not aware of the existence of CVEs allegedly related to H.264

Allegedly? This isn't a court, it's a DISCUSSion:-)

> and its transport specs.  Thanks for pointing it out.
> 
> Upon brief review, none of the 34 CVEs relate to H.264 over RTP in
> any way, shape, or form.

So you'd not previously searched for related CVEs? That is one good
way to find out what implementers have done wrong in the past and
so are likely to do again. Not the only way, but one.

> 
> Every one relates to an implementation problem, and NOT to a
> specification problem.  Some of them clearly are mischaracterized by
> the search engine or the filers; certainly, problems with SMTP, RTSP,
> file name parsing etc. have nothing to do with video codecs or RTP.
> They are irrelevant here.  Many appear to be duplicates, or at least
> reporting the same source of a problem leading to different
> vulnerabilities.  Of those directly related to codec implementations,
> they pretty much all refer to vulnerabilities that open when feeding
> either non-compliant, or compliant but “evil” bitstreams to decoders.
> Those are the only ones we possibly could do something about.

That's fair. And what guidance is offered for that last kind of case?
(Other than the one discussed below.)

> 
> The second paragraph (one of those you labelled as “boilerplate”)
> deals with exactly those vulnerabilities and recommends a solution.

To an extent. DAO and data integrity do help yes, but RTP will often
be used without those. At that point the implementer needs guidance.
And the issue is less with "overload" and more with allowing remote
code execution.

> 
> There is very little we can recommend beyond what is in the second
> paragraph to a codec implementer regarding the decoder’s reaction to
> non-compliant bitstreams.  The only thing that comes to mind is to
> say that even in case of applied end-to-end security attackers may
> feed non-compliant bitstreams to an unaware system, and that system
> better does not crash and burn over those bitstreams but gracefully
> discard them or something.  With respect to “evil” bitstreams, we
> could point out that the video codec standardization community has
> long recognized their existence (for reasons completely unrelated to
> security), and publishes a set of test “evil” bitstreams designed to

I didn't know that. Do those include cases that might lead to remote
code execution? (Just wondering.) Have you pointers?

> check many of the stress points of a decoder implementation.  It’s
> quite obvious that many of the open source folks building decoders,
> as well as some early commercial H.264 decoder implementations, did
> not run tests against those bitstreams--or ran the tests, knew about
> the vulnerability, but decided that performance is more important
> than stability and security against theoretical threat scenarios.  We
> could point out that it is prudent for a decoder implementer to run
> its implementation at least against published known “evil”
> bitstreams. 

I think that would be worthwhile, esp. if as you say it could
have helped avoid some implementation errors.

> But that’s all we can possibly do with respect to codec
> implementations.

That's where I'm not sure and where the 99 pages aspect comes in.
You've described so much about this, I can't see why you omit the
security considerations.

> 
> All the shmuh above is probably useful input for the security
> considerations section of IETF video codecs as designed by netvc.
> They do not necessarily belong to an RTP payload spec, but if you
> prefer we will add them nevertheless.
> 
> 
> 
>> If the authors here are asserting that this presumably more complex
>> codec will have few vulnerabilities, then I would welcome seeing
>> the justification for that statement.
> 
> No one but us (in the third paragraph) has pointed out a security
> vulnerability in spec space.  We can’t really combat shoddy
> implementation practices.

Combat is the wrong word. What's needed here is to document the
security considerations of the specification text that's in this
document.


> 
> 
>> (Note: In some cases with payload drafts, authors can fairly claim
>> that the I-D does not describe the payload in detail and hence that
>> the security considerations text need not be comprehensive as
>> implementers are expected to find that elsewhere. In this case, the
>> 99 pages strongly argues otherwise IMO - there are plenty of ways
>> to go badly wrong implementing what is stated in this draft.)
> 
> Agreed in principle.  But we are not aware of anything that can go
> wrong within the protocol designed in the 99 pages beyond what we
> pointed out in the fourth paragraph--and yes, we looked.

Again, if you could point out where that was described to the WG
that'd be great.

> 
> 
> 
>> To summarise: "MUST exercise caution" is not useful, can't you
>> translate that into actionable advice to an implementer based on
>> experience with security issues found with this and similar
>> codecs?
> 
> So let’s look in more detail at the “MUST exercise caution”.  The
> offending paragraph reads: “Decoders MUST exercise caution with
> respect to the handling of user data SEI messages, particularly if
> they contain active elements, and MUST restrict their domain of
> applicability to the presentation of the bitstream.” We point out one
> H.264/H.265 codec specific potential vulnerability perhaps not well
> understood by some people (as it is pretty exotic)--the “user data
> Supplementary Enhancement Information (SEI) message”.  Video codec
> implementers--the main audience for this spec--know what SEI messages
> are, but not necessarily this one.  The user data SEI allows an
> encoder to embed into the video bitstream pretty much any bitstring
> up to a certain (in many cases quite large) length.  Machine code,
> Javascript, ..., anything.  Obviously, this can be dangerous.
> However, it is also useful and used in practice in some of today’s
> systems (though not necessarily with active content).  Insofar, we do
> not require a receiver to discard this potentially dangerous (but
> often quite useful) data.  Instead, we say that a decoder must be
> careful around these bits, and restrict their use to the rendering
> subsystem.  If implemented correctly, the worse that would happen is
> a messed up video picture. You suggest that “MUST exercise caution”
> is not actionable, and upon reflection we agree.  Informative
> language appears to be more appropriate.  We could remove the
> “caution” language and just stay with the clearly actionable other
> remedy, or we could soften the “caution” language to something like
> “decoder implementers have to weight the potential benefits of using
> active content against their dangers.  One way to do so is to
> restrict the domain of applicability to the presentation of the
> bitstream.”.  Does this address your concern?

I'm not sure the latter (alone) is much better tbh. What's needed is
something that a developer who is implementing the rest of this draft
can easily comprehend. Explaining the trade-offs is a fine thing to
do, but in the above, many implementers won't weigh costs/benefits,
they'll just write their code;-)

So explaining the attack (in a sentence), mentioning the trade offs
(in a sentence) and then saying something like "to be safe do X."
In this case, I'm not sure "restrict domain..." is a correct X, since
active content could e.g. take over the entire screen in which case
there is no really safe way to handle it unless you first check the
specific content. I'd say that disabling all active content is
really the only safe instruction that can be given, if that is
practical (I'm not sure if it is).

> 
> 
>> 
>> (2) This is just a process nit probably. The shepherd write-up
>> doesn't mention the Nokia IPR declaration.  Were the WG also ok
>> with that one? The write-up seems to pre-date that latest IPR
>> declaration, which is from a company that seems to employ one of
>> the authors. That is odd timing really so can someone explain the
>> sequence of events and why all is well?
> 
> I will leave the chairs and Ben to sort this out.  However, I
> remember quite clearly that I myself pointed out during IETF payload
> sessions the Nokia IPR at least once, including my understanding of
> the scope of protection and its limitation to certain optional modes
> of operation.  I don’t believe that these discussions were minuted,
> but they probably can be found in audio records.  In other words,
> yes, the Nokia IPR was raised in the WG, and in a level of detail
> rather uncommon for such discussions.  Really, no regular follower of
> the payload WG should be surprised of this.

Ben is handling this separately.

> 
>> 
>> 
>> ----------------------------------------------------------------------
>>
>> 
COMMENT:
>> ----------------------------------------------------------------------
>>
>>
>>
>>
>> 
- General: I was puzzled as to why there is so much text
>> that is presumably non-normative explanatory text covering what is
>> elsewehere in (I guess) ITU documents. It seems like there is a
>> lot, but not enough, here for an implementer.
> 
> The WG heard this comment before and discussed it. 

Cool - can you also send me a point to that discussion so I can
understand it better? (It still puzzles me tbh.)

> It is my
> understanding of the WG consensus that the level of detail is about
> right.  For your information, the H.265 version 1 spec is an
> extremely dense document (much denser than the payload format) and,
> set out in 10 pt. Times Roman, spans 300 pages.  We cover the core
> codec only superficially, but dig in a bit deeper in those aspects
> where we added signaling support for capability exchange (for
> example, parallelization tools).
> 
>> 
>> - 4.1: " The assignment of an RTP payload type for this new packet
>> format is outside the scope of this document and will not be
>> specified here. " Huh? That's confusing. For me at least.
> 
> This is common text among many other RTP payload formats.
> Historically, RTP payload formats (or RTP profiles) hard-coded the
> RTP payload type for a media format.  Today, the assignment of the
> RTP payload type is handled dynamically by the call control protocol,
> for example by SIP.  I don’t think that this paragraph is confusing
> to anyone familiar with RTP.
> 
>> 
>> - p75 - why would md5 ever be most-preferred these days? Better to
>> not say that, even in an example. Even better would be to deprecate
>> md5 even for this non-security purpose to simplify code-audit. Or,
>> if there is some reason why e.g. sha256 isn't suited then
>> explaining that would also help for code-audits.
> 
> The code point your comment relates to is available to negotiate the
> capability to include a certain SEI message (namely the decoded
> picture hash (DPH) SEI message) into the H.265 bitstream.  We are
> therefore restricted to the syntax of that SEI message.  The DPH
> offers the choice of an MD5 checksum, a 16 bit CRC or a 32 bit
> add-until wraparound checksum.  Please refer to the cited paragraphs
> in H.265 to the available options.  Of these, we recommend the most
> sophisticated one (and this is inline with current implementation
> practice; for example, the H.265 reference code includes DPHs using
> the MD5 checksum). Speculating a bit why ITU and ISO/IEC uses these
> (and not other, more sophisticated) checksums, I guess MD5 was chosen
> as it is sufficient for the purpose.  Including the DPH is mostly a
> crude error detection tool.  It’s unrelated to security.  In any
> case, it’s not up to a RTP payload format document to question the
> design choices made by the payload standardizers.

Bummer that they've kept md5.

FWIW, in such cases I'd choose crc32 myself. Using md5 causes
problems (i.e. cost) for folks doing security code audits when
the auditor has to have it explained/documented why there is any
md5 code still being used and why that isn't security sensitive
and how one ensures no security sensitive bits of code re-use that
md5 implementation. (It's generally a bad plan to continue to use
broken crypto for anything I think. But people do do it.)

Cheers,
S.


>