fix(key_map): do not fulfill BIP32 requests outside descriptor path - #1004
Conversation
Remove the early `Xpriv::get_key` return in `DescriptorSecretKey::get_key`. That path matched on fingerprint only and ignored derivation path / wildcard, so a master xprv in e.g. `.../0/*` would sign sibling paths like `.../9/7`. Rely on `matches()` only. Assisted-by: Grok 4.5
Regression: master xprv descriptors such as `.../0/*` must not fulfill `KeyRequest::Bip32` for sibling paths like `.../9/7`. Assisted-by: Grok 4.5
|
@apoelstra friendly ping. I think this is important to fix asap |
|
Yeah, I think you're right. I'm torn about whether it's reasonable for a user to rely on this -- it seems like if you only want to sign with a certain key, you should only make that key available to your signer. But certainly, if a user does rely on this, we shouldn't be surprising them like that. In general, Miniscript has a bias toward "if the user has asked to sign, do anything possible to make the signing happen" on the assumption that if the user isn't allowed to make that decision, presumably they're physically unable to make that decision. |
|
Sorry for the long delay. Should be ACKed/merged today. If you could backport this to 12.x and 13.x (target branches |
9667f8e test(key_map): reject sibling BIP32 path for master xprv (Noah Joeris) 9bad254 fix(key_map): do not fulfill BIP32 requests outside descriptor path (Noah Joeris) Pull request description: Backport of #1004. Fixes bypassing descriptor path validation for `GetKey` on sibling paths. ACKs for top commit: apoelstra: ACK 9667f8e; successfully ran local tests Tree-SHA512: a9079045c341e5fc45cfa322c07633395993700b30ea4f22622b78efdcef6b545a3cd5fa18ca2d64371f3f3927768c9642f38107f73220c2435bdf49b96ab8a9
651715a test(key_map): reject sibling BIP32 path for master xprv (Noah Joeris) 4973cd4 fix(key_map): do not fulfill BIP32 requests outside descriptor path (Noah Joeris) Pull request description: Backport of #1004. Fixes bypassing descriptor path validation for `GetKey` on sibling paths. ACKs for top commit: apoelstra: ACK 651715a; successfully ran local tests Tree-SHA512: 75a4b3574b35d570aadfae3f0f47efe698554af3e07d34aab745c74f83e92eed114a37d983fb21aec05bad33535a1295fe2ff5340d54fd01939b4e712834d166
2 participants
Description
GetKeybypassed descriptor path validation on fingerprint match, allowing a master xprv for…/0/*to sign sibling paths like…/9/7.Remove the early return and require
matches()before key derivation. Adds a regression test.