From e073b1d343c9e6c02606320f50ea61a4085b42df Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 6 May 2022 16:58:10 -0700 Subject: [PATCH] macaroons: futher abstract NewService from root key store impl In this commit, we modify the `macaroons.NewService` consturctor to accept the main interface rather than the raw DB. This allows us to use other backends other than bolt or the kvdb interface to store the macaroon root keys. We also create a new ExtendedRootKeyStore interface that implements some of the more advanced features we use such as macaroon encryption and password rotation. --- config_builder.go | 6 +- docs/release-notes/release-notes-0.16.0.md | 4 +- macaroons/service.go | 82 +++++++++++++++++----- macaroons/service_test.go | 17 +++-- walletunlocker/service.go | 6 +- 5 files changed, 91 insertions(+), 24 deletions(-) diff --git a/config_builder.go b/config_builder.go index 05f8ab168..d43834e0d 100644 --- a/config_builder.go +++ b/config_builder.go @@ -394,8 +394,12 @@ func (d *DefaultWalletImpl) BuildWalletConfig(ctx context.Context, var macaroonService *macaroons.Service if !d.cfg.NoMacaroons { // Create the macaroon authentication/authorization service. + rootKeyStore, err := macaroons.NewRootKeyStorage(dbs.MacaroonDB) + if err != nil { + return nil, nil, nil, err + } macaroonService, err = macaroons.NewService( - dbs.MacaroonDB, "lnd", walletInitParams.StatelessInit, + rootKeyStore, "lnd", walletInitParams.StatelessInit, macaroons.IPLockChecker, macaroons.CustomChecker(interceptorChain), ) diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index a1b768018..b43b951a6 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/docs/release-notes/release-notes-0.16.0.md @@ -5,7 +5,9 @@ * [Fixed error typo](https://github.com/lightningnetwork/lnd/pull/6659). +* [The macaroon key store implementation was refactored to be more generally useable](https://github.com/lightningnetwork/lnd/pull/6509). + # Contributors (Alphabetical Order) * Carla Kirk-Cohen * ErikEk - +* Olaoluwa Osuntokun diff --git a/macaroons/service.go b/macaroons/service.go index 49cb1c6da..0c3df7a38 100644 --- a/macaroons/service.go +++ b/macaroons/service.go @@ -5,7 +5,6 @@ import ( "encoding/hex" "fmt" - "github.com/lightningnetwork/lnd/kvdb" "google.golang.org/grpc/metadata" "gopkg.in/macaroon-bakery.v2/bakery" "gopkg.in/macaroon-bakery.v2/bakery/checkers" @@ -41,13 +40,43 @@ type MacaroonValidator interface { requiredPermissions []bakery.Op, fullMethod string) error } +// ExtendedRootKeyStore is an interface augments the existing +// macaroons.RootKeyStorage interface by adding a number of additional utility +// methods such as encrypting and decrypting the root key given a password. +type ExtendedRootKeyStore interface { + bakery.RootKeyStore + + // Close closes the RKS and zeros out any in-memory encryption keys. + Close() error + + // CreateUnlock calls the underlying root key store's CreateUnlock and + // returns the result. + CreateUnlock(password *[]byte) error + + // ListMacaroonIDs returns all the root key ID values except the value + // of encryptedKeyID. + ListMacaroonIDs(ctxt context.Context) ([][]byte, error) + + // DeleteMacaroonID removes one specific root key ID. If the root key + // ID is found and deleted, it will be returned. + DeleteMacaroonID(ctxt context.Context, rootKeyID []byte) ([]byte, error) + + // ChangePassword calls the underlying root key store's ChangePassword + // and returns the result. + ChangePassword(oldPw, newPw []byte) error + + // GenerateNewRootKey calls the underlying root key store's + // GenerateNewRootKey and returns the result. + GenerateNewRootKey() error +} + // Service encapsulates bakery.Bakery and adds a Close() method that zeroes the // root key service encryption keys, as well as utility methods to validate a // macaroon against the bakery and gRPC middleware for macaroon-based auth. type Service struct { bakery.Bakery - rks *RootKeyStorage + rks bakery.RootKeyStore // ExternalValidators is a map between an absolute gRPC URIs and the // corresponding external macaroon validator to be used for that URI. @@ -67,17 +96,12 @@ type Service struct { // not harmful. Default checkers, such as those for `allow`, `time-before`, // `declared`, and `error` caveats are registered automatically and don't need // to be added. -func NewService(db kvdb.Backend, location string, statelessInit bool, - checks ...Checker) (*Service, error) { - - rootKeyStore, err := NewRootKeyStorage(db) - if err != nil { - return nil, err - } +func NewService(keyStore bakery.RootKeyStore, location string, + statelessInit bool, checks ...Checker) (*Service, error) { macaroonParams := bakery.BakeryParams{ Location: location, - RootKeyStore: rootKeyStore, + RootKeyStore: keyStore, // No third-party caveat support for now. // TODO(aakselrod): Add third-party caveat support. Locator: nil, @@ -98,7 +122,7 @@ func NewService(db kvdb.Backend, location string, statelessInit bool, return &Service{ Bakery: *svc, - rks: rootKeyStore, + rks: keyStore, ExternalValidators: make(map[string]MacaroonValidator), StatelessInit: statelessInit, }, nil @@ -204,13 +228,21 @@ func (svc *Service) CheckMacAuth(ctx context.Context, macBytes []byte, // Close closes the database that underlies the RootKeyStore and zeroes the // encryption keys. func (svc *Service) Close() error { - return svc.rks.Close() + if boltRKS, ok := svc.rks.(ExtendedRootKeyStore); ok { + return boltRKS.Close() + } + + return nil } // CreateUnlock calls the underlying root key store's CreateUnlock and returns // the result. func (svc *Service) CreateUnlock(password *[]byte) error { - return svc.rks.CreateUnlock(password) + if boltRKS, ok := svc.rks.(ExtendedRootKeyStore); ok { + return boltRKS.CreateUnlock(password) + } + + return nil } // NewMacaroon wraps around the function Oven.NewMacaroon with the defaults, @@ -239,7 +271,11 @@ func (svc *Service) NewMacaroon( // ListMacaroonIDs returns all the root key ID values except the value of // encryptedKeyID. func (svc *Service) ListMacaroonIDs(ctxt context.Context) ([][]byte, error) { - return svc.rks.ListMacaroonIDs(ctxt) + if boltRKS, ok := svc.rks.(ExtendedRootKeyStore); ok { + return boltRKS.ListMacaroonIDs(ctxt) + } + + return nil, nil } // DeleteMacaroonID removes one specific root key ID. If the root key ID is @@ -247,19 +283,31 @@ func (svc *Service) ListMacaroonIDs(ctxt context.Context) ([][]byte, error) { func (svc *Service) DeleteMacaroonID(ctxt context.Context, rootKeyID []byte) ([]byte, error) { - return svc.rks.DeleteMacaroonID(ctxt, rootKeyID) + if boltRKS, ok := svc.rks.(ExtendedRootKeyStore); ok { + return boltRKS.DeleteMacaroonID(ctxt, rootKeyID) + } + + return nil, nil } // GenerateNewRootKey calls the underlying root key store's GenerateNewRootKey // and returns the result. func (svc *Service) GenerateNewRootKey() error { - return svc.rks.GenerateNewRootKey() + if boltRKS, ok := svc.rks.(ExtendedRootKeyStore); ok { + return boltRKS.GenerateNewRootKey() + } + + return nil } // ChangePassword calls the underlying root key store's ChangePassword and // returns the result. func (svc *Service) ChangePassword(oldPw, newPw []byte) error { - return svc.rks.ChangePassword(oldPw, newPw) + if boltRKS, ok := svc.rks.(ExtendedRootKeyStore); ok { + return boltRKS.ChangePassword(oldPw, newPw) + } + + return nil } // RawMacaroonFromContext is a helper function that extracts a raw macaroon diff --git a/macaroons/service_test.go b/macaroons/service_test.go index aaaf8be0a..18bcf00ea 100644 --- a/macaroons/service_test.go +++ b/macaroons/service_test.go @@ -59,10 +59,13 @@ func TestNewService(t *testing.T) { tempDir, db := setupTestRootKeyStorage(t) defer os.RemoveAll(tempDir) + rootKeyStore, err := macaroons.NewRootKeyStorage(db) + require.NoError(t, err) + // Second, create the new service instance, unlock it and pass in a // checker that we expect it to add to the bakery. service, err := macaroons.NewService( - db, "lnd", false, macaroons.IPLockChecker, + rootKeyStore, "lnd", false, macaroons.IPLockChecker, ) require.NoError(t, err, "Error creating new service") defer service.Close() @@ -106,8 +109,10 @@ func TestValidateMacaroon(t *testing.T) { // First, initialize the service and unlock it. tempDir, db := setupTestRootKeyStorage(t) defer os.RemoveAll(tempDir) + rootKeyStore, err := macaroons.NewRootKeyStorage(db) + require.NoError(t, err) service, err := macaroons.NewService( - db, "lnd", false, macaroons.IPLockChecker, + rootKeyStore, "lnd", false, macaroons.IPLockChecker, ) require.NoError(t, err, "Error creating new service") defer service.Close() @@ -154,8 +159,10 @@ func TestListMacaroonIDs(t *testing.T) { // Second, create the new service instance, unlock it and pass in a // checker that we expect it to add to the bakery. + rootKeyStore, err := macaroons.NewRootKeyStorage(db) + require.NoError(t, err) service, err := macaroons.NewService( - db, "lnd", false, macaroons.IPLockChecker, + rootKeyStore, "lnd", false, macaroons.IPLockChecker, ) require.NoError(t, err, "Error creating new service") defer service.Close() @@ -186,8 +193,10 @@ func TestDeleteMacaroonID(t *testing.T) { // Second, create the new service instance, unlock it and pass in a // checker that we expect it to add to the bakery. + rootKeyStore, err := macaroons.NewRootKeyStorage(db) + require.NoError(t, err) service, err := macaroons.NewService( - db, "lnd", false, macaroons.IPLockChecker, + rootKeyStore, "lnd", false, macaroons.IPLockChecker, ) require.NoError(t, err, "Error creating new service") defer service.Close() diff --git a/walletunlocker/service.go b/walletunlocker/service.go index e473a0c5b..157ef086d 100644 --- a/walletunlocker/service.go +++ b/walletunlocker/service.go @@ -807,8 +807,12 @@ func (u *UnlockerService) ChangePassword(ctx context.Context, // then close it again. // Attempt to open the macaroon DB, unlock it and then change // the passphrase. + rootKeyStore, err := macaroons.NewRootKeyStorage(u.macaroonDB) + if err != nil { + return nil, err + } macaroonService, err := macaroons.NewService( - u.macaroonDB, "lnd", in.StatelessInit, + rootKeyStore, "lnd", in.StatelessInit, ) if err != nil { return nil, err