-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Pin lock - RFC #622
base: master
Are you sure you want to change the base?
Pin lock - RFC #622
Conversation
Signed-off-by: Dado Mista <[email protected]>
Signed-off-by: Dado Mista <[email protected]>
When calling LOCK_STATUS we now also return another flag to indicate when the pin is 0. Signed-off-by: Dado Mista <[email protected]>
Odometer in the app isn't entered in meters, so it's tricky to produce an exact match. Check for match within 500m...
Signed-off-by: Dado Mista <[email protected]>
LOCK_STATUS now also returns whether lock_on_boot is set to true Bugfix in check-writelock function Signed-off-by: Dado Mista <[email protected]>
this is interesting for who uses ebike-scooters and leave them parked or use them in crowded areas where other people could be malicious. |
Agreed - in the Float Package for example we have a feature to disable the whole board. Combined with the PIN lock it would be impossible for a 3rd party to get the vehicle running again, without disassembling it and accessing USB or the debug pins. |
Great feature. First of all my question is why is the official UUID pairing even a thing if it's client side? That renders it completely pointless and it should just be entirely removed if that's how it functions IMHO. Second question, is there a timeout of how many pin combinations can be attempted before requiring a power cycle (i.e. preventing a brute-force attack)? |
First question, idk - not my implementation. It was developed back when VESC Tool was the only tool/app available to prevent accidental pairing of the wrong remote. Second, we can consider making it more hacker proof later, right now the aim is to keep it simple and idiot proof. |
I submitted a PR to @surfdado branch for this btw: surfdado/bldc#4 "Adds a new command to COMM_WRITE_UNLOCK_CMD where you can append the pin before the actual command and payload which will handle writelock unlocking/locking while handling the command. Calls the command processing function recursively like COMM_FORWARD_CAN. Fixed some vulnerabilities with stack overflow by limiting recursion depth (which was also in COMM_FORWARD_CAN) as well as fixing brute force attempts for writelock." As bldc gets more popular in semi-commercial applications (such as the Floatwheel), I do think it's worth putting some time aside for discussion and development of some basic security features in this safety critical system. |
I'll check it out as soon as I get a chance! |
would like this to be added officially would save a lot of confusion when connecting with multiple boards around |
This isn't meant to be merged yet, mainly posted it for people to comment on or test independently.
Basic idea is to introduce a simple method to prevent accidental writes to the wrong VESC by using a 4-digit PIN to write-lock your firmware. Unlike pairing it is actually enforced by the firmware, so rogue 3rd party apps cannot circumvent it. Also, a 4-digit PIN is easy to remember.
PIN is stored in EEPROM just like the Odometer, however this implementation does clear the PIN during firmware writes.
Functionality is fully described here:
https://pev.dev/t/new-feature-spec-pin-locking/815/4