SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
699 stars 161 forks source link

README note: don't mount .img before signature verification #539

Closed akarve closed 5 months ago

akarve commented 6 months ago

Description

Clarify instructions for new users. If you happen to double click the .img before running the verification code on a Mac then verification fails and it looks scary for no reason.

This pull request is categorized as a:

Checklist

So I did run pytest and two tests are failing, which I'm glad to fix but have nothing to do with my PR. See comments for more.

If you modified or added functionality/workflow, did you add new unit tests?

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

akarve commented 6 months ago

Here's what happens when I run pytest on an M1 chip. Two QR code unit tests fail:

Note: I needed to conda install zbar" outside of any requirements.txt provided as apparently that didn't get installed with pyzbar

============================= test session starts ==============================
platform darwin -- Python 3.10.14, pytest-6.2.4, py-1.11.0, pluggy-0.13.1
rootdir: /Users/akarve/code/seedsigner, configfile: pytest.ini, testpaths: tests
plugins: cov-4.1.0
collected 87 items

tests/test_bip85.py .                                                    [  1%]
tests/test_controller.py .....                                           [  6%]
tests/test_decodepsbtqr.py ............                                  [ 20%]
tests/test_embit_utils.py .....                                          [ 26%]
tests/test_encodepsbtqr.py ......                                        [ 33%]
tests/test_flows.py ........                                             [ 42%]
tests/test_flows_psbt.py ..                                              [ 44%]
tests/test_flows_seed.py ..............                                  [ 60%]
tests/test_flows_settings.py .....                                       [ 66%]
tests/test_flows_tools.py ....                                           [ 71%]
tests/test_flows_view.py ....                                            [ 75%]
tests/test_mnemonic_generation.py ........                               [ 85%]
tests/test_psbt_parser.py .                                              [ 86%]
tests/test_seed.py .                                                     [ 87%]
tests/test_seedqr.py .FF                                                 [ 90%]
tests/test_settings.py ......                                            [ 97%]
tests/test_settingsqr_decoder.py ..                                      [100%]

=================================== FAILURES ===================================
______________________ test_compact_seedqr_encode_decode _______________________

    def test_compact_seedqr_encode_decode():
        """ Should encode 24- and 12- word mnemonics to CompactSeedQR format and decode
            them back again to their original mnemonic seed phrase.
        """
        # 24-word seed
>       run_encode_decode_test(os.urandom(32), mnemonic_length=24, qr_type=QRType.SEED__COMPACTSEEDQR)

tests/test_seedqr.py:59: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

entropy = b'\xebb\xa5\xb1Qm\x80\xd9d\xf0\xbe\xdd\xd9\xa21\x1b\xcb\xb9\x9a,\x11)\xfa\x196\xc42o\xcd\x11\xc0\xad'
mnemonic_length = 24, qr_type = 'seed__compactseedqr'

    def run_encode_decode_test(entropy: bytes, mnemonic_length, qr_type):
        """ Helper method to re-run multiple variations of the same encode/decode test """
        print(entropy)
        seed_phrase = bip39.mnemonic_from_bytes(entropy).split()
        print(seed_phrase)
        assert len(seed_phrase) == mnemonic_length

        e = EncodeQR(seed_phrase=seed_phrase, qr_type=qr_type)
        data = e.next_part()
        print(data)

        qr = QR()
        image = qr.qrimage(
            data=data,
            width=240,
            height=240,
            border=3
        )

        decoder = DecodeQR()
        status = decoder.add_image(image)
>       assert status == DecodeQRStatus.COMPLETE
E       assert <DecodeQRStatus.INVALID: 5> == <DecodeQRStatus.COMPLETE: 3>
E        +  where <DecodeQRStatus.COMPLETE: 3> = DecodeQRStatus.COMPLETE

tests/test_seedqr.py:34: AssertionError
----------------------------- Captured stdout call -----------------------------
b'\xebb\xa5\xb1Qm\x80\xd9d\xf0\xbe\xdd\xd9\xa21\x1b\xcb\xb9\x9a,\x11)\xfa\x196\xc42o\xcd\x11\xc0\xad'
['twin', 'benefit', 'hobby', 'people', 'submit', 'hole', 'need', 'blind', 'tape', 'snack', 'middle', 'daring', 'romance', 'snack', 'raccoon', 'cement', 'wheel', 'situate', 'rain', 'gospel', 'wolf', 'material', 'actor', 'release']
b'\xebb\xa5\xb1Qm\x80\xd9d\xf0\xbe\xdd\xd9\xa21\x1b\xcb\xb9\x9a,\x11)\xfa\x196\xc42o\xcd\x11\xc0\xad'
____________________ test_compact_seedqr_handles_null_bytes ____________________

    def test_compact_seedqr_handles_null_bytes():
        """ Should properly encode a CompactSeedQR with null bytes (b'\x00') in the input
            entropy and decode it back to the original mnemonic seed.
        """
        # 24-word seed, null bytes at the front
        entropy = b'\x00' + os.urandom(31)
>       run_encode_decode_test(entropy, mnemonic_length=24, qr_type=QRType.SEED__COMPACTSEEDQR)

tests/test_seedqr.py:72: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

entropy = b'\x004B\x82\xeeb\x95\xb2S\xa6V_\xda\xb5{\xc5\x05\x92h%\x7f\xbd0\x0cs\xde\xe6\xb9\x84GJ\x97'
mnemonic_length = 24, qr_type = 'seed__compactseedqr'

    def run_encode_decode_test(entropy: bytes, mnemonic_length, qr_type):
        """ Helper method to re-run multiple variations of the same encode/decode test """
        print(entropy)
        seed_phrase = bip39.mnemonic_from_bytes(entropy).split()
        print(seed_phrase)
        assert len(seed_phrase) == mnemonic_length

        e = EncodeQR(seed_phrase=seed_phrase, qr_type=qr_type)
        data = e.next_part()
        print(data)

        qr = QR()
        image = qr.qrimage(
            data=data,
            width=240,
            height=240,
            border=3
        )

        decoder = DecodeQR()
        status = decoder.add_image(image)
>       assert status == DecodeQRStatus.COMPLETE
E       assert <DecodeQRStatus.INVALID: 5> == <DecodeQRStatus.COMPLETE: 3>
E        +  where <DecodeQRStatus.COMPLETE: 3> = DecodeQRStatus.COMPLETE

tests/test_seedqr.py:34: AssertionError
----------------------------- Captured stdout call -----------------------------
b'\x004B\x82\xeeb\x95\xb2S\xa6V_\xda\xb5{\xc5\x05\x92h%\x7f\xbd0\x0cs\xde\xe6\xb9\x84GJ\x97'
['ability', 'pear', 'pass', 'system', 'citizen', 'summer', 'excess', 'skull', 'garlic', 'stick', 'galaxy', 'meat', 'float', 'cross', 'nominee', 'waste', 'copy', 'glory', 'waste', 'sniff', 'obscure', 'casual', 'clean', 'usage']
b'\x004B\x82\xeeb\x95\xb2S\xa6V_\xda\xb5{\xc5\x05\x92h%\x7f\xbd0\x0cs\xde\xe6\xb9\x84GJ\x97'
=========================== short test summary info ============================
FAILED tests/test_seedqr.py::test_compact_seedqr_encode_decode - assert <Deco...
FAILED tests/test_seedqr.py::test_compact_seedqr_handles_null_bytes - assert ...
========================= 2 failed, 85 passed in 0.93s =========================
kdmukai commented 5 months ago

Note the manual installation instructions here: https://github.com/SeedSigner/seedsigner/blob/dev/docs/manual_installation.md#install-zbar

Errors reading Compact SeedQR format most likely point to an incompatibility with reading raw byte data format out of a QR code.

Also verify that you're running with our fork of pyzbar: https://github.com/SeedSigner/seedsigner/blob/dev/docs/manual_installation.md#install-zbar

akarve commented 5 months ago

Thanks. I'm running the tests locally, as opposed to in Docker. I was able to brew install zbar and can verify I'm on your fork of pyzbar as that's what's in requirements.txt. But getting the same errors. Should I break this out into a separate docs PR for getting Mac unit tests running and unencumber this PR?