multi: make macaroon DB remote compatible

The macaroon root keys should also be stored to the remote database if a
replicated backend such as etcd is used.
This commit refactors the macaroons service and wallet unlocker to
accept a kvdb backend directly instead of creating the bolt instance
automatically.

macaroons/service.go

@@ -4,9 +4,6 @@ import (
 	"context"
 	"encoding/hex"
 	"fmt"
-	"os"
-	"path"
-	"time"
 
 	"github.com/lightningnetwork/lnd/kvdb"
 	"google.golang.org/grpc/metadata"
@@ -17,10 +14,6 @@ import (
 )
 
 var (
-	// DBFilename is the filename within the data directory which contains
-	// the macaroon stores.
-	DBFilename = "macaroons.db"
-
 	// ErrMissingRootKeyID specifies the root key ID is missing.
 	ErrMissingRootKeyID = fmt.Errorf("missing root key ID")
 
@@ -68,34 +61,17 @@ type Service struct {
 	StatelessInit bool
 }
 
-// NewService returns a service backed by the macaroon Bolt DB stored in the
-// passed directory. The `checks` argument can be any of the `Checker` type
-// functions defined in this package, or a custom checker if desired. This
-// constructor prevents double-registration of checkers to prevent panics, so
-// listing the same checker more than once is 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(dir, location string, statelessInit bool,
-	dbTimeout time.Duration, checks ...Checker) (*Service, error) {
+// NewService returns a service backed by the macaroon DB backend. The `checks`
+// argument can be any of the `Checker` type functions defined in this package,
+// or a custom checker if desired. This constructor prevents double-registration
+// of checkers to prevent panics, so listing the same checker more than once is
+// 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) {
 
-	// Ensure that the path to the directory exists.
-	if _, err := os.Stat(dir); os.IsNotExist(err) {
-		if err := os.MkdirAll(dir, 0700); err != nil {
-			return nil, err
-		}
-	}
-
-	// Open the database that we'll use to store the primary macaroon key,
-	// and all generated macaroons+caveats.
-	macaroonDB, err := kvdb.Create(
-		kvdb.BoltBackendName, path.Join(dir, DBFilename), true,
-		dbTimeout,
-	)
-	if err != nil {
-		return nil, err
-	}
-
-	rootKeyStore, err := NewRootKeyStorage(macaroonDB)
+	rootKeyStore, err := NewRootKeyStorage(db)
 	if err != nil {
 		return nil, err
 	}

macaroons/service_test.go

@@ -33,7 +33,7 @@ var (
 // default password of 'hello'. Only the path to the temporary
 // DB file is returned, because the service will open the file
 // and read the store on its own.
-func setupTestRootKeyStorage(t *testing.T) string {
+func setupTestRootKeyStorage(t *testing.T) (string, kvdb.Backend) {
 	tempDir, err := ioutil.TempDir("", "macaroonstore-")
 	if err != nil {
 		t.Fatalf("Error creating temp dir: %v", err)
@@ -55,21 +55,20 @@ func setupTestRootKeyStorage(t *testing.T) string {
 	if err != nil {
 		t.Fatalf("error creating unlock: %v", err)
 	}
-	return tempDir
+	return tempDir, db
 }
 
 // TestNewService tests the creation of the macaroon service.
 func TestNewService(t *testing.T) {
 	// First, initialize a dummy DB file with a store that the service
 	// can read from. Make sure the file is removed in the end.
-	tempDir := setupTestRootKeyStorage(t)
+	tempDir, db := setupTestRootKeyStorage(t)
 	defer os.RemoveAll(tempDir)
 
 	// 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(
-		tempDir, "lnd", false, kvdb.DefaultDBTimeout,
-		macaroons.IPLockChecker,
+		db, "lnd", false, macaroons.IPLockChecker,
 	)
 	if err != nil {
 		t.Fatalf("Error creating new service: %v", err)
@@ -117,11 +116,10 @@ func TestNewService(t *testing.T) {
 // incoming context.
 func TestValidateMacaroon(t *testing.T) {
 	// First, initialize the service and unlock it.
-	tempDir := setupTestRootKeyStorage(t)
+	tempDir, db := setupTestRootKeyStorage(t)
 	defer os.RemoveAll(tempDir)
 	service, err := macaroons.NewService(
-		tempDir, "lnd", false, kvdb.DefaultDBTimeout,
-		macaroons.IPLockChecker,
+		db, "lnd", false, macaroons.IPLockChecker,
 	)
 	if err != nil {
 		t.Fatalf("Error creating new service: %v", err)
@@ -175,14 +173,13 @@ func TestValidateMacaroon(t *testing.T) {
 func TestListMacaroonIDs(t *testing.T) {
 	// First, initialize a dummy DB file with a store that the service
 	// can read from. Make sure the file is removed in the end.
-	tempDir := setupTestRootKeyStorage(t)
+	tempDir, db := setupTestRootKeyStorage(t)
 	defer os.RemoveAll(tempDir)
 
 	// 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(
-		tempDir, "lnd", false, kvdb.DefaultDBTimeout,
-		macaroons.IPLockChecker,
+		db, "lnd", false, macaroons.IPLockChecker,
 	)
 	require.NoError(t, err, "Error creating new service")
 	defer service.Close()
@@ -208,14 +205,13 @@ func TestDeleteMacaroonID(t *testing.T) {
 
 	// First, initialize a dummy DB file with a store that the service
 	// can read from. Make sure the file is removed in the end.
-	tempDir := setupTestRootKeyStorage(t)
+	tempDir, db := setupTestRootKeyStorage(t)
 	defer os.RemoveAll(tempDir)
 
 	// 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(
-		tempDir, "lnd", false, kvdb.DefaultDBTimeout,
-		macaroons.IPLockChecker,
+		db, "lnd", false, macaroons.IPLockChecker,
 	)
 	require.NoError(t, err, "Error creating new service")
 	defer service.Close()

macaroons/store.go

@@ -66,7 +66,6 @@ type RootKeyStorage struct {
 }
 
 // NewRootKeyStorage creates a RootKeyStorage instance.
-// TODO(aakselrod): Add support for encryption of data with passphrase.
 func NewRootKeyStorage(db kvdb.Backend) (*RootKeyStorage, error) {
 	// If the store's bucket doesn't exist, create it.
 	err := kvdb.Update(db, func(tx kvdb.RwTx) error {
@@ -78,7 +77,10 @@ func NewRootKeyStorage(db kvdb.Backend) (*RootKeyStorage, error) {
 	}
 
 	// Return the DB wrapped in a RootKeyStorage object.
-	return &RootKeyStorage{Backend: db, encKey: nil}, nil
+	return &RootKeyStorage{
+		Backend: db,
+		encKey:  nil,
+	}, nil
 }
 
 // CreateUnlock sets an encryption key if one is not already set, otherwise it
@@ -97,7 +99,7 @@ func (r *RootKeyStorage) CreateUnlock(password *[]byte) error {
 		return ErrPasswordRequired
 	}
 
-	return kvdb.Update(r, func(tx kvdb.RwTx) error {
+	return kvdb.Update(r.Backend, func(tx kvdb.RwTx) error {
 		bucket := tx.ReadWriteBucket(rootKeyBucketName)
 		if bucket == nil {
 			return ErrRootKeyBucketNotFound
@@ -153,7 +155,7 @@ func (r *RootKeyStorage) ChangePassword(oldPw, newPw []byte) error {
 		return ErrPasswordRequired
 	}
 
-	return kvdb.Update(r, func(tx kvdb.RwTx) error {
+	return kvdb.Update(r.Backend, func(tx kvdb.RwTx) error {
 		bucket := tx.ReadWriteBucket(rootKeyBucketName)
 		if bucket == nil {
 			return ErrRootKeyBucketNotFound
@@ -225,7 +227,7 @@ func (r *RootKeyStorage) Get(_ context.Context, id []byte) ([]byte, error) {
 		return nil, ErrStoreLocked
 	}
 	var rootKey []byte
-	err := kvdb.View(r, func(tx kvdb.RTx) error {
+	err := kvdb.View(r.Backend, func(tx kvdb.RTx) error {
 		bucket := tx.ReadBucket(rootKeyBucketName)
 		if bucket == nil {
 			return ErrRootKeyBucketNotFound
@@ -276,7 +278,7 @@ func (r *RootKeyStorage) RootKey(ctx context.Context) ([]byte, []byte, error) {
 		return nil, nil, ErrKeyValueForbidden
 	}
 
-	err = kvdb.Update(r, func(tx kvdb.RwTx) error {
+	err = kvdb.Update(r.Backend, func(tx kvdb.RwTx) error {
 		bucket := tx.ReadWriteBucket(rootKeyBucketName)
 		if bucket == nil {
 			return ErrRootKeyBucketNotFound
@@ -319,7 +321,7 @@ func (r *RootKeyStorage) GenerateNewRootKey() error {
 	if r.encKey == nil {
 		return ErrStoreLocked
 	}
-	return kvdb.Update(r, func(tx kvdb.RwTx) error {
+	return kvdb.Update(r.Backend, func(tx kvdb.RwTx) error {
 		bucket := tx.ReadWriteBucket(rootKeyBucketName)
 		if bucket == nil {
 			return ErrRootKeyBucketNotFound
@@ -341,7 +343,13 @@ func (r *RootKeyStorage) Close() error {
 		r.encKey.Zero()
 		r.encKey = nil
 	}
-	return r.Backend.Close()
+
+	// Since we're not responsible for _creating_ the connection to our DB
+	// backend, we also shouldn't close it. This should be handled
+	// externally as to not interfere with remote DB connections in case we
+	// need to open/close the store twice as happens in the password change
+	// case.
+	return nil
 }
 
 // generateAndStoreNewRootKey creates a new random RootKeyLen-byte root key,
@@ -377,7 +385,7 @@ func (r *RootKeyStorage) ListMacaroonIDs(_ context.Context) ([][]byte, error) {
 
 	// Read all the items in the bucket and append the keys, which are the
 	// root key IDs we want.
-	err := kvdb.View(r, func(tx kvdb.RTx) error {
+	err := kvdb.View(r.Backend, func(tx kvdb.RTx) error {
 
 		// appendRootKey is a function closure that appends root key ID
 		// to rootKeySlice.
@@ -426,7 +434,7 @@ func (r *RootKeyStorage) DeleteMacaroonID(
 	}
 
 	var rootKeyIDDeleted []byte
-	err := kvdb.Update(r, func(tx kvdb.RwTx) error {
+	err := kvdb.Update(r.Backend, func(tx kvdb.RwTx) error {
 		bucket := tx.ReadWriteBucket(rootKeyBucketName)
 
 		// Check the key can be found. If not, return nil.

macaroons/store_test.go

@@ -54,6 +54,7 @@ func openTestStore(t *testing.T, tempDir string) (func(),
 
 	cleanup := func() {
 		_ = store.Close()
+		_ = db.Close()
 	}
 
 	return cleanup, store
@@ -108,6 +109,7 @@ func TestStore(t *testing.T) {
 	require.Equal(t, macaroons.ErrAlreadyUnlocked, err)
 
 	_ = store.Close()
+	_ = store.Backend.Close()
 
 	// Between here and the re-opening of the store, it's possible to get
 	// a double-close, but that's not such a big deal since the tests will
@@ -206,6 +208,8 @@ func TestStoreChangePassword(t *testing.T) {
 	// after closing.
 	err = store.Close()
 	require.NoError(t, err)
+	err = store.Backend.Close()
+	require.NoError(t, err)
 
 	err = store.CreateUnlock(&newPw)
 	require.Error(t, err)