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

Magnus Westerlund <magnus.westerlund@ericsson.com> Wed, 10 June 2015 15:50 UTC

Return-Path: <magnus.westerlund@ericsson.com>
X-Original-To: rtcweb@ietfa.amsl.com
Delivered-To: rtcweb@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 45B4A1B2F36; Wed, 10 Jun 2015 08:50:06 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.201
X-Spam-Level:
X-Spam-Status: No, score=-4.201 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yW6jVFde9fQL; Wed, 10 Jun 2015 08:50:03 -0700 (PDT)
Received: from sessmg23.ericsson.net (sessmg23.ericsson.net [193.180.251.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id AF7701B2F24; Wed, 10 Jun 2015 08:50:02 -0700 (PDT)
X-AuditID: c1b4fb2d-f794d6d000004501-fc-55785ca81856
Received: from ESESSHC014.ericsson.se (Unknown_Domain [153.88.253.125]) by sessmg23.ericsson.net (Symantec Mail Security) with SMTP id CB.3B.17665.8AC58755; Wed, 10 Jun 2015 17:50:00 +0200 (CEST)
Received: from [127.0.0.1] (153.88.183.153) by smtp.internal.ericsson.com (153.88.183.62) with Microsoft SMTP Server id 14.3.210.2; Wed, 10 Jun 2015 17:50:00 +0200
Message-ID: <55785CA7.4090005@ericsson.com>
Date: Wed, 10 Jun 2015 17:49:59 +0200
From: Magnus Westerlund <magnus.westerlund@ericsson.com>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: Chris Inacio <inacio@cert.org>, "secdir@ietf.org" <secdir@ietf.org>, "iesg@ietf.org" <iesg@ietf.org>, "draft-ietf-rtcweb-rtp-usage-23@tools.ietf.org" <draft-ietf-rtcweb-rtp-usage-23@tools.ietf.org>, "rtcweb@ietf.org" <rtcweb@ietf.org>
References: <24C0D45F-DBF0-43A4-A2D6-B086F7EC368F@cert.org>
In-Reply-To: <24C0D45F-DBF0-43A4-A2D6-B086F7EC368F@cert.org>
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 8bit
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrNLMWRmVeSWpSXmKPExsUyM+Jvre6KmIpQgw/vtCzmtMZbzPgzkdli 4dqTjBZr/7WzW3xY+JDFgdVjxgYfjyVLfjJ5fLn8mS2AOYrLJiU1J7MstUjfLoEr4+PTRSwF Cx0rpvflNDCeN+5i5OSQEDCROLrqPjOELSZx4d56ti5GLg4hgaOMEq8v72SBcJYzSnSv28oG UsUroC3x78sTFhCbRUBVYtW8g+wgNpuAhcTNH41gNaICURJTH69jgagXlDg58wnYIBGBb0BT t05nBEkIC9hILJrxD6xBSMBaYufCR0CDODg4geK/upVAwsxAM2fOP88IYctLNG+dzQxRri3R 0NTBOoFRYBaSFbOQtMxC0rKAkXkVo2hxanFxbrqRsV5qUWZycXF+nl5easkmRmDoHtzyW3cH 4+rXjocYBTgYlXh4FWaVhwqxJpYVV+YeYpTmYFES552xOS9USCA9sSQ1OzW1ILUovqg0J7X4 ECMTB6dUA2PT8uoKs4y4WSdyOWzP3u6qiti1Mq9zupO9wUIzlzVJ/m5at1+4yTEvu6t19Psl GU0G70Obg3kK5k6az55q5sexI/H8hkeSE1uOPfmyauo5v8qP2xgzE9t5XX1erT+lO2vzWu3W gE1HMmfuzLnbM31B4qIguYmuTP9/JcqGLO5g8I2aM+9RgpISS3FGoqEWc1FxIgDDl9LSPgIA AA==
Archived-At: <http://mailarchive.ietf.org/arch/msg/rtcweb/pR2SNeZpFwNpyp5cbcDs7uYm2HA>
Subject: Re: [rtcweb] sector review of draft-ietf-rtcweb-rtp-usage-23
X-BeenThere: rtcweb@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Real-Time Communication in WEB-browsers working group list <rtcweb.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rtcweb>, <mailto:rtcweb-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/rtcweb/>
List-Post: <mailto:rtcweb@ietf.org>
List-Help: <mailto:rtcweb-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rtcweb>, <mailto:rtcweb-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 10 Jun 2015 15:50:06 -0000

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 inacio@cert.org
>
>
>

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: magnus.westerlund@ericsson.com
----------------------------------------------------------------------