Re: [rtcweb] Comments on draft-ietf-rtcweb-jsep-06

Justin Uberti <juberti@google.com> Fri, 14 March 2014 16:48 UTC

Return-Path: <juberti@google.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 9FDC41A018A for <rtcweb@ietfa.amsl.com>; Fri, 14 Mar 2014 09:48:43 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.924
X-Spam-Level:
X-Spam-Status: No, score=-1.924 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FM_FORGED_GMAIL=0.622, HTML_MESSAGE=0.001, NORMAL_HTTP_TO_IP=0.001, RP_MATCHES_RCVD=-0.547, 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 fFh657izQ6oq for <rtcweb@ietfa.amsl.com>; Fri, 14 Mar 2014 09:48:36 -0700 (PDT)
Received: from mail-vc0-x229.google.com (mail-vc0-x229.google.com [IPv6:2607:f8b0:400c:c03::229]) by ietfa.amsl.com (Postfix) with ESMTP id 646DB1A016F for <rtcweb@ietf.org>; Fri, 14 Mar 2014 09:48:36 -0700 (PDT)
Received: by mail-vc0-f169.google.com with SMTP id ik5so3109314vcb.0 for <rtcweb@ietf.org>; Fri, 14 Mar 2014 09:48:29 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; bh=HTypKET3gkcrip/S+Nb9rol80ovJoZ9lzOp1eGWHHMs=; b=fHCy+/IxtThTFtKDZLzDdUv6aSW1YIa9MWnMBifQDmwT+S4AvIzOLKwcrTxou9i1u4 jv1hUwktZXY+jf9yYshN9+uwNLiwRO9aeHxVDh1HbySrA7Nygvh/8OzI7/hUYh6fkm8D dUSAsxfl0xT6/eCTJiCRwjH8oVnSK4H34iet/g5LMuXI4AG6kKLhZEgZXeg8jPCCgp0N mkBKtYQst4PIXhWpAcyx11x7tQYk4kOltiBda4uC3pyOeH8qh4IdgDdu9hJnomsNarhc Q4J6VOxxhGHKouIJ3TWLL/gdgYyIjZD8xcVaX5zFhaSOjIFy81VsKubOpvxqPxxReJb1 aSRw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=HTypKET3gkcrip/S+Nb9rol80ovJoZ9lzOp1eGWHHMs=; b=hE3tIfTwMgA42W/crHLyUTQ2bF+Jdu9SOyTSFYcSPzvulm+CYWf38TImpH2ArJW/MG pcr3//jGP+TB7oXfHvirwuxXKJ8irmLna0X+4YN1mf9tTsgOmGX/2HxbAsyBPHGoMw79 2LZ8LXLf3w8yI/kGCJgOrzd3WSIprhs5KvSJtzIHKr5/+VdLsZ5pj/DunB20qRkBbKTC q3yFFYKhGw910UxikhCdQmcWwgcvCGClyuM+JC84DsFDn5fobVZBo8TnJeTftIjKd7Kn OZRciq1g9RN+5ECLAGpyxUyocQ6S4ZilD97QY4v9xXB9gy3QkcF+PyJLAk1CtbW7mDE1 1MRA==
X-Gm-Message-State: ALoCoQnQaq/9vW/qFaT/DirnOZk1SRnhWj1BgIHxTTcaJRi6fvQ3UyvINCSgvkgIZ8Cel++QaFICFVGb1GWuraBpW1A6OylDtT2dx+T7IrjSw6LQGpHBS79VFeQpD1N+csjrPMd/UIOVuKqtSDz4MVofBwU4j8R6/JR4/3rEuUImhm3fcxpV0Mr6Rb1n/akS48nLI77g6jYB
X-Received: by 10.221.74.65 with SMTP id yv1mr1034191vcb.31.1394815709069; Fri, 14 Mar 2014 09:48:29 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.52.26.43 with HTTP; Fri, 14 Mar 2014 09:48:07 -0700 (PDT)
In-Reply-To: <5322FE7F.9040007@ericsson.com>
References: <5322FE7F.9040007@ericsson.com>
From: Justin Uberti <juberti@google.com>
Date: Fri, 14 Mar 2014 09:48:07 -0700
Message-ID: <CAOJ7v-0aA753PWhULBjiwhXv+bz4CRK+0GPVn7zsvzdg53uPcg@mail.gmail.com>
To: Magnus Westerlund <magnus.westerlund@ericsson.com>
Content-Type: multipart/alternative; boundary=001a11365638dddf9f04f493d6f3
Archived-At: http://mailarchive.ietf.org/arch/msg/rtcweb/E4xwSZkIb3bqodW_5A7nzmuw4eE
Cc: draft-ietf-rtcweb-jsep@tools.ietf.org, "rtcweb@ietf.org" <rtcweb@ietf.org>
Subject: Re: [rtcweb] Comments on draft-ietf-rtcweb-jsep-06
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: Fri, 14 Mar 2014 16:48:43 -0000

Thanks for the detailed review. We will work through these comments over
the next few days.


On Fri, Mar 14, 2014 at 6:05 AM, Magnus Westerlund <
magnus.westerlund@ericsson.com> wrote:

> Hi,
>
> I did review the JSEP -06 prior to the meeting. Now I have had time to
> write-up my comments.
>
> 1) Section 1):
>
> [W3C.WD-webrtc-20111027]
> This is very old version of the API, please update to the currently
> relevant version.
>
> 2) Section 3.4.1.1:
> The m-line index is a zero-based index, referring to the Nth m-line in
> the SDP.
>
> a) I think the above is a bit confusing. As a zero-based index would
> mean that the first m= section would have index 0. Thus, the referring
> to the Nth m-line in the SDP is unclear. Is N, the provided index, in
> which case it would be the N+1th m-line. Please rewrite.
>
> b) I also wonder if one has to be clearer on what "the SDP" is. I assume
> it is the one that has been generated by the agent, rather than the
> latest sent or received?
>
> 3) Section 3.4.1.1:
> "The MID uses the "media stream identification", as defined in
>    [RFC5888] , to identify the m-line."
>
> a spurious space after ref.
>
> 4) JSEP applications typically inform the browser to begin ICE gathering
>    via the information supplied to setLocalDescription, as this is where
>    the app specifies the number of media streams to gather candidates
>    for.
>
> I find the statement that ICE candidate gathering starts first with
> setLocalDescription. Doesn't the created offer already contain local
> candidates, thus one must have already gathered some candidates? To me
> it appears reasonable to start gathering earlier, rather than later.
> Thus, as soon as one create the PeerConnection object and have added any
> MST to it or a data channel, one can start gathering.
>
> 5) Section 3.5.2:
>    In the parallel forking case where the Javascript application wishes
>    to simultaneously exchange media with multiple peers, the flow is
>    slightly more complex, but the Javascript application can follow the
>    strategy that [RFC3960] describes using UPDATE.  (It is worth noting
>    that use cases where this is the desired behavior are very unusual.)
>
> I mostly react to the last sentence within the paragraph. I think the
> poster child for semi-parallel forking was the MMORG that like to
> establish PCs with anyone that is coming close in the virtual world.
> Thus either needs to have a number of PCs on standby or have just one,
> but treat that as parallel forking. And if you like to discourage the
> later, then I think you might need to at least inform about the
> possibility to have stand-by PCs.
>
> 6. Section 3.6:
>
>    In the event that the local application state is reinitialized,
>    either due to a user reload of the page, or a decision within the
>    application to reload itself (perhaps to update to a new version), it
>    is possible to keep an existing session alive, via a process called
>    "rehydration".  The explicit goal of rehydration is to carry out this
>    session resumption with no interaction with the remote side other
>    than normal call signaling messages.
>
> I think this paragraph makes two erronous statements:
> a) "keep an existing session alive"
> I think "resume" an existing session would be more correct. Because it
> would be broken when the reload happens.
>
> b) "with no interaction with the remote side"
> My understanding is that due to DTLS and key handling a DTLS
> renegotiation will be required with the peer. Isn't also a signalling
> exchange likely to be required due to change of local DTLS cert as well
> as ICE candidates as parameters? Any other parameters that will be
> required to exchanges when rehydrating?
>
> 7. Section 3.6:
> Next, a new offer is
>    generated by the new PeerConnection; this offer will have new ICE and
>    possibly new DTLS-SRTP certificate fingerprints (since the old ICE
>    and SRTP state has been lost).
>
> I think this may require quite a bit more details. A forward pointer to
> where you will specify that?
>
> 8. Section 3.6:
>    [OPEN ISSUE: EKR proposed an alternative rehydration approach where
>    the actual internal PeerConnection object in the browser was kept
>    alive for some time after the web page was killed and provided some
>    way for a new page to acquire the old PeerConnection object.]
>
> Can't we declare this open issue closed? I have seen no details on the
> alternative proposal. Thus, lets work with the one that is present.
>
> 9. Section 4.1.1:
> There is no discussion of how the bundle policies relate to the Data
> Channels? Are they included or do they have a specific set of policies.
> Please extend this to discuss this.
>
> 10. Section 4.1.3:
>
> This text doesn't appear to contain a requirement on that one MUST have
> called setRemote prior to being able to call it.
>
> 11. Section 4.1.4.1:
>    While this could also be done with an inactive "pranswer", followed
>    by a sendrecv "answer", the initial "pranswer" leaves the offer-
>    answer exchange open, which means the caller cannot send an updated
>    offer during this time.
>
> Should one mention that also the callee can't send an offer without
> doing a "final" answer?
>
> 12. Section 4.1.4.1
>
> By the time the human has accepted the call and
>    triggered the new offer, it is likely that the ICE and DTLS
>    handshaking for all the channels will already be set up.
>
> Wouldn't the handshaking have "finished" rather than "set up"?
>
> 13. Section 4.1.4.2:
>    Rollback can only be used to cancel proposed changes; there is no
>    support for rolling back from a stable state to a previous stable
>    state.
>
> What is meant with "proposed changes"? From my side it is not clear how
> much you can do after having performed a SetLocal or SetRemote. I think
> these needs to be treated separately.
>
> For an offerer, they can createOffer, SetLocal, send to peer, receive
> reject and then undo the SetLocal. That is straight forward, and if they
> done the SetRemote, they would be consider in a new Stable state and
> can't roll back anymore?
>
> For the answering side, it receive an offer, does SetRemote,
> CreateAnswer, determines that it doesn't like it, the can undo the
> SetRemote and send a reject message to offerer.
>
> But what about this case? it receive an offer, does SetRemote,
> CreateAnswer, SetLocal and sends the message, then receive a reject from
> the peer, because it didn't like the answer? Are the answerer then in a
> stable state that can't be unrolled, or?
>
> 14. Section 4.1.5:
>    This API indirectly controls the candidate gathering process.  When a
>    local description is supplied, and the number of transports currently
>    in use does not match the number of transports needed by the local
>    description, the PeerConnection will create transports as needed and
>    begin gathering candidates for them.
>
> Another part of an earlier question regarding what triggers ICE gathering?
>
> 15. Section 4.1.7:
>    TODO: Do we need to expose accessors for both the current and
>    proposed local description?
>
> This needs to be resolved, I don't know if I can answer this question.
>
> 16. Section 4.1.9:
>
> Should this section discuss that it might be browser that has overriding
> controls regarding if other candidates then relay ones may be provided,
> i.e. privacy mode?
>
> 17. Section 4.1.9:
>    Regardless of the configuration, the gathering process collects all
>    available candidates, but excluded candidates will not be surfaced in
>    onicecandidate callback or used for connectivity checks.
>
> Doesn't this result in both resource consumption and that the configured
> TURN server will learn the external IP address and port for the client?
>
> 18. Section 5.2.1:
>
> Structure in relation between RTP media and DATA channel. Maybe you
> should try to find a structure that makes the difference between the two
> types of transport more clear, and what applies to the perspective.
>
> 19. Section 5.2.1:
>
>
>    o  For the current default candidate, a "c=" line, as specific in
>       [RFC5245], Section 4.3., paragraph 6.  If no candidates have been
>       gathered yet, the default candidate should be set to the 'null'
>       value defined in [I-D.ietf-mmusic-trickle-ice], Section 5.1.
>
> What about host candidates?
>
> 20. Section 5.2.1:
> The CNAME must be generated
>       in accordance with draft-rescorla-random-cname-00.  [OPEN ISSUE:
>       How are CNAMEs specified for MSTs?  Are they randomly generated
>       for each MediaStream?  If so, can two MediaStreams be synced?]
>
> I think you should point to the RTP usage for this particular piece.
> Because we do have text there both for the CNAME generation and how
> CNAMES among MSTs and between PeerConnection. Likely to be changed soon
> based on the mailing list discussion.
>
> 21. Section 5.2.1:
>    o  [OPEN ISSUE: Handling of a=imageattr]
>
> Yes, I think imageattr should be supported to give guidance on suitable
> resolutions that an endpoint like to receive for a particular MST.
>
> 22. Section 5.2.1:
>    Once all m= sections have been generated, a session-level "a=group"
>    attribute MUST be added as specified in [RFC5888].  This attribute
>    MUST have semantics "BUNDLE", and MUST include the mid identifiers of
>    each m= section.
>
> Isn't this overriding BUNDLE as currently specified, forcing everything
> into a single bundle group. Thus, preventing the bundle policies to work.
>
> 23. Section 5.2.2:
>
>    o  If any MediaStreamTracks have been added, and there exist m=
>       sections of the appropriate media type with no associated
>       MediaStreamTracks
>
> Isnt' the last a "MediaStreamTrack" rather than plural ones?
>
> 24. 5.2.3.3.  VoiceActivityDetection
>
>    If the "VoiceActivityDetection" constraint is specified, with a value
>    of "true", the offer MUST indicate support for silence suppression by
>    including comfort noise ("CN") codecs for each supported clock rate,
>    as specified in [RFC3389], Section 5.1.
>
> As currently specified this text requires the usage of a comfort noise
> codec, currently there is no such on required. I think the "MUST" needs
> to be rewritten.
>
> 25. Section 5.3.1:
>                                                                   Note
>       that for simplicity, the answerer MAY use different payload types
>       for codecs than the offerer, as it is not prohibited by
>       Section 6.1.
>
> I react to the word simplicity here. I never consider usage of different
> PTs per direction anything simple. But, yes it is allowed by RFC 3264.
> But I kind of protest to encourage the usage.
>
> 26. Section 5.3.2 etc.
>
> When will we see a first draft of the text in the sections:
> 5.3.2, 5.3.3., 5.4, 5.5, 5.6, 5.7?
>
> I really like to see it prior to the Interim meeting.
>
> 27. Section 6:
>    Note: This section is still very early and is likely to significantly
>    change as we get a better understanding of a) the use cases for this
>    b) the implications at the protocol level c) feedback from
>    implementors on what they can do.
>
> Shouldn't this mention W3C discussion also?
>
> 28. Section 7 - Security Considerations:
>
> First of all there are no references to the Security Considerations or
> architecture document.
>
> Secondly, I would like to have some security discussion on significant
> issues with JSEP itself. Resource attacks on the local endpoint for
> example?
>
> 29. section 10
>
> There are a couple of draft references that are either published RFCs or
> at least WG versions. Please update the reference list.
>
> 30. Section 10.2:
> Looking at the spec, I think the following references are normative:
>
>    [I-D.ietf-mmusic-trickle-ice]
>               Ivov, E., Rescorla, E., and J. Uberti, "Trickle ICE:
>               Incremental Provisioning of Candidates for the Interactive
>               Connectivity Establishment (ICE) Protocol", draft-ietf-
>               mmusic-trickle-ice-00 (work in progress), March 2013.
>
>    [I-D.ietf-rtcweb-data-protocol]
>               Jesup, R., Loreto, S., and M. Tuexen, "WebRTC Data Channel
>               Protocol", draft-ietf-rtcweb-data-protocol-04 (work in
>               progress), February 2013.
>
>
> 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
> ----------------------------------------------------------------------
>
> _______________________________________________
> rtcweb mailing list
> rtcweb@ietf.org
> https://www.ietf.org/mailman/listinfo/rtcweb
>