Bluetooth-Devices / bthome-ble

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

Encryption rolling #20

Closed nagyrobi closed 8 months ago

nagyrobi commented 1 year ago

Is your feature request related to a problem? Please describe. Please add some kind of rolling key mechanism. Eg. when sending an event or the same value repeatedly via BTHome, will the ecryption result in the same data? If yes, then it needs some extra random stuff added else it can be spoofed...

Describe the solution you'd like When the raw data is being sniffed from the air, and re-transmitted from a different device, with the same mac address, is there a way to determine that some manipulation has been done?

Additional context Presence detection can suffer from this. A legitimate token could be spoofed by re-transmitting the same data from a different device. The data inside the packet is encrypted, that's fine, and the contents can't be seen by the different device, however, the HA system will think that the original device transmitted it...

Follow up from https://github.com/stumpylog/bthome-weather-station/issues/1

Ernst79 commented 1 year ago

Isn't the 4 byte counter taking care of that? The counter should be a changing number (4 bytes) in each advertisement. We have added some more info about the encryption in the V2 docs (will be released this week, so I only have a github link to the docs for now)

https://github.com/home-assistant/bthome.io/blob/7f30b11e5f4c66f9022744cd2d671932e2489314/encryption.html#L144

I'm not totally familiar with encryption, so please correct me if I'm wrong, but I thought the counter should be a changing value in each message. However, I'm not sure what happens if a BTHome receiver receives two exactly the same messages (encrypted with the same counter value)

nagyrobi commented 1 year ago

And what happens if the sender is outside the range of the receiver, while sending data packages?

Counter increases on senders side, but not on receivers side...?

Ernst79 commented 1 year ago

And what happens if the sender is outside the range of the receiver, while sending data packages?

It will just not be received, but a following message will still have a different counter, so it can be parsed.

Counter increases on senders side, but not on receivers side...?

I think the receiver should check that the counter has changed compared to the previous counter. However, I'm not sure if this is already checked in the decryption process, or if this should be added to the BTHome code.

pvvx commented 1 year ago

With the same data and the same counters, the code in the packed will be equal. A packet with broken data or a counter is not decrypted - unpacking return error. BLE advertising transmission interval with counter change for new data in a typical minimum of 2.5 sec. Usually from 2.54 = 10 sec, i.e. equal to the set measurement interval at default settings. As a result, before repeating the counter value, it will pass: 10 seconds 4294967296 (32-bit counter) = 1362 years. Additionally, the transmitted values (temperature, humidity, ...) change, which introduces additional uncertainty for decrypting the key.

Presence detection can suffer from this. A legitimate token could be spoofed by re-transmitting the same data from a different device. The data inside the packet is encrypted, that's fine, and the contents can't be seen by the different device, however, the HA system will think that the original device transmitted it...

BLE in advertising mode lacks the two-way communication and acknowledgment needed to implement more complex algorithms. SoCs for BLE have small software and hardware resources for encryption, and the basis of BLE is Low Energy. The end result is a compromise...

bdraco commented 1 year ago

With HomeKit encryption, which also has a counter, we try to decrypt with value we expect the counter to be, then we try one back, and then we try the next 30 increments before declaring desync and connecting back to the accessory to ask it to regenerate the key.

bdraco commented 1 year ago

https://github.com/Jc2k/aiohomekit/blob/065994eb754f168139a91f5b1ce258c4cbc36345/aiohomekit/controller/ble/pairing.py#L571

pvvx commented 1 year ago

The amount of code for HomeKit in the chip is more than twice that of BLE. The consumption of a device with HomeKit firmware is much higher. The cost of assembly increases. These are serious limitations for DIY.

bdraco commented 1 year ago

I'm not thinking it would be a good idea to implement the HomeKit stack. I'm only trying to offer a solution to when the counter is out of sync.

pvvx commented 1 year ago

Not suitable for wearable sensors = "Failed to set broadcast key, try un-paring and re-pairing the accessory."

A sufficient condition is that the new counter is larger.

Ernst79 commented 1 year ago

yes, for wearables, that will be a problem. If you are out for 5 minutes, you will have to repair. But checking on an increasing number should be workable.

nagyrobi commented 1 year ago

I was thinking for a use case like a garage door opener, when broadcast (and a code roll) would only occur on a button press...

Ernst79 commented 1 year ago

Should still work if we only check on an increase of the counter. As long as it is higher, it is ok. Question can be if this adds much security

bdraco commented 1 year ago

I'd be a little concerned about using it as a garage door remote, because you could interfere, capture the ble packet, then replay it later.

but probably unlikely .. https://xkcd.com/538/

nagyrobi commented 1 year ago

How do simple 433MHz rolling code garage door openers work?

Ernst79 commented 1 year ago

@pvvx Do you know why we have this cipher.update(b"\x11") in the decryption code? This requires everyone to implement this in their sensor firmware as well, in the encryption process, but I don't see the benefit?

From the docs, I understand that this is just some header, that is not encrypted at all (see below).

I'm thinking about removing it in BTHome V2, as it is probably not used yet in real life (HA support is planned for 2022.12).

I remember that MiBeacon also uses this.

Do you agree that this can be removed?

update(data)

Authenticate those parts of the message that get delivered as is, without any encryption (like headers). It is similar to the update() method of a MAC object. Note that all data passed to encrypt() and decrypt() get automatically authenticated already.

Parameters: | data (bytes) – the extra data to authenticate -- | --
update(data)[¶](https://pycryptodome.readthedocs.io/en/latest/src/cipher/modern.html#update) Authenticate those parts of the message that get delivered as is, without any encryption (like headers). It is similar to the update() method of a MAC object. Note that all data passed to encrypt() and decrypt() get automatically authenticated already. Parameters: data (bytes) – the extra data to authenticate
pvvx commented 1 year ago

Removing cipher.update(b"\x11") will cause the decryption function to respond: Decryption failed: MAC check failed In the en/crypted message, "Mic" is changed.

Ernst79 commented 1 year ago

But if you remove it both in the encryption and decryption function, it works ok.

In the fix-remove-cipher-update branch, I've updated the example in which the cipher.update(b"\x11") is removed both in the encryption and decryption code.

https://github.com/Bluetooth-Devices/bthome-ble/blob/fix-remove-cipher-update/src/bthome_ble/bthome_v2_encryption.py

pvvx commented 1 year ago

This means that all users will have to flash new firmware on already flashed devices? :)


I think the receiver should check that the counter has changed compared to the previous counter. However, I'm not sure if this is already checked in the decryption process, or if this should be added to the BTHome code.

In mijia, the counter is stored in Flash approximately every 512 steps. When the battery is replaced, it transmits the previous multiple of 512. Replacing the counter in some sensors is equal to the step of transferring new data. For some, it's 15 minutes. It will take several days until the counter crosses the threshold of the old value. In some firmwares (and sources found) with a cold start, it seems that there is an addition to the counter at once 512 steps.

kzyapkov commented 1 year ago

Users will need to flash new firmware with BTHome v2 support anyway, it adds a leading byte with flags/version and a new data format. So dropping the 0x11 "authenticated data" byte from the CCM cypher, while a breaking change from v1, is not the only one. Crypto will work with or without it, it just needs to be settled. @pvvx , what was the rationale for having this byte in the first place?

As for the counter, we intend to use the 4 bytes as a timestamp in seconds (2.5s increments were suggested here but seconds seem simpler). This will be used for the "garage door opener" scenario -- observers will limit the validity of the event packet in time.

Ernst79 commented 1 year ago

This means that all users will have to flash new firmware on already flashed devices? :)

Only for BTHome V2. I will leave it in V1 as it is now, so V1 devices won't stop working.

        if sw_version == 1:
            cipher.update(b"\x11")

V2 is totally rewritten, with a new UUID, so for V2 you will have to flash the device anyways.

pvvx commented 1 year ago

what was the rationale for having this byte in the first place?

In order to use the same encoding procedures as the mijia version -> to reduce the size of the code.

As for the counter, we intend to use the 4 bytes as a timestamp in seconds

"Time stamp in seconds" is not suitable for a door or motion sensor in combination with a thermometer and other sensors. Messages with new data are transmitted more than once per second. 'Trigger events' are usually transmitted by multiple BLE advertisements (6..16 bursts) in 50ms increments. Other measurements are transmitted in large and stable steps.

If the packet size does not allow to contain all the data, then the message is divided into several packets - advertisements. This option can change the counter more often than 1 second.

Plus, most BLE chips do not have clock quartz and they calculate the time approximately, with poor accuracy, from the built-in RC generator. Discrepancies can reach tens of minutes per day, depending on temperature and other factors.

Ernst79 commented 1 year ago

In order to use the same encoding procedures as the mijia version -> to reduce the size of the code.

I still propose to remove it than (in V2) as there is no real need to add this byte. Codewise, this should not be a big change on the pvvx/atc firmware code, I assume.

if advertising_format != BTHome_V2:
    cipher.update(b"\x11")
pvvx commented 1 year ago

V2 will consume more battery power due to the additional version flag. :)


When will LE Coded PHY (Long Range) advertising be accepted on the main channels? BT5.0 at June 16, 2016 !

Ernst79 commented 1 year ago

V2 will consume more battery power due to the additional version flag. :)

Why?, we add one byte for the version flag, but the first byte of each measurement is removed (the byte indicating the length and format). In the end, the length of the advertisement will be shorter or equal.

Example for temperature + humidity

V1: 1C18 2302C409 0303BF13 V2: FCD2 40 02C409 03BF13

pvvx commented 1 year ago

But we add 3 more bytes of flags to the general message...

Ernst79 commented 1 year ago

Ah, yes, but that isn't related to V1 vs V2. Both will have the same issue.

I will ask @bdraco on discord if he has an idea, as that looks more like something that is defined in the core HA bluetooth integration. I think ble_monitor will just work fine without 020106 in active mode. But lets discuss that in the other topic.

pvvx commented 1 year ago

"Time stamp in seconds" is not

Addition: Current implementations can send up to 6 different messages per second. Of these, 4 trigger events - a pack of 5 messages with a step of up to 50 ms, a message with counters overflow, one typical announcement about the general state of the sensors. Those. transmissions use 6 counters per second...

Clarification: For example, if the reed switch fires more than 5 ms, then one more pack of advertisements will appear, and the old pack of advertisements about the past state will break off. As a result, the modified Xiaomi LYWSD03MMC and with the addition of a reed switch can transmit up to 200 advertisements 'open-close' with signature counter changes per second.

sdrsdr commented 1 year ago

After some brainstorming a idea is formed: use 3bytes lower part of 32bit unix timestamp plus one byte counter-in-the-second this allows for 256 unique packets in second with compromise that a packet can be replayed nearly every 9th year. And if you re-key your device before 9 years have passed you're safe from replay attacks

Ernst79 commented 1 year ago

cipher.update(b"\x11") method has been removed for encryption/decryption in V2 in release V2.3.0. V1 will remain as it is now, no changes there.

pvvx commented 1 year ago

A year later, Bluetooth SIG standardized similar data encryption in advertising in the "Bluetooth Basic Specification version 5.4".

CCM is the Counter with CBC-MAC block cipher mode. It is used with a 128-bit block cipher such
as AES to encrypt and authenticate messages. Authentication is achieved by including a calculated
message authentication code (MAC) which the Bluetooth Core Specification calls a MIC (Message
Integrity Check).

PS: I wonder how much longer everyone will have to wait for "Periodic Advertising with Responses (PAwR)" to work on Linux, if the Bluetooth 5.0 version released in 2016 is not supported today?

esev commented 12 months ago

Would it be possible for the documentation to be updated to reflect that the counter is currently not being validated? Being familiar with CCM, I was using this implementation with the expectation that the counter was being validated to prevent replay attacks. It was surprising/unexpected to see this issue.

Ernst79 commented 12 months ago

Yes, will do that. I’m not so familiar with CCM encryption, we basically copied the encryption from xiaomi MiBeacon, Xiaomi’s BLE format.

Are you willing to add this validation to BTHome? Or can you explain to me how this validation should work?, such that I can implement it.

pvvx commented 12 months ago

To begin with, it is advisable to enter something like this: If the counter value is less than the previous one, then print a "Warning".

esev commented 12 months ago

Context: I came across this issue while looking into details of how the Shelly BLU Button1 works with Home Assistant. I found that despite the Button sending increasing counter values, nothing seems to be implementing the replay protection feature supported by AES-CCM.

To implement the replay protection features of AES-CCM, the counter values need to be checked to ensure they aren't being reused. Typically the counter is an ever increasing integer. Only accepting messages with counter values that are greater than the previous value is sufficient.

However, this library doesn't have any persistence logic, it currently doesn't store values across reboots for example. So I'm not sure if this is the correct place to implement the feature. That's why I suggested a documentation update, so clients using the library know that this needs to be implemented on their end.

For my specific use-case, I just want to know that the button has been pressed. Using a change to the 'Packed Id' sensor as a trigger, and verifying it was greater than the previous value, worked for me. But this is very specific to my use-case and to the Shelly device. https://gist.github.com/esev/49b0ced41ac32906b7de796c0b81e1f7#file-garaage_remote-py-L83

It would be great if this feature was included at a lower level where it could be supported generically for all devices that support V2 encryption. As I mentioned, I don't know if this logic belongs in this library, or in the client of the library. But from a Home Assistant user perspective, I'd like to have:

Some suggestions for changes to the specification to improve interoperability:

  1. Describe that the counter should be increasing in value. That ensures the counter is easy to verify. The current specification only says they must be unique. It is described as a "counter" though, so it may already be clear how it should be implemented. So I don't feel too strongly that this needs to be clarified.
  2. It looks like the counter is currently meant to be an unsigned 32-bit little-endian integer. The specification doesn't mention the least significant byte should come first. Add this info to the specification.
Ernst79 commented 12 months ago

Clear, not sure if I'm able to implement it all, but let's start with the docs.

These are done

image

Ernst79 commented 12 months ago

Created a first step based on @pvvx his suggestion to add a warning in PR #92

This should log warning if the encryption counter is ~equal or~ decreased.

esev commented 12 months ago

It looks like the counter is currently meant to be an unsigned 32-bit little-endian integer.

I'm kind of confused on this now. I had seen this logic, and that lead me to believe the value was little-endian. Please correct me if I've gotten this wrong.

https://github.com/Bluetooth-Devices/bthome-ble/blob/4cb68589dabe995df04bb6abf2cbc9a1e62d9687/src/bthome_ble/parser.py#L260

I think I had confused this for the SensorDeviceClass.PACKET_ID. My apologies if I have gotten this wrong!

esev commented 12 months ago

Ok. I think count_id is in little-endian. I added a log message after this line to confirm. Sorry for my confusion above. I just didn't want to lead anyone astray if I had misinterpreted the code.

https://github.com/Bluetooth-Devices/bthome-ble/blob/4cb68589dabe995df04bb6abf2cbc9a1e62d9687/src/bthome_ble/parser.py#L558

_LOGGER.error("count_id=%s", repr(count_id))

For my LYWSD03MMC sensors, these are counter values.

[bthome_ble.parser] count_id=b'\xa0\x19\x00\x00'
[bthome_ble.parser] count_id=b'\xa1\x19\x00\x00'

For my Shelly BLU Button1 sensors, these are timestamp values (they're always increasing values, so that's okay).

[bthome_ble.parser] count_id=b'\xce\xe5%e'
[bthome_ble.parser] count_id=b'A\xe6%e'

Parsed as a little-endian unsigned int, these are:

>>> int.from_bytes(b'\xce\xe5%e', "little")
1696982478
>>> import time
>>> time.time()
1696982567.8120687
>>> int.from_bytes(b'A\xe6%e', "little", signed=False)
1696982593