Subject: Re: [AVTCORE] Review of draft-ietf-avtcore-rfc5764-mux-fixes-02

From: "Gonzalo Salgueiro (gsalguei)" <[email-masked]>
Date: Mon, 19 Oct 2015 22:16:08 +0000

Hi Magnus - 

We have published an -03 attempting to address your comments.  Please have a look and share any additional comments you have.

Thanks,

Gonzalo

> On Sep 17, 2015, at 4:10 PM, Marc Petit-Huguenin <[email-masked]> wrote:
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Hi Magnus,
> 
> Thanks for the review, and very sorry for the delay in responding to that.
> 
> See comments and answers below.
> 
> On 06/23/2015 06:24 AM, Magnus Westerlund wrote:
>> Hi,
>> 
>> I did a review of draft-ietf-avtcore-rfc5764-mux-fixes-02 and have
>> some comments:
>> 
>> A. Abstract:
>> 
>> It overrides the guidance from SRTP Extension for DTLS [RFC5764],
>> which suffered from three issues described and fixed in this
>> document.
>> 
>> There are four issues listed in Section 1.
> 
> It's back to 3, see below.
> 
>> 
>> B. Section 1, bullet 1 and 2:
>> 
>> "It implicitly allocated codepoints ..."
>> 
>> I am not particular happy about this formulation as it is only part
>> of the truth. I think the authors should try to write a separate
>> paragraph about the issue of overlapping value ranges, and the need
>> to maintain the separated for any usage where multiplexing could
>> occur. Thus this leading to an limitation of the usage of codepoints
>> in the STUN/TURN as well as TLS protocol.
> 
> I added some text to this effect.
> 
>> 
>> C. Section 1, bullet 4:
>> 
>> The current ranges are not efficiently allocated making it harder to
>> introduce new protocols that might require multiplexing.
>> 
>> I am not quite certain what this bullet tries to really say. What is
>> allocated in ranges, is the ranges available for registration that
>> doesn't reserve space for future protocols, or something else?
> 
> Not sure either.  I removed the bullet point.
> 
>> 
>> D. Section 1:
>> 
>> These flaws in the demultiplexing scheme were unavoidably inherited 
>> by other documents, such as [RFC7345] and 
>> [I-D.ietf-mmusic-sdp-bundle-negotiation].  These will need to be 
>> corrected with the updates this document provides.
>> 
>> It is unclear what the second "These" refers to, I guess "flaws" not
>> the documents.
> 
> No, the documents will need to be corrected to match the RFC-to-be.  Text updated.
> 
>> 
>> E. Section 1.1:
>> 
>> Also here I think the document could benefit from a more analytic and
>> less confrontational language. It is better to write that
>> "Multiplexing limits the available range for new STUN methods." than
>> "... entirely obliterating those STUN method codepoints".
> 
> But "limits" is not the truth either.  From The Concise Oxford Dictionary:
> 
> limit: n. 2 a restriction on the size or amount of something.
> v. set or serve as limit to.
> 
> Here we are talking about not just a restriction in size for the range, but the total elimination of the range.
> 
> I will rewrite to "[...] eliminating the possibility of having STUN method codepoints assigned by Designated Expert (i.e., the range 0x800 - 0xFFF)."
> 
>> 
>> F. Section 1.2:
>> 
>> I want to remind the authors that this document will go to TLS WG
>> during WG last call, and would recommend that one avoids to dictate
>> to that WG how their situation are:
>> 
>> "Unlike STUN, TLS is a mature protocol that is already well
>> established and widely implemented and thus we expect only relatively
>> few new codepoints to be assigned in the future."
>> 
>> Considering the quite significant work ongoing on TLS, I think you
>> might be wrong in this analysis.
>> 
>> Focus on the facts and the goal here. I think the goal is to avoid
>> having future assignment of functions cause failure in the
>> multiplexing scheme.
> 
> I removed the sentence altogether.
> 
>> 
>> G. Section 1.
>> 
>> I am missing a clearer description of the long term goal with this
>> update. To my understanding it is to avoid future allocation in any
>> of the affected protocols to result in that multiplexing becomes
>> impossible when using that feature.
>> 
>> A second goal appear to enable the possibility that additional
>> protocols should be able to be given a range for them to multiplex. I
>> think this second one can result in more discussion, as there are
>> multiple people that express a preference for more explicit
>> demultiplexing.
>> 
>> In addition I think the method for achieving these goals should be
>> made more clear. That is to make it clear in the IANA registries
>> which values that are subject to risks for collisions and where
>> coordination between protocols may be required. That is at least what
>> I thought was the conclusion of the Dallas meeting.
> 
> Added the following tentative paragraphs in the introduction that states these goals:
> 
> "The first goal of this document are to make sure that future
> allocations in any of the affected protocols are done with the full
> knowledge on their impact on multiplexing.  This is achieved by
> modifying the IANA registries with instructions for coordination
> between the protocols at risk.
> 
> A second goal is to permit to add new protocols to the list of
> multiplexed protocols in a manner that does not break existing
> implementations."
> 
>> 
>> H. Section 1.2: 0-19    Reserved (MUST be allocated with Standards
>> Action)
>> 
>> As I EKR commented, as these values anyway require Standards action
>> this is meaningless. I think something more like (Requires
>> coordination, see RFCXXXX). Where RFCXXXX is this document would be
>> more acceptable and also much more informative.
> 
> Done.
> 
>> 
>> This relates to what I see as the need for this document to much
>> better explanation of the goals and future considerations needed to
>> avoid failure in allocations.
>> 
>> I. Section 1.3:
>> 
>> An implementation that uses the source IP address and port to 
>> identify TURN channel messages MAY not need to restrict the channel 
>> numbers to the above range.
>> 
>> I think we need to have discussion about this. I personally think it
>> makes much more sense to not limit the other protocols due to TURN,
>> as an TURN client MUST know which address the TURN server uses. Thus
>> limiting TURN channel demultiplexing based on source address appears
>> trivial.
>> 
>> If I misunderstanding this, then I think this section must before
>> giving any recommendation make it clear why src address is not
>> sufficent and we actually want to consume part of the value space for
>> this?
>> 
>> Note, I do think it is valuable to discuss TURN channels here, but I
>> think it needs to reconsider what the message should be.
> 
> As for the content of this paragraph, it should not have said "MAY" here, as all the 1.x sections are non-normative. Section 3 should have been modified to say that the receiver may first use to source IP/port to demultiplex the TURN channel packets.
> 
> But we need a WG decision on this.  If we say that Channel demux is optionally done using the source IP/port then we still have to reserve the 64-79 prefix for receivers that decide to not use this.
> 
> If on the other hand we decide to not reserve the 64-79 prefix for the TURN channel demux, then demuxing using the source IP/port is a MUST.
> 
> For now I just removed the whole sentence in the text.
> 
>> 
>> J. Section 1.4:
>> 
>> This is insufficient to explain any issues why the order of testing
>> do matters?
> 
> I do not understand the question.
> 
>> 
>> K. Section 3:
>> 
>> When new values or ranges are added, they MUST be tested in
>> ascending order.
>> 
> 
> This text should not talk about adding new values/ranges. Changed the text to:
> 
> "The various range values for the first byte MUST be tested in ascending order."
> 
>> I am missing the text that makes it clear how one would go about to
>> add allocate a new value or range when proposing to add a new
>> protocol.
> 
> One just add it to the respective IANA registry.  The registries will be modified to remind the registry expert that mux/demux should be taken in account.
> 
>> 
>> L. Section 5.
>> 
>> It doesn't simply update existing registries. It does currently
>> specifies that TURN channels can be demultiplexed. Are there
>> implications of that?
> 
> For now I am assuming that only demuxing is used for TURN channel, so I updated the text for this.
> 
> I also updated the text to also say that the testing order is now specified.
> 
>> 
>> M. Section 6:
>> 
>> I think all the "Reserved" messages are the wrong ones. I think they
>> should say "Reserved (DTLS-SRTP multiplexing collision avoidance, see
>> RFC XXXX)
> 
> Done.
> 
>> 
>> N. Section 6.1:
>> 
>> I think this needs to be more explicit about what it re-assigns. It
>> needs to say that the values available for allocation i.e. 0-FFF are
>> being changed to instead have the following ranges.
>> 
>> Then I think a resulting table for the ranges would be good.
>> 
> 
> This is already explained in section 1.1, and the resulting table is also in 1.1 (which is referenced in the text in 6.1).  Do you think that the instructions to IANA should have this level of detail?
> 
> - -- 
> Marc Petit-Huguenin
> Email: [email-masked]
> Blog: http://blog.marc.petit-huguenin.org
> Profile: http://www.linkedin.com/in/petithug
> 
> 
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
> 
> iQIcBAEBCAAGBQJV+x4lAAoJECnERZXWan7E3G0P/1Gzg6OGRaT48DxnyF/2Tq/p
> pg9b32J5OMGMP9BGXnlD9MVXuT3TbYg/EcOCjADSTGSxw6yIWiBca65rW4EXci5f
> VRi6OyXBBpXGzRLGJd2Gj7Tw2n5d9yO1I9ECxFDWdm0BzADIRCsUn+5MJIA8CHCk
> iGhFxKDKEGE2+ltQZf7+1XA0kHk363RyjnG1t9y6+6aJdN5F8WBEgNRc53GowPeW
> b19lzn40tHkzvG6jCMPjSmsx1zKDVbJxb8xGwp/Yid29zw9qWCjs3kgzK6iEdbsy
> ai4IsbxB7co9yw9C6LvzEFZPmHJMgDooJxkB1hJWBz0gAarkhCqsgbHfKGZgD9yw
> rJwE9TL9mX4OuCAV0lyUXC5uNrbJlL+2HhXeZmQ7Oh3PE+C+IBuLlOtgY+b/6L0y
> litA5X4k8qR71ocYLwTM3Oy+5hEapUs3ZVoE4Kz1Mt3/Ic5ryhg4Nk1gSwjWHOky
> 32APLxuV7YCGNGWKnj+NPRb1LKNwp7QMr6lcglacjWNubiEj/9TZmSBm0eese0bL
> WWTiX4a9kPMbyanZ6f1UnMXzdunw/8F2SLotG686quIovSM30pvfZWoUBYhXkUjI
> 9ejEHaVdBxYRh2Zq+t8mz+Vj6jHbm2bQ7bL4Q7VYn5YG5GTTzT5s2z4lz7UMTG2N
> JEXyPcPeZCIDircn4WMM
> =/7Es
> -----END PGP SIGNATURE-----
> 
> _______________________________________________
> Audio/Video Transport Core Maintenance
> [email-masked]
> https://www.ietf.org/mailman/listinfo/avt

---

From: "Gonzalo Salgueiro (gsalguei)" <[email-masked]>
Date: Mon, 19 Oct 2015 22:19:58 +0000

Hi Paul - 

We have published an -03 addressing Magnus’ comments.  We don’t believe this has addressed your concern, so we have a TODO note in the draft to still address your comment.  We aren’t sure how best to resolve this yet.  Any specific text proposals based on -03 would be very helpful at this point.

Thanks,

Gonzalo

> On Sep 25, 2015, at 10:25 AM, Paul Kyzivat <[email-masked]> wrote:
> 
> On 9/25/15 6:33 AM, Marc Petit-Huguenin wrote:
>> Hi Paul,
>> On 09/18/2015 10:17 AM, Paul Kyzivat wrote:
>>> On 9/17/15 4:10 PM, Marc Petit-Huguenin wrote:
>>> 
>>>>> K. Section 3:
>>>>> 
>>> I just reviewed this, and I have no idea what it means. I see nothing
>>> about Figure 3 that requires testing to be done in a particular
>>> order. The *text* suggests an if/then/else or switch statement
>>> implementation. But the code generated for a switch statement may
>>> well be a lookup table and so have no particular ordering.
>>> 
>>> The only time ordering would matter would be if the ranges
>>> overlapped. And IIUC the goal here that the ranges *not* overlap.
>> 
>> The reason we added this was to plan when a new overlapping range is added (mostly because someone ignored the advices provided by this draft).  If the algorithm says today that range must be checked in ascending order, then *all* implementations will work (or, most likely, fail) in a similar way.  That sentence is meant to explicitly prevent using a lookup table or perfect hashing or whatever technique that would prevent things to fail identically everywhere.
>> 
>> That said the sentence can probably be improved.
> 
> Yes.
> 
> Do you consider overlapping ranges to be an error, and want the error to be handled deterministically? Or are you thinking that there may be overt changes that legally define overlapping ranges and want to make the meaning deterministic?
> 
> If there is a deterministic choice when ranges overlap, then that seems to effectively update one of the ranges so that they don't overlap.
> 
> 	Thanks,
> 	Paul
>

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 20 Oct 2015 07:19:41 +0000

Hi,

I assume we don't need to update BUNDLE because of this, as BUNDLE doesn't specify the details of the multiplexing, but simply refers to RFC 5764 (which means it implicitly refers to this draft):

BUNDLE currently says:

	"Section 5.1.2 of [RFC5764] describes a mechanism to identify the
	   protocol of a received packet among the STUN, Datagram Transport
	   Layer Security (DTLS) and SRTP protocols (in any combination).  If an
	   offer or answer includes bundled "m=" lines that represent these
	   protocols, the offerer or answerer MUST support the mechanism
	   described in [RFC5764], and no explicit negotiation is required in
	   order to indicate support and usage of the mechanism."

Regards,

Christer

-----Original Message-----
From: avt [mailto:[email-masked]] On Behalf Of Gonzalo Salgueiro (gsalguei)
Sent: 20. lokakuuta 2015 1:20
To: Paul Kyzivat
Cc: Marc Petit-Huguenin; [email-masked]
Subject: Re: [AVTCORE] Review of draft-ietf-avtcore-rfc5764-mux-fixes-02

Hi Paul - 

Thanks,

Gonzalo

>>> I just reviewed this, and I have no idea what it means. I see 
>>> nothing about Figure 3 that requires testing to be done in a 
>>> particular order. The *text* suggests an if/then/else or switch 
>>> statement implementation. But the code generated for a switch 
>>> statement may well be a lookup table and so have no particular ordering.
>>> 
> Audio/Video Transport Core Maintenance [email-masked]