Subject: Re: [rtcweb] sector review of draft-ietf-rtcweb-rtp-usage-23

From: Magnus Westerlund <[email-masked]>
Date: Wed, 10 Jun 2015 17:49:59 +0200

Hi,

(Adding RTCWEB WG list as this discusses changes to the draft)

Thanks for the very detailed review. We will include all relevant 
changes in an update that will be submitted after the IESG call tomorrow.

WG, this is your chance to scream if there are bad changes here.

Chris Inacio skrev den 2015-06-09 17:13:
>
>
> 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.

Yes, but it actually specifies which particular generation algorithm to 
use. RFC7022 does contain more than one.

I propose that we change this text to say:

Section 4.9 of this memo mandates generation of short-term persistent 
RTCP CNAMES, as specified in RFC7022, resulting in untraceable CNAME 
values that alleviate this risk.

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.

If I understand this correctly, you want something like the below added 
to the security consideration section. I suggest to add this last in the 
section.

    In multi-party communication scenarios using RTP Middleboxes, a lot
    of trust is placed on these middleboxes to preserve the sessions
    security.  The middlebox needs to maintain the confidentiality,
    integrity and perform source authentication.  As discussed in
    Section 12.1.1 the middlebox can perform checks that prevents any
    endpoint participating in a conference to impersonate another.  Some
    additional security considerations regarding multi-party topologies
    can be found in [I-D.ietf-avtcore-rtp-topologies-update].

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

Yes, we will use endpoint

>
> 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.”
>

Yes.

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

This is intended as an informational reference, thus I propose to change 
this to "mandates" thus avoiding the RFC2119 terms.

>
> page 7:
>
> "This to ensure robust handling of future extensions…” to “This is to
> ensure…”

Sure.

>
> 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?

Yes, it is intentional.

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

Addressed in -24 version.

>
> page 20:
>
> “heterogeneous network environments using a variety set of link
> technologies…” get rid of “set”.
>
Yes.

> 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]”.
>

They are both relevant references when understanding what RTCP XR are 
and what metrics one should consider.

I propose to add a "see" so that the results become "... framework, see 
[RFC3611][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.

I have added also MediaStreamTrack to the Terminology section (3). And 
populated both MediaStream and MediaStreamTrack with text from this 
paragraph. I have however not removed any text from this paragraph in 
Section 11.

>
> page 26:
>
> “force an end-point to to disrupt”  only one “to”.

Ok

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

Changed this to:

This is motivating the use of a single CNAME in Section 4.9.

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

ok

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

Fixed

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

Ok

> 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 ….”

I propose:

To maintain a coherent mapping of the relationship between RTP sessions 
and RTCPeerConnection objects it is recommend that this is implemented 
as several individual RTP sessions.

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

ok

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

Yes

> page 38:
> “combing” to “combining”

ok

>
>
> -- Chris Inacio [email-masked]
>
>
>

Cheers

Magnus Westerlund

----------------------------------------------------------------------
Services, Media and Network features, Ericsson Research EAB/TXM
----------------------------------------------------------------------
Ericsson AB                 | Phone  +46 10 7148287
Färögatan 6                 | Mobile +46 73 0949079
SE-164 80 Stockholm, Sweden | mailto: [email-masked]
----------------------------------------------------------------------

---

From: Adam Roach <[email-masked]>
Date: Wed, 10 Jun 2015 11:30:33 -0500

On 6/10/15 10:49, Magnus Westerlund wrote:
> This is intended as an informational reference, thus I propose to 
> change this to "mandates" thus avoiding the RFC2119 terms.

RFC 2119 doesn't remove the words "require", "must", "should", "may" and 
"recommend" from the English language. If all you mean is the ordinary 
word "require," (rather than the 2119 term "REQUIRE"), then "require" is 
just fine.

/a

---

From: Colin Perkins <[email-masked]>
Date: Wed, 10 Jun 2015 19:25:15 +0100

On 10 Jun 2015, at 17:30, Adam Roach <[email-masked]> wrote:
>> This is intended as an informational reference, thus I propose to change this to "mandates" thus avoiding the RFC2119 terms.
> 
> RFC 2119 doesn't remove the words "require", "must", "should", "may" and "recommend" from the English language. If all you mean is the ordinary word "require," (rather than the 2119 term "REQUIRE"), then "require" is just fine.

The RFC 2119 term is "REQUIRED", not "REQUIRES" or "REQUIRE" anyway, but we have explicitly avoided lower-case versions of RFC 2119 terms in the draft, precisely to avoid any possible confusion.

-- 
Colin Perkins
https://csperkins.org/

---

From: Chris Inacio <[email-masked]>
Date: Wed, 10 Jun 2015 18:34:20 +0000

> On Jun 10, 2015, at 9:30 AM, Adam Roach <[email-masked]> wrote:
> 

That was exactly my comment and you have made a thoughtful decision -- I'm good. 

--
Chris Inacio

---

From: Chris Inacio <[email-masked]>
Date: Wed, 10 Jun 2015 20:31:22 +0000

> On Jun 10, 2015, at 8:49 AM, Magnus Westerlund <[email-masked]> wrote:
> 
> Hi,
> 
> Thanks for the very detailed review. We will include all relevant changes in an update that will be submitted after the IESG call tomorrow.
> 
> Yes, but it actually specifies which particular generation algorithm to use. RFC7022 does contain more than one.
> 
> Section 4.9 of this memo mandates generation of short-term persistent RTCP CNAMES, as specified in RFC7022, resulting in untraceable CNAME values that alleviate this risk.
> 

looks good.

> If I understand this correctly, you want something like the below added to the security consideration section. I suggest to add this last in the section.
> 

That would be excellent.

> This is intended as an informational reference, thus I propose to change this to "mandates" thus avoiding the RFC2119 terms.
I’m good with that.

> They are both relevant references when understanding what RTCP XR are and what metrics one should consider.
> 
> I propose to add a "see" so that the results become "... framework, see [RFC3611][RFC6792].
> 

I think I was misunderstood, likely my fault.  In the PDF, the RFC3611 seemed to be a true reference in the document; but RFC6792 did NOT seem to be a reference.  Maybe an issue in the actual XML, not the resulting PDF/TXT.  It may be nothing.

> I have added also MediaStreamTrack to the Terminology section (3). And populated both MediaStream and MediaStreamTrack with text from this paragraph. I have however not removed any text from this paragraph in Section 11.
> 
> 
sounds good.

> This is motivating the use of a single CNAME in Section 4.9.
:)

> To maintain a coherent mapping of the relationship between RTP sessions and RTCPeerConnection objects it is recommend that this is implemented as several individual RTP sessions.
> 

looks good.

>> -- Chris Inacio [email-masked]

great.  It is a very well written draft.

--
Chris Inacio
[email-masked]

---

From: "DRAGE, Keith (Keith)" <[email-masked]>
Date: Thu, 11 Jun 2015 08:08:08 +0000

I support Colin on this.

Using lower case versions of RFC 2119 terms causes its own confusions.

It is usually easy to maintain clarity while avoiding this.

Keith

> -----Original Message-----
> From: rtcweb [mailto:[email-masked]] On Behalf Of 
> Colin Perkins
> Sent: 10 June 2015 19:25
> To: Adam Roach
> Cc: [email-masked]; [email-masked]; [email-masked]; 
> [email-masked]; Chris Inacio
> Subject: Re: [rtcweb] sector review of draft-ietf-rtcweb-rtp-usage-23
> 
> >>> "This specification requires the usage of a single CNAME 
> when sending
> >>> RTP Packet Streams..."   should the "require" be "REQUIRE"?
> >> 
> >> This is intended as an informational reference, thus I 
> propose to change this to "mandates" thus avoiding the RFC2119 terms.
> > 
> > RFC 2119 doesn't remove the words "require", "must", 
> "should", "may" and "recommend" from the English language. If 
> all you mean is the ordinary word "require," (rather than the 
> 2119 term "REQUIRE"), then "require" is just fine.
> 
> The RFC 2119 term is "REQUIRED", not "REQUIRES" or "REQUIRE" 
> anyway, but we have explicitly avoided lower-case versions of 
> RFC 2119 terms in the draft, precisely to avoid any possible 
> confusion.
> 
> --
> Colin Perkins
> https://csperkins.org/
> 
> 
> 
> 
> _______________________________________________
> rtcweb mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtcweb
>

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 11 Jun 2015 10:20:51 +0200

Hi

Snipping it down to a single thing I want to comment on.

> I think I was misunderstood, likely my fault. In the PDF, the
> RFC3611  seemed to be a true reference in the document; but RFC6792 did NOT seem
to be a reference. Maybe an issue in the actual XML, not the resulting
PDF/TXT. It may be nothing.

I haven't looked at the PDF generated at all before. It appear as a bug 
in the generation. Both these are xref items in the XML source. So it 
must be something when putting two xref after each other and no display 
text. I have reported this as a bug in the XML2RFC tracker.

cheers

Magnus Westerlund

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 11 Jun 2015 10:25:28 +0200

> Using lower case versions of RFC 2119 terms causes its own confusions.

I have heard some people argue that also the lower case usage have the 
RFC 2119 meaning. The RFC 2119 to some degree support such an 
interpretation, but is not more explicit than this.

    In many standards track documents several words are used to signify
    the requirements in the specification.  These words are often
    capitalized.  This document defines these words as they should be
    interpreted in IETF documents.

> It is usually easy to maintain clarity while avoiding this.

Yes, and the rewrite here was trivial and conveys the same meaning.

Cheers

Magnus Westerlund