betaflight-configurator
betaflight-configurator copied to clipboard
betaflight
Add support for getting/setting elrs uid via receivers tab
Dependent on https://github.com/betaflight/betaflight/pull/11192
Adds support for getting and setting elrs passphrase via receivers tab:
- Allows entering passphrase directly - uid bytes are displayed below entry
- Calls msp API to get/set uid
- Saves mapping of uid byte string to passphrase into localstorage so it can properly display any passphrases its seen
UI when passphrase is known (passphrase retrieved from storage):
UI when passphrase is not known (not present in local storage):
UI when disabled (not spi rx, or older version of betaflight)
Kudos, SonarCloud Quality Gate passed! 
0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell
No Coverage information
0.0% Duplication
![sonarqubecloud[bot] avatar](https://avatars.githubusercontent.com/in/12526?v=4 "Published by a pub.dev verified publisher"){kind=small}
I don't know how ELRS works, but some doubts:
- What happens if the user changes the passphrase in another computer / device / CLI /etc.? I suppose this will show a bad passphrase when connected again?
- If the passphrase is unknown, there is no way to change it? I see you hide the input.
Seems a little confusing to me. If we can retrieve it from the FC/RX, I think is better to simply mask the passphrase with '*****' and let it change for a new value under user petition. A kind of write only value.
As I say, I don't know about ELRS so maybe my thoughts are wrong.
@McGiverGim: elrs stores a 6 byte hash of the passphrase. In this change, if the actual passphrase is unknown, it displays a representation of that hash, and the save button just saves the hash it read upon load unless the user changes it. It might be less confusing to mask it out in that case, maybe via a placeholder.
I do like the idea of showing the actual passphrase if we know it, because I can plug my FC into betaflight and see if it's set correctly for that FC. I think the most common case would be someone wants to set all of their quads to the exact same passphrase, and this gives an easy way to see whether it's correct.
Sorry for my late reply, I was out ;)
I'm more of the opinion that if it is a hash, it is a one-way password and is better to not store it, simplifying the things. But I don't use the system, so if others developers/users think is ok, then I will not oppose to it.
@jeffpearce I think having indication that there is a password, but it's only hash is better than hiding? We'd need to indicate that it will overwrite the existing password, but it would also help since user would see about the current state not just overwrite what's there already.
Im waiting for elrs tx/rx this week, but i should adquire and spi rx fc to start testing that and how it works.
AUTOMERGE: (FAIL)
- github identifies PR as mergeable -> FAIL
- assigned to a milestone -> PASS
- cooling off period lapsed -> PASS
- commit count less or equal to three -> PASS
-
Don't mergelabel NOT found -> PASS - at least one
RN:label found -> PASS -
Testedlabel found -> FAIL - assigned to an approver -> FAIL
- approver count at least three -> FAIL
Kudos, SonarCloud Quality Gate passed!
0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells
No Coverage information
0.0% Duplication
@chmelevskij:
I think having indication that there is a password, but it's only hash is better than hiding? We'd need to indicate that it will overwrite the existing password, but it would also help since user would see about the current state not just overwrite what's there already.
I'm sorry, but I don't understand the suggestion. What I'm considering right now is always showing the hash (since we read that from Betaflight), but not attempting to convert it back the the plaintext password. If the user types in a new password, they would see the hash change, and we could even treat it differently (bold, different color) if they do that. Does that sound along the lines of what you're suggesting, or have I misunderstood?
we could even treat it differently (bold, different color) if they do that.
yep, something to show that there is existing and it will be overwritten.
How about showing only the hash, but add a 'Generate' button (or similar) that prompts for the password (pop up), the hash is then replaced if the person selects ok.
md5 should be replaced with sha-256 :joy:
https://www.npmjs.com/package/sha.js https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf
@haslinghuis if depends on what you’re doing with it. If you’re implementing a password scheme or trying to keep something secret, md5 isn’t safe. If, on the other hand, you’re just using it as a hash function to convert an arbitrarily long string to something of known length as we are here, it’s a good choice. This needs to match the value stored in the transmitter, which is the md5 hash of the passphrase as entered in the expresslrs configurator.
AES-256 should be military-grade encryption. Just testing the waters if this would be possible.
@chmelevskij this is only done in the configurator. Both the rx and tx store this hash value, and if they match, the tx will talk to that rx - it's a convenient substitute for binding. This change is only applicable to SPI receivers, as the ELRS configurator handles storing it on an external rx.
I'm not sure if I made it clear, but inputting this hash value is already available via the Betaflight CLI. This is meant to make the process a little more user friendly. Currently, you need to go the the ELRS website and input your passphrase; it creates the hash and gives you the CLI command for storing it on the FC.
@jeffpearce, I just had a quick look. When you think PR is ready to merge please make sure it's only one commit (squashed). Thanks!
We are using less and not using css anymore, maybe that is breaking the build?
yarn gulp release --android doesn't work.
Kudos, SonarCloud Quality Gate passed!
0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells
No Coverage information
0.0% Duplication
Cordova doesn't like crypto-js for some reason. IDK if it needs babel, or if it's something else. I could work around this by copying the MD5 code, but that's not my preferred solution.
Do you want to test this code? Here you have an automated build: Betaflight-Configurator-Linux WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@jeffpearce not sure here, but have no problem with alternatives here. Could make alternative for affected Operating Systems only or even as unsupported (yet).
Hey @jeffpearce does it work by just having the code in repo?
It wouldn't be the first package we do this to. Given that we are multiplatform and quite complex it's often easier that than trying to fix it upstream.
@chmelevskij i think I have a solution. I’ll do a bit more testing and push my changes tomorrow if I don’t find any problems
Do you want to test this code? Here you have an automated build: Betaflight-Configurator-Android Betaflight-Configurator-Linux Betaflight-Configurator-macOS Betaflight-Configurator-Windows WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!
FWIW and if this is still under debate (pr 11192), i think it is really un-important to do any privacy masks in the configurator. any ELRS "exploit" concerns are not of any essence here, IMHO.
QuickSilver allows pilot to type the phrase visible in plaintext and the hash is visible as plaintext as well. Privacy is not a real factor.
if it's really desired, then so be it, but diff/dump bind-code is plaintext.
