walletunlocker+lnd: add command line flag to allow passing admin macaroon after wallet creation #1288
Conversation
guggero
force-pushed
the
stateless-init
branch
3 times, most recently
from
8a457dd to
d88e737
Compare
guggero
force-pushed
the
stateless-init
branch
3 times, most recently
from
7e87918 to
b48f6f0
Compare
guggero
force-pushed
the
stateless-init
branch
2 times, most recently
from
9a4e4c6 to
d570156
Compare
Roasbeef
added
P2
guggero
force-pushed
the
stateless-init
branch
9 times, most recently
from
3391512 to
0580880
Compare
|
This PR has gotten a lot more complicated than initially thought... According to @Roasbeef (#1236 (comment)), this was a mistake. @neogeno is helping me with the testing, thanks a lot for that! |
guggero
force-pushed
the
stateless-init
branch
3 times, most recently
from
f697e5c to
1e6e526
Compare
guggero
force-pushed
the
stateless-init
branch
from
cfb836d to
5d54497
Compare
| // to delete them here and they will be recreated during normal startup | ||
| // later. If they are missing, this is only an error if the | ||
| // stateless_init flag was not set. | ||
| if in.NewMacaroonRootKey || in.StatelessInit { |
There was a problem hiding this comment.
Don't we always need to do this for any change password request?
There was a problem hiding this comment.
No, not anymore. Before, we just deleted the whole macaroon DB, effectively invalidating all macaroons. Therefore it did make sense to delete the macaroon files themselves as well. But now, we only re-generate the root key if specifically requested. So we only need to remove the macaroon files on disk if the root key is changed or if the user tries to switch to stateless.
| } | ||
| return nil, err | ||
| } | ||
| err = macaroonService.ChangePassword(privatePw, in.NewPassword) |
There was a problem hiding this comment.
Why do we need to now explicitly perform this new password change at the macaroon service level? If we crash at this point, but the above succeeds, then the user may end up in a situation where they can't start up (?) or just aren't able to use their macaroons. I guess in either case, then they can just remove their on-disk macaroons, then have things just "work".
There was a problem hiding this comment.
Before we just deleted the whole macaroon DB. But since we now want to support only changing the password, without invalidating all macaroons, we have to perform the actual password change on the DB level.
Indeed, if something goes wrong here, it could result in the wallet DB password being changed but the macaroon DB password not. The user then still has the option to completely remove the macaroon DB and all macaroon files to resolve this. Not sure if we need to address this in this PR?
guggero
force-pushed
the
stateless-init
branch
3 times, most recently
from
827ca50 to
2cf4f20
Compare
guggero
force-pushed
the
stateless-init
branch
from
2cf4f20 to
bfbda5b
Compare
guggero
force-pushed
the
stateless-init
branch
from
bfbda5b to
98d3cb6
Compare
…locker This commit adds the --stateless_init flag to all three wallet unlocker operations. Once you initialize a wallet stateless, you need to set this flag for every further wallet unlocker operation. Otherwise you risk non-encrypted macaroon information to leak to the underlying system.
To make sure no macaroons are created anywhere if the stateless initialization was requested, we keep the requested initialization mode in the memory of the macaroon service.
As a preparation for the next commit where we add proper wallet unlocker shutdown handling, we move the calls that require cleanup down after the creation of the wallet unlocker service itself.
Because we'll need to return the macaroon through the wallet unlocker we cannot shut down its service before we have done so, otherwise we'll end up in a deadlock. That's why we collect all shutdown tasks and return them as a function that can be called after we've initialized the macaroon service.
To prepare for new tests to be added, we first rewrite the existing tests to use the require library and get rid of some smaller linter issues.
This will prevent the subservers from writing macaroons to disk when the stateless_init flag is set to true. It accomplishes this by storing the StatelessInit value in the Macaroon Service.
guggero
force-pushed
the
stateless-init
branch
from
98d3cb6 to
5aa0d26
Compare
Fixes #1236.
The new flag
--stateless_initof the startup commandscreate,unlockandchangepasswordinstructs the daemon to not create any macaroon files in its file system but instead return the admin macaroon in the response.The
lnclithen prints the macaroon to the standard output (binary serialized in hex format) or writes it to the file specified with the--save_toparameter.Some of the integration test code is also contained in PR #1152. If either of these two PRs get merge, I'll rebase/fix the other one.