Skip to content
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

[Feature] Add MicroSD PSBT File support for SeedSignerOS #281

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

newtonick
Copy link
Collaborator

Design and workflow considerations and tradeoffs

  • I did not implement a UI to browse a file system with directories. For simplicity sake, the application looks for PSBT files in binary or base64 on the root level of the microsd card directory structure and listed them in the UI in alphabetical order.
  • When a unsigned PSBT file is read from the microsd it will only write out the signed psbt file to microsd. The same for reading a PSBT via QR. There is no option to read a PSBT file from microsd and then display the signed PSBT as a QR. If you start with a PSBT file you'll end with a signed PSBT file.
  • PSBT files written to microsd are not trimmed
  • The file name of a signed PSBT is appended with "signed" to the original file name. So "Test 123.psbt" unsigned PSBT will be written to the microsd card as "Test 123 signed.psbt". There is no option currently to name the signed psbt output file.
  • The list of items that cause the file list of PSBT's to refresh
    1. Inserting MicroSD card
    2. Enabling Micro PSBT in settings
    3. After successful signing of PSBT
  • The MicroSD PSBT Setting is visbile in Raspberry Pi OS even though the feature is explicitly disabled unless the hostname is seedsigner-os
  • The review and signing of the PSBT is 100% in memory to avoid time-of-check-time-of-use style of attacks

Testing/Draft status

  • Still needs further review and testing

Easiest way to test right now is to build a image and pull code from my repo/branch

git clone --recursive -b restructure https://github.com/newtonick/seedsigner-os.git seedsigner-os-test
cd seedsigner-os-test; SS_ARGS="--pi0 --app-repo=https://github.com/newtonick/seedsigner.git --app-branch=microsd-psbt-files" docker-compose up -d

This works on my M1 Mac and takes about 40 minutes to build the image.

@SeedSigner
Copy link
Owner

Really appreciate your work on this feature, I am excited to give it a try.

We might consider some kind of admonition screen? I feel as though the process of moving a PSBT with a memory card incurs some additional risk we may want to make users aware of? Open to other perspectives on this if they are out there.

@newtonick
Copy link
Collaborator Author

Really appreciate your work on this feature, I am excited to give it a try.

We might consider some kind of admonition screen? I feel as though the process of moving a PSBT with a memory card incurs some additional risk we may want to make users aware of? Open to other perspectives on this if they are out there.

@SeedSigner definitely open to ideas. The feature is disabled by default as implemented. I'm not sure what specific risks we would want to call out in a warning screen.

@SeedSigner
Copy link
Owner

This gets a bit into the weeds, but for me its an unknown whether mounting the volume on the MicroSD, or accessing / interacting with the volume in other ways (file creation, renaming, etc.) could trigger malicious code that could also be present on the memory card. Removable media containing a file system would seem in general to have a broader attack surface than the QR exchange process. With the OS in RAM, its likely to only impact the current SeedSigner session, but we may seek out someone with more deeply relevant expertise as we consider the best way to activate and message about this issue.

@newtonick newtonick added the enhancement New feature or request label Feb 28, 2023
@newtonick
Copy link
Collaborator Author

rebased and resolved conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants