1458624 - Firefox on Linux sends signal strength to Google geolocation API as a percentage, not RSSI dBm

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect, P3)

Description

[zibri]

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

Steps to reproduce:

I checked Firefox (linux version on ubuntu) and I found out it sends the wrong data to google location API.



Actual results:

Firefox sends signalStrenght in a value that is between 0 and 100 (or 0 and 70 on some wifi cards) which is the normal value of signal quality you can find also in iwlist scan report.
This is WRONG.



Expected results:

Google geolocate API needs the signalStrenght expressed in dB.
Google chrome uses this simple formula:

signalStrenght=(signal_quality / 2 -100)

Also note that since some wifi cards express the quality as a number between 0 and 70 (or other number),
you should ask linux os what is the "max quality".

To do so, you must:

		bzero(&iwrequest, sizeof(struct iwreq));
		iwrequest.u.data.pointer = buff;
		iwrequest.u.data.flags = 0;
		iwrequest.u.data.length = buffsize;
		strcpy(iwrequest.ifr_name, wifi_devicename);
		if (ioctl(iwsd, SIOCGIWRANGE, &iwrequest) < 0) {
			die(wifi_device_name);
		}

		struct iw_range *range = (struct iw_range *)buff;

		maxqual = range->max_qual.qual;

P.S.
If you need more informations about this, contact me.
I also think this is eligible for the bug bounty program, by the way.

Comment 1

[zibri]

In the end, the final formula will be:

signal_strenght = (signal_quality * 100 / max_quality / 2) - 100;

Comment 2

[Josh Matthews [:jdm]]

Is there a public source for this information?

Comment 3

[zibri]

Sure.
You could have easily found it using goole:

https://developers.google.com/maps/documentation/geolocation/intro


macAddress: (required) The MAC address of the WiFi node. It's typically called a BSS, BSSID or MAC address. Separators must be : (colon).
signalStrength: The current signal strength measured in dBm.
age: The number of milliseconds since this access point was detected.
channel: The channel over which the client is communicating with the access point.
signalToNoiseRatio: The current signal to noise ratio measured in dB.

An example WiFi access point object is shown below.

{
  "macAddress": "00:25:9c:cf:1c:ac",
  "signalStrength": -43,
  "age": 0,
  "channel": 11,
  "signalToNoiseRatio": 0
}

P.S.
For accurate results put "signalToNoiseRatio" at -2 or don't put it at all.
age, always 0.
channel is optional.

Comment 4

[zibri]

So? Did you check or I have to bring more proof?
I logged firefox https traffic and I know it sends the quality instead of signal in dB.

Comment 5

[Chris Peterson [:cpeterson]]

Thanks for reporting this bug, zibri. I see that Firefox on Linux does indeed pass the D-Bus AccessPoint's "Strength" property:

https://searchfox.org/mozilla-central/rev/b28b94dc81d60c6d9164315adbd4a5073526d372/netwerk/wifi/nsWifiScannerDBus.cpp#289-297

which is signal strength expressed as a percentage, not in dBm:

https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.AccessPoint.html#gdbus-property-org-freedesktop-NetworkManager-AccessPoint.Strength

Firefox on Windows, macOS, FreeBSD, and Solaris correctly reports signal strength in dBm. Here are the platform-specific calls to setSignal():

https://searchfox.org/mozilla-central/search?case=true&regexp=true&q=%5CbsetSignal%5Cb

This Linux D-Bus code landed in bug 668194. A follow-up bug 724496 was filed at the time about this code only reporting one access point. Is that still happening?

Comment 6

[Chris Peterson [:cpeterson]]

I don't have a Linux build environment, but the fix for this bug should be pretty easy. We can convert the signal strength percentage to a fake RSSI value:

https://en.wikipedia.org/wiki/Received_signal_strength_indication

RSSI is the relative received signal strength in a wireless environment, in arbitrary units. RSSI is an indication of the power level being received by the receive radio after the antenna and possible cable loss. Therefore, the higher the RSSI number, the stronger the signal. Thus, when an RSSI value is represented in a negative form (e.g. −100), the closer the value is to 0, the stronger the received signal has been.

In my home, I see strong Wi-Fi signals are about -40 dBm and weak signals are about -80 dBm. So we can just slide the 0-100% value down -100 points to simulate an RSSI value. A 100% signal would be converted to 0 dBm and a 0% signal to -100 dBm in this function:

https://searchfox.org/mozilla-central/rev/11cfa0462a6b5d8c5e2111b8cfddcf78098f0141/netwerk/wifi/nsWifiScannerDBus.cpp#270-272

uint8_t strength;
dbus_message_iter_get_basic(&variant, &strength);
MOZ_ASSERT(strength >= 0 && strength <= 100, "strength should be a percentage");
int fakeRSSI = strength - 100; // [-100, 0] pseudo-dBm
ap->setSignal(fakeRSSI);

Comment 7

[zibri]

(In reply to Chris Peterson [:cpeterson] from comment #6)

I don't have a Linux build environment, but the fix for this bug should be pretty easy. We can convert the signal strength percentage to a fake RSSI value:

https://en.wikipedia.org/wiki/Received_signal_strength_indication

RSSI is the relative received signal strength in a wireless environment, in arbitrary units. RSSI is an indication of the power level being received by the receive radio after the antenna and possible cable loss. Therefore, the higher the RSSI number, the stronger the signal. Thus, when an RSSI value is represented in a negative form (e.g. −100), the closer the value is to 0, the stronger the received signal has been.

In my home, I see strong Wi-Fi signals are about -40 dBm and weak signals are about -80 dBm. So we can just slide the 0-100% value down -100 points to simulate an RSSI value. A 100% signal would be converted to 0 dBm and a 0% signal to -100 dBm in this function:

https://searchfox.org/mozilla-central/rev/11cfa0462a6b5d8c5e2111b8cfddcf78098f0141/netwerk/wifi/nsWifiScannerDBus.cpp#270-272

uint8_t strength;
dbus_message_iter_get_basic(&variant, &strength);
MOZ_ASSERT(strength >= 0 && strength <= 100, "strength should be a percentage");
int fakeRSSI = strength - 100; // [-100, 0] pseudo-dBm
ap->setSignal(fakeRSSI);

Did you read my original post? https://bugzilla.mozilla.org/show_bug.cgi?id=1458624#c0

I wrote it there one year ago!

signalStrenght=(signal_quality / 2 -100)

  bzero(&iwrequest, sizeof(struct iwreq));
  iwrequest.u.data.pointer = buff;
  iwrequest.u.data.flags = 0;
  iwrequest.u.data.length = buffsize;
  strcpy(iwrequest.ifr_name, wifi_devicename);
  if (ioctl(iwsd, SIOCGIWRANGE, &iwrequest) < 0) {
  	die(wifi_device_name);
  }

  struct iw_range *range = (struct iw_range *)buff;

  maxqual = range->max_qual.qual;

Comment 8

[Chris Peterson [:cpeterson]]

Sorry! I read your original comment last year and didn't read it all this time as I was reviewing some old geolocation bugs. The Google Chrome formula you describe is better (and avoids the edge case of sending signalStrength == 0 that might cause unexpected results from the geolocation service).

Comment 9

[zibri]

;-)

Comment 10

[sagudev]

I think this will be good first bug for me. Can someone assign this to me?

Comment 11

[Chris Peterson [:cpeterson]]

(In reply to sagudev from comment #10)

I think this will be good first bug for me. Can someone assign this to me?

Hi Samo! Thanks for your help!

Have built and run Firefox on your machine yet? That's the first step. Here are instructions for getting the code and building on Linux:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build/Linux_and_MacOS_build_preparation

I can assign the bug to you after you have a patch ready for code review. That's the usual process for first bugs.

Comment 12

[sagudev]

I just compiled Firefox and try https://www.w3schools.com/html/tryit.asp?filename=tryhtml5_geolocation
I capture traffic and Firefox only connect to location.services.mozilla.com so is this bug still existing.

Comment 13

[sagudev]

I think the problem is that I did not use Google API key.

Comment 14

[sagudev]

I have all implemented except max quality. I did some test and it seems it is not needed.

Probably https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.AccessPoint.html#gdbus-property-org-freedesktop-NetworkManager-AccessPoint.Strength always use 100 as max quality.

When I ran command: iwconfig wlan0 | grep -i --color quality and compere dB with first formula (signal_quality / 2 -100) and with second formula ((signal_quality * 100 / max_quality / 2) - 100) it seems that first formula without max quality is more accurate than second formula. Even in chromium they did not calculate with max quality (https://cs.chromium.org/chromium/src/services/device/geolocation/wifi_data_provider_linux.cc?q=/org/freedesktop/NetworkManager&sq=package:chromium&dr=C&l=292).

Comment 15

[sagudev]

Attachment: patch_geoloc_api.diff

Comment 16

[Chris Peterson [:cpeterson]]

(In reply to sagudev from comment #12)

I just compiled Firefox and try https://www.w3schools.com/html/tryit.asp?filename=tryhtml5_geolocation
I capture traffic and Firefox only connect to location.services.mozilla.com so is this bug still existing.

This is expected. At this time, Firefox uses location.services.mozilla.com (Mozilla Location Service aka MLS) instead of Google's location service (aka GLS). You shouldn't need an API key to test with MLS.

(In reply to sagudev from comment #14)

I have all implemented except max quality. I did some test and it seems it is not needed.

Probably https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.AccessPoint.html#gdbus-property-org-freedesktop-NetworkManager-AccessPoint.Strength always use 100 as max quality.

When I ran command: iwconfig wlan0 | grep -i --color quality and compere dB with first formula (signal_quality / 2 -100) and with second formula ((signal_quality * 100 / max_quality / 2) - 100) it seems that first formula without max quality is more accurate than second formula. Even in chromium they did not calculate with max quality (https://cs.chromium.org/chromium/src/services/device/geolocation/wifi_data_provider_linux.cc?q=/org/freedesktop/NetworkManager&sq=package:chromium&dr=C&l=292).

Interesting. What is the reported max_quality value? If it is 100, then the first and second formulas should produce the same result. If max_quality is not 100, then what are the results of the two different formulas?

Comment 17

[Chris Peterson [:cpeterson]]

Comment on attachment 9078013 [details] [diff] [review]
patch_geoloc_api.diff

Review of attachment 9078013 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! Your patch looks good to me. There is just one typo: `signal_strenght` should be spelled `signal_strength`.

Also, do you mind resubmitting your patch using Phabricator (Mozilla's code review server) instead of a .diff patch file? Here are instructions for creating a Phabricator account and uploading your patch:

https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

It's pretty easy with the documentation and Mozilla's `mach` tool. If you plan to contribute more Firefox bug fixes, Phabricator will make reviewing and checking in your patches much easier for code reviewers. If you don't have the time now, that's OK. I can fix the typo and check in your patch for you. Just let me know. :)

::: netwerk/wifi/nsWifiScannerDBus.cpp
@@ +271,3 @@
>          dbus_message_iter_get_basic(&variant, &strength);
> +        int signal_strenght = (strength / 2) - 100;  // strength to dB
> +        ap->setSignal(signal_strenght);

`signal_strenght` should be spelled `signal_strength`.

Comment 18

[sagudev]

(In reply to Chris Peterson [:cpeterson] from comment #16)

Interesting. What is the reported max_quality value? If it is 100, then the first and second formulas should produce the same result. If max_quality is not 100, then what are the results of the two different formulas?
I did not checked for reported max_quality in code. I used linux command ´iwconfig Interface-Name-Here´ and result was Link Quality=x/70, so maximum quality is 70.

Comment 19

[sagudev]

Also, do you mind resubmitting your patch using Phabricator (Mozilla's code
review server) instead of a .diff patch file? Here are instructions for
creating a Phabricator account and uploading your patch:

https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

OK, I will try.

Comment 20

[sagudev]

Attachment: Bug 1458624 - Firefox on Linux now sends signal strength to geolocation API as RSSI dBm instead of a percentage. r=cpeterson

Comment 21

[Chris Peterson [:cpeterson]]

(In reply to sagudev from comment #19)

Also, do you mind resubmitting your patch using Phabricator (Mozilla's code
review server) instead of a .diff patch file? Here are instructions for
creating a Phabricator account and uploading your patch:

https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

OK, I will try.

Thanks. Your patch looks good. One small issue: your patch doesn't include your name and email address. You might need to set the username variable in your ~/.hgrc config file like this:

https://www.mercurial-scm.org/wiki/QuickStart#Setting_a_username

I can fix your patch's username when I check it in. Do you want to be credited with your Bugzilla account's name and email address sagudev <[e-mail]>?

I will run your patch through our Linux tests before checking it in:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1717bd16412ac9ba5fb911004092299e41045ce4

Comment 22

[sagudev]

Thanks. Your patch looks good. One small issue: your patch doesn't include your name and email address. You might need to set the username variable in your ~/.hgrc config file like this:

https://www.mercurial-scm.org/wiki/QuickStart#Setting_a_username

I can fix your patch's username when I check it in. Do you want to be credited with your Bugzilla account's name and email address sagudev <[e-mail]>?

Yes, please.

Comment 23

[sagudev]

Attachment: Bug 1458624 - Firefox on Linux now sends signal strength to Google geolocation API as RSSI dBm instead of a percentage. r=cpeterson

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d4306a95a09
Firefox on Linux now sends signal strength to geolocation API as RSSI dBm instead of a percentage. r=cpeterson

Comment 25

[Chris Peterson [:cpeterson]]

I will close this and push data to original one. Sorry for trouble. I'm still learning phabricator.

No problem at all. That's why we have bug mentors to help contributors learn these tools in their first bugs. :)

I see you mentioned arc diff in a Phabricator comment. I recommend using the moz-phab script in the future. moz-phab is a wrapper around arc that makes uploading new and revised patches easier.

https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#using-moz-phab

Comment 26

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/3d4306a95a09

Comment 27

[sagudev]

@cpeterson Thanks for helping me on first bug.

Comment 28

[Chris Peterson [:cpeterson]]

You're welcome! Thanks for fixing this bug. If you're looking for another bug, check out Mozilla's "Codetribute" website to search for interesting bugs with mentors:

https://codetribute.mozilla.org/