Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue818 tls #830

Merged
merged 6 commits into from
Oct 10, 2017
Merged

Issue818 tls #830

merged 6 commits into from
Oct 10, 2017

Conversation

ekr
Copy link
Contributor

@ekr ekr commented Aug 26, 2017

No description provided.

@ekr ekr requested review from fluffy and juberti August 26, 2017 17:28
@@ -412,9 +414,11 @@ def simple_example(draft):
'ice_pwd': 'mqyWsAjvtKwTGnvhPztQ9mIf', 'dtls_dir': 'passive' }
]
fp1 = '19:E2:1C:3B:4B:9F:81:E6:B8:5C:F4:A5:A8:D8:73:04:BB:05:2F:70:9F:04:A9:0E:05:E9:26:33:E8:70:88:A2'
tid1 = '91bbf309c0990a6bec11e38ba2933cee'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these simply be monotonically increasing integers, for simplicity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are required to have 120 bits of entropy, so they have to be long. but I could do H(integer) if you like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, ok. Do you know why? The dtls-sdp document doesn't provide any rationale for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luckily, we don't need to decide this for this document/PR because clearly long is OK. Do you want me to change them to be H(integer)?

@@ -3540,12 +3540,12 @@ candidate:1 1 UDP 1694498815 192.0.2.33 10000 typ host

<t>tls-id value, which MUST be set according to
<xref target="I-D.ietf-mmusic-dtls-sdp" />, Section 5. If
this is a re-offer and the tls-id value is different
this is a re-offer or a response to a re-offer
and the tls-id value is different
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the remote DTLS id changes, and the local side is not performing an ICE restart, what then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's apparently forbidden:
When a new DTLS association is established over an unordered
transport, in order to disambiguate any packets associated with the
newly established DTLS association, at least one of the endpoints
MUST allocate a completely new set of ICE candidates which were not
recently used for any other DTLS association. This means the
answerer cannot initiate a new DTLS association unless the offerer
initiated ICE restart [I-D.ietf-ice-rfc5245bis]. If the answerer
wants to initiate a new DTLS association, it needs to initiate an ICE
restart and a new offer/answer exchange on its own. However, an ICE
restart does not by default require a new DTLS association to be
established.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the answerer can change the tls-id, even if the offerer did not, but only if ICE is already restarting.

Seems kind of strange to permit this corner case; do you know what the thinking is here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I do not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I would suggest that we should instead explode if the TLS id changes on the remote side during an ICE restart and the offerer didn't change it.

Copy link
Contributor

@fluffy fluffy Sep 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so not sure I care about the following case but consider it ... the browser is using webrtc to connect to a farm of servers that are all streaming the music on hold or a football game or whatever ... session is set up and browser does an ICE restart as the wifi interface just came up on mobile device browser is running on. The offer does not change tls-id. Now the call lands on a new server and it needs new TLS session. The answer replies with a new tls-id. Seems like this should be OK

@fluffy
Copy link
Contributor

fluffy commented Aug 28, 2017

In section 5.10, the text that says to tear down the old TLS connections seems like it needs a tweek. For the case of a PR_ANS, I would expect it to not get torn down until the resources are freed when the ANS happens.

Copy link
Contributor

@fluffy fluffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

@ekr
Copy link
Contributor Author

ekr commented Sep 1, 2017

See: cdh4u/draft-dtls-sdp#37

@ekr
Copy link
Contributor Author

ekr commented Sep 29, 2017

We had a call to discuss and I think reached resolution on the SDP issue. I will prepare a PR and send to the list for approval. Separately we need to close on tls-id size, but that need not hold JSEP if we are willing to have the examples use the longer size (which we all agree is permitted)

@ekr
Copy link
Contributor Author

ekr commented Sep 29, 2017

Update: we had a call to discuss this and I think have resolution on the SDP semantics. I will prepare a PR for that and send to lists. Separately, we have to discuss the S

@@ -3041,6 +3041,12 @@ candidate:1 1 UDP 1694498815 192.0.2.33 10000 typ host
section is bundled into another m= section, it still MUST
NOT contain any ICE credentials.</t>

<t>Each "a=tls-id" line MUST stay the same unless the
offeror's "a=tls-id" line changed, in which case a new
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

offerer

@@ -3041,6 +3041,12 @@ candidate:1 1 UDP 1694498815 192.0.2.33 10000 typ host
section is bundled into another m= section, it still MUST
NOT contain any ICE credentials.</t>

<t>Each "a=tls-id" line MUST stay the same unless the
offeror's "a=tls-id" line changed, in which case a new
"a=tls-id" line MUST be added, as described in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new line? Or a new value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right

present, MUST be the same as in the offer.</t>
values. Note that although JSEP requires that answers
change the tls-id value if and only if the offerer does,
answerer, non-JSEP answerers are permitted to change the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove 'answerer,'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops.

change the tls-id value if and only if the offerer does,
answerer, non-JSEP answerers are permitted to change the
tls-id as long as the offer contained an ICE restart.
Thus, JSEP implementations which process DTLS data prior
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to fingerprint changes as well; suggest moving this to 5.11, where we talk about the need to start a new DTLS connection if either the tls-id or fingerprint changes.

Copy link
Contributor Author

@ekr ekr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Juberti, PTAL

present, MUST be the same as in the offer.</t>
values. Note that although JSEP requires that answers
change the tls-id value if and only if the offerer does,
answerer, non-JSEP answerers are permitted to change the
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops.

@@ -3041,6 +3041,12 @@ candidate:1 1 UDP 1694498815 192.0.2.33 10000 typ host
section is bundled into another m= section, it still MUST
NOT contain any ICE credentials.</t>

<t>Each "a=tls-id" line MUST stay the same unless the
offeror's "a=tls-id" line changed, in which case a new
"a=tls-id" line MUST be added, as described in
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right

@juberti juberti merged commit e0a7461 into rtcweb-wg:master Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants