lnbits / lnbits

LNbits, free and open-source Lightning wallet and accounts system.
https://lnbits.com
MIT License
1.03k stars 336 forks source link

[BUG] Encrypted Macaroon and Admin Macaroon problem for LndWallet #2649

Closed vicariousdrama closed 3 weeks ago

vicariousdrama commented 1 month ago

Describe the bug

LndWallet (GRPC) can't update Admin macaroon after initialization

In lnbits/lnbits/wallets/lndgrpc.py, the macaroon is assigned the value of settings.lnd_grpc_macaroon which is set during initialization.

The Admin Macaroon field is available in the UI for setting lnd_grpc_admin_macaroon, but will have no impact since lnd_grpc_macaroon is already set.

So the only way to update the macaroon appears to be via the Encrypted Macaroon option.

The guidance for this according to the docs (https://docs.lnbits.org/guide/wallets.html) is to run poetry run python lnbits/wallets/macaroon/macaroon.py. On a stock clone setup, this unfortunately results in an error as loguru is not found by default. While external, even after fulfilling that dependency, the dependencies for lnbits itself wont be met without building the whole project. To rectify, I can spin up a docker instance of lnbits and run this

cd /app
poetry run python lnbits/wallets/macaroon/macaroon.py

And then paste in the hexadecimal for the macaroon. Next, it wants a password because AESCipher wasn't initialized with a key.
Providing no password produces an encrypted value, but this isn't going to get very far as the decrypt operation also wants to prompt for a password. Indeed, on startup, providing a value for Encrypted Macaroon will result in this error

Enter macaroon decryption password:/usr/local/lib/python3.10/getpass.py:91: GetPassWarning: Can not control echo on the terminal.
  passwd = fallback_getpass(prompt, stream)
Warning: Password input may be echoed.

To Reproduce Steps to reproduce the behavior:

  1. Initialize an LNbits instance with LndWallet settings
  2. Attempt to change the macaroon
  3. New macaroon cannot take effect, either by using Admin Macaroon field (because its subordinate to the encrypted macaroon field), or the Encrypted Macaroon field (because of the bug with password handling)

Expected behavior

If a HEX value for macaroon or file path is provided in Admin Macaroon field, it should update the lnd_grpc_macaroon value in addition ot the lnd_grpc_admin_macaroon.

It should be possible to provide an Encrypted macaroon with no password, or else a default internal password should be used or perhaps based on the AUTH_SECRET_KEY. In any event, encrypting documentation will need to be updated to reflect what value the user should specify for password.

Screenshots If applicable, add screenshots to help explain your problem.

image

Desktop (please complete the following information):

Additional context It's possible that this is related to #1906. To be clear, if you initialize the instance with the macaroon you intend to use, it seems to be fine. I've been using an instance for over a month with only invoice support. It was only when I wanted to add the ability to Send and Track Payments with a new macaroon that I uncovered this in my setup.

vicariousdrama commented 1 month ago

I'm able to build lnbits now, and this is what I'm seeing after changing from a virtually stock setup (only LNBITS_ADMIN_UI=true) that started as VoidWallet with defaults for others, and then attempted to configure LndWallet with the Admin Macaroon

image

In the UI, there is no longer a reference to this value '/home/bob/.lnd/data/chain/bitcoin/mainnet/admin.macaroon'

vicariousdrama commented 1 month ago

After setting to an invoice only macaroon using encrypted macaroon input with the fix in the referenced PR image

vicariousdrama commented 1 month ago

After setting to macaroon with required payment permissions and paying an invoice on the same LND node. This isn't being done as an internal lnbits instance, but rather from one lnbits to another lnbits referencing the same funding source image

This proves that the encrypted macaroon value was picked up with changes in referenced PR

vicariousdrama commented 1 month ago

And completing a payment image

motorina0 commented 3 weeks ago

Can this be closed?