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

Twilio advertises incorrect stun urls #1934

Closed
fippo opened this issue Nov 19, 2022 · 17 comments
Closed

Twilio advertises incorrect stun urls #1934

fippo opened this issue Nov 19, 2022 · 17 comments
Assignees

Comments

@fippo
Copy link

fippo commented Nov 19, 2022

@makarandp0 @manjeshbhargav please route to the right team

https://www.twilio.com/docs/stun-turn shows a STUN server with a query component:

      "urls": "stun:global.stun.twilio.com:3478?transport=udp"

and I have seen this in a recent webrtc-internals dump as well.
This is not allowed in stun urls per
https://www.rfc-editor.org/rfc/rfc7064#section-3.1
This will likely break in Chrome 110 (February 2023) when
https://chromiumdash.appspot.com/commits?commit=f4d463ca875f51c26a7de0e594eb1516f3e60111&platform=Windows
ships.

While you are updating the docs please also remove the .url which was removed from the spec prior to 2016

@fippo fippo changed the title twilio vends incorrect stun urls Wwilio advertises incorrect stun urls Nov 19, 2022
@fippo fippo changed the title Wwilio advertises incorrect stun urls Twilio advertises incorrect stun urls Nov 19, 2022
@seancoleman2
Copy link
Contributor

Hi @fippo, thanks for surfacing this one. We are going to notify customers of the upcoming URL change and plan to make this change shortly before Chrome 110 is released.

@markbackman
Copy link

Hi @seancoleman2, do you have an update on this issue? Chrome 110 is scheduled for release on Feb 1, 2023. Without a fix, Twilio's STUN and TURN service will break for Chrome 110 and later.

@seancoleman2
Copy link
Contributor

Hi @markbackman - the change needed to the Network Traversal Service will be deployed on Jan 31st, before Chrome 110 early stable release goes out on Feb 1st. An email will be sent out to NTS customers with more details tomorrow.

Note for others following:

@AeroNotix
Copy link

No-one else feel like this is too short a timeframe to be rolling this out in? I got an email today and the rollout is at the end of the month.

What is the harm in Chrome silently allowing completely benign parameters to exist in the URLs? The review ( https://chromium-review.googlesource.com/c/chromium/src/+/4037887 ) looks to have barely any discussion, besides someone mentioning it could break twilio and ending up here.

Not sure I fully understand why this change needs to happen, ... at all really.

@seancoleman2
Copy link
Contributor

@AeroNotix admittedly this issue was where we first became aware of the Chrome 110 change's impact on the NTS service, so I do understand the timeframe is far from ideal. We were hoping that it wouldn't impact many, if any, customers since the token endpoint is intended to take the response values as is. With that being said, we feared some customers could be parsing the query params from the URIs and that's why we wanted to send an email. Curious, will this update require a change on your end?

I also cannot speak to the reasoning/urgency of this change from Chrome besides what is in PR you linked. Maybe @fippo can chime in there.

@AeroNotix
Copy link

@seancoleman2 oh please do not take the fact I am commenting here to mean that I lay any blame or ownership on yourself, or twilio in general.

I am commenting here because I know how absolutely futile it would be to try to discuss with Googlers on their own forum. Hence starting a conversation here.

@fippo
Copy link
Author

fippo commented Jan 19, 2023

The change was intended as a low-priority fix for a set of failing spec compliance tests. Nobody knew upfront that https://xkcd.com/1172/ would apply.

@AeroNotix talking with Googlers instead of about them gives amazing results.

@AeroNotix
Copy link

The change was intended as a low-priority fix for a set of failing spec compliance tests. Nobody knew upfront that https://xkcd.com/1172/ would apply.

Are you joking?

talking with Googlers instead of about them gives amazing results.

Beg to differ. This isn't my first rodeo.

@AeroNotix
Copy link

For the record, none of my systems were affected by this in a negative way. I just think the poor planning and rollout is just so indicative of Googlers it needed more attention than it seemed to be getting.

@harshit-j
Copy link

harshit-j commented Jan 20, 2023

We also got this email today. We're using Conversations SDK and Programmable Video SDK on client side and Twilio Node SDK on the backend side. We're not doing any direct API calls to Twilio or doing any interpretation of above mentioned query param ourselves. So do we assume that we'll not be affected?

@carl-meyer-paxton
Copy link

Hi, thanks for commenting on this thread, the discussion has been very useful.
Can anyone link any documentation or article that explains the nature of the change a bit more clearly (or maybe explains what is allowed and what is not)?
We have a client-side library that makes stun requests to Twilio servers and I want to make sure this sudden chrome update won't impact our library.

@seancoleman2
Copy link
Contributor

seancoleman2 commented Jan 20, 2023

@harshit-j if you are not using the Network Traversal Service, then you should not have gotten an email (could have been a mistake on our end) and do not need to worry about this change. One nuance is that if you are using the RTC Diagnostics SDK, you may be gathering NTS credentials to run the testMediaConnectionBitrate test. But regardless, if you aren't doing any interpretation of the query params (likely case), then you won't be affected.

@carl-meyer-paxton here are the contents of the email:

Ahoy!

You’re receiving this email from Twilio because you have used Twilio’s Network Traversal Service in the past 30 days.

What do you need to know?

Effective January 31, 2023, the STUN URI syntax returned by the Network Traversal Service will be updated to comply with RFC 7064 (see section-3.1).

Specifically, when generating a token using the Network Traversal Service the query parameters will be removed from the STUN URIs returned in the response.

For example,

{“urls”:"stun:global.stun.twilio.com:3478?transport=udp"}

will be:

{“urls”:"stun:global.stun.twilio.com:3478”}

Why is this happening?

The upcoming release of Chrome 110 will no longer accept STUN URIs that include the query parameters in the syntax (f4d463c). Thus, this change is required to ensure your WebRTC or VOIP application functions as intended for users on Chrome 110+.

What do you need to do?

It is possible, although unlikely, that your application has logic that is parsing the query parameters from the STUN URIs returned in the token response. We recommend you check your application code to ensure this is not the case.

The values returned when generating a token are intended to be used as is to configure your client, so we don’t expect any action will be needed.

What if you don’t take action?

If your application is using the NTS tokens as outlined in the documentation, then there will be no impact; your application will continue to function normally.

If your application has logic that is expecting the query parameters and parsing the URI values from the response, it is possible you could experience errors. The impact of those errors will be dependent on your implementation.

Sincerely,

Team Twilio

@seancoleman2 seancoleman2 self-assigned this Jan 20, 2023
@seancoleman2
Copy link
Contributor

One additional update.

To provide an option to test the change / verify no impact before the 31st in a pre-production environment, we are going to deploy this behind a feature flag next week (default disabled) and can enable it on a per-account SID basis.

If you are interested, email me at scoleman at twilio.com with account SID(s) that you want to test it on (ie. your dev/staging environments) and I will enable the flag for you. It's currently in our QE process, but we are targeting next Wednesday (1/25) to make this available.

Again, we believe it's very unlikely there will be any impact given the nature of the service + what is outlined in the documentation but we can't be 100% certain.

@AeroNotix
Copy link

That's very forward-thinking @seancoleman2. Interesting how a different billion-dollar company couldn't come up with the same thing 🤷🏻.

Will reach out via email!

@alvestrand
Copy link

That's very forward-thinking @seancoleman2. Interesting how a different billion-dollar company couldn't come up with the same thing 🤷🏻.

Google recommends that application vendors and concerned users test with canary and beta versions of Chrome.

@seancoleman2
Copy link
Contributor

Closing this issue as we deployed this change yesterday. Thanks all for the help!

@gedeonagmas
Copy link

gedeonagmas commented Mar 12, 2024

just uninstall and reinstall your simple-peer library to the latest one in my case it was 9.11.1

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

No branches or pull requests

10 participants