Jade and normal multisig compatibility #22

stepansnigirev · 2021-09-13T17:10:16Z

Are there plans to add support for standard multisig wallets?

I see that you have support for Green-specific 2-of-2 or 2-of-3 or 2-of-2+csv, but would be nice to be able to use Jade in standard multisig with other hardware wallets.

For example, software wallet could pass receiving and change descriptors to the HW and it could verify that all scriptpubkeys in inputs and change outputs are derived from these descriptors.

JamieDriver · 2021-09-20T13:27:14Z

This is something in the pipeline for an upcoming release.
The current thinking is that you would have to upload/inspect/confirm a descriptor for the multisig on Jade. Once registered, you can then generate receive addresses, validate change outputs and sign multisigs based on that descriptor.
(Initially btc only - liquid later)

JamieDriver · 2021-10-05T08:56:51Z

Merged to master, for the next fw release, 0.1.30

JamieDriver · 2021-10-28T09:32:44Z

0.1.30 is now available. Cheers.

JamieDriver · 2021-10-28T16:13:14Z

NOTE: fyi support has also been added to the relevant HWI PR

JamieDriver · 2021-11-26T09:36:53Z

fyi: the next release of the Jade fw should add support for bip67/sortedmulti(), and a PR to enable that on HWI will follow.
Then hopefully this item should be fully addressed. Cheers.

JamieDriver · 2021-12-23T14:13:17Z

fyi: Jade fw 0.1.32 is now available, which supports sortedmulti(). An HWI PR will be opened early in the new year.

stepansnigirev · 2021-12-23T15:31:00Z

Awesome, thank you!

georgantas · 2021-12-27T18:01:32Z

Interesting article: https://shiftcrypto.ch/blog/the-pitfalls-of-multisig-when-using-hardware-wallets/ Wonder if multisig with Jade is vulnerable to some of these attacks.

JamieDriver · 2021-12-28T12:55:30Z

Thanks.
So with Jade we opted to be quite strict in this regard - in order to use multisig the user first needs to register the setup on Jade - ie. as a separate registration step the user inspects/confirms:

all co-signer xpubs (the hw checks that at least one belongs the Jade)
threshold
script type
(and now) whether bip67 key sorting applies

Once that has been confirmed, future requests to get receive addresses or change addresses will use the registered setup - the only additional data passed per request is the bip32 path suffixes. The full key derivation paths are validated against bip45/48/87, and that the final element is within a reasonable range. If this validation fails, a warning is displayed and the user must explicitly check/confirm the path.

We believe this should cover the known attack scenarios - but obviously are happy to hear if you think otherwise! ;-)

stepansnigirev · 2021-12-28T14:19:29Z

I am trying to implement that and currently have a problem registering multisig on the device, here are the parameters for register_multisig json-rpc request:

{
'network': 'testnet',
'multisig_name': 'hwi41bdc647542b',
'descriptor': {
  'variant': 'wsh(multi(k))',
  'sorted': True,
  'threshold': 2,
  'signers': [
    {'fingerprint': '396779c9', 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDCGbvM6UyTcRHu4rXgSPnMhaHuNec86ME1Ap9CuqBbhNpwdR28a7MRQoeKeqizAcSb3MKFD47jHg2bYUR6u95tD3K5grTAFXHvhWwELX6C2', 'path': []},
    {'fingerprint': 'fb7c1f11', 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7', 'path': []},
    {'fingerprint': '47fc1ba1', 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD', 'path': []}
  ]
  }
}

I receive an error:

JadeError: -32602 - Failed to extract valid co-signers from parameters (Data: None)

stepansnigirev · 2021-12-28T14:37:59Z

The same happens with the current master of HWI when I am trying to display address for unsorted multisig descriptor:

hwi --chain regtest -f 396779c9 displayaddress --desc "wsh(multi(2,[396779c9/48h/1h/0h/2h]tpubDCGbvM6UyTcRHu4rXgSPnMhaHuNec86ME1Ap9CuqBbhNpwdR28a7MRQoeKeqizAcSb3MKFD47jHg2bYUR6u95tD3K5grTAFXHvhWwELX6C2/0/0,[fb7c1f11/48h/1h/0h/2h]tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7/0/0,[47fc1ba1/48h/1h/0h/2h]tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD/0/0))#5a3f60y9"
{"error": "Failed to validate multisig co-signers", "code": -7}

JamieDriver · 2021-12-28T18:26:09Z

Is one of the cosigners derived from the Jade key/fingerprint - and it is correct ? ie. the jade master key plus the derivation given, definitely yields that xpub ? Jade itself and HWI have tests which [should] be covering the above.
Ok, I can see the -f matches the first key... I'll switch that for my test key and see what happens.

JamieDriver · 2021-12-28T18:51:24Z

OK, I loaded the test seed (from the jade tests) into jade and got the xpub we want to use:
(fyi the 'set-seed' call is only available with a test/debug build of the firmware)

TEST_SEED = bytes.fromhex('b90e532426d0dc20fffe01037048c018e940300038b165c211915c672e07762c')
jade.set_seed(TEST_SEED, temporary_wallet=True)

print(jade.get_xpub('localtest', [2147483696, 2147483649, 2147483648, 2147483650]))

to get:
tpubDEAjmvwVDj4aTdNLyLdoVGdEuRf8ZMsZEun6Aa2uMHa3DFNSzXQFt8DY62gi37RuaPTcpoNFage2h6dPHABssCnWcp2j6srQTpWbGEKHgU1

I then switched out the first signer fingerprint/xpub of your multisig to my jade fingerprint/xpub, and got the following:

(HWI)$ hwi enumerate
[{"type": "jade", "model": "jade", "path": "/dev/ttyUSB0", "needs_pin_sent": false, "needs_passphrase_sent": false, "fingerprint": "1273da33"}]

(HWI)$ hwi --chain regtest -f 1273da33 displayaddress --desc "wsh(multi(2,[1273da33/48h/1h/0h/2h]tpubDEAjmvwVDj4aTdNLyLdoVGdEuRf8ZMsZEun6Aa2uMHa3DFNSzXQFt8DY62gi37RuaPTcpoNFage2h6dPHABssCnWcp2j6srQTpWbGEKHgU1/0/0,[fb7c1f11/48h/1h/0h/2h]tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7/0/0,[47fc1ba1/48h/1h/0h/2h]tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD/0/0))"

{"address": "bcrt1qegfvejxlklt72peyhjyg9f0zm579ep0wv9lqqqg4ek75zk8yhpasp5vr8t"}

JamieDriver · 2021-12-28T19:11:29Z

With your first example, I added my Jade test key, so the multisig became a 2-of-4:

    multisig = {
      'network': 'testnet',
      'multisig_name': 'hwi41bdc647542b',
      'descriptor': {
        'variant': 'wsh(multi(k))',
        'sorted': True,
        'threshold': 2,
        'signers': [
          {'fingerprint': bytes.fromhex('1273da33'), 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDEAjmvwVDj4aTdNLyLdoVGdEuRf8ZMsZEun6Aa2uMHa3DFNSzXQFt8DY62gi37RuaPTcpoNFage2h6dPHABssCnWcp2j6srQTpWbGEKHgU1', 'path': []},
          {'fingerprint': bytes.fromhex('396779c9'), 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDCGbvM6UyTcRHu4rXgSPnMhaHuNec86ME1Ap9CuqBbhNpwdR28a7MRQoeKeqizAcSb3MKFD47jHg2bYUR6u95tD3K5grTAFXHvhWwELX6C2', 'path': []},
          {'fingerprint': bytes.fromhex('fb7c1f11'), 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7', 'path': []},
          {'fingerprint': bytes.fromhex('47fc1ba1'), 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD', 'path': []}
        ]
      }
    }
    print(jade._jadeRpc('register_multisig', params=multisig))

(Note the fingerprints are sent as bytes, not hex strings)

Sending: {'method': 'register_multisig', 'id': '512432', 'params': {'network': 'testnet', 'multisig_name': 'hwi41bdc647542b', 'descriptor': {'variant': 'wsh(multi(k))', 'sorted': True, 'threshold': 2, 'signers': [{'fingerprint': '1273da33', 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDEAjmvwVDj4aTdNLyLdoVGdEuRf8ZMsZEun6Aa2uMHa3DFNSzXQFt8DY62gi37RuaPTcpoNFage2h6dPHABssCnWcp2j6srQTpWbGEKHgU1', 'path': []}, {'fingerprint': '396779c9', 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDCGbvM6UyTcRHu4rXgSPnMhaHuNec86ME1Ap9CuqBbhNpwdR28a7MRQoeKeqizAcSb3MKFD47jHg2bYUR6u95tD3K5grTAFXHvhWwELX6C2', 'path': []}, {'fingerprint': 'fb7c1f11', 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7', 'path': []}, {'fingerprint': '47fc1ba1', 'derivation': [2147483696, 2147483649, 2147483648, 2147483650], 'xpub': 'tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD', 'path': []}]}}} as cbor of size 848

<pause while confirmed on hw>

Received msg: {'id': '512432', 'result': True}

JamieDriver · 2021-12-28T19:12:41Z

fyi: I do have 'Add api/cbor-message documentation' as an outstanding task!

georgantas · 2021-12-28T23:20:12Z

Thank you for the clarification, Jamie. I had one more doubt, but I created a new issue not to go off-topic on this issue: #29

stepansnigirev · 2021-12-29T09:58:41Z

@JamieDriver this is very strange, I tried again, the same issue. Firmware version 0.1.32.
Here is the python code I use - it gets xpub from the device, adds it to cosigners and tries to register multisig.
Can you try running it yourself? Could it be that production and debug firmware behave differently?

from jadepy import JadeAPI
from hwilib import _base58 as base58

NETWORK = "testnet"
HARD = 0x80000000
PATH = [HARD+48, HARD+1, HARD, HARD+2]

params = {
'network': NETWORK,
'multisig_name': 'tmp',
'descriptor': {
  'variant': 'wsh(multi(k))',
  'sorted': True,
  'threshold': 2,
  'signers': [
    {'fingerprint': 'fb7c1f11', 'derivation': PATH, 'xpub': 'tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7', 'path': []},
    {'fingerprint': '47fc1ba1', 'derivation': PATH, 'xpub': 'tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD', 'path': []}
  ]}
}

with JadeAPI.create_serial("/dev/ttyUSB0") as d:
    d.auth_user(NETWORK)
    xpub = d.get_xpub(NETWORK, PATH)
    print(xpub)
    child0 = d.get_xpub(NETWORK, [0])
    fgp = base58.get_xpub_fingerprint(child0)
    print(fgp.hex())
    params["descriptor"]["signers"].append({
        "fingerprint": fgp.hex(),
        "derivation": PATH,
        "xpub": xpub,
        "path": [],
    })
    print(d._jadeRpc('register_multisig', params=params))

JamieDriver · 2021-12-29T11:04:47Z

As I say above (and as in my example) fingerprint should be bytes, not hex-string.
ie:

@@ -16,6 +16,6 @@
   'threshold': 2,
   'signers': [
-    {'fingerprint': 'fb7c1f11', 'derivation': PATH, 'xpub': 'tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7', 'path': []},
-    {'fingerprint': '47fc1ba1', 'derivation': PATH, 'xpub': 'tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD', 'path': []}
+    {'fingerprint': bytes.fromhex('fb7c1f11'), 'derivation': PATH, 'xpub': 'tpubDExnGppazLhZPNadP8Q5Vgee2QcvbyAf9GvGaEY7ALVJREaG2vdTqv1MHRoDtPaYP3y1DGVx7wrKKhsLhs26GY263uE6Wi3qNbi71AHZ6p7', 'path': []},
+    {'fingerprint': bytes.fromhex('47fc1ba1'), 'derivation': PATH, 'xpub': 'tpubDFa3rRFxivQmR3dcgfStqsry6QXHrjpV1FjyXKE8BroySBRTL1SD8ddh2sWXUZxF6wRZufhpkaiR9qqmc7mzEUr4ddvpN7ZHkvJ7vTbgRJD', 'path': []}
   ]}
 }
@@ -29,5 +29,5 @@
     print(fgp.hex())
     params["descriptor"]["signers"].append({
-        "fingerprint": fgp.hex(),
+        "fingerprint": fgp,
         "derivation": PATH,
         "xpub": xpub,

with these changes, your script works for me.

I know I need to generate some documentation so totally my bad. It is high on my list for the new year!

stepansnigirev · 2021-12-29T13:33:35Z

Sorry, I missed it. Works now, thanks a lot!

georgantas · 2021-12-30T02:06:32Z

Thanks. So with Jade we opted to be quite strict in this regard - in order to use multisig the user first needs to register the setup on Jade - ie. as a separate registration step the user inspects/confirms:
* all co-signer xpubs (the hw checks that at least one belongs the Jade)

* threshold

* script type

* (and now) whether bip67 key sorting applies
Once that has been confirmed, future requests to get receive addresses or change addresses will use the registered setup - the only additional data passed per request is the bip32 path suffixes. The full key derivation paths are validated against bip45/48/87, and that the final element is within a reasonable range. If this validation fails, a warning is displayed and the user must explicitly check/confirm the path.

We believe this should cover the known attack scenarios - but obviously are happy to hear if you think otherwise! ;-)

I think I thought of a scenario.

I noticed that to get an address, the computer sends the following to the Jade:

INFO:jade:Sending: {'method': 'get_receive_address', 'id': '468131', 'params': {'network': 'mainnet', 'paths': [[0, 2], [0, 2], [0, 2]], 'multisig_name': 'hwi'}} as cbor of size 108

However, I see that the Jade accepts paths that don't match like:

INFO:jade:Sending: {'method': 'get_receive_address', 'id': '865589', 'params': {'network': 'mainnet', 'paths': [[0, 1], [0, 2], [0, 3]], 'multisig_name': 'hwi'}} as cbor of size 108

which I think is non-standard in most companion wallets.

Could an attacker pass something like [[0, 2333], [0,3999], [0,4383]] and perform a type of ransomware attack on get_address or sign_tx, where they will only reveal the paths in return for some BTC?

I saw that see there's actually some validation on paths here:

Jade/main/wallet.c

Line 343 in 34a1814

bool wallet_is_expected_multisig_path(

, but I don't think this case is covered. I think this wouldn't be a problem for 2 of 3 multisig, as the victim could easily brute force the paths because there's a check on MAX_PATH_PTR of 10000, so 10000^3=10^12 combinations to try (for comparison, bitcoin network hashrate is in the order of 10^20 Hashes/second), but this might be a problem for larger multisigs (I see up to 8 signers are supported). Should we be checking that the paths are all equal to each other for multisig wallets?

JamieDriver · 2021-12-31T16:28:58Z

Might be a good idea. Let's continue discussion on #30 - cheers.

stepansnigirev changed the title ~~Jade and normal multisig~~ Jade and normal multisig compatibility Sep 13, 2021

georgantas mentioned this issue Dec 28, 2021

Does the Jade check the existence of a change address in the output of a transaction? #29

Closed

stepansnigirev closed this as completed Dec 29, 2021

georgantas mentioned this issue Dec 30, 2021

Display a warning if multisig suffix paths don't match #30

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jade and normal multisig compatibility #22

Jade and normal multisig compatibility #22

stepansnigirev commented Sep 13, 2021

JamieDriver commented Sep 20, 2021 •

edited

Loading

JamieDriver commented Oct 5, 2021 •

edited

Loading

JamieDriver commented Oct 28, 2021

JamieDriver commented Oct 28, 2021

JamieDriver commented Nov 26, 2021

JamieDriver commented Dec 23, 2021

stepansnigirev commented Dec 23, 2021

georgantas commented Dec 27, 2021 •

edited

Loading

JamieDriver commented Dec 28, 2021

stepansnigirev commented Dec 28, 2021 •

edited

Loading

stepansnigirev commented Dec 28, 2021

JamieDriver commented Dec 28, 2021 •

edited

Loading

JamieDriver commented Dec 28, 2021 •

edited

Loading

JamieDriver commented Dec 28, 2021

JamieDriver commented Dec 28, 2021

georgantas commented Dec 28, 2021

stepansnigirev commented Dec 29, 2021

JamieDriver commented Dec 29, 2021

stepansnigirev commented Dec 29, 2021

georgantas commented Dec 30, 2021 •

edited

Loading

JamieDriver commented Dec 31, 2021

Jade and normal multisig compatibility #22

Jade and normal multisig compatibility #22

Comments

stepansnigirev commented Sep 13, 2021

JamieDriver commented Sep 20, 2021 • edited Loading

JamieDriver commented Oct 5, 2021 • edited Loading

JamieDriver commented Oct 28, 2021

JamieDriver commented Oct 28, 2021

JamieDriver commented Nov 26, 2021

JamieDriver commented Dec 23, 2021

stepansnigirev commented Dec 23, 2021

georgantas commented Dec 27, 2021 • edited Loading

JamieDriver commented Dec 28, 2021

stepansnigirev commented Dec 28, 2021 • edited Loading

stepansnigirev commented Dec 28, 2021

JamieDriver commented Dec 28, 2021 • edited Loading

JamieDriver commented Dec 28, 2021 • edited Loading

JamieDriver commented Dec 28, 2021

JamieDriver commented Dec 28, 2021

georgantas commented Dec 28, 2021

stepansnigirev commented Dec 29, 2021

JamieDriver commented Dec 29, 2021

stepansnigirev commented Dec 29, 2021

georgantas commented Dec 30, 2021 • edited Loading

JamieDriver commented Dec 31, 2021

JamieDriver commented Sep 20, 2021 •

edited

Loading

JamieDriver commented Oct 5, 2021 •

edited

Loading

georgantas commented Dec 27, 2021 •

edited

Loading

stepansnigirev commented Dec 28, 2021 •

edited

Loading

JamieDriver commented Dec 28, 2021 •

edited

Loading

JamieDriver commented Dec 28, 2021 •

edited

Loading

georgantas commented Dec 30, 2021 •

edited

Loading