1383814 - IceServers configuration cuts off the username if it is longer than 513 bytes, and stops sending allocate request if the username is > 932 bytes

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[Steven Tang]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.55 Safari/537.36 Vivaldi/1.92.904.3

Steps to reproduce:

1. Init peerconnection with iceservers containing a turn server config that has a username longer than 512 bytes
2. createOffer, wait for candidates and getDescription
3. And noticed no turn allocations succeeded.
4. captured a pcap with wireshark and noticed FF omitted the username when sending the allocate request for turn 

iceServers = [
{
  credential: "credentials = 50 bytes",
  username: "LONG_STRING > 512 bytes",
  urls: ["turn:turn.url?transport=udp"]
}
];


Actual results:

It looks like FF has an arbitrary limit on the username (512 bytes) according to experiments. And this limit is 1. too short if the username is a generated public key. 2. this limit is not documented anywhere.


Expected results:

The same configuration work as expected with Chrome and the allocation request for turn sends the username properly.

Comment 1

[Steven Tang]

An update to the description:

1. FF cuts off the username (to 513 bytes) if the username is > 513 bytes but < 933 bytes

in wireshark, the allocate request has:
    Attribute Type: USERNAME (0x0006)
    Attribute Length: 513
    Username [truncated]: 
    Padding: 3

2. FF would no longer send the allocate request after the first challenge with nonce if the username is >= 933 bytes

in wireshark, after the initial turn allocate request, the server rejects with 401, and no more allocate with the nonce and username is sent.

Comment 2

[Nils Ohlmeier [:drno]]

The problem is that the spec mandates a limit of 512 bytes https://tools.ietf.org/html/rfc5389#section-15.3

So Chrome is violating the spec if it sends out the username attribute with more then 512 bytes length.

Therefore I'm leaning towards closing this as invalid.

Comment 3

[Steven Tang]

Thansk Nils, cannot really argue about that one. I guess the only thing I could say is it is an arbitrary limit in a dated RFC, and this username field is the only field that is configurable with WebRTC and is also sent to the server, and if one would like to implement any custom authentication carrying the data in the username field will be largely limited by this.

If our decision is to not make any changes to this, can FF dev team look at rejecting invalid server configuration instead of silently cutting it off or not sending the allocate request?

Comment 4

[Nils Ohlmeier [:drno]]

Attachment: Bug 1383814: throw exception for too long TURN username.

Review commit: https://reviewboard.mozilla.org/r/161196/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/161196/

Comment 5

[Nils Ohlmeier [:drno]]

(In reply to Steven Tang from comment #3)
> Thansk Nils, cannot really argue about that one. I guess the only thing I
> could say is it is an arbitrary limit in a dated RFC, and this username
> field is the only field that is configurable with WebRTC and is also sent to
> the server, and if one would like to implement any custom authentication
> carrying the data in the username field will be largely limited by this.

Steven: the reason for having such limitations in specs is so that receivers can build in buffers with limited sizes into their software.

I guess on one hand it is good to have a limitation in specs like in this case. But I can also understand that people want to try to new things which are blocked by these limitations.

Byron, Ekr: what do you think about extending this?

> If our decision is to not make any changes to this, can FF dev team look at
> rejecting invalid server configuration instead of silently cutting it off or
> not sending the allocate request?

That's easy. Patch attached. Thanks for reporting.

Comment 6

[Eric Rescorla (:ekr)]

I think Firefox should conform to the spec. If people don't like it, they can get the spec changed.

Comment 7

[Nils Ohlmeier [:drno]]

Steven in case it helps you to modify Firefox locally and do some experiments with longer username: the limit (without my new patch) is define here http://searchfox.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/stun/stun_msg.h#42

Comment 8

[Steven Tang]

Thanks Nils! Will try that!

Thanks Eric! Like mentioned, I cannot really argue about that, just thought rules are made by people, and changing an RFC is not a trivial ask and is a long process. We will try to get our standards team to follow up on that. FF as a tool though, if we could potentially offer some extra features (especially in the security aspect) without compromising the overall idea of the RFC, it might still be a good thing :). That said, I fully understand from your point of view, and appreciate everyone that looked into this.

Steven

Comment 9

[Eric Rescorla (:ekr)]

Steven, I'm a bit confused as to why this limit is causing you a problem. What is it you are trying to put in this field specifically?

Comment 10

[Steven Tang]

Hi Eric, I'm not allowed to disclose the full details. But, the username field is the only field we could access via the WebRTC APIs to send to the server (our custom turn server, which can be deployed to anywhere without having access to the actual token issuer), and we rely on this field to carry some custom authentication information and the auth-token that could grow to a limit that is bigger than 512 bytes (2k I think). We had our client fully tested on Chrome and is trying to expand the support to FF and other browsers.

BTW, I am aware that the WebRTC community is making the o-auth standards for turn authentication. But AFAIK, it does not expand the STUN protocol and does not send the token over, which means the turn server has to have full access to the token issuer.

I hope this clarifies things.

Comment 11

[Michael Froman [:mjf]]

Comment on attachment 8890112 [details]
Bug 1383814: throw exception for too long TURN username.

https://reviewboard.mozilla.org/r/161196/#review166784

Looks good to me.

Comment 12

[Eric Rescorla (:ekr)]

(In reply to Steven Tang from comment #10)
> Hi Eric, I'm not allowed to disclose the full details. But, the username
> field is the only field we could access via the WebRTC APIs to send to the
> server (our custom turn server, which can be deployed to anywhere without
> having access to the actual token issuer), and we rely on this field to
> carry some custom authentication information and the auth-token that could
> grow to a limit that is bigger than 512 bytes (2k I think).

My question is: why is it so big? You said something about public keys, but
if you use EC you should be able to make these smaller. Note that it's not
good to have the username this big anyway because you are going to start
bumping up against UDP packet size limits.


> We had our
> client fully tested on Chrome and is trying to expand the support to FF and
> other browsers.
> 
> BTW, I am aware that the WebRTC community is making the o-auth standards for
> turn authentication. But AFAIK, it does not expand the STUN protocol and
> does not send the token over, which means the turn server has to have full
> access to the token issuer.
> 
> I hope this clarifies things.

Comment 13

[Steven Tang]

Hi Eric, sorry, I mentioned public keys as an example. We have asked our token issuer partner to see if we could minify the palyload. That's one of the first things we approached after hitting this limit. But as it stands now it is big, I know. AFAIK, there are some user information, some key ids, and the token encrypted by the public keys, which comes within a json object. And the entire payload is then base64 encoded as a string. I'm not sure we could get it down to 512 bytes at the moment, given how big it is now, and the user info length is very flexible, and the encryption method we're using also adds quite a few overhead. But we're definitely trying to squeeze it as we speak, and we haven't given up on FF yet.

Steven

Comment 14

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/530e3098f9c1
throw exception for too long TURN username. r=mjf

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/530e3098f9c1