cyberman54 / ESP32-Paxcounter

Wifi & BLE driven passenger flow metering with cheap ESP32 boards
https://cyberman54.github.io/ESP32-Paxcounter/
Other
1.73k stars 405 forks source link

Vendor Filter ignores devices on filter #675

Closed Eldoprano closed 3 years ago

Eldoprano commented 3 years ago

Hi. I was trying to understand how MAC hashing works in here, and i found this part of the code, were if the Mac Filter is on, the code should ignore every MAC except the ones on the list.

https://github.com/cyberman54/ESP32-Paxcounter/blob/7f20c198b968ae338273207d1edba70c035ddf06/src/macsniff.cpp#L137-L139

I can't test this code right now, but, shouldn't we return 0 when the search returns vendors.end()? Like this:

if (std::find(vendors.begin(), vendors.end(), __builtin_bswap32(*oui) >> 8) == vendors.end()) 
   return 0;

Because that would mean that we ignore when it wasn't found, not when it was found.

cyberman54 commented 3 years ago

Thank you very much for this heads up concerning a piece of code, which grew over time, but never was deeply sanitized. You are right, we need to invert the condition. The root cause here is that the control flow in macsniff.cpp changed. Originally, there was an if clause around the vendorfilter. This was removed later, causing this major bug to come in, while nobody recognized this - yet. Perhaps most users don't use the vendorfilter.

cyberman54 commented 3 years ago

fixed by https://github.com/cyberman54/ESP32-Paxcounter/commit/e4270cb473ecf58d5428c8fad14531210d128000

Eldoprano commented 3 years ago

I finally got a LoPy to test this, and apparently this line is also filtering out Bluetooth MAC Addresses. I'm not sure if the list that's used in this project also has Bluetooth MAC Addresses, but at least for me it filters out all my Bluetooth devices, counting just WiFi MAC Adresses. And it's also filtering out the ENS mac addresses..

https://github.com/cyberman54/ESP32-Paxcounter/blob/e4270cb473ecf58d5428c8fad14531210d128000/src/macsniff.cpp#L137-L139

I tried to solve this by first checking if the MAC Address comes from WiFi:

if (MacBuffer.sniff_type == MAC_SNIFF_WIFI && std::find(vendors.begin(), vendors.end(),
                          __builtin_bswap32(*oui) >> 8) == vendors.end())
    return 0;

And now i get the Bluetooth MAC Addresses counted, but strangely not the ENS Mac Addresses (Both Bluetooth and ENS MAC Addresses do get counted when the filter is OFF).

cyberman54 commented 3 years ago

Can @spmrider help here?

spmrider commented 3 years ago

The vendor list hasn't been updated for a while and for the moment I suggest not to use it. Basically it should also cover Bluetooth - but due to merely all modern devices using MAC address randomization the vendor list does not really make sense these days. We're currently testing new algorithms and preparing a complete rewrite of the PaxCounter part, expected to be released in January as a library (which this project will then include). We assume we will completely drop the vendor list implementation and still get better results.