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

Xor seedsave #94

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Xor seedsave #94

wants to merge 4 commits into from

Conversation

dipunm
Copy link

@dipunm dipunm commented Apr 29, 2022

Thanks for contributing to COLDCARD source code, please be as descriptive as possible.

By submitting this work you agree to the Individual Contributor License Agreement (CLA)


I hope this is useful and not controversial, I am building on the seed xor work (love it btw).

TODO list:
[x] Implement loading from file
[x] Implement saving to file
[x] Manually test edge cases
[x] Make existing tests pass
[ ] Create new tests to cover new functionality

This PR will introduce saving and restoring XOR from sd cards.

The idea here, is that similar to passphrases, you can store one seed on the device, and another on an SD card.
When you wish to sign, you must log in using your pin, then you must restore the seed xor from file and you may then sign a transaction, bringing a more secure alternative to the passphrase.

Of course, this is not a replacement for backing up the XOR seed words.

When creating an XOR file, you are told to have your seed words ready, this is a hint to have an already valid seed word. As the ColdCard is not creating the words, there is no need for a quiz.

Loading from SD Card is not enabled if you have no existing seed words (eg. first time use). This makes no sense anyway as the file is encrypted using an existing xpriv, so if you don't have an existing xpriv, we can't decrypt the file anyway.

When you are restoring seed XOR words, you will be asked whether you wish to "enter manually" or load from sd card.

The code is defensive, and if reading, decrypting or parsing the file fails, then the user will see a message, no action was taken, and they will be able to continue where they left off.

One thing that is novel here, is that once the XOR has been applied, you cannot decrypt the file anymore as we will always use the currently in use private key to decrypt and parse any xor files.

A user can store multiple xor files on multiple sd cards and restore them, or they can put them all on one sd card.

You are NOT offered to save your XOR to file after creating one, that feels complicated. Instead, you are able to create XOR files at any time through a separate menu.

@dipunm
Copy link
Author

dipunm commented Apr 29, 2022

I have no idea why there are these other commits in this PR, I have run git fetch --all, the branch should be rebased on master. Any ideas?

@doc-hex
Copy link
Contributor

doc-hex commented Apr 29, 2022

  • we just recently merged all the mk4 code onto master, maybe that's an issue?

@dipunm
Copy link
Author

dipunm commented Apr 29, 2022

nvm, I wasn't looking properly, my commits were off another branch, not master. That has been resolved now.

@dipunm
Copy link
Author

dipunm commented Apr 29, 2022

Let me know if this is valuable, I plan to write something along the lines of these tests:

#def test_import_xor_file()
# TODO: 
# - test when sdcard is missing
# - test when no files found
# - test when 1 file found
# - test when 2 files found
# - test choosing a bad file

#def test_export_xor_file()
# TODO:
# - test when sdcard is missing
# - test when no files exist
# - test when conflicting file exists
# - test when using tmp_secret

#def test_import_xor_file_and_manual()

Not sure if all of the use cases will be testable though, for example, I don't know if I can make a test assuming that there is no SD card since the tests don't control the emulator's startup.

@doc-hex
Copy link
Contributor

doc-hex commented Apr 29, 2022

It is useful. We were planning to do something similar, except with just XOR of 1.. so you might call it "ephemeral seeds"

@dipunm
Copy link
Author

dipunm commented May 1, 2022

I deleted and re-init the repo now that I've rebased, I'm trying to get the thing to build.

I am getting the following errors when I run pip install -r requirements.txt

  error: subprocess-exited-with-error
  
  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  ╰─> [8 lines of output]
      running bdist_wheel
      running build
      running build_py
      running build_ext
      building 'smartcard.scard._scard' extension
      swigging smartcard/scard/scard.i to smartcard/scard/scard_wrap.c
      swig -python -outdir smartcard/scard -DPCSCLITE -o smartcard/scard/scard_wrap.c smartcard/scard/scard.i
      error: command 'swig' failed: No such file or directory
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for pyscard
  Running setup.py clean for pyscard
Failed to build pyscard
Installing collected packages: pyscard, pydes, pycoin, pyaes, pbkdf2, ndeflib, namedlist, libusb1, iniconfig, toml, six, pyusb, pyparsing, py, pluggy, Pillow, numpy, nfcpy, mnemonic, hidapi, click, attrs, zbar-py, packaging, onetimepass, ndef, ecdsa, pytest, ckcc-protocol
  Running setup.py install for pyscard ... error
  error: subprocess-exited-with-error
  
  × Running setup.py install for pyscard did not run successfully.
  │ exit code: 1
  ╰─> [10 lines of output]
      running install
      /home/dipunm/Projects/coldcard/firmware/ENV/lib/python3.10/site-packages/setuptools/command/install.py:34: SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip and other standards-based tools.
        warnings.warn(
      running build
      running build_py
      running build_ext
      building 'smartcard.scard._scard' extension
      swigging smartcard/scard/scard.i to smartcard/scard/scard_wrap.c
      swig -python -outdir smartcard/scard -DPCSCLITE -o smartcard/scard/scard_wrap.c smartcard/scard/scard.i
      error: command 'swig' failed: No such file or directory
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: legacy-install-failure

× Encountered error while trying to install package.
╰─> pyscard

note: This is an issue with the package mentioned above, not pip.
hint: See above for output from the failure.

Any idea why/what I need to install to get this to build now?

Thanks @doc-hex

I am running Arch Linux by the way, but if there is a specific piece of debian software that I need, I can usually find it's equivalent in the Arch package repository.

@dipunm
Copy link
Author

dipunm commented May 1, 2022

NVM, new dependency required from linux repository: swig

@dipunm
Copy link
Author

dipunm commented May 31, 2022

I'm struggling to get tests passing. I've tried the following commands:

pytest --sim
pytest --sim -m "not bitcoind and not qrcode and not unfinalized"
pytest --sim -m qrcode

I had to make a change to conftest.py around line 534:

            try:
                m = cap_menu()
            except RuntimeError:
                continue         

This was because after running the tests in test_addr.py, the tests in test_address_explorer.py expect that after pressing x, you should be presented with a menu, but you are presented with a QR code instead so the tests fail when running after one another. This change allows the tests to press x repeatedly until a menu is presented without exceeding the 10 iteration limit. (for i in range(10):)

Now I am struggling with tests that assume NFC is enabled.

I can of course run tests by file, however, I\d like to understand how should I be ensuring that my tests are well written and will not break other tests?

Also, is there a way to write a test that can force the simulator to think it has no SD card without requiring manual intervention?

Thanks.

@scgbckbone
Copy link
Collaborator

with regards to SD card - what you're looking for is --eject passed to simulator. Also check unix/README.md for more simulator options.

Recently we've also added testing/run_sim_tests.py (git pull --rebase upstream/master)which is basically a retry test runner. Check it out, maybe you're just experiencing intermittent failures - that can be resolved with running concrete test against fresh simulator.

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

Successfully merging this pull request may close these issues.

3 participants