We should not use deprecated IPV6 addresses while gathering candidates
Categories
(Core :: WebRTC: Networking, defect, P2)
Tracking
()
backlog | webrtc/webaudio+ |
People
(Reporter: bwc, Assigned: bwc, NeedInfo)
References
Details
Attachments
(4 files, 1 obsolete file)
It looks like we can use ioctl with SIOCGIFAFLAG_IN6 to get the flags for an address, and test for the IN6_IFF_DEPRECATED flag. This will help in certain pathological situations where deprecated temporary IPV6 addresses are not being cleaned up properly, resulting in the use of an excessive number of IPV6 addresses while gathering.
Updated•9 years ago
|
Comment 1•9 years ago
|
||
there are a lot of examples indeed out there about how to do that: http://lists.freebsd.org/pipermail/freebsd-net/2010-July/025820.html Now the question is more (for me) where is the code we should patch? It looks like most of the candidate gathering is delegated to lib nicer in media/mtransport/third_party/nICEr/src ? grep -R -l AF_INET6 * ice/ice_candidate.c net/transport_addr.c net/transport_addr_reg.c stun/addrs.c stun/ifaddrs-android.c
Assignee | ||
Comment 2•9 years ago
|
||
The interface gathering code lives in media/mtransport/third_party/nICEr/src/stun/addrs.c, and this is also where all of the ioctl calls live right now. There is also media/mtransport/gonk_addrs.cpp that is only used on Firefox OS (most commonly referred to as B2G), but that does not seem to support IPV6 anyway. Also, don't forget about WIN32 here. I have not looked yet whether it has a concept of deprecated addresses.
Comment 3•7 years ago
|
||
Mass change P2->P3 to align with new Mozilla triage process.
I was just about to report this as well. This needs to be fixed. As people are using more video conferencing these days, leaking historical IPv6 addresses out to random people is a VERY bad thing to do.
I just looked a little bit closer. It's worse than this. It leaks ALL the IPv6 addresses, even the non-temporary ones. So on my MBP (MacOSX), it leaks the MAC address of my machine, allowing full tracking over the entire lifetime of my machine.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 6•4 years ago
|
||
Yeah, draft-ietf-rtcweb-transports (sec 3.3) says we should discard permanent IPv6 addresses if there are non-deprecated temporary ones present. Presumably, when operating in mode 2 we want to ensure that we prefer temp addresses also. That still allows the use of permanent addresses if there are no temporary addresses; is that the intent?
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
While we're at it, we probably want to avoid using things like IFA_F_OPTIMISTIC, IFA_F_DADFAILED/IN6_IFF_DUPLICATED, IFA_F_TENTATIVE/IN6_IFF_TENTATIVE, IN6_IFF_DETACHED, and IN6_IFF_ANYCAST.
Comment 8•4 years ago
|
||
So the filtering out of DAD failures and whatnot seems fine, but it's worth poking at the principles behind mode X.
Mode 2 says that you provide local IPs, but only for the route you are using. So in that case, it's probably not terrible to use a permanent address, because that is analogous in many ways to a v4 address, I think can be equated to a permanent address. Obviously, we can (and should) prefer a temporary address, but we don't need to insist on it.
I would probably apply the same logic to mode 1. There is no value to us in using a permanent address when a temporary address exists. These interactions are inherently temporary.
Obviously, modes 3 and 4 should not provide any local address, temporary or permanent.
(The deprecated temporary is a curious thing. Following the spec seems right: filter them out at first, keep one if it becomes deprecated and you are doing an ICE restart. I would add: ...only if it was on the nominated path. So if you had used that path, you should ADD the new non-deprecated temporary address. However, if you have no non-deprecated temporary addresses on that interface, then you provide the permanent address, just as you would at the start.)
Assignee | ||
Comment 9•4 years ago
|
||
Retaining deprecated interfaces that are currently nominated would require a whole bunch of plumbing, and that's a MAY level thing, so I'm not sure it would be worth our time. The only time that makes much sense to me is if we need to choose between continuing to use a deprecated temp addy and a permanent one, but it is unclear when this would actually happen in practice. If temp addresses are being used, how often do we end up in a scenario when our last temp addy is deprecated, and no new temp addy exists?
Comment 10•4 years ago
|
||
Yeah, I'm speaking from the position of ideals. If a restart gets a whole new set of addresses, without a deprecated temporary address, then I think things will work. It's a slight regression, but as you say, there is no significant risk that you have to fall back to a permanent address anyway.
Assignee | ||
Comment 11•4 years ago
•
|
||
Ok, in order to get this information on linux, we need to use netlink. We have a perfectly good netlink-based implementation of getifaddrs here, that we only use on android:
I'm going to try renaming this from ifaddrs-android to ifaddrs-netlink, and try using it on linux too. If that works ok, then I will refactor it to handle flag checking as it goes. If that all works, I will then implement an ifaddrs-bsd variant, based on getifaddrs and ioctl with SIOCGIFAFLAG_IN6.
Assignee | ||
Comment 12•4 years ago
|
||
We might also be able to ask Necko to supply these addresses, instead of NrIceCtx. https://searchfox.org/mozilla-central/rev/7fd1c1c34923ece7ad8c822bee062dd0491d64dc/media/mtransport/ipc/StunAddrsRequestParent.cpp#149
Worth looking into.
Assignee | ||
Comment 13•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=72427756feef866f065aa2c23cb88719fe5fc152
Assignee | ||
Comment 14•4 years ago
|
||
On the subject of Necko, is there anything we need to do to support RFC 6724? Is that all handled at the kernel level? Would there be any value in modifying NetlinkService to expose interface details like the IFA_F_* flags?
Assignee | ||
Comment 15•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f56b8401979c77efa644cb4d4bfc3532d14840d7
Assignee | ||
Comment 16•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e84cc5f049ae1fbe957545158c247aa5888da28
Assignee | ||
Comment 17•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=187b98dda928bbdf705783bd17e0d3e59738a840
Assignee | ||
Comment 18•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c465808801340c9af38630f68020eacdf137811
Assignee | ||
Comment 19•4 years ago
•
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a94fe0665f9f24ba6c9ee2ecc19cdaa07a3dabda gtest: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbd2e20608821c7b1462bf787bca9ad69f77005c
Assignee | ||
Comment 20•4 years ago
•
|
||
With windows build fixes https://treeherder.mozilla.org/#/jobs?repo=try&revision=92c69444effda79f337c6bbe7a225751a2c59a00 gtest: https://treeherder.mozilla.org/#/jobs?repo=try&revision=486e3e7595068ff353122a198ed4a14fbc041469
Assignee | ||
Comment 21•4 years ago
|
||
Could you try this binary (you said you were testing on OS X) and see if it only uses the privacy addresses?
Assignee | ||
Comment 22•4 years ago
|
||
Trying to find some others who have access to an IPv6 environment for testing. Try at least has IPv6 addresses, but I don't have any visibility into whether it uses privacy addresses or not.
Assignee | ||
Comment 23•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1b9a535d827a45d6387437384514ef3fbb1f2b3
Assignee | ||
Comment 24•4 years ago
|
||
Nevermind, that binary was not working correctly.
Assignee | ||
Comment 25•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc14b582b8a1ab407206dade16267129eee670c7
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 26•4 years ago
|
||
Ok, go ahead and try this one: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/fn8qWmZHRYSTbgIFvWHXGw/runs/1/artifacts/public/build/target.dmg
Assignee | ||
Comment 27•4 years ago
|
||
Dan, can you give the linux and windows binaries a try?
Assignee | ||
Comment 29•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa4fb062cfa0a3ca65e020c433c1c7faa62e7d27
Assignee | ||
Comment 30•4 years ago
|
||
Most recent build seems to work on Linux and Windows (thanks Dan!).
Assignee | ||
Comment 31•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b640eb7855453398c7a361622aaf282c1a1f940a
Assignee | ||
Comment 32•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b178a0a13fc5ec808292e4480698a64fdd0ba4cc
Assignee | ||
Comment 33•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=20aa488fe93cd6a7b49bcb8006ba068b099b198b
Assignee | ||
Comment 34•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=af8ecaa7220ddbf587df80fedefedb8395b4b52b
Assignee | ||
Comment 35•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=efcdcd5a9a2ade642a9a4d46aa5cb1757a36e603
Assignee | ||
Comment 36•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f332ee268469ed176d1a46ece0c69529b877f778
Assignee | ||
Comment 37•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f25b9fccb0edc7a09879ac73cf5b38a132970dc9
Assignee | ||
Comment 38•4 years ago
|
||
Assignee | ||
Comment 39•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dd3a3851868a4fa04a44fedb024d9aa1026a0d8
Assignee | ||
Comment 40•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=26dac46e686aba05ff3b77747d89208fb93108b2
Assignee | ||
Comment 41•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd38d834a38ae5ce237d99e6c101c5007095db4e
Assignee | ||
Comment 42•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=888c580b5c12df3305ddb50e18cf0815cc884a15
Assignee | ||
Comment 43•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd046f14dab5744b4866c26352f797088ee02bde
Updated•4 years ago
|
Assignee | ||
Comment 44•4 years ago
|
||
Depends on D74693
Updated•4 years ago
|
Assignee | ||
Comment 45•4 years ago
|
||
Depends on D75947
Assignee | ||
Comment 46•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=15bd96bee4ffad40adffe02136fbb2db2c8084bd
Assignee | ||
Comment 47•4 years ago
|
||
Assignee | ||
Comment 48•4 years ago
|
||
Depends on D76042
Updated•4 years ago
|
Assignee | ||
Comment 49•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b7c8ac0bb87fab9dd65dec6d0cd8065e17ce86e
Comment 50•4 years ago
•
|
||
I am sorry for such a late reply. Honestly I don't know the answer off-hand, so I'll forward to Dragana and Michal, in case they know.
The question asked is in comment 14.
Assignee | ||
Comment 51•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d4367403d7167cfcd4d5e81afb999ce1d1fa209
Assignee | ||
Comment 52•4 years ago
|
||
I'm going to wait until after the merge to land this.
Comment 53•4 years ago
|
||
Pushed by bcampen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a759f9651479 Rename a function to better reflect what it does, and fix a bug where handling of teredo and mac-based IPv6 was mixed up. r=mjf https://hg.mozilla.org/integration/autoland/rev/a74c46669606 Add a flags field to nr_local_addr so IPv6 addresses can be marked as temporary, and filter non-temp IPv6 if temp IPv6 are available. r=mjf https://hg.mozilla.org/integration/autoland/rev/24a4472452b8 Move platform-specific code from addrs.c into separate files. r=mjf https://hg.mozilla.org/integration/autoland/rev/99cce372d20c Teach platform-specific code to filter out inappropriate IPv6 addresses, and mark temp addresses. r=mjf
Comment 54•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a759f9651479
https://hg.mozilla.org/mozilla-central/rev/a74c46669606
https://hg.mozilla.org/mozilla-central/rev/24a4472452b8
https://hg.mozilla.org/mozilla-central/rev/99cce372d20c
Updated•4 years ago
|
Comment 55•4 years ago
|
||
(In reply to Byron Campen [:bwc] from comment #14)
Would there be any value in modifying NetlinkService to expose interface details like the IFA_F_* flags?
Where it would be used? NetlinkService is used solely for calculating network ID and determining link status. But if some of the information is useful anywhere else, we can expose it.
Assignee | ||
Comment 56•4 years ago
|
||
(In reply to Michal Novotny [:michal] from comment #55)
(In reply to Byron Campen [:bwc] from comment #14)
Would there be any value in modifying NetlinkService to expose interface details like the IFA_F_* flags?
Where it would be used? NetlinkService is used solely for calculating network ID and determining link status. But if some of the information is useful anywhere else, we can expose it.
If that is all it is used for, then moving that capability there would probably not be of much use. If, someday, there's some other code that needs more in-depth network interface information, we could think about moving it there.
Updated•3 years ago
|
Description
•