Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No pairing is needed to re-flash watch #1814

Open
1 task done
annoyinganime opened this issue Jul 21, 2023 · 24 comments
Open
1 task done

No pairing is needed to re-flash watch #1814

annoyinganime opened this issue Jul 21, 2023 · 24 comments
Labels
enhancement Enhancement to an existing app/feature

Comments

@annoyinganime
Copy link

Verification

  • I searched for similar issues and found none was relevant.

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
Copy link
Contributor

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
Copy link
Author

annoyinganime commented Jul 21, 2023 via email

@minacode
Copy link
Contributor

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
Copy link
Author

annoyinganime commented Jul 21, 2023

@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
Copy link
Contributor

Ah, you mean #1264?

@Avamander Avamander changed the title No pairing is needed to re-flash watch (Security issue!) No pairing is needed to re-flash watch Jul 21, 2023
@Avamander
Copy link
Collaborator

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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

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

@mark9064
Copy link
Member

mark9064 commented Jul 23, 2023

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
Copy link
Contributor

minacode commented Jul 23, 2023

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
Copy link
Member

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
Copy link
Collaborator

JF002 commented Aug 6, 2023

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
Copy link
Member

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
Copy link

LunNova commented Oct 11, 2023

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
Copy link

DavisNT commented Oct 16, 2023

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:

  • Enabled
  • Disabled
  • Enabled till reboot

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
Copy link

DavisNT commented May 22, 2024

@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
Copy link

DavisNT commented Jun 24, 2024

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

@mark9064
Copy link
Member

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
Copy link

DavisNT commented Jun 30, 2024

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

@minacode
Copy link
Contributor

minacode commented Jul 1, 2024

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
Copy link

DavisNT commented Jul 3, 2024

@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
Copy link
Contributor

minacode commented Jul 3, 2024

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

@mark9064 mark9064 added the enhancement Enhancement to an existing app/feature label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing app/feature
Projects
None yet
Development

No branches or pull requests

10 participants