Re: [MMUSIC] Review draft-ietf-mmusic-sdp-mux-attributes-06

[Suhas Nandakumar <[e-mail]>]

Hello Thomas

  Sorry for the delayed response. Please see responses inline.

Thanks
Suhas

On Fri, Jan 9, 2015 at 7:52 AM, Stach, Thomas <[e-mail]>
wrote:

>  Suhas,
>
>
>
> I did a review of draft-06.
>
> Since started late, I only came until the end of session 6.
>
> It looks already quite mature, but I have the following comments.
>

[Suhas] Many thanks.

>
>
> Major (more or less)
>
>
>
> 5.7 a=crypto
>
> I don’t think the text goes far enough.
>
> The category TRANSPORT can only be applied if one assumes that all bundled
> m-lines contain the a=crypto attribute and together with the same crypto
> suite.
>
> Otherwise, if you want to bundle m-lines offering SRTP with some that
> offer RTP, the RTP m-lines would have inherit the SRTP master key and would
> be encrypted.
>
> One resolution could be a restriction to bundle only m-lines that all
> offer SRTP and use the same SRTP keying mechanism.
>

[Suhas]. This was the assumption made as required by Section 10.1.1 of
BUNDLE draft which mandates the proto value for all the m=lines BUNDLED
MUST be same. Thus bundling RTP/AVP with RTP/SAVP for example is not
allowed by design.


> Alternatively, you could also apply the NORMAL category and use the key
> material as provided for the corresponding m-line, if any.
>
> The correct key material for a specific RTP stream would have to be
> determined via the RTP header extension from draft-bundle.
>
> Using NORMAL in such a way would also allow for bundling of RTP and SRTP
> m-lines, even if the keying material is provided by different key exchange
> mechanism like MIKEY and SDP Security Descriptions.
>

[Suhas] - I would prefer deferring this to future specifications on
defining the procedures to use different keying mechanisms for SRTP keys in
a given Bundle group.

OTOH even if there are m=lines that differ in the keying mechanisms, the
category TRANSPORT implies that the keying mechanism corresponding the
BUNDLE address needs to be considered. So the Offerer/Answerer has a choice
on choosing which m=line's BUNDLE-tag appears first in the BUNDLE group.


>
>
> 5.14 a=rtcp attribute
>
> The category needs to be changed from IDENTICAL to TRANSPORT.
>
> Otherwise it conflicts with
> https://tools.ietf.org/html/draft-ietf-mmusic-sdp-bundle-negotiation-14#section-10.3.2
>
> In case of a=rtcp-mux draft-bundle prescribes a=rtcp with a port identical
> to the RTP port and deliberately avoided choosing identical RTP ports in
> the m-lines.
>
> If ICE is used, a=rtcp would carry the default RTCP candidate, which again
> would not necessarily be identical for all m-lines.
>

[Suhas] : Completely agree. Thanks for catching this. I will update the
draft to reflect the same

>
>
> 5.35 a=key-mgmt attribute
>
>
>
> The table is a bit misleading as it seems to imply that there is a a=mikey
> attribute, where the prtcl-id  value in the a=key-mgmt attribute is
> actually meant. Similar as for section 5.7 the prtcl-id  would need to be
> the same for all bundled m-lines.
>
> The key-mgmt-data part (i.e. keying material) of the a=key-mgmt attribute
> could however still be different for each m-lines similar as for the
> a=crypto attribute.
>
> Due to the similarities with a=crypto (or a=fingerprint in section 5.36) I
> think that TRANSPORT (or NORMAL as proposed above) is a better fit than
> IDENTICAL.
>

>
> 5.39 ZRTP
>
> Similar to section 5.7, the RTP header extension from draft-bundle would
> allow that the a=ZRTP-hash can be associated to the corresponding media
> stream. With that I don’t see why this can’t be changed from NOT
> RECOMMENDED to NORMAL.
>
> TRANSPORT couldn’t probably used as not all m-lines are required to use
> a=ZRTP-hash
>

[Suhas] :  NOT RECOMMENDED still seems to be appropriate for the very same
reason that some of the media lines might not have zrtp-hash undefined.
However, in future if there exists a specification that describes the
header extension for using zrtp-hash in BUNDLE scenario, we can always
upgrade the mux category referencing that specification.



>  5.45 IKE
>
> Would similar consideration as for section 5.7 apply to a=
> psk-fingerprint?
>

[Suhas] - IDENTICAL seems  to be appropriate to ensure that we end up using
only one IPSec association for all of the "m=" lines thus simplifying the
design.

>
>
> I haven’t looked into the 3GPP stuff in 5.49-5.53.
>
>
>
> Minor
>
>
>
> Section 4.9 Category TBD
>
> Do we need this? It would look awkward to see that in a IANA registry.
>
>
>
> TBD seems to apply to attributes related to draft-ietf-rmt-flute-sdp-03
>
> According to
> https://datatracker.ietf.org/doc/draft-ietf-rmt-flute-sdp/history/  that
> draft seems to be dead.
>
> So, it might not do any harm if we put it into the NOT RECOMMENDED
> category.
>
>
>
> Furthermore TBD applies to  a=chatroom  and a= 3gpp.iut.replication.
>
> a=chatroom is defined in
> http://tools.ietf.org/html/draft-ietf-simple-chat-18#section-8 which in
> the RFC Editor’s queue.
>
> Since this attribute is related to MSRP it might as well fit into the
> NORMAL category as was applied for e.g. section 5.57.
>
> I haven’t thought much about this, however.
>
>
>
> a= 3gpp.iut.replication is defined in
> http://www.etsi.org/deliver/etsi_ts/124300_124399/124337/12.00.00_60/ts_124337v120000p.pdf
>
> Maybe somebody with IMS expertise can advise here.
>
> Otherwise, since the default category for TBD is NOT RECOMMENDED we could
> use this as initial value, subject to change.
>
> BTW: This spec also defines a=3gpp.iut.controllee which is not (yet?)
> registered in IANA.
>
>
>

[Suhas] TBD was added as a way to indicate the attributes for which we
don't have much information at this point of thw draft submission or to
cater to those attributes that might get added by the time this draft gets
a RFC number. Hence I would prefer to keep it that way even though I agree
with your point that it looks bit awkward



>
>
> Nits
>
> Section 4.4
>
>    Thus the calculated AS value would be 256+64 bytes for the given
>
>    example.
>
> The default unit would be kilobits per second, not bytes
>

[Suhas] .. Thanks , will update as suggested.


>
> Section 4.5
>
> The link for  [I-D.ietf-mmusic-sdp-bundle-negotiation] does not work
> correctly.
>
>
>

[Suhas] .. Will update the reference appropriately.

>  … the mid appearing first on the a=group:BUNDLE line …
>
> Draft-bundle now calls this “BUNDLE-tag”
>
>
>
> Section 6.3
>
> The note in the table entry for bwtype:TIAS does not parse.
>
>
[Suhas] .. Thanks , will fix it

>
>
> That’s all up to section 6.
>
> I’ll try to go through the remainder of the draft asap and would follow up.
>
>
>
> Regards
>
> Thomas
>
>
>