From f362f7670b584c113e7d8d93e133448b465d12b8 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 24 Jul 2020 00:26:59 +0800 Subject: [PATCH] macaroons: specify root key ID in bakery --- macaroons/README.md | 20 ++++++- macaroons/context.go | 44 +++++++++++++++ macaroons/service.go | 43 ++++++++++++++ macaroons/service_test.go | 95 +++++++++++++++++++++++++++++-- macaroons/store.go | 114 +++++++++++++++++++++++++++++++++++--- macaroons/store_test.go | 46 ++++++++++++--- 6 files changed, 341 insertions(+), 21 deletions(-) create mode 100644 macaroons/context.go diff --git a/macaroons/README.md b/macaroons/README.md index 67ef9ab4..8e379c0a 100644 --- a/macaroons/README.md +++ b/macaroons/README.md @@ -95,8 +95,8 @@ command line. Users can create their own macaroons with custom permissions if the provided default macaroons (`admin`, `invoice` and `readonly`) are not sufficient. -For example, a macaroon that is only allowed to manage peers would be created -with the following command: +For example, a macaroon that is only allowed to manage peers with a default root +key `0` would be created with the following command: `lncli bakemacaroon peers:read peers:write` @@ -114,3 +114,19 @@ removing all three default macaroons (`admin.macaroon`, `invoice.macaroon` and `readonly.macaroon`, **NOT** the `macaroons.db`!) from their `data/chain///` directory inside the lnd data directory and restarting lnd. + + +## Root key rotation + +To manage the root keys used by macaroons, there are `listmacaroonids` and +`deletemacaroonid` available through gPRC and command line. +Users can view a list of all macaroon root key IDs that are in use using: + +`lncli listmacaroonids` + +And remove a specific macaroon root key ID using command: + +`lncli deletemacaroonid root_key_id` + +Be careful with the `deletemacaroonid` command as when a root key is deleted, +**all the macaroons created from it are invalidated**. \ No newline at end of file diff --git a/macaroons/context.go b/macaroons/context.go new file mode 100644 index 00000000..c73b0841 --- /dev/null +++ b/macaroons/context.go @@ -0,0 +1,44 @@ +package macaroons + +import ( + "context" + "fmt" +) + +var ( + // RootKeyIDContextKey is the key to get rootKeyID from context. + RootKeyIDContextKey = contextKey{"rootkeyid"} + + // ErrContextRootKeyID is used when the supplied context doesn't have + // a root key ID. + ErrContextRootKeyID = fmt.Errorf("failed to read root key ID " + + "from context") +) + +// contextKey is the type we use to identify values in the context. +type contextKey struct { + Name string +} + +// ContextWithRootKeyID passes the root key ID value to context. +func ContextWithRootKeyID(ctx context.Context, + value interface{}) context.Context { + + return context.WithValue(ctx, RootKeyIDContextKey, value) +} + +// RootKeyIDFromContext retrieves the root key ID from context using the key +// RootKeyIDContextKey. +func RootKeyIDFromContext(ctx context.Context) ([]byte, error) { + id, ok := ctx.Value(RootKeyIDContextKey).([]byte) + if !ok { + return nil, ErrContextRootKeyID + } + + // Check that the id is not empty. + if len(id) == 0 { + return nil, ErrMissingRootKeyID + } + + return id, nil +} diff --git a/macaroons/service.go b/macaroons/service.go index 825c6435..823ef0fc 100644 --- a/macaroons/service.go +++ b/macaroons/service.go @@ -20,6 +20,13 @@ 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") + + // ErrDeletionForbidden is used when attempting to delete the + // DefaultRootKeyID or the encryptedKeyID. + ErrDeletionForbidden = fmt.Errorf("the specified ID cannot be deleted") ) // Service encapsulates bakery.Bakery and adds a Close() method that zeroes the @@ -197,3 +204,39 @@ func (svc *Service) Close() error { func (svc *Service) CreateUnlock(password *[]byte) error { return svc.rks.CreateUnlock(password) } + +// NewMacaroon wraps around the function Oven.NewMacaroon with the defaults, +// - version is always bakery.LatestVersion; +// - caveats is always nil. +// In addition, it takes a rootKeyID parameter, and puts it into the context. +// The context is passed through Oven.NewMacaroon(), in which calls the function +// RootKey(), that reads the context for rootKeyID. +func (svc *Service) NewMacaroon( + ctx context.Context, rootKeyID []byte, + ops ...bakery.Op) (*bakery.Macaroon, error) { + + // Check rootKeyID is not called with nil or empty bytes. We want the + // caller to be aware the value of root key ID used, so we won't replace + // it with the DefaultRootKeyID if not specified. + if len(rootKeyID) == 0 { + return nil, ErrMissingRootKeyID + } + + // // Pass the root key ID to context. + ctx = ContextWithRootKeyID(ctx, rootKeyID) + + return svc.Oven.NewMacaroon(ctx, bakery.LatestVersion, nil, ops...) +} + +// 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) +} + +// DeleteMacaroonID removes one specific root key ID. If the root key ID is +// found and deleted, it will be returned. +func (svc *Service) DeleteMacaroonID(ctxt context.Context, + rootKeyID []byte) ([]byte, error) { + return svc.rks.DeleteMacaroonID(ctxt, rootKeyID) +} diff --git a/macaroons/service_test.go b/macaroons/service_test.go index dd90ad5f..32f2327c 100644 --- a/macaroons/service_test.go +++ b/macaroons/service_test.go @@ -10,6 +10,7 @@ import ( "github.com/lightningnetwork/lnd/channeldb/kvdb" "github.com/lightningnetwork/lnd/macaroons" + "github.com/stretchr/testify/require" "google.golang.org/grpc/metadata" "gopkg.in/macaroon-bakery.v2/bakery" "gopkg.in/macaroon-bakery.v2/bakery/checkers" @@ -72,8 +73,13 @@ func TestNewService(t *testing.T) { } // Third, check if the created service can bake macaroons. - macaroon, err := service.Oven.NewMacaroon( - context.TODO(), bakery.LatestVersion, nil, testOperation, + _, err = service.NewMacaroon(context.TODO(), nil, testOperation) + if err != macaroons.ErrMissingRootKeyID { + t.Fatalf("Received %v instead of ErrMissingRootKeyID", err) + } + + macaroon, err := service.NewMacaroon( + context.TODO(), macaroons.DefaultRootKeyID, testOperation, ) if err != nil { t.Fatalf("Error creating macaroon from service: %v", err) @@ -117,8 +123,8 @@ func TestValidateMacaroon(t *testing.T) { } // Then, create a new macaroon that we can serialize. - macaroon, err := service.Oven.NewMacaroon( - context.TODO(), bakery.LatestVersion, nil, testOperation, + macaroon, err := service.NewMacaroon( + context.TODO(), macaroons.DefaultRootKeyID, testOperation, ) if err != nil { t.Fatalf("Error creating macaroon from service: %v", err) @@ -141,3 +147,84 @@ func TestValidateMacaroon(t *testing.T) { t.Fatalf("Error validating the macaroon: %v", err) } } + +// TestListMacaroonIDs checks that ListMacaroonIDs returns the expected result. +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) + 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, macaroons.IPLockChecker) + require.NoError(t, err, "Error creating new service") + defer service.Close() + + err = service.CreateUnlock(&defaultPw) + require.NoError(t, err, "Error unlocking root key storage") + + // Third, make 3 new macaroons with different root key IDs. + expectedIDs := [][]byte{{1}, {2}, {3}} + for _, v := range expectedIDs { + _, err := service.NewMacaroon(context.TODO(), v, testOperation) + require.NoError(t, err, "Error creating macaroon from service") + } + + // Finally, check that calling List return the expected values. + ids, _ := service.ListMacaroonIDs(context.TODO()) + require.Equal(t, expectedIDs, ids, "root key IDs mismatch") +} + +// TestDeleteMacaroonID removes the specific root key ID. +func TestDeleteMacaroonID(t *testing.T) { + ctxb := context.Background() + + // 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) + 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, macaroons.IPLockChecker) + require.NoError(t, err, "Error creating new service") + defer service.Close() + + err = service.CreateUnlock(&defaultPw) + require.NoError(t, err, "Error unlocking root key storage") + + // Third, checks that removing encryptedKeyID returns an error. + encryptedKeyID := []byte("enckey") + _, err = service.DeleteMacaroonID(ctxb, encryptedKeyID) + require.Equal(t, macaroons.ErrDeletionForbidden, err) + + // Fourth, checks that removing DefaultKeyID returns an error. + _, err = service.DeleteMacaroonID(ctxb, macaroons.DefaultRootKeyID) + require.Equal(t, macaroons.ErrDeletionForbidden, err) + + // Fifth, checks that removing empty key id returns an error. + _, err = service.DeleteMacaroonID(ctxb, []byte{}) + require.Equal(t, macaroons.ErrMissingRootKeyID, err) + + // Sixth, checks that removing a non-existed key id returns nil. + nonExistedID := []byte("test-non-existed") + deletedID, err := service.DeleteMacaroonID(ctxb, nonExistedID) + require.NoError(t, err, "deleting macaroon ID got an error") + require.Nil(t, deletedID, "deleting non-existed ID should return nil") + + // Seventh, make 3 new macaroons with different root key IDs, and delete + // one. + expectedIDs := [][]byte{{1}, {2}, {3}} + for _, v := range expectedIDs { + _, err := service.NewMacaroon(ctxb, v, testOperation) + require.NoError(t, err, "Error creating macaroon from service") + } + deletedID, err = service.DeleteMacaroonID(ctxb, expectedIDs[0]) + require.NoError(t, err, "deleting macaroon ID got an error") + + // Finally, check that the ID is deleted. + require.Equal(t, expectedIDs[0], deletedID, "expected ID to be removed") + ids, _ := service.ListMacaroonIDs(ctxb) + require.Equal(t, expectedIDs[1:], ids, "root key IDs mismatch") +} diff --git a/macaroons/store.go b/macaroons/store.go index 68d3c8b6..97baae49 100644 --- a/macaroons/store.go +++ b/macaroons/store.go @@ -1,6 +1,7 @@ package macaroons import ( + "bytes" "context" "crypto/rand" "fmt" @@ -21,11 +22,9 @@ var ( // rootKeyBucketName is the name of the root key store bucket. rootKeyBucketName = []byte("macrootkeys") - // defaultRootKeyID is the ID of the default root key. The first is + // DefaultRootKeyID is the ID of the default root key. The first is // just 0, to emulate the memory storage that comes with bakery. - // - // TODO(aakselrod): Add support for key rotation. - defaultRootKeyID = []byte("0") + DefaultRootKeyID = []byte("0") // encryptedKeyID is the name of the database key that stores the // encryption key, encrypted with a salted + hashed password. The @@ -42,6 +41,10 @@ var ( // ErrPasswordRequired specifies that a nil password has been passed. ErrPasswordRequired = fmt.Errorf("a non-nil password is required") + + // ErrKeyValueForbidden is used when the root key ID uses encryptedKeyID as + // its value. + ErrKeyValueForbidden = fmt.Errorf("root key ID value is not allowed") ) // RootKeyStorage implements the bakery.RootKeyStorage interface. @@ -157,8 +160,7 @@ func (r *RootKeyStorage) Get(_ context.Context, id []byte) ([]byte, error) { // RootKey implements the RootKey method for the bakery.RootKeyStorage // interface. -// TODO(aakselrod): Add support for key rotation. -func (r *RootKeyStorage) RootKey(_ context.Context) ([]byte, []byte, error) { +func (r *RootKeyStorage) RootKey(ctx context.Context) ([]byte, []byte, error) { r.encKeyMtx.RLock() defer r.encKeyMtx.RUnlock() @@ -166,8 +168,19 @@ func (r *RootKeyStorage) RootKey(_ context.Context) ([]byte, []byte, error) { return nil, nil, ErrStoreLocked } var rootKey []byte - id := defaultRootKeyID - err := kvdb.Update(r, func(tx kvdb.RwTx) error { + + // Read the root key ID from the context. If no key is specified in the + // context, an error will be returned. + id, err := RootKeyIDFromContext(ctx) + if err != nil { + return nil, nil, err + } + + if bytes.Equal(id, encryptedKeyID) { + return nil, nil, ErrKeyValueForbidden + } + + err = kvdb.Update(r, func(tx kvdb.RwTx) error { ns := tx.ReadWriteBucket(rootKeyBucketName) dbKey := ns.Get(id) @@ -215,3 +228,88 @@ func (r *RootKeyStorage) Close() error { } return r.Backend.Close() } + +// ListMacaroonIDs returns all the root key ID values except the value of +// encryptedKeyID. +func (r *RootKeyStorage) ListMacaroonIDs(_ context.Context) ([][]byte, error) { + r.encKeyMtx.RLock() + defer r.encKeyMtx.RUnlock() + + // Check it's unlocked. + if r.encKey == nil { + return nil, ErrStoreLocked + } + + var rootKeySlice [][]byte + + // 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 { + + // appendRootKey is a function closure that appends root key ID + // to rootKeySlice. + appendRootKey := func(k, _ []byte) error { + // Only append when the key value is not encryptedKeyID. + if !bytes.Equal(k, encryptedKeyID) { + rootKeySlice = append(rootKeySlice, k) + } + return nil + } + + return tx.ReadBucket(rootKeyBucketName).ForEach(appendRootKey) + }) + if err != nil { + return nil, err + } + + return rootKeySlice, nil +} + +// DeleteMacaroonID removes one specific root key ID. If the root key ID is +// found and deleted, it will be returned. +func (r *RootKeyStorage) DeleteMacaroonID( + _ context.Context, rootKeyID []byte) ([]byte, error) { + + r.encKeyMtx.RLock() + defer r.encKeyMtx.RUnlock() + + // Check it's unlocked. + if r.encKey == nil { + return nil, ErrStoreLocked + } + + // Check the rootKeyID is not empty. + if len(rootKeyID) == 0 { + return nil, ErrMissingRootKeyID + } + + // Deleting encryptedKeyID or DefaultRootKeyID is not allowed. + if bytes.Equal(rootKeyID, encryptedKeyID) || + bytes.Equal(rootKeyID, DefaultRootKeyID) { + + return nil, ErrDeletionForbidden + } + + var rootKeyIDDeleted []byte + err := kvdb.Update(r, func(tx kvdb.RwTx) error { + bucket := tx.ReadWriteBucket(rootKeyBucketName) + + // Check the key can be found. If not, return nil. + if bucket.Get(rootKeyID) == nil { + return nil + } + + // Once the key is found, we do the deletion. + if err := bucket.Delete(rootKeyID); err != nil { + return err + } + rootKeyIDDeleted = rootKeyID + + return nil + }) + if err != nil { + return nil, err + } + + return rootKeyIDDeleted, nil +} diff --git a/macaroons/store_test.go b/macaroons/store_test.go index b6898df6..2bf18010 100644 --- a/macaroons/store_test.go +++ b/macaroons/store_test.go @@ -51,13 +51,45 @@ func TestStore(t *testing.T) { t.Fatalf("Error creating store encryption key: %v", err) } - key, id, err := store.RootKey(context.TODO()) + // Check ErrContextRootKeyID is returned when no root key ID found in + // context. + _, _, err = store.RootKey(context.TODO()) + if err != macaroons.ErrContextRootKeyID { + t.Fatalf("Received %v instead of ErrContextRootKeyID", err) + } + + // Check ErrMissingRootKeyID is returned when empty root key ID is used. + emptyKeyID := []byte{} + badCtx := macaroons.ContextWithRootKeyID(context.TODO(), emptyKeyID) + _, _, err = store.RootKey(badCtx) + if err != macaroons.ErrMissingRootKeyID { + t.Fatalf("Received %v instead of ErrMissingRootKeyID", err) + } + + // Create a context with illegal root key ID value. + encryptedKeyID := []byte("enckey") + badCtx = macaroons.ContextWithRootKeyID(context.TODO(), encryptedKeyID) + _, _, err = store.RootKey(badCtx) + if err != macaroons.ErrKeyValueForbidden { + t.Fatalf("Received %v instead of ErrKeyValueForbidden", err) + } + + // Create a context with root key ID value. + ctx := macaroons.ContextWithRootKeyID( + context.TODO(), macaroons.DefaultRootKeyID, + ) + key, id, err := store.RootKey(ctx) if err != nil { t.Fatalf("Error getting root key from store: %v", err) } - rootID := id - key2, err := store.Get(context.TODO(), id) + rootID := id + if !bytes.Equal(rootID, macaroons.DefaultRootKeyID) { + t.Fatalf("Root key ID doesn't match: expected %v, got %v", + macaroons.DefaultRootKeyID, rootID) + } + + key2, err := store.Get(ctx, id) if err != nil { t.Fatalf("Error getting key with ID %s: %v", string(id), err) } @@ -100,12 +132,12 @@ func TestStore(t *testing.T) { t.Fatalf("Received %v instead of ErrPasswordRequired", err) } - _, _, err = store.RootKey(context.TODO()) + _, _, err = store.RootKey(ctx) if err != macaroons.ErrStoreLocked { t.Fatalf("Received %v instead of ErrStoreLocked", err) } - _, err = store.Get(context.TODO(), nil) + _, err = store.Get(ctx, nil) if err != macaroons.ErrStoreLocked { t.Fatalf("Received %v instead of ErrStoreLocked", err) } @@ -115,7 +147,7 @@ func TestStore(t *testing.T) { t.Fatalf("Error unlocking root key store: %v", err) } - key, err = store.Get(context.TODO(), rootID) + key, err = store.Get(ctx, rootID) if err != nil { t.Fatalf("Error getting key with ID %s: %v", string(rootID), err) @@ -125,7 +157,7 @@ func TestStore(t *testing.T) { key2, key) } - key, id, err = store.RootKey(context.TODO()) + key, id, err = store.RootKey(ctx) if err != nil { t.Fatalf("Error getting root key from store: %v", err) }