lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.69k stars 2.08k forks source link

Wallet recovery with passphrase is not working #3168

Closed ShahanaFarooqui closed 5 years ago

ShahanaFarooqui commented 5 years ago

initwallet REST API is working without passphrase but it fails with passphrase, with 'invalid passphrase' error message.

I opened first issue about this API #3128 (https://github.com/lightningnetwork/lnd/issues/3128) almost 10 days back also and i am testing these scenarios from almost last 15 days. I found that initwallet REST API is behaving strangly.

If I generate the genseed with passphrase through lncli and use this cipher_seed_mnemonic array to recover the wallet through initwallet REST API, then it works perfectly. But, if i will generate the genseed with passphrase through REST and use that cipher_seed_mnemonic array to recover with initwallet REST then i am getting 'invalid passphrase' error.

Your environment version of lnd: 0.6.0-beta which operating system: Windows version bitcoind: v0.17.1

Steps to reproduce 1) Call GET /v1/genseed with aezeed_passphrase value. 2) Call POST /v1/initwallet with wallet_password. it will initialize the wallet successfully. 3) Delete the wallet manually. 4) Call POST /v1/initwallet with wallet_password, aezeed_passphrase and cipher_seed_mnemonic array generated from step 1.

Initialize wallet with below details: Method: POST, url: https://192.168.1.8:8080/v1/initwallet, payload: { "wallet_password":"c3VoZWIxMjM=", "cipher_seed_mnemonic":["absent","stuff","layer","holiday","slam","pelican","drift","heart","prepare","casual","treat","liar","small","venue","fragile","wise","frown","safe","brief","cherry","pigeon","pole","sound","rocket"], "aezeed_passphrase":"dGhpc2lzbXlwYXNzcGhyYXNl" }

Expected behaviour It should successfully recover the wallet.

Actual behaviour Failed response with { "error": "invalid passphrase", "code": 2 }

I have tested initwallet with multiple scenarios and all the scenarios are working with GRPC but not with REST.

Roasbeef commented 5 years ago

Do you understand the difference between the aezeed_passphrase and the wallet_password? Only the wallet_password is actually required.

Perhaps it's an issue of encoding (base64) between the wallet calls? Will try to reproduce on our end.

ShahanaFarooqui commented 5 years ago

We are not using passphrase when initializing the wallet, using it only to recover the wallet. And if it is not even required for recovery then can you please explain the use case where passphrase should be passed to initwallet API?

wpaulino commented 5 years ago

I don't think there's an issue here given that things are working fine with gRPC. Like @Roasbeef mentioned, it is most likely an encoding issue of the distinct passphrases.

We are not using passphrase when initializing the wallet, using it only to recover the wallet. And if it is not even required for recovery then can you please explain the use case where passphrase should be passed to initwallet API?

wallet_password is the password to unlock the wallet. This does not serve a purpose other than encrypting your secrets on-disk. It is always required when using initwallet.

aezeed_passphrase is an additional optional passphrase that only applies to your seed and not your wallet.

saubyk commented 5 years ago

The scenario that we are testing is:

The scenario works when the seed and the passphrase are generated with grpc. The recovery step fails when the seed and passphrase generated via REST APIs are used.

ShahanaFarooqui commented 5 years ago

@ashwilly, we didn’t have any funds to recover as we were on testnet. We are developing an application called ‘Ride the Lightning’ (RTL) and we wanted to include create new wallet feature for our users but unfortunately we are also worried about the recovery. We will appreciate any help on the issue :). Thanks.

Roasbeef commented 5 years ago

Was this eventually resolved w/ proper API usage?

ShahanaFarooqui commented 5 years ago

No, it is still an open issue and our user will still be unable to recover the wallet with passphrase.

guggero commented 5 years ago

Can you please try if base64 encoding the wallet_password and aezeed_passphrase helps? Or passing in a Buffer into the request of walletUnlocker.initWallet instead of a string?

ShahanaFarooqui commented 5 years ago

@guggero We worked on this issue almost 5 months back and as long as i can recall we were passing both in base64. But, let me recheck it once again. I will update you by today eod.

ShahanaFarooqui commented 5 years ago

@guggero Appologies for the delayed reply. Previously, with LND's old version, we were able to initialize the wallet with passphrase and were only unable to recover it. But, this time, we are not even able to initialize the wallet with passphrase.

When we tried to initialize the wallet with passphrase, we got the error as: {"error":"invalid passphrase","code":2}.

Below are the steps to reproduce the issue:

1) Called genseed with passphrase as "Test": https://192.168.x.x:8080/v1/genseed?"aezeed_passphrase"=Test

2) Received response as: {"cipher_seed_mnemonic":["able","flower","income","fantasy","kidney","arrest","oxygen","elite","dice","mule","height","broken","marriage","island","rural","unfair","ticket","valid","awesome","poet","acquire","either","embody","chief"],"enciphered_seed":"AEsxyimHphjnkkA9cimqjkiE7O9naOG+GEJTkCKOEhk+"}

3) Called initwallet API with "TestPassword" as password, "Test" as passphrase and above mnemonic value as cipher_seed_mnemonic. URL: https://192.168.x.x:8080/v1/initwallet options.form: {"wallet_password":"VGVzdFBhc3N3b3Jk","cipher_seed_mnemonic":["able","flower","income","fantasy","kidney","arrest","oxygen","elite","dice","mule","height","broken","marriage","island","rural","unfair","ticket","valid","awesome","poet","acquire","either","embody","chief"],"aezeed_passphrase":"VGVzdA=="}

Please note that password and passphrase both are passing base64 values of "TestPassword" and "Test".

guggero commented 5 years ago

@ShahanaFarooqui thank you very much for the detailed steps. I was able to reproduce the issue.

The problem why recovery doesn't work is because the password set in genseed is ignored. It looks like if you pass the parameters in that way, they are just ignored. You need to set them as normal query parameters. For example:

curl -k https://localhost:8099/v1/genseed?aezeed_passphrase=dGVzdA== (base64 for "test").

But this will lead to the following error message: {"error":"unsupported field type uint8","code":3}

This is a problem with the grpc-gateway library we use for the automatic translation between gRPC and REST.

I created a PR to update the library to a version that doesn't have this bug: #3650.

ShahanaFarooqui commented 5 years ago

@guggero Thanks a lot for fixing it quickly :), hope it will be merged and released soon.

We also tried to use the query params the usual way (without quotes) but due to the above error, we had to pass them in quotes.

Also, the REST documentation, https://api.lightning.community/rest/index.html#v1-genseed, is advising to send the passphrase in string format NOT in base64. So, please confirm once again if we should use it in string/base64 format?

guggero commented 5 years ago

I think if there are no unexpected side effects of the fix then it can be merged quite quickly.

Correct, placing double quotes around the parameter name makes the error disappear. Because it is then just ignored by the REST gateway.

Please note that the REST documentation is auto-generated (I think from the rpc.swagger.json) and the data types noted there are referring to JSON data types (of which there is only really string, number and boolean). So it is not very precise. Maybe it helps to look at the rpc.swagger.json file directly:

    "/v1/genseed": {
      "get": {
        ...
        "operationId": "GenSeed",
       ...
        },
        "parameters": [
          {
            "name": "aezeed_passphrase",
            "description": "*\naezeed_passphrase is an optional user provided passphrase that will be used\nto encrypt the generated aezeed cipher seed.",
            "in": "query",
            "required": false,
            "type": "string",
            "format": "byte"
          },
          ...
        ],

You can see that the type is string but the format is really byte. And the default encoding for a byte array is base64 in the library we use. So short answer: Yes, it needs to be a base64 string whenever the Swagger format is byte.

ShahanaFarooqui commented 4 years ago

@guggero, One last question :). Should the passphrase be used till seed generation only or we should pass it to init wallet API also? The documentation shows it as optional. But, if the passphrase is already being used to generate the seed then will it again be required at initwallet? Our flow diagram as per the documentation can be found here.

guggero commented 4 years ago

If the user sets a password and you send it with genseed then it is also required in initwallet. Otherwise lnd won't be able to decrypt the 24 words you send in initwallet. If the user chooses to not set a password then it must be empty for both calls.

ShahanaFarooqui commented 4 years ago

@guggero & @Roasbeef, We pulled the master branch and tested the passphrase fix thoroughly and it is working perfectly now. Thanks.

weltitob commented 3 months ago

Hello eventough this issue is closed since you guys figured once how to use the api to obtain a seedphrase and then call initwallet i am reaching out as this servcie seems to be broken? Can you maybe provide me with an examplecode or smth how you guys did this? This is my issue: https://github.com/lightninglabs/lightning-terminal/issues/805