[Wallet] add option for a custom extended master privat key (xpriv) #8735

pull jonasschnelli wants to merge 3 commits into bitcoin:master from jonasschnelli:2016/09/hd_set_seed changing 5 files +127 −20

jonasschnelli commented at 9:11 AM on September 15, 2016: contributor

At the moment, the HD seed cannot be set by the user. It will always be generated by Bitcoin-Core (CKey::MakeNewKey()).

This PR adds a startup argument -hdxpriv where users can set their custom hd extended master private key. The argument takes only affect during the creation of a new wallet.dat and there will be a warning if the argument is present during loading an exiting wallet.

If a custom extended master private key has been set, it will be kept when encrypting the wallet (with log-printing a warning).
Add support for IsValid(), SetNull() for CExtKey, allow serialization of uninitialized keys 91d014e4ae
[Wallet] Add support for a custom extended private master key in CHDChain 5793c735ea
jonasschnelli added the label Wallet on Sep 15, 2016
[Wallet] allow setting the master extended private key (xpriv) during HD wallet creation d772d3bb21
jonasschnelli force-pushed on Sep 15, 2016
btcdrak commented at 3:03 PM on September 15, 2016: contributor

As it stands this will leak the xpriv since the xpriv will be in the command history. The import of the xpriv should be interactive and should be encrypted before being stored.

I know previously we would not recommend forcing encryption because it could lead to funds loss, here the user already has the xpriv so it's already backed up.
paveljanik commented at 3:11 PM on September 15, 2016: contributor

@btcdrak We can change this to accept the seed only from stdin (-stdin).
jonasschnelli commented at 3:27 PM on September 15, 2016: contributor

Yes. I'm aware of the security weakness of accepting a xpriv over a startup argument. A previous proposed solution would be to support a tool called bitcoin-wallet which could create, alter and encrypt wallet without the need of bitcoind (this would by my preferred solution but can be added later).

I think accepting a xpriv over a configuration value is not extremely bad. If an attacker can access bitcoin.conf he might also be capable of grabbing your RPC password and dump your wallet to /tmp.
btcdrak commented at 3:35 PM on September 15, 2016: contributor

Personally I prefer a separate interactive import tool and a Qt GUI option which I think would be used more in any case. FWIW, I think importing via a bitcoin.conf option is even worse.
btcdrak commented at 3:36 PM on September 15, 2016: contributor

@btcdrak We can change this to accept the seed only from stdin (-stdin).

Sounds good, that relieves my concerns.

in src/wallet/walletdb.h:54 in d772d3bb21

  46 | @@ -47,8 +47,12 @@ class CHDChain
  47 |  public:
  48 |      uint32_t nExternalChainCounter;
  49 |      CKeyID masterKeyID; //!< master key hash160
  50 | +    CExtKey customMasterKey;
  51 | +
  52 | +    static const int VERSION_BASE = 1;
  53 | +    static const int VERSION_WITH_CUSTOM_MASTER_KEY = 2;
  54 | +    static const int CURRENT_VERSION = VERSION_WITH_CUSTOM_MASTER_KEY;

TheBlueMatt commented at 6:09 PM on September 29, 2017:

Why add a new version? Why not just overwrite more akin to the way #11085 works?

TheBlueMatt commented at 6:09 PM on September 29, 2017: member

Needs rebase.
sipa commented at 5:08 PM on March 6, 2018: member

Concept ACK, but I think this needs significant reworking to interact with multiwallet.
jnewbery commented at 5:24 PM on April 3, 2018: member

This would possibly be easier to implement using bitcoin-wallet-tool (https://github.com/bitcoin/bitcoin/pull/8745)
dancju commented at 2:00 PM on May 28, 2018: none

I am confused. Is there any workaround that allows me importing xpriv or seed or mnemonic into bitcoind?
MarcoFalke added the label Needs rebase on Jun 6, 2018
DrahtBot commented at 11:20 PM on November 8, 2018: member

There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened.
DrahtBot added the label Up for grabs on Nov 8, 2018
DrahtBot closed this on Nov 8, 2018
laanwj removed the label Needs rebase on Oct 24, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

Labels