RPC/Wallet: Refuse to dumpprivkey for derived addresses #11802

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:dumpprivkey_noderived changing 3 files +18 −0

luke-jr commented at 8:31 PM on November 30, 2017: member

The specific ECDSA "private key" of a derived key is merely a midstate of the signature algorithm. It doesn't really make sense to dump it.

RPC/Wallet: Refuse to dumpprivkey for derived addresses

The specific ECDSA "private key" of a derived key is merely a midstate of the signature algorithm. It doesn't really make sense to dump it.

38efd220c4

fanquake added the label RPC/REST/ZMQ on Nov 30, 2017
fanquake added the label Wallet on Nov 30, 2017

in src/wallet/rpcdump.cpp:582 in 38efd220c4

 577 | @@ -578,6 +578,9 @@ UniValue dumpprivkey(const JSONRPCRequest& request)
 578 |      if (!IsValidDestination(dest)) {
 579 |          throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
 580 |      }
 581 | +    if (pwallet->IsKeyDerived(dest)) {
 582 | +        throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to a key (derived)");

promag commented at 1:03 AM on December 1, 2017:

Add test?

promag commented at 1:07 AM on December 1, 2017: member

Fix test/functional/import-rescan.py.
jonasschnelli commented at 6:18 AM on December 1, 2017: contributor

Concept ACK. Needs travis fixing.
TheBlueMatt commented at 2:57 PM on December 5, 2017: member

NACK - I dont see why a user shouldn't be able to export the private key of a key they just getnewaddress'd. Not only is this a huge API change, but why should that not be allowed?
luke-jr commented at 5:22 PM on December 5, 2017: member

For a HD wallet, there is only one private key: the seed. Everything else is merely a midstate of the signing algorithm.
sipa commented at 5:40 PM on December 5, 2017: member

@luke-jr For non-hardened derivation I would agree with that, as leaking a child is effectively equivalent to leaking the whole tree. But I disagree that this is desirable for hardened derivation - that derivation is designed to result in standalone usable keys without security implications.
ryanofsky commented at 6:03 PM on December 5, 2017: member

Maybe disallowing by default and adding a force option would be a way of warning users who may be unintentionally dumping derived keys, while not getting in the way of users who do want this.
luke-jr closed this on Feb 26, 2018
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels