cdh4u / draft-sdp-bundle

1 stars 9 forks source link

Ambiguities are possible if "m=" sections are moved between BUNDLE groups. #26

Closed taylor-b closed 6 years ago

taylor-b commented 7 years ago

Suppose I'm bundling A and B together in one group, and C and D together in a separate group. Now, in a subsequent offer, I start bundling A/C together, and B/D together. Depending on the protocol(s) used, it may be ambiguous which underlying transport objects are being changed. Is A joining C's group, or is C joining A's group?

Practical example where this matters: ICE. If I change the ufrag/password to do an ICE restart at the same time as I'm shuffling "m=" sections, ICE is supposed to send media using the previously selected candidate pair until a new pair is formed. But which previously selected pair should it use for the "A/C" group? The "A/B" group's previously selected pair, or the "C/D" group's? The same issue exists for TCP connection reuse.

The fundamental issue is that there's no reliable way to identify BUNDLE groups between subsequent offer/answer exchanges, if this kind of shuffling is allowed. We can fix this pretty easily by just disallowing this type of shuffling.

cdh4u commented 7 years ago

Is this issue BUNDLE specific?

Assume you have two un-bundled m- lines, A and B. Then you shuffle the addresses. Don't you have the same issue?

taylor-b commented 7 years ago

Continuing the example of ICE: that's not possible, since the ICE ufrag/password are chosen randomly.

So you could not go from:

m=audio 9 ...
a=ice-ufrag:foo
...
m=video 9 ...
a=ice-ufrag:bar

To:

m=audio 9 ...
a=ice-ufrag:bar
...
m=video 9 ...
a=ice-ufrag:foo

But even if you did, it wouldn't be ambiguous which previously-selected ICE candidate pair should be used, because there's a fixed mapping between ICE checklists and media descriptions.

cdh4u commented 7 years ago

Aren't there some requirements regarding the uniqueness-in-time-and-space when generating ICE ufrags and/or passwords?

taylor-b commented 7 years ago

Just that they're chosen randomly.

cdh4u commented 7 years ago

If I remember correctly, Roman told me there are some requirements on the randomness.

taylor-b commented 7 years ago

Right, that's what I'm saying. Specifically:

The ice-ufrag attribute MUST contain at least 24 bits of randomness, and the ice-pwd attribute MUST contain at least 128 bits of randomness.

I'm not seeing how this relates to the reported issue, though.

cdh4u commented 7 years ago

My point is that it is very unlikely that the scenario you describe would occur.

taylor-b commented 7 years ago

The shuffling scenario? Then can we just say it's not supported? Meaning: a subsequent may move m= sections from one BUNDLE group to another, but it must not simultaneously move them in and out.

cdh4u commented 7 years ago

Do we really need to say that? Implementors need to THINK about things that can go wrong :)

taylor-b commented 7 years ago

Well... I'm an implementer, and thinking about it, which is why I brought it up. The problems in my opinion are:

NOTE: If an "m=" line, when being moved out of a BUNDLE group, is added to another BUNDLE group, the offerer applies the procedures in [Section 8.5.2] to the "m=" line.

cdh4u commented 7 years ago

There will be a group:BUNDLE attribute for each bundle group. So, when you are adding an m- line to a bundle group, you place the identification-tag in the attribute associated with that bundle group - no matter whether the m- line previously belonged to another bundle group or not.

taylor-b commented 7 years ago

There will be a group:BUNDLE attribute for each bundle group

How does an answerer know which "group:BUNDLE" attribute is "associated with" which BUNDLE group from the previous offer/answer exchange? That's the issue here.

cdh4u commented 7 years ago

The identification-tag value for each m- line needs to stay the same (maybe that's something we need to specify), so that the answerer can see that the m- line is moving from one bundle group to another.

taylor-b commented 7 years ago

The identification-tag value for each m- line needs to stay the same (maybe that's something we need to specify)

Hmm, it appears that's only a SHOULD in RFC5888. I agree we need to specify this.

the answerer can see that the m- line is moving from one bundle group to another

This is not possible in the original example:

Suppose I'm bundling A and B together in one group, and C and D together in a separate group. Now, in a subsequent offer, I start bundling A/C together, and B/D together

Initial offer:

AB - Group 1 CD - Group 2

Subsequent offer:

AC - Could be group 1, if the groups are trading B and C. Or group 2, if they're trading A and D. BD - Could be group 2, if the groups are trading B and C. Or group 1, if they're trading A and D.

Which group is which? Is it based on ordering of "a=group" attributes? That would be one fix. "Groups offered in a subsequent offer MUST appear in the same order as previously established groups. New groups MUST appear after existing groups. This allows groups to be identified between offer/answer exchanges, even in cases that involve moving 'm=' sections between groups."

cdh4u commented 7 years ago

I am not sure whether the groups need to be in a particular "order". The first tag in each group:BUNDLE attribute defines the properties (BUNDLE address etc) of the bundle group associated with that attribute, and all other tags associated with the same attribute share those properties.

So, in your example: after the initial offer, A and C will define the properties for each bundle group, and after the subsequent offer A and B will do it.

If the answerer can not process a subsequent offer like this (e.g., due to technical reasons, or whatever) it should simply reject it. I do not think we need to forbid it in the standard.

taylor-b commented 7 years ago

There's no ambiguity about which properties apply to which groups. There's ambiguity about which groups are which between successive offers/answers, which is a problem for connection-oriented protocols. I'm not sure how to make this more clear... Does this diagram help?

screen shot 2017-02-12 at 10 11 17 am
taylor-b commented 7 years ago

Here's another example if that helps:

Initial offer from endpoint 1:

a=group:BUNDLE A B
a=group:BUNDLE C D
m=image 10000 TCP
c=10.0.0.1
a=mid:A
a=setup:passive
m=image 20000 TCP
c=10.0.0.1
a=mid:B
a=setup:passive
m=image 30000 TCP
c=10.0.0.1
a=mid:C
a=setup:passive
m=image 40000 TCP
c=10.0.0.1
a=mid:D
a=setup:passive

Answer from endpoint 2:

a=group:BUNDLE A B
a=group:BUNDLE C D
m=image 9 TCP
c=10.0.0.2
a=mid:A
a=setup:active
m=image 9 TCP
c=10.0.0.2
a=mid:B
m=image 9 TCP
c=10.0.0.2
a=mid:C
a=setup:active
m=image 9 TCP
c=10.0.0.2
a=mid:D

Subsequent offer from endpoint 2:

a=group:BUNDLE C B
a=group:BUNDLE A D
m=image 9 TCP
c=10.0.0.2
a=mid:A
a=setup:active
a=connection:existing
m=image 9 TCP
c=10.0.0.2
a=mid:B
m=image 9 TCP
c=10.0.0.2
a=mid:C
a=setup:active
a=connection:existing
m=image 9 TCP
c=10.0.0.2
a=mid:D

The answerer knows that the offerer wants to re-use the two existing connections. But which does it want to re-use for the "B C" group, and which does it want to re-use for the "A D" group?

cdh4u commented 7 years ago

There may be something I am missing here, but when you send the the subsequent offer the suggested connections for the BUNDLE groups will be according to the address:port of C and A. If the address:port does not correspond to an existing connection a new connection will have to be created.

taylor-b commented 7 years ago

according to the address:port of C and A

The ports of A and C are both "9". Assume they have the same address as well (I'll add the "c=" lines to the example, I had just omitted them for brevity).

Let me try explaining it another way: Pre-BUNDLE, there are SDP semantics that rely on using the "m=" section index to identify a transport. With BUNDLE, the BUNDLE group identifies the transport instead. But this can't be done when there's ambiguity about which BUNDLE group is which.

Your point seems to be "you can use the address to identify the transport instead", but I've been talking specifically about situations where you can't.

cdh4u commented 7 years ago

I don't think you can add an m- line with port 9 to a BUNDLE group, and request the address:port of that m- line to be used as BUNDLE address. Within a BUNDLE group, one m- line always defines the transport for that group.

So, perhaps there are some restrictions that need to be done, but I don't think a general rule saying that an m- line must not be moved from one BUNDLE group to another is the right way.

taylor-b commented 7 years ago

I don't think you can add an m- line with port 9 to a BUNDLE group, and request the address:port of that m- line to be used as BUNDLE address

If we added this rule, then BUNDLE would not be compatible with ICE or TCP, or anything else that might not use the "port" field.

I don't think a general rule saying that an m- line must not be moved from one BUNDLE group to another is the right way.

There are other ways we could resolve the ambiguity, as I suggested earlier. But using the address:port is not a generally applicable way.

cdh4u commented 7 years ago

I don't think you can add an m- line with port 9 to a BUNDLE group, and request the address:port of that m- line to be used as BUNDLE address

If we added this rule, then BUNDLE would not be compatible with ICE or TCP, or anything else that might not use the "port" field.

What I meant was that the port needs to be seen somewhere: either in the m- line, or in the candidate attribute (in case of ICE).

I don't think a general rule saying that an m- line must not be moved from one BUNDLE group to another is the right way.

There are other ways we could resolve the ambiguity, as I suggested earlier. But using the address:port is not a generally applicable way.

The BUNDLE address for a group is the address:port associated with the first identification-tag in the group:BUNDLE attribute associated with the group. That address:port may be taken from c- or m- lines, and/or ICE candidates, but it needs to be somewhere.

taylor-b commented 7 years ago

What I meant was that the port needs to be seen somewhere: either in the m- line, or in the candidate attribute (in case of ICE).

A unique port? That's a problem when the port is "9". As I said in my previous comment:

There are other ways we could resolve the ambiguity, as I suggested earlier. But using the address:port is not a generally applicable way.

Do we accept that limitation, or deal with it? If we accept it, it should at least be called out. Currently, BUNDLE does not say that the "address:port" is used to identify BUNDLE groups between successive offers/answers, and that implementations must avoid ambiguous situations.

cdh4u commented 7 years ago

I do agree that we need to say something - I am just trying to figure out what :)

cdh4u commented 6 years ago

It has been clarified that m= sections can not be moved directly between BUNDLE groups. An m= section needs to first be moved out of a BUNDLE group before it can be added to another BUNDLE group.