Bluetooth-Devices / bthome-ble

Parser for BTHome BLE devices
https://bthome.io/
MIT License
70 stars 15 forks source link

Encryption counter is lower than or equal to the previous value #99

Closed rrooggiieerr closed 8 months ago

rrooggiieerr commented 9 months ago

Describe the bug I'm getting a lot of warnings that the encryption counter is lower than or equal to the previous value.

2024-01-09 14:35:48.273 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (293542) is lower than or equal to the previous value (293542). The data might be compromised. BLE advertisement will be skipped.
2024-01-09 14:36:38.313 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (293551) is lower than or equal to the previous value (293551). The data might be compromised. BLE advertisement will be skipped.
2024-01-09 14:37:08.269 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (293556) is lower than or equal to the previous value (293556). The data might be compromised. BLE advertisement will be skipped.
2024-01-09 14:43:08.532 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (293621) is lower than or equal to the previous value (293621). The data might be compromised. BLE advertisement will be skipped.
2024-01-09 14:45:58.650 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (293652) is lower than or equal to the previous value (293652). The data might be compromised. BLE advertisement will be skipped.
2024-01-09 14:46:28.652 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (293657) is lower than or equal to the previous value (293657). The data might be compromised. BLE advertisement will be skipped.
2024-01-09 14:47:08.673 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (293664) is lower than or equal to the previous value (293664). The data might be compromised. BLE advertisement will be skipped.
2024-01-09 14:47:23.447 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (296106) is lower than or equal to the previous value (296106). The data might be compromised. BLE advertisement will be skipped.

To Reproduce Steps to reproduce the behavior:

Additional context I'm using 3 BLE Proxies and my Raspberry Pi also has a BLE receiver. Maybe different devices are receiving the BLE transmission and forwarding it to the integration? If that's the root cause the maybe only print a warning when the encryption counter is lower than to the previous value, and ignore when equal?

Ernst79 commented 8 months ago

Hmm, it should check the following condition first, which should filter out your use case.

        if (
            self.last_service_info
            and new_encryption_counter == last_encryption_counter
            and service_info.service_data == self.last_service_info.service_data
            and self.bindkey_verified is True
        ):

So if

it should only log a debug message, not a warning.

I'm not quite understand which condition isn't met in your case.

Ernst79 commented 8 months ago

Are you able to (and willing to) modify some code in the bthome-ble pypi package manually (on your system)? If so, you could modify the following line in parser.py of the bthome_ble pypi package?

https://github.com/Bluetooth-Devices/bthome-ble/blob/13b987c60b763646ba2a7998b977d445e437bd2e/src/bthome_ble/parser.py#L611-L616

to the following

                _LOGGER.warning(
                    "The new encryption counter (%i) is lower than or equal to the previous value "
                    "(%i). The data might be compromised. BLE advertisement will be skipped."
                    "self.last_service_info (%s) "
                    "service_info.service_data (%s) "
                    "self.last_service_info.service_data (%s) "
                    "self.bindkey_verified (%s) ",
                    new_encryption_counter,
                    last_encryption_counter,
                    self.last_service_info,
                    service_info.service_data,
                    self.last_service_info.service_data,
                    self.bindkey_verified,
                )
rrooggiieerr commented 8 months ago

Thanks for explaining what's coing on in the code. I am able to change the code, but it's a bit complicated to do that on on my live HAOS system, so have to do that on my development system. Hopefully I can reproduce the situation.

Some additional on my setup: I'm using 5 of those Xiaomi Mijia Bluetooth Sensors and the latest version of the bthome firmware I just restarted one of these sensors and now get additional warnings:

2024-01-13 12:45:11.417 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (353235) is lower than or equal to the previous value (353235). The data might be compromised. BLE advertisement will be skipped.
2024-01-13 12:49:41.629 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (353284) is lower than or equal to the previous value (353284). The data might be compromised. BLE advertisement will be skipped.
2024-01-13 12:50:01.015 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (2) is lower than or equal to the previous value (91302). The data might be compromised. BLE advertisement will be skipped.
2024-01-13 12:50:18.448 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (5) is lower than or equal to the previous value (91302). The data might be compromised. BLE advertisement will be skipped.

I'm actually starting to think some of my devices use the same encryption key. I used the default encryption key that came with the device. Also one of my sensors has stopped updating since I upgraded to HA Core 2024.1.3 last evening:

Screenshot 2024-01-13 at 14 04 27

Removing the battery from the failing device (Living Room) removes the warnings with the lower numbered encryption counters from the log.

Maybe above information is helpful for you, I'm looking forward to your ideas

rrooggiieerr commented 8 months ago

I've checked the bindkey of all my devices and they are unique for each device.

I notice that after removing the battery the encryption counter starts back at 0, making the new encryption counter lower than the previous value. How is this covered in the code?


2024-01-13 15:52:06.124 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (42) is lower than or equal to the previous value (369). The data might be compromised. BLE advertisement will be skipped.
2024-01-13 15:53:11.034 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (54) is lower than or equal to the previous value (369). The data might be compromised. BLE advertisement will be skipped.
2024-01-13 15:53:23.529 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (55) is lower than or equal to the previous value (369). The data might be compromised. BLE advertisement will be skipped.
2024-01-13 15:56:05.966 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (84) is lower than or equal to the previous value (369). The data might be compromised. BLE advertisement will be skipped.
2024-01-13 15:56:18.441 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (87) is lower than or equal to the previous value (369). The data might be compromised. BLE advertisement will be skipped.```
Ernst79 commented 8 months ago

At the moment, we only allow this if the counter has reached the end of possible values.

https://github.com/Bluetooth-Devices/bthome-ble/blob/170877504fdf1a57b9cc87d84fe77c770a0c3710/src/bthome_ble/parser.py#L605

in all other cases, a restart of HA is necessary to restart the verification of the counter. So, after a restart, everything should be back to normal. I think it will be good if I add that to the logging.

bdraco commented 8 months ago

I wonder if it only makes sense to enforce the counter if the device sets some type of bit indicating that it wants that level of security, as I think the likelihood of an attack on a temperature sensor is probably quite low, but if the device was some type of locking mechanism, or otherwise, it might make a much better target.

If it's wildly off, we could assume a new cycle is states if the bit isn't set. Another option would be to trigger reauthentication and ask them if they'd like to allow the device to reset the counter, but that might just be annoying.

The other concern is that once we see the bit, it can never be unset by a forged advertisement.

rrooggiieerr commented 8 months ago

Ok, I have restarted HA and now don't get any more lower than the previous values. Would reloading the integration also have worked?

When the integration detects a lower than or equal to the previous value situation the integration is actually registering the device as being available and updating the temperature, but with the last known value, like in attached screenshot. Instead I think it should be set as being unavailable.

Screenshot 2024-01-13 at 20 24 45\ Here the blank parts of the graph is where I had removed the battery fromf the device, and the device was thus unavailable. While the horizontal lines are where I had put back the battery in the device and the device was detected as available but previous values were being logged due to the encryption counter being reset. Where the graph starts logging sensible data is when I have restarted HA.

I do get some equal to the previous values. I'm suspecting one or two devices which are the culprit but will look into that tomorrow.

Ernst79 commented 8 months ago

These are two different things.

  1. lower counter due to a battery change. The counter starts again at zero. HA sees a lower counter value and reports that as being suspicious and doesn’t parse the message. A restart of HA will cause HA to not have a previous counter value, and will process the first message. After that, it will check the counter value, compared to the first counter value after the restart. This should be an increase, so your sensor will work again.
  2. the same counter value. This message is strange, as explained before, as the log message would normally be filtered out by the first check in the code (if the counter value is the same and the service load is the same, skip the message, without a warning, as it is just a message it has already be seen). The only way to figure out why in your case you do see a warning, if adding the modification to the code, as suggested earlier, I’m afraid.
rrooggiieerr commented 8 months ago

Yes, I understand these are two different things.

Regarding the equal value encryption counter, I can not reproduce the issue on my dev environment (Macbook Air using the built in Bluetooth and 1 BLE Proxy configured).

I also tried something else; when I disable the onboard Bluetooth of my live HA system, and thus only use BLE Proxies, the encryption counter warnings for equal vallues disappear.

rrooggiieerr commented 8 months ago

Ok, I have been able to reproduce the issue on my dev environment:

2024-01-16 11:02:27.126 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (119) is lower than or equal to the previous value (119). The data might be compromised. BLE advertisement will be skipped.self.last_service_info (<habluetooth.models.BluetoothServiceInfoBleak object at 0x1657e78c0>) service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xb8\xda*Z\x8d\xb3f\xf8w\x00\x00\x00\xf0|\xcfS'}) self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xb5\n!\xe4\x8bw\x00\x00\x00\xa7|a)'}) self.bindkey_verified (True)
Ernst79 commented 8 months ago

Thanks, I see that the new service_data is different compared to the old service data.

service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xb8\xda*Z\x8d\xb3f\xf8w\x00\x00\x00\xf0|\xcfS'}) 
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xb5\n!\xe4\x8bw\x00\x00\x00\xa7|a)'}) 

So, the counter is the same, but the service data is different. That is the reason you get a warning. Question is, why is the service data different, while the counter is the same?

rrooggiieerr commented 8 months ago

That's the question indeed.

Hereby some others that got triggered.

2024-01-16 11:59:56.645 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (42712) is lower than or equal to the previous value (42712). The data might be compromised. BLE advertisement will be skipped.self.last_service_info (<habluetooth.models.BluetoothServiceInfoBleak object at 0x10833b5c0>) service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xc14\xddw-R}@\xd8\xa6\x00\x00=4\xb0\x97'}) self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xcc\xec\xd6\xa7)\xd8\xa6\x00\x00H\xd9\xf9\x80'}) self.bindkey_verified (True)
2024-01-16 13:12:03.410 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (1517) is lower than or equal to the previous value (1517). The data might be compromised. BLE advertisement will be skipped.self.last_service_info (<habluetooth.models.BluetoothServiceInfoBleak object at 0x1665ff140>) service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b"A\x94f'P\xf5>^}\xed\x05\x00\x00\xd1#\xc4\xf8"}) self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x99\x1d/\x9f\xf3\xed\x05\x00\x00a*l\x05'}) self.bindkey_verified (True) 
2024-01-16 13:13:03.348 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (1527) is lower than or equal to the previous value (1527). The data might be compromised. BLE advertisement will be skipped.self.last_service_info (<habluetooth.models.BluetoothServiceInfoBleak object at 0x10833a7c0>) service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x9d\xb4\xa7Q\x8b\xf1\x9f\xbe\xf7\x05\x00\x00\x81\xe7\xd0;'}) self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x90\xce\xaf\xa0\x8d\xf7\x05\x00\x00\x8b"\x9f\xfc'}) self.bindkey_verified (True)
service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xc14\xddw-R}@\xd8\xa6\x00\x00=4\xb0\x97'})
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xcc\xec\xd6\xa7)\xd8\xa6\x00\x00H\xd9\xf9\x80'})

service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b"A\x94f'P\xf5>^}\xed\x05\x00\x00\xd1#\xc4\xf8"})
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x99\x1d/\x9f\xf3\xed\x05\x00\x00a*l\x05'})

service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x9d\xb4\xa7Q\x8b\xf1\x9f\xbe\xf7\x05\x00\x00\x81\xe7\xd0;'})
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x90\xce\xaf\xa0\x8d\xf7\x05\x00\x00\x8b"\x9f\xfc'})

Meanwhile on my live HA installation I haven't received the warning since I've disabled the internal Bluetooth.

Ernst79 commented 8 months ago

Can you send me your encryption key, such that I can check the decrypted content?

Edit. The counter is indeed the same

**77000000**

41 b8 da2a5a8db366f8 **77000000** f07ccf53
41 b5 0a21e48b **77000000** a77c6129

**d8a60000**

41 c1 34dd772d527d40 **d8a60000** 3d34b097
41 cc ecd6a729 **d8a60000** 48d9f980
rrooggiieerr commented 8 months ago

Or maybe you can tell me how to decrypt the content?

Getting back on the disabled internal Bluetooth on my live HA system, I actually did get 2 warnings since I disabled the internal Bluetooth

2024-01-15 22:09:48.923 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (34586) is lower than or equal to the previous value (34586). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:10:15.639 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (33792) is lower than or equal to the previous value (33792). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:12:15.614 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (33813) is lower than or equal to the previous value (33813). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:15:25.740 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (33846) is lower than or equal to the previous value (33846). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:15:58.626 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (34652) is lower than or equal to the previous value (34652). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:16:05.859 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (33853) is lower than or equal to the previous value (33853). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:16:18.566 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (34655) is lower than or equal to the previous value (34655). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:16:25.851 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (33856) is lower than or equal to the previous value (33856). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:17:08.556 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (34664) is lower than or equal to the previous value (34664). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:17:35.935 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (33867) is lower than or equal to the previous value (33867). The data might be compromised. BLE advertisement will be skipped.
2024-01-15 22:18:15.881 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (33873) is lower than or equal to the previous value (33873). The data might be compromised. BLE advertisement will be skipped.
2024-01-16 01:14:46.804 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (36552) is lower than or equal to the previous value (36552). The data might be compromised. BLE advertisement will be skipped.
2024-01-16 07:20:07.797 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (40408) is lower than or equal to the previous value (40409). The data might be compromised. BLE advertisement will be skipped.
2024-01-16 11:32:11.440 WARNING (MainThread) [bthome_ble.parser] The new encryption counter (43038) is lower than or equal to the previous value (43038). The data might be compromised. BLE advertisement will be skipped.

I disabled the bluetooth around 22:18 and before that time the warning was there at least once a minute. And only 3 times since that time where one time the encryption counter was lower(!) than the previous value. So that might be interesting too

Ernst79 commented 8 months ago

The decryption is explained here. https://bthome.io/encryption/

There is also a link to a python script you can use at the bottom of that page. https://github.com/Bluetooth-Devices/bthome-ble/blob/main/src/bthome_ble/bthome_v2_encryption.py

Note the d2fc that is added in the example in front of the service data (d2fc is the UUID of BTHome).

rrooggiieerr commented 8 months ago

Ok, so should I do something like:

bindkey = binascii.unhexlify(<HEX bind key goes here>)
mac = binascii.unhexlify(<HEX mac address goes here>)
payload = b'A\x90\xce\xaf\xa0\x8d\xf7\x05\x00\x00\x8b"\x9f\xfc'
decrypt_aes_ccm(key=bindkey, mac=mac, data=payload)

?

Ernst79 commented 8 months ago

Yes, but with one small change. You need to add \xd2\xfc in front of the payload. You can do that as follows

In the payload, replace A with \x41 (which is the same) payload = b'\x41\x90\xce\xaf\xa0\x8d\xf7\x05\x00\x00\x8b"\x9f\xfc'

Next, add '\xd2\xfc` in front of the bytestring payload = b'\xd2\xfc\x41\x90\xce\xaf\xa0\x8d\xf7\x05\x00\x00\x8b"\x9f\xfc'

rrooggiieerr commented 8 months ago

Thanks, I just figured that out ;-)

rrooggiieerr commented 8 months ago

For the first two cases

service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xb8\xda*Z\x8d\xb3f\xf8w\x00\x00\x00\xf0|\xcfS'}) 
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xb5\n!\xe4\x8bw\x00\x00\x00\xa7|a)'}) 

I get: Decryption succeeded, decrypted data: 013202ae0703a816 Temperature: 5.62 Humidity: 7.75

Decryption succeeded, decrypted data: 0ce2091001 Temperature: 25.3 Humidity: 0.01

This is my living room sensor and at the given time 11:02 temperature was about 19.7 and humidity about 57% So both values are off

rrooggiieerr commented 8 months ago

The other cases:

service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xc14\xddw-R}@\xd8\xa6\x00\x00=4\xb0\x97'})
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\xcc\xec\xd6\xa7)\xd8\xa6\x00\x00H\xd9\xf9\x80'})

Decryption succeeded, decrypted data: 012302c00503211b Temperature: 5.47 Humidity: 7.73

Decryption succeeded, decrypted data: 0cfb091001 Temperature: 25.55 Humidity: 0.01

service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b"A\x94f'P\xf5>^}\xed\x05\x00\x00\xd1#\xc4\xf8"})
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x99\x1d/\x9f\xf3\xed\x05\x00\x00a*l\x05'})

Decryption succeeded, decrypted data: 013002df07035316 Temperature: 5.6 Humidity: 7.75

Decryption succeeded, decrypted data: 0c4b0a1001 Temperature: 26.35 Humidity: 0.01

service_info.service_data           ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x9d\xb4\xa7Q\x8b\xf1\x9f\xbe\xf7\x05\x00\x00\x81\xe7\xd0;'})
self.last_service_info.service_data ({'0000fcd2-0000-1000-8000-00805f9b34fb': b'A\x90\xce\xaf\xa0\x8d\xf7\x05\x00\x00\x8b"\x9f\xfc'})

Decryption succeeded, decrypted data: 013002e107035016 Temperature: 5.6 Humidity: 7.75

Decryption succeeded, decrypted data: 0c4a0a1001 Temperature: 26.34 Humidity: 0.01

Ernst79 commented 8 months ago

The decryption is ok, but the script is not really aimed to translate the data to something else than temperature and humidity. It was only an example to show the encryption and decryption.

But let me explain the data 013002e107035016, see also https://bthome.io/format/

0130 = object type 01 = battery, 30 in hex is 48 in dec --> 48% 02e107 = object type 02 = temperature, 07e1 in hex is 2017 in dec --> 2017 0.01 = 20.17 deg 035016 = object type 03 = humidity, 1650 in hex is 5712 in dec --> 5712 0.01 = 50.17 % RH

This look fine to me.

Now the next package with the same number 0c4a0a = object type 0c = voltage, 0a4a in hex is 2634 in dec --> 2634 * 0.001 = 2.634 volt 1001 = object type 10= Power, 01 means "On"

So, I think I understand what happens. I assume that this is a LYWSD03MMC sensor that is flashed with pvvx firmware. It sends two data packages, one with battery, temperature and humidity, and one with voltage and power. But both advertisements get the same encryption counter, which is not allowed. I think we should raise an issue at the pvvx repository.

rrooggiieerr commented 8 months ago

Thanks for explaining. 20 degrees and 50% humidity sounds realistic. Package sizes were also different, so I was already wondering how they could contain the same information. Interesting to see that I'm the first one who discovers this while these sensors and firmware are quite popular (I assumed?). Yes, I'm using 5 LYWSD03MMCs and the pvvx firmware. I should have mentioned that in my issue. Maybe add hardware and firmware make and version to the issue template for future raised issues?

There are still some questions crossing my mind:

I understand you might not be able to answer all these questions, but these are now just coming op after reading your explanation.

My thought would be that BLE is a fire and forget kind of messages, and not two way communication between sensor and server, and due to poor reception most of the messages get lost or crippled anyways so those that do make it to the server only incidentally are both the temp and battery data with the same encryption counter.

I'll raise an issue at the pvvx firmware Github and will refer to this issue. Hopefully this gets resolved soon and we'll have a more robust product in the end. Do you own one of these LYWSD03MMC sensors with pvvx firmware? Otherwise I'm happy to send you one (order one on Aliexpress for you), so you've got something to test with.

Then something else about replacing the battery. I know different issue. I found out that reloading the integrations also resets the encryption counter in the integration, so that's a lot more convenient than restarting HA all together. Just so you know.

rrooggiieerr commented 8 months ago

FYI: https://github.com/pvvx/ATC_MiThermometer/issues/468

pvvx commented 8 months ago

A long message is divided into groups and signed by one counter. The counter is an indication that this is a single event measurements. If the counter changes on blocks, then the opportunity to obtain a time reference for measurements is lost.

For example, 100 takes are transmitted in 10 minutes between measurement events. If the first part of the message is transmitted 50 times and signed by counter 1, and the second by counter 2, then the difference between receiving the information will be 5 minutes. If the first and second parts alternate, the data will be received and recorded immediately.

Cryptography is not affected in any way if different data is transmitted on the same meter.

Ernst79 commented 8 months ago

Ah, yes, I agree. If you would use counter 1 and 2 for part 1 and 2 of the data, and it receives the message with counter 2 first, e.g. due to bad reception, it will ignore all messages with counter 1, as the counter is lower. This is not what we want.

I'll create a fix, such that it only checks that the counter is equal or increasing.

rrooggiieerr commented 8 months ago

Ok, so that seems to be resolved then, great work! I've closed the issue in the pvvx ATC_MiThermometer repository and will close this one once I've been able to verify it on my dev system

Ernst79 commented 8 months ago

Yes, I think it is. you can test it by rewriting the two checks to


        # filter advertisements that are exactly the same as the previous advertisement
        if (
            self.last_service_info
            and service_info.service_data == self.last_service_info.service_data
            and self.bindkey_verified is True
        ):
            _LOGGER.debug(
                "The service data is the same as the previous service data. Skipping "
                "this BLE advertisement.",
            )
            raise ValueError

        # filter advertisements with decreasing encryption counter.
        if (
            new_encryption_counter < last_encryption_counter
            and self.bindkey_verified is True
        ):
            if new_encryption_counter < 100 and last_encryption_counter >= 4294967195:
                # the counter has (most likely) restarted from 0 after reaching the highest number.
                self.encryption_counter = new_encryption_counter
            else:
                # in all other cases, we assume the data has been comprimised and skip the
                # advertisement
                _LOGGER.warning(
                    "The new encryption counter (%i) is lower than the previous value (%i). "
                    "The data might be compromised. BLE advertisement will be skipped.",
                    new_encryption_counter,
                    last_encryption_counter,
                )
                raise ValueError
        else:
            self.encryption_counter = new_encryption_counter
pvvx commented 8 months ago

Ah, yes, I agree. If you would use counter 1 and 2 for part 1 and 2 of the data, and it receives the message with counter 2 first, e.g. due to bad reception, it will ignore all messages with counter 1, as the counter is lower. This is not what we want.

That's not the problem. If each transmitted block is signed by a new counter, the counter will quickly overflow. There will be several counters for the same data - this makes it possible to quickly obtain the encryption key.

In another case, different (unknown) data are signed by the same counter. That is, we need to look at which option is worse for cryptography...

PS: With the advent of PAwR (and BLE advertising encryption standard) in BT5.4, these problems are a thing of the past. Only relevant for Linux users, since Bluez does not even support BT5.0.