InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.64k stars 904 forks source link

No pairing is needed to re-flash watch #1814

Open annoyinganime opened 11 months ago

annoyinganime commented 11 months ago

Verification

Introduce the issue

Yesterday, when I was flashing my watch using WatchMate I found something that looks like a severe security issue. I disabled my phone's bluetooth (I read that when BLE connection is active no other device can connect to that BLE device), opened WatchMate on my laptop, selected there my pinetime watch, then selected "flashing", chose new firmware and began the process. And while the progress bar slowly filled, it dawned on me: in no point in time I EVER touch my watch. I just basically connected, and started flashing. So looks like if you keep watch disconnected with bluetooth still enabled anyone in range can connect and flash watch with potentially malicious firmware.

Preferred solution

What I suggest is any pairing process on watch side, even pressing simple "YES" on new connection would be good enough

Version

v1.13.0

everypizza1 commented 11 months ago

Sometimes I will shut down my phone overnight and then reconnect to gadgetbridge in the morning. It's much easier than pressing yes on the watch.

annoyinganime commented 11 months ago

@everypizza1, you didn't understand me, I want it to be done during first-time pairing, and not at every reconnect. I shut down my phone every night, and that's exactly an issue I'm describing: when your phone is disabled, anyone can connect to your watch without your consent, and upload malicious firmware to your watch

minacode commented 11 months ago

When pairing was introduced not all companions implemented it, so there had to be a phase where you could (but don't have to) pair your watch. This is where we currently still are. If I remember correctly, you get an encrypted connection while being paired.

Which companions support pairing right now and which don't? I think at least with iOS there is no pairing available.

annoyinganime commented 11 months ago

@minacode I am not talking about established connection, and pairing for encryption, and maybe I'm just bad at explaining, what I want to be implemented is something like: At first connection to new device, whether the connection is encrypted or not, infinitime asks user if new connection should be trusted, and if user presses "yes' - at adds MAC address to "known devices" list. So if there is a new connection from previously unknown device - user has to explicitly allow it. From my point of view this process can be implemented on watch, and no modifications for companion apps is needed

minacode commented 11 months ago

Ah, you mean #1264?

Avamander commented 11 months ago

In order for there to be a security issue there has to be either a risk with an impact that can manifest, or it has to bypass some existing guarantee. I don't see either of those being true.

Asking for consent before an update would be a reasonable feature request.

But it has to overweigh the risks of someone being unable upload a fix (for the touchscreen driver for example) or otherwise recover from a bad state.

FintasticMan commented 11 months ago

When using BLE secure pairing, you need to fill in a code that is displayed on the PineTime's screen. That seems like it would resolve this issue. As @minacode said, currently secure pairing isn't forced. It should be possible to either force a secure connection, or only allow firmware updates when on a secure connection.

evergreen22 commented 11 months ago

OP said "I disabled my phone's bluetooth (I read that when BLE connection is active no other device can connect to that BLE device)...then selected "flashing", chose new firmware and began..."

OP then said "So looks like if you keep watch disconnected with bluetooth still enabled anyone in range can connect".

What the OP read is true, but was misunderstood. The connection to disable is the watch, not the phone. When the OP disabled their phone's bluetooth, Infinitime disconnected and resumed advertising. When Infinitime is advertising, ANY device can request a connection. If the OP does not want this behavior, they have two choices

  1. Do NOT disable their phone's bluetooth so that the phone and Infinitime are still connected. The watch only supports a single connection.
  2. Disable BLE on the watch, therby preventing any connection to the Infinitime.

As stated by @Avamander this is not a security defect, it is how Infinitime is designed to work.

mark9064 commented 11 months ago

Since recovery mode exists, anonymous connections during normal operation could probably be disabled which resolves the root issue. It also cleans up other problems relating to open access of the watch.

And if anyone needs anonymous connections for development reasons (it's useful for me when debugging the PPG remotely), they can revert the change so that's covered too

minacode commented 11 months ago

As I already mentioned, there is no pairing support for iOS so I think that would be too aggressive.

mark9064 commented 11 months ago

Ah I missed that, that's a bummer. What's limiting support here? Struggling to find up to date resources on it, what I can find says that iOS should support all pairing modes (other than OOB)

Edit: Are you referring to section 41.10 here?

minacode commented 11 months ago

There is #920. It's some time since I last followed this. At the time we tried to implement Notifications (and eventually media control) for iOS and hoped that it would solve the problem. Sadly we failed, because developing against two devices with bad live-debugging is not fun 😄

After reading the specification again, I wonder how hard this would be:

If, for security reasons, the accessory requires a bonded relationship with the Central, the Peripheral should reject the ATT request using the Insufficient Authentication error code, as appropriate. As a result, the device may proceed with the necessary security procedures.

mark9064 commented 11 months ago

Reading that issue is honestly painful, why have apple got to make this so hard. And not even being able to tell if you're paired?? Like come on!

I don't know enough about the BLE spec to figure out if returning insufficient auth when not paired is something you can just do or if it requires encryption / other stuff, but if we could slap that on all / most of the characteristics then it sounds like that would work

JF002 commented 11 months ago

With secure bonding, InfiniTime accepts the connection from the phone only if the pin displayed by InfiniTime is entered in the phone companion app. This allows the user of the watch to accept BLE connections only from their own devices.

However, as others have already said, secure bonding is not enforced in InfiniTime right now, which means that InfiniTime will gladly accept any non-secure connection from any other device. I agree that this is not the most secure choice, but we had to do this while transitioning from the non-secure mode (that was implemented since the beginning of InfiniTime) to the new secure mode added afterwards : we didn't want users to be locked out of their watch because their companion app does not implement secure bonding.

Since then, I think most of the companion apps implement secure bonding, except InfiniLink (which is still looking for developers and maintainers) and maybe Watchmate.

FintasticMan commented 11 months ago

Perhaps we can make only the DFU characteristic encrypted, so that most features in companion apps that don't support secure pairing still work, but flashing firmware doesn't. Unfortunately I'm busy for a couple of weeks, and I'm not very familiar with BLE, so it might take some time before I can have a look into that.

LunNova commented 8 months ago

I'd prefer if pairing is only required for DFU too. I've found gadgetbridge's connection to my watch very unreliable in paired mode, works fine without pairing. Until that's fixed this change would be a big downgrade.

DavisNT commented 8 months ago

I am actually working on two PRs with two security features for this issue.

Do these two features sound like a good solution for this issue (and overall security improvements)?

1. Bluetooth "Restricted" mode.

In Settings - Bluetooth menu a third option "Restricted" would appear. When selected it would enforce to use existing Bluetooth bond (saved pairing) and prevent anonymous access as well as creation of any new bonds or pairings. Dynamic encryption key update would still work, repeated pairings would be disabled.

I thing a dedicated option would be a good first step towards securing connections, because it would allow to maintain maximum compatibility with existing companion apps as well as allow to lock down Bluetooth for users who want that.

2. Setting to control DFU (firmware update) and file transfer services.

In Settings - Firmware menu a new radio button group controlling access to DFU and FS services would appear. It would have options:

Maybe accessing the disabled services would also trigger a notification.

I think a separate option to disable these services would make InfiniTime a lot more secure, because these services are tho most sensitive, they aren't often needed and Bluetooth is not always intuitive and bug free (e.g. on InfiniTime v1.13 official build I noticed that once my phone had managed to connect to PineTime and send notifications despite Bluetooth setting having been set to Disabled; also restricting new pairings and bondings is much harder than I expected).

DavisNT commented 1 month ago

@JF002, @FintasticMan, @Avamander A question to maintainers: Are the two pull requests described in previous comment welcome? If I will rebase and update #1891, will it be merged?

DavisNT commented 1 week ago

@JF002, @FintasticMan, @Avamander Any feedback from project maintainers would be appreciated.

mark9064 commented 1 week ago

I'm not a core maintainer but I think in concept this looks great. All of the core devs are unfortunately tied up right now so you probably won't get a response in the short term (I'd love to be proven wrong though!)

I'd go ahead with rebasing if you can, as it means people can at least test your PR and feedback on it (I'd try it out for sure)

DavisNT commented 4 days ago

@mark9064 I have rebased and updated #1891, please test and report the results!

minacode commented 3 days ago

Also no maintainer, but I like your two ideas :)

Maybe we can simplify it and let the Bluetooth settings screen have the following options:

  1. on (radio button)
  2. off (radio button)
  3. encrypted (check button)
  4. file access (check button)

Functionality is the same, but it's more compact. Also "restricted" in contrast to "on" sounds to me like I would be missing something. "Encrypted" makes people want it.

We could go even further and reverse the button functionality and call it "allow unsafe".

DavisNT commented 1 day ago

@minacode Technically "encrypted" wouldn't be the correct term. Now, when the watch is paired to phone (or any other device) using the 6 digit PIN the connection is already encrypted. However the problem is that any other device can still "call Bluetooth services" on the watch, without any sort of authentication or authorization.

Regarding the settings, I would like to keep the "Till reboot" option for DFU and file access. I think this will perfectly fit the need for 99% of the users who want to restrict the access. At least I always use it before updating firmware, because after a successful firmware update the watch reboots and reboot resets the setting back to "Disabled" state.

minacode commented 1 day ago

I see, thank you for clarifying that for me. Now I support your idea as you stated it :)