-
Notifications
You must be signed in to change notification settings - Fork 222
BLE Pairing and Encryption #156
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
Conversation
Initial - encrypted read
With write encryptionn requirement
Write encryption & visible LTK / IRK
Fixed packet fragmentation
Android bugfix
Fix timing issue during pairing, add pairing control/status methods
Secure random & reject unknown LTK
Pairing code & IOCaps set via implementation of callbacks
Pull request in relation to: #36 and unknownconstant#11
shemminger
commented
Mar 20, 2021
Not all devices want to support pairing. This code doesn't seem to do that.
I would like if it were possible to respond "Pairing not supported" as described in Bluetooth Core Spec C.5.1
Happy to take a look - which bit of the spec are you looking at for this? There are a number of volumes which have a C.5.1.
The code in this PR currently only supports the peripheral device, so if the device using this code does not support pairing then it shouldn't be set to require encryption. I understand for BLE that the central will instigate encryption only after the peripheral has sent an error response indicating insufficient encryption or authentication.
Volume 3, H 3.5.1 describes bonding flags in the pairing procedure. When testing I found that these flags did not affect whether an iPhone stored pairing data or not but I would welcome more info if you're able to test this or have any other insights.
shemminger
commented
Mar 22, 2021
A little more background. I am building an Ardunio BLE device which emulates a proprietary BLE device that provides steering angle control for the bike training game Zwift. The protocol is already reverse engineered (so thats easy). And the Nano BLE has magnetic sensor, and that can be used as directional input. This is just a dumb BLE device (similar to Battery monitor etc).
The code works talking to Android, and probably MacOs. But does not work with Windows 10 the application connects, but then gets no data. I suspect the lack of pairing support. Doing full encryption is overkill for this application. Wondered if this would be enough to just respond with "not supported"
shemminger
commented
Mar 23, 2021
I did some more experiments (and tried to learn about the twisted maze of BLE security).
It looks like just setting 0 in the auth request bits in the pairing response is enough to get what I need (pairing without encrypt).
BUT the value is hard coded in this version (see HCI.localAuthReq); looks like you weren't finished with that bit yet.
What would the API be for an application to override the default authentication request settings.
Something like:
BLE.setLocalAuth(AuthReq)?
CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.
✅ eltos
❌ unknownconstant
unknownconstant seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.
Compile is catching this warning:
/home/shemminger/Arduino/libraries/ArduinoBLE/src/utility/L2CAPSignaling.cpp: In member function 'virtual void L2CAPSignalingClass::handleSecurityData(uint16_t, uint8_t, uint8_t*)':
/home/shemminger/Arduino/libraries/ArduinoBLE/src/utility/L2CAPSignaling.cpp:296:13: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized]
for(int i; i<6; i++) peerAddress[5-i] = identityAddress->address[i];
^
Looks like missing i = 0
Lots of other warnings now introduced.
/home/shemminger/Arduino/libraries/ArduinoBLE/src/utility/L2CAPSignaling.cpp: In member function 'virtual void L2CAPSignalingClass::handleSecurityData(uint16_t, uint8_t, uint8_t*)':
/home/shemminger/Arduino/libraries/ArduinoBLE/src/utility/L2CAPSignaling.cpp:271:8: warning: unused variable 'pairingFailed' [-Wunused-variable]
} *pairingFailed = (PairingFailed*)data;
^~~~~~~~~~~~~
/home/shemminger/Arduino/libraries/ArduinoBLE/src/utility/L2CAPSignaling.cpp:320:7: warning: unused variable 'readPublicKeyCommand' [-Wunused-variable]
} readPublicKeyCommand = {
^~~~~~~~~~~~~~~~~~~~
/home/shemminger/Arduino/libraries/ArduinoBLE/src/utility/L2CAPSignaling.cpp: In member function 'virtual void L2CAPSignalingClass::smCalculateLTKandConfirm(uint16_t, uint8_t*)':
/home/shemminger/Arduino/libraries/ArduinoBLE/src/utility/L2CAPSignaling.cpp:421:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
for(int i=0; i<sizeof(Eb); i++){
~^~~~~~~~
Hoping to get some time in the next couple of days to take a look at this properly
Hi @shemminger , I've not been able to reproduce these errors using the standard Arduino compiler. Is this what you have used, or are you using another toolchain?
When cloning this library on another computer, a number of errors were generated as I had mistakenly included both this version, and the official release. It may be worth checking this?
shemminger
commented
Apr 26, 2021
via email
AmirMahdiNassiri
commented
Jun 1, 2021
Any update on when we can have this feature released as an official release?
I don't know, there seems to be a licence issue. I've signed but it states it's still pending which may be preventing a merge. We may be waiting on the Arduino team to review the code.
I'm so sorry for jumping late into this. It looks like there's an issue with CLAbot since I do see that the user @unknownconstant signed the CLA, but we can easily force that.
On the other hand, this pull request contains commits also from @eltos who doesn't seem to have gone through CLA. @eltos, it would be very nice if you could spend a few seconds for that! :)
@alranel CLA signed ✅
muadiber
commented
Sep 29, 2021
@alranel can you merge pull request?
brontesprocessing
commented
Sep 30, 2021
@alranel Any chance to have it merged any time soon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the examples with my Android phone and a Nano 33 BLE and they worked like a charm! :)
brontesprocessing
commented
Oct 11, 2021
@alranel It looks like the Pull Request is approved. What happens next? Is it possible that new release will be published soon? Thank you in advance!
Sorry for being so slow - just one last confirmation: did you write all this code or did you take any part of it from other sources?
Looking forward to merge this. :)
Yes, the code is either original, refactored from this repo, or in the case of a couple of the crypto functions e.g. AES_CMAC is taken from rfc4493, and Bluetooth toolbox functions are implementations of the published Bluetooth LE spec.
Do we progress or other implementation is needed?
Unsure - AFAIK it's just waiting on being merged
The features provided by this pull request would be great to have, I look forward to it being merged.
Is this still waiting on resolution of a license issue?
@alranel would you be so kind to advise?
Yup we were still waiting for the CLA to be signed by all the commit authors but it looks like it was and the bot is not actually behaving correctly... I'm going to merge it anyway 😉
For future reference, the reason the CLA check was not passing is because the check is done against the owner of each commit in the PR. The CLA is signed by your GitHub user account, so the commit can only be verified when its email address (as set via the user.email
Git configuration key) is associated with the GitHub account.
You can see that some of the commits from @unknownconstant had one email address configuration. For example:
and other commits had another. For example:
The bot attempts to communicate this situation in the comments:
unknownconstant seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
5 years ago, support for encryption was added by arduino-libraries#156. Currently, the BLEEncryption tag needs to be added to require proper encrypted bluetooth pairing. It is possible to add this tag to all other kinds of characteristics except String characteristics. BLEStringCharacteristic's constructor calls BLECharacteristic's constructor, and BLECharacteristic uses a uint16_t instead of a uint8_t. This PR simply changes the properties attribute of BLEStringCharacteristic to match the width of the permissions attribute of BLECharacteristic (from 8 to 16 bits) to support the BLEPermission tags in BLEProperty.h.
This pull request is for an implementation of the BLE Security Manager which allows arduino devices to pair & encrypt connections using the HCI specification. Specifically, it implements Secure Connect / Numeric comparison.
The branch appears stable but I'm aware it may need some refactoring. An example sketch is included to show how to pair devices.
The branch supports encryption instigated by the central when the arduino is the peripheral.