1304920 - Non ICE binding requests include fingerprint

Status: RESOLVED FIXED

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

Description

[Nils Ohlmeier [:drno]]

The binding request from Firefox STUN server to query for server reflexive candidates contains the fingerprint attribute.

RFC 5389 says the fingerprint is added to allow implementations which multiplex multiple protocols on the same port to differentiate the protocols. That is clearly not the case when querying your public IP and port. RFC 5389 says:
"Since the stand-alone server only runs STUN, FINGERPRINT provides no benefit."

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: Bug 1304920: remove fingerprint attribute from plain binding requests.

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

Comment 2

[Byron Campen [:bwc]]

Comment on attachment 8794018 [details]
Bug 1304920: remove fingerprint attribute from plain binding requests.

https://reviewboard.mozilla.org/r/80580/#review79380

::: media/mtransport/third_party/nICEr/src/stun/stun_build.h:43
(Diff revision 1)
>  
>  #define NR_STUN_MODE_STUN               1
>  #ifdef USE_STUND_0_96
>  #define NR_STUN_MODE_STUND_0_96         2    /* backwards compatibility mode */
>  #endif /* USE_STUND_0_96 */
> +#define NR_STUN_MODE_STUN_NO_ATTR       3

NO_AUTH?

Comment 3

[Byron Campen [:bwc]]

Comment on attachment 8794018 [details]
Bug 1304920: remove fingerprint attribute from plain binding requests.

https://reviewboard.mozilla.org/r/80580/#review79382

gtest looks sad, I'm guessing our server code insists on there being a fingerprint?

Comment 4

[Nils Ohlmeier [:drno]]

Comment on attachment 8794018 [details]
Bug 1304920: remove fingerprint attribute from plain binding requests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80580/diff/1-2/

Comment 5

[Nils Ohlmeier [:drno]]

(In reply to Byron Campen [:bwc] from comment #3)
> gtest looks sad, I'm guessing our server code insists on there being a
> fingerprint?

I think that is an actual bug in our STUN TCP implementation. Let me know if I should submit that in a separate bug report for better tracking.

Comment 6

[Nils Ohlmeier [:drno]]

Comment on attachment 8794018 [details]
Bug 1304920: remove fingerprint attribute from plain binding requests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80580/diff/2-3/

Comment 7

[Byron Campen [:bwc]]

Comment on attachment 8794018 [details]
Bug 1304920: remove fingerprint attribute from plain binding requests.

https://reviewboard.mozilla.org/r/80580/#review79688

Comment 8

[Nils Ohlmeier [:drno]]

Comment on attachment 8794018 [details]
Bug 1304920: remove fingerprint attribute from plain binding requests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80580/diff/3-4/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5abb97190394
remove fingerprint attribute from plain binding requests. r=bwc

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5abb97190394