diff --git a/external/libngu b/external/libngu index 74b373c2..ddfd4764 160000 --- a/external/libngu +++ b/external/libngu @@ -1 +1 @@ -Subproject commit 74b373c2b7c92e6e903be22da773bad3f0daa09b +Subproject commit ddfd47644962fe20301466199da90a6f292732af diff --git a/releases/ChangeLog.md b/releases/ChangeLog.md index 047a88d2..7c1d8ac4 100644 --- a/releases/ChangeLog.md +++ b/releases/ChangeLog.md @@ -1,4 +1,4 @@ -## 4.1.2 - Jun XX, 2021 +## 4.1.2 - July XX, 2021 - Enhancement: Shows QR code with BIP-85 derived entropy value if you press (3) while value shown on-screen. Thanks to [@opennoms](https://twitter.com/openoms) for idea. @@ -8,7 +8,11 @@ - Seed words, during picking process (before the quiz) - Stored seed words: Advanced > Danger Zone > Seed Functions > View Seed Words - TXID of just-signed transaction (64 hex digits) - - Backup password (12 words) which is only case safe to save as a photo on your phone! + - Backup password (12 words) which is the only safe QR to save as a photo on your phone!!! +- Enhancement: We now grind a nonce so that our signatures are always 71 bytes or shorter. + This may save a byte or two in transaction size (and miner fees), and makes our signatures + identical to those produced by BitcoinCore, improving anonymity on-chain. Thanks to + [@craigraw](https://twitter.com/craigraw) for detecting this. ## 4.1.1 - April 30, 2021 diff --git a/shared/auth.py b/shared/auth.py index 6b5f1d6b..2e6d467b 100644 --- a/shared/auth.py +++ b/shared/auth.py @@ -147,7 +147,7 @@ def sign_message_digest(digest, subpath, prompt): sv.register(pk) dis.progress_bar_show(.75) - rv = ngu.secp256k1.sign(pk, digest).to_bytes() + rv = ngu.secp256k1.sign(pk, digest, 0).to_bytes() dis.progress_bar_show(1) diff --git a/shared/psbt.py b/shared/psbt.py index e0fdd91e..2b0fc62d 100644 --- a/shared/psbt.py +++ b/shared/psbt.py @@ -1517,21 +1517,30 @@ class psbtObject(psbtProxy): #print(" digest %s" % b2a_hex(digest).decode('ascii')) # Do the ACTUAL signature ... finally!!! - result = ngu.secp256k1.sign(pk, digest).to_bytes() + + # We need to grind sometimes to get a positive R + # value that will encode (after DER) into a shorter string. + # - saves on miner's fee (which might be expected/required) + # - blends in with Bitcoin Core signatures which do this + for retry in range(10): + result = ngu.secp256k1.sign(pk, digest, retry).to_bytes() + + # convert signature to DER format + #assert len(result) == 65 + r = result[1:33] + s = result[33:65] + der_sig = ser_sig_der(r, s, inp.sighash) + + if len(der_sig) <= 71: + # odds of needing retry: just under 50% I think + break # private key no longer required stash.blank_object(pk) stash.blank_object(node) del pk, node, pu, skp - #print("result %s" % b2a_hex(result).decode('ascii')) - - # convert signature to DER format - assert len(result) == 65 - r = result[1:33] - s = result[33:65] - - inp.added_sig = (which_key, ser_sig_der(r, s, inp.sighash)) + inp.added_sig = (which_key, der_sig) success.add(in_idx) diff --git a/shared/usb.py b/shared/usb.py index ea64c627..113854c6 100644 --- a/shared/usb.py +++ b/shared/usb.py @@ -644,7 +644,7 @@ class USBHandler: pk = sv.node.privkey() sv.register(pk) - signature = ngu.secp256k1.sign(pk, self.session_key).to_bytes() + signature = ngu.secp256k1.sign(pk, self.session_key, 0).to_bytes() assert len(signature) == 65 diff --git a/testing/README.md b/testing/README.md index 92d64345..1e6d8256 100644 --- a/testing/README.md +++ b/testing/README.md @@ -21,9 +21,19 @@ None of this code ships on the product itself, but it does get used for testing --dev --manual -s -- test all QR code relates cases with: +## Marked Test Cases + +- test all QR code related cases with: + py.test -m qrcode +- txn signing where an unfinalized PSBT is created (low-R tests) + + py.test -m unfinalized + +- "bitcoind" which means test would be skipped if you don't have bitcoin core + running locally (on testnet) + ## PSBT reference files - examples with `IN_REDEEM_SCRIPT`: diff --git a/testing/conftest.py b/testing/conftest.py index 07abe50b..c738faab 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -586,6 +586,13 @@ def set_encoded_secret(sim_exec, sim_execfile, simulator, reset_seed_words): # - actually need seed words for all tests reset_seed_words() +@pytest.fixture(scope="function") +def use_mainnet(settings_set): + def doit(): + settings_set('chain', 'BTC') + yield doit + settings_set('chain', 'XTN') + @pytest.fixture(scope="function") def set_seed_words(sim_exec, sim_execfile, simulator, reset_seed_words): # load simulator w/ a specific bip32 master key @@ -1033,10 +1040,15 @@ def end_sign(dev, need_keypress): # skip checks; it's text return psbt_out + sigs = [] + if not finalize: - if in_psbt: - from psbt import BasicPSBT - assert BasicPSBT().parse(in_psbt) != None + from psbt import BasicPSBT + tp = BasicPSBT().parse(psbt_out) + assert tp is not None + + for i in tp.inputs: + sigs.extend(i.part_sigs.values()) else: from pycoin.tx.Tx import Tx # parse it @@ -1045,6 +1057,11 @@ def end_sign(dev, need_keypress): t = Tx.from_bin(res) assert t.version in [1, 2] + # TODO: pull out signatures from signed txn, but pycoin not helpful on that + + for sig in sigs: + assert len(sig) <= 71, "overly long signature observed" + return psbt_out return doit diff --git a/testing/pytest.ini b/testing/pytest.ini index 86e820c5..5b8f72f0 100644 --- a/testing/pytest.ini +++ b/testing/pytest.ini @@ -6,3 +6,4 @@ markers = onetime: test cant be combined with any others, likely needs board reset veryslow: test takes more than 30 minutes realtime qrcode: test uses or tests QR related features + unfinalized: test cases produces an unfinalized PSBT diff --git a/testing/test_msg.py b/testing/test_msg.py index ab350410..3e3dda1c 100644 --- a/testing/test_msg.py +++ b/testing/test_msg.py @@ -222,4 +222,47 @@ def test_sign_msg_microsd_fails(dev, sign_on_microsd, msg, concern, no_file, tra assert concern in story +@pytest.mark.parametrize('msg,num_iter,expect', [ + ('Test2', 1, 'IHra0jSywF1TjIJ5uf7IDECae438cr4o3VmG6Ri7hYlDL+pUEXyUfwLwpiAfUQVqQFLgs6OaX0KsoydpuwRI71o='), + ('Test', 2, 'IDgMx1ljPhLHlKUOwnO/jBIgK+K8n8mvDUDROzTgU8gOaPDMs+eYXJpNXXINUx5WpeV605p5uO6B3TzBVcvs478='), + ('Test1', 3, 'IEt/v9K95YVFuRtRtWaabPVwWOFv1FSA/e874I8ABgYMbRyVvHhSwLFz0RZuO87ukxDd4TOsRdofQwMEA90LCgI='), +]) +def test_low_R_cases(msg, num_iter, expect, dev, set_seed_words, use_mainnet, need_keypress): + # Thanks to @craigraw of Sparrow for this test case, copied from: + # + + set_seed_words('absent essay fox snake vast pumpkin height crouch silent bulb excuse razor') + use_mainnet() + path = "m/44'/0'/0'/0/0" # first address, P2PKH + addr_fmt = AF_CLASSIC + + #addr = dev.send_recv(CCProtocolPacker.show_address(path, addr_fmt), timeout=None) + #assert addr == '14JmU9a7SzieZNEtBnsZo688rt3mGrw6hr' + + msg = msg.encode('ascii') + dev.send_recv(CCProtocolPacker.sign_message(msg, path, addr_fmt=addr_fmt), timeout=None) + + need_keypress('y') + + done = None + while done == None: + time.sleep(0.050) + done = dev.send_recv(CCProtocolPacker.get_signed_msg(), timeout=None) + + assert len(done) == 2, done + got_addr, raw = done + + assert got_addr == '14JmU9a7SzieZNEtBnsZo688rt3mGrw6hr' + assert 40 <= len(raw) <= 65 + + sig = str(b64encode(raw), 'ascii').replace('\n', '') + + if num_iter != 1: + # I have gotten these cases to pass, but I didn't want to keep the code + # that grinded for low R in message signing... Ok for txn signing, but + # needless delay for message signing. + raise pytest.xfail('no code') + + assert sig == expect + # EOF diff --git a/testing/test_multisig.py b/testing/test_multisig.py index 2004c69d..f64bf975 100644 --- a/testing/test_multisig.py +++ b/testing/test_multisig.py @@ -1169,6 +1169,7 @@ def fake_ms_txn(): return doit +@pytest.mark.unfinalized @pytest.mark.parametrize('addr_fmt', [AF_P2SH, AF_P2WSH, AF_P2WSH_P2SH] ) @pytest.mark.parametrize('num_ins', [ 2, 15 ]) @pytest.mark.parametrize('incl_xpubs', [ False, True, 'no-import' ]) @@ -1196,6 +1197,7 @@ def test_ms_sign_simple(N, num_ins, dev, addr_fmt, clear_ms, incl_xpubs, import_ else: try_sign(psbt) +@pytest.mark.unfinalized @pytest.mark.parametrize('num_ins', [ 15 ]) @pytest.mark.parametrize('M', [ 2, 4, 1]) @pytest.mark.parametrize('segwit', [True, False]) @@ -1409,8 +1411,9 @@ def test_make_airgapped(addr_fmt, acct_num, goto_home, cap_story, pick_menu_item need_keypress('x') -@pytest.mark.parametrize('addr_style', ["legacy", "p2sh-segwit", "bech32"]) +@pytest.mark.unfinalized @pytest.mark.bitcoind +@pytest.mark.parametrize('addr_style', ["legacy", "p2sh-segwit", "bech32"]) def test_bitcoind_cosigning(dev, bitcoind, import_ms_wallet, clear_ms, explora, try_sign, need_keypress, addr_style): # Make a P2SH wallet with local bitcoind as a co-signer (and simulator) # - send an receive various diff --git a/testing/test_sign.py b/testing/test_sign.py index 882658bc..7b0d2b58 100644 --- a/testing/test_sign.py +++ b/testing/test_sign.py @@ -124,6 +124,7 @@ def test_psbt_proxy_parsing(fn, sim_execfile, sim_exec): rb = BasicPSBT().parse(open(rb, 'rb').read()) assert oo == rb +@pytest.mark.unfinalized def test_speed_test(request, fake_txn, is_mark3, start_sign, end_sign, dev, need_keypress): import time # measure time to sign a larger txn @@ -248,6 +249,7 @@ def test_real_signing(fake_txn, try_sign, dev, num_ins, segwit, decode_with_bitc if segwit: assert all(x['txinwitness'] for x in decoded['vin']) +@pytest.mark.unfinalized # iff we_finalize=F @pytest.mark.parametrize('we_finalize', [ False, True ]) @pytest.mark.parametrize('num_dests', [ 1, 10, 25 ]) @pytest.mark.bitcoind @@ -386,6 +388,7 @@ def test_sign_example(set_master_key, sim_execfile, start_sign, end_sign): #assert 'require subpaths to be spec' in str(ee) +@pytest.mark.unfinalized def test_sign_p2sh_p2wpkh(match_key, start_sign, end_sign, bitcoind): # Check we can finalize p2sh_p2wpkh inputs right. @@ -420,6 +423,7 @@ def test_sign_p2sh_p2wpkh(match_key, start_sign, end_sign, bitcoind): assert network == signed +@pytest.mark.unfinalized def test_sign_p2sh_example(set_master_key, sim_execfile, start_sign, end_sign, decode_psbt_with_bitcoind, offer_ms_import, need_keypress, clear_ms): # Use the private key given in BIP 174 and do similar signing # as the examples. @@ -687,6 +691,7 @@ def test_sign_multisig_partial_fail(start_sign, end_sign): assert 'None of the keys involved' in str(ee) +@pytest.mark.unfinalized def test_sign_wutxo(start_sign, set_seed_words, end_sign, cap_story, sim_exec, sim_execfile): # Example from SomberNight: we can sign it, but signature won't be accepted by @@ -873,8 +878,9 @@ def KEEP_test_random_psbt(try_sign, sim_exec, fname="data/ .psbt"): assert 'led to wrong pubkey for input' in msg -@pytest.mark.parametrize('num_dests', [ 1, 10, 25 ]) @pytest.mark.bitcoind +@pytest.mark.unfinalized +@pytest.mark.parametrize('num_dests', [ 1, 10, 25 ]) def test_finalization_vs_bitcoind(match_key, check_against_bitcoind, bitcoind, start_sign, end_sign, num_dests): # Compare how we finalize vs bitcoind ... should be exactly the same txn @@ -1114,7 +1120,7 @@ def test_bip143_attack_data_capture(num_utxo, segwit_in, try_sign, fake_txn, set time.sleep(.1) title, story = cap_story() assert 'TXID' in title, story - txid = story.strip() + txid = story.strip().split()[0] assert hist_count() in {128, hist_b4+num_utxo+num_inp_utxo} @@ -1170,7 +1176,7 @@ def test_txid_calc(num_ins, fake_txn, try_sign, dev, segwit, decode_with_bitcoin title, story = cap_story() assert '0' in story assert 'TXID' in title, story - txid = story.strip() + txid = story.strip().split()[0] if 1: # compare to PyCoin @@ -1189,6 +1195,7 @@ def test_txid_calc(num_ins, fake_txn, try_sign, dev, segwit, decode_with_bitcoin assert decoded['txid'] == txid +@pytest.mark.unfinalized # iff partial=1 @pytest.mark.parametrize('encoding', ['binary', 'hex', 'base64']) #@pytest.mark.parametrize('num_outs', [1,2,3,4,5,6,7,8]) @pytest.mark.parametrize('num_outs', [1,2]) @@ -1212,6 +1219,7 @@ def test_sdcard_signing(encoding, num_outs, del_after, partial, try_sign_microsd _, txn, txid = try_sign_microsd(psbt, finalize=not partial, encoding=encoding, del_after=del_after) +@pytest.mark.unfinalized @pytest.mark.parametrize('num_ins', [2,3,8]) @pytest.mark.parametrize('num_outs', [1,2,8]) def test_payjoin_signing(num_ins, num_outs, fake_txn, try_sign, start_sign, end_sign, cap_story):