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

[Magnus Westerlund <[e-mail]>]

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  [phone]
Färögatan 6                 | Mobile [phone]
SE-164 80 Stockholm, Sweden | mailto: [e-mail]
----------------------------------------------------------------------