selfcustody / krux

Open-source signing device firmware for Bitcoin
https://selfcustody.github.io/krux/
Other
179 stars 36 forks source link

Better inform the user the risks of using encrypted mnemonic stored on flash or SD card #299

Closed tadeubas closed 7 months ago

tadeubas commented 10 months ago

As of 23.09.0 Krux allows users to store an encrypted mnemonic on flash or SD card. It is also possible to delete the stored mnemonic. But as @jdlcdl investigations pointed out https://github.com/selfcustody/krux/discussions/269 the delete word may pass the wrong idea to the user. Users need to be aware that deleting a mnemonic stored on flash will not remove completely the mnemonic from flash, but will only mark the content as deleted and an attacker can obtain that mnemonic later with physical access to the device. It is important to notice that this issue may affect the storage on SD card too.

I know we can add more info on the docs but my point is that we can't rely only on the docs to inform the user, we need to add a warn message every time the user will store or delete a mnemonic from flash or SD storage. This is specially important because the user may test this functionality with their real mnemonic and a poor key (just to see how encrypted mnemonic works), then later delete the mnemonic hoping everything is ok and the device or the SD card are clean. As @jdlcdl pointed to me "delete really isn't delete" and we need to make that clear to the user, tell them what Krux does (and maybe what they should do).

odudex commented 10 months ago

As Krux features grow it will get harder to forecast every possibility of usage and how users understand the inner workings and the risks of the tools they use. We must aim for a balanced set of instructions and restrictions. Too much of them and the UX will be compromised, to little and we may have risks and bad usage materializing. We will not be able to teach about filesystems through M5stickV screen, so it is important to be objective and minimalist at the device's UI, while keeping details in documentation. I believe the "delete" word is still appropriate, as it does the same thing as it does in any OS, where there's no warning someone could retrieve that information using memory sweeping tools. Having that said, i think we could:

tadeubas commented 10 months ago

I would suggest some additions / changes:

jdlcdl commented 10 months ago

I like the above ideas of better messaging. I'm not sure about exact wording.

In tight screen space, I wonder if a softer verb than "delete", like remove (or drop, unindex, unlink), might hint that it's not destructive delete; elsewhere when it is, stronger verbs like: wipe, secure delete, scrub will be seen and considered, then used whenever appropriate. Same in different languages as translators know best (seems challenging). Maybe there are icons that can express the nuance.

odudex commented 10 months ago

@jdlcdl I agree we could replace the word "delete" with "remove" when deleting removing encrypted mnemonics. @tadeubas I don't agree warnings are needed both when creating and deleting removing saved encrypted mnemonics. I think it is an exaggeration. If Krux will be giving an "opinion" about how strong encryption keys are, to be consistent, we should also create metrics and speculate about strength of mnemonics and passphrases, etc. and I don't think we should go this way, which is a endless path, with Krux prioritizing being some kind tutor instead of a simple and clean tool. I still believe choosing an objective, and not "FUDing" sentence when removing mnemonics is enough. At the documentation, we can give more details about how storage works, risks and good practices.

jdlcdl commented 10 months ago

tl;dr on a gist I'll share below:

It appears that an "msdos" (as in mkdosfs) filesystem on a "W95 FAT32" MBR partition does NOT treat overwriting a file similar to how it happens in spiffs. Rather, on the sdcard when I alter settings.json or seeds.json, it truly feels like 0x00 bytes are overwritten on top of bytes that were deleted... and appending doesn't just abandon a copy and write the entire file into a new page like happens on spiffs.

Similar to spiffs, a delete (as in rm filename) on the sdcard filesystem is effectively unlinking the filename, abandoning the bytes which are left on the sdcard. Different from spiffs, when the deleted filename is re-created, it appears right where it was originally with new contents.

More research is warranted, but mnemonics that are on sdcards and later deleted seem to be less of a concern than I originally thought. My tests do not exhaust what happens when files get fragmented and scattered, so Im not sure how long this convenient pattern plays out. Also, I don't know if my choice of "mkdosfs" and "W95 FAT32" are the only valid choices and if other valid choices act similar or different.

[updated]: I've since done a little bit more research, and found that overwriting files is much better than in spiffs, but i can already see that it's less than perfect... abandoned some bytes... the gist includes this new info.

I'd still side towards "learn how to dd if=/dev/zero of=/dev/sdcard on your computer!"

https://gist.github.com/jdlcdl/2939dec9bf2dc5bcaf1dac1801f3c21d

odudex commented 10 months ago

Another jaw dropping investigation @jdlcdl! Even with some lost bytes left behind, this is much better than spiffs and than I expected.

jdlcdl commented 10 months ago

My latest theory, because I just noticed that in both sdcard files, abandoned bytes after a "write" (as opposed to os.remove()) are those in a 512 byte sector that used to be needed but not after the file has reduced in size.

As if "Never use os.remove('seeds.json'), and when writing to seeds.json, pad the json.dumps() result with spaces so it's at least the original file size." might be an option for deleting seeds from the sdcard???

That sounds so crazy. What's next "and when expanding the size of a file, first write existing size as all spaces, sync the filesystem (if buffering might be a thing), then write the new bigger file."???

If it worked, it would only work for writes done by krux to the sdcard. It couldn't possibly be a solution for what the user might do to the sdcard outside of krux.

Tagging @qlrd to this topic: I wonder your thoughts regarding the best way to make it easy for the user to prepare/scrub an sdcard that will be used by krux (for firmware or other krux uses). Might it be within the krux_installer domain?

tadeubas commented 7 months ago

Done in release 24.03.0