trustcrypto / OnlyKey-Firmware

The OnlyKey Firmware runs on the OnlyKey itself and provides the core functionality of OnlyKey.
https://docs.crp.to/firmware.html
212 stars 40 forks source link

"space backslash char" in password field leads to stripped output #126

Closed euidzero closed 2 years ago

euidzero commented 2 years ago

After upgrading to v2.1.1-prodc from beta8 the sequence : "space backslash char" leads to stripped output :

"test\ktest" is correctly output "test \ktest" is output as "testt" "test \kabc" is output as "test"

FW OnlyKey v2.1.1-prodc App : App v5.3.3

Note: I made sure sysadmin mode is disabled. Same issue with FW 2.1.0

onlykey commented 2 years ago

This is because the " \"combination is used to start sysadmin mode operation. We tried to make this a combination that is not common and not likely to interfere with any known use cases. Without sysadmin mode enabled the only options that can be used are TAB and RETURN which are " \t" and " \r".

euidzero commented 2 years ago

It would be great to allow the same behaviour as beta8 when sysadmin mode is disabled. The key is advertised as acting like a keyboard and send all possible single key codes. This is no more true.

Upgrading the fw broke silently existing entries containing this sequence.

Any chance to fix this in future upgrade? Thx

Le mer. 15 sept. 2021 à 15:14, onlykey @.***> a écrit :

This is because the " " combination is used to start sysadmin mode operation. We tried to make this a combination that is not common and not likely to interfere with any known use cases. Without sysadmin mode enabled the only options that can be used are TAB and RETURN which are " \t" and " \r".

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/trustcrypto/OnlyKey-Firmware/issues/126#issuecomment-920007145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADLUHAXREBGUX54AJSKVRLUCCLZXANCNFSM5EB467DA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

onlykey commented 2 years ago

Is there a specific requirement to be able to send these specific keys? I am not aware of a use case. If there is we can certainly consider this. The other consideration is that in every case there needs to be a way for user's to send a TAB or RETURN as this is a feature used by a large number of users.

euidzero commented 2 years ago

space and \ are valid characters for a passphrase and " \" is a valid sequence too in this context.

onlykey commented 2 years ago

We will leave this issue open to see if anyone else is affected but currently the only limitation is you can't use the " \"combination as a password in a slot. This is a required limitation so you can in a password specify custom keystrokes like TAB and RETURN.