Last Call Review of draft-ietf-rtcweb-rtp-usage-23
review-ietf-rtcweb-rtp-usage-23-secdir-lc-inacio-2015-06-10-00

Request Review of draft-ietf-rtcweb-rtp-usage
Requested rev. no specific revision (document currently at 26)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2015-06-09
Requested 2015-05-15
Authors Colin Perkins, Magnus Westerlund, Joerg Ott
Draft last updated 2015-06-10
Completed reviews Genart Last Call review of -23 by Suresh Krishnan (diff)
Genart Telechat review of -24 by Suresh Krishnan (diff)
Secdir Last Call review of -23 by Christopher Inacio (diff)
Assignment Reviewer Christopher Inacio
State Completed
Review review-ietf-rtcweb-rtp-usage-23-secdir-lc-inacio-2015-06-10
Reviewed rev. 23 (document currently at 26)
Review result Has Nits
Review completed: 2015-06-10

Review
review-ietf-rtcweb-rtp-usage-23-secdir-lc-inacio-2015-06-10


I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.


BLUF:  This draft is ready to go with some NITS.


This draft is an overarching set of guidelines of the use and application of RTP and RTCP as it applies to WebRTC and the related W3C API.  The W3C API is not described in this document, but references to functionality requirements for the API are made.

This draft is extremely well written.  At the same time, however, it reads like an encyclopedia of references and requirements across 35 different normative other referenced drafts.  Correspondingly, I did NOT read through all of the referenced drafts, nor did I believe that it was necessary in this case to do so.

The security considerations section was comprehensive and security impacts were taken into account throughout this draft.  I have two small NIT’s about the security section that I would like improved, and I feel these are rather small:  First, in the paragraph in the security section that starts “RTCP packets convey a Canonical Name…”  the authors state that the CNAME generation algorithm in described in section 4.9 – it isn’t, section 4.9 references RFC7022 for the generation algorithm.  Second, the last paragraph on page 39, starting with “Providing source authentication in multi-party…” ends the page with a large security warning.   Please include a reference in that paragraph in the security considerations and possibly to the appropriate draft/RFC which discusses that issue in some more depth.


general NITS:

I believe there are multiple versions of “end point” used in the document End Point, end-point, etc.  Those should be harmonized.


page 4:

Section 2 Rationale

“This builds on the past 20 years development of RTP to mandate the use of extensions that have shown widespread utility” 

can this be reworded to “This builds on the past 20 years of RTP development to mandate the use of extensions that have shown widespread utility.”


page 6:

“This specification requires the usage of a single CNAME when sending RTP Packet Streams…”   should the “require” be “REQUIRE”?

page 7:

"This to ensure robust handling of future extensions…” to “This is to ensure…”

page 14:

In reference to the paragraph that starts with “While the use of IP multicast...”.  There is no MAY/SHOULD/SHALL/REQUIRE etc. in the paragraph, but the last sentence does seem to imply an implementation requirement.  Was that intentional?

page 16:

“This can be various reasons for this:…”  to “There can be various reasons for this:…”

page 20:

“heterogeneous network environments using a variety set of link technologies…” get rid of “set”.

page 23:

“…supported by the RTCP Extended Reports (XR) framework [RFC3611][RFC6792].”   RFC3611 seems to be a full fledged reference while RFC6792 seems to just be text of “[RFC6792]”.

page 25:

First paragraph of section 11 defines a number of WebRTC API terms, PLEASE move those (far) forward in the document.
“The MediaStreamTracks within a MediaStream need to be possible to play out synchronized.”  rewrite to “The MediaStreamTracks within a MediaStream may need to be synchronized during play back.”  or something similar.

page 26:

“force an end-point to to disrupt”  only one “to”.

“This is motivating the discussion in Section 4.9”, (yes, now I’m really getting picky,) but the discussion was already motivated.  :)

page 27:

“Optimizations of this method is clearly possible and …”  “is” -> “are”

“receiving multiple MediaStreamTracks, where each of different MediaStreamTracks (and …” to “ … where each of *the* different MediaStreamTracks … “

“This later is relevant to handle some cases of legacy interop.” to “The later is relevant … legacy interoperability.”

page 28:

“. . . Endpoints can configure and use RTP sessions is outlined.”  change “is” to “are”

“ . . . it is common to use one RTP session for each type of media sources (e.g. . . .”  change “sources” to “source”

page 31:

“To maintain a coherent mapping between the relation between RTP sessions and RTCPeerConnection objects it is …”  rewrite to maybe

“To maintain a coherent mapping between the relationship of RTP sessions and RTCPeerConnection objects it is ….”

page 32:

“This scenario also results in that an end-point’s feedback or requests goes to the mixer…”  change “goes” to “go”.

“necessarily be explicitly visible any RTP and RTCP traffic …” to “necessarily be explicitly visible to any RTP and RTCP traffic”.

page 38:

“combing” to “combining”



--
Chris Inacio
inacio at cert.org