dbinfrago / libpax

Apache License 2.0
21 stars 13 forks source link

don't count unrandomized ble macs & bugfix LSB ble mac #15

Closed cyberman54 closed 2 years ago

cyberman54 commented 2 years ago

1) Checking universal bit is now happening centralized in mac_add(), thus working for wifi and ble macs in same manner. Bugfix: Before ble macs where not checked for universal bit.

2) Bugfix: BLE macs appear in little endian format, thus LSB processing is required. But they were processed as MSB. Which means that all octets of ble macs were swapped.

cyberman54 commented 2 years ago

ping /cc @oliverbrandmueller @FlorianLudwig

oliverbrandmueller commented 2 years ago

I'll have @FlorianLudwig @lifezoned4 check the code technically; I'd merge into master and start a comparative test in a suitable environment (two counters, one with old, one with new code) before publishing a minor release.

cyberman54 commented 2 years ago

There well be another small commit to this, later this day.

lifezoned4 commented 2 years ago

@oliverbrandmueller @cyberman54

Code LGTM but still the unit tests have to be adjusted because now the bit filter is in mac_add. The PR needs at least something as this:

index 512015f..174cb40 100644
--- a/test/libpax_test_cases.cpp
+++ b/test/libpax_test_cases.cpp
@@ -10,7 +10,7 @@
 */
 void test_mac_add_bytes() {
   libpax_counter_reset();
-  uint8_t test_mac_addr[6] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0};
+  uint8_t test_mac_addr[6] = {0x0b, 0x01, 0x0, 0x0, 0x0, 0x0};
   test_mac_addr[4] = 0x01;
   test_mac_addr[5] = 0x01;
   mac_add(test_mac_addr, MAC_SNIFF_WIFI);
@@ -39,6 +39,9 @@ void test_collision_add() {
   libpax_counter_reset();
   uint8_t test_mac_addr[6];

+  test_mac_addr[0] = 0x0b;
+  test_mac_addr[1] = 0x10;
+
   uint16_t *test_mac_addr_p = (uint16_t *)(test_mac_addr + 4);
   *test_mac_addr_p = 1;
   for (int i = 0; i < 1000; i++) {
@@ -66,7 +69,7 @@ void test_counter_reset() {
   libpax_counter_reset();
   TEST_ASSERT_EQUAL(0, libpax_wifi_counter_count());

-  uint8_t test_mac_addr[6] = {1, 1, 1, 1, 1, 1};
+  uint8_t test_mac_addr[6] = {0x0b, 0x01, 1, 1, 1, 1};
   mac_add(test_mac_addr, MAC_SNIFF_WIFI);
   TEST_ASSERT_EQUAL(1, libpax_wifi_counter_count());
cyberman54 commented 2 years ago

Test case changes added to PR.

lifezoned4 commented 2 years ago

Code still LGTM. In my opinion this can be merged. @oliverbrandmueller

oliverbrandmueller commented 2 years ago

merged

cyberman54 commented 2 years ago

thx! platformio registration still missing?