From f9d99153e04cfa2fead593f57f0278fe2cf6f784 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 17 May 2023 11:47:43 +0200 Subject: [PATCH] macaroons: let ChangePassword re-encrypt all root keys The ChangePasswords method should re-encrypt all the root keys found in the store, not just the default root key. --- macaroons/store.go | 64 ++++++++++++++++++++++++++++------------- macaroons/store_test.go | 22 ++++++-------- 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/macaroons/store.go b/macaroons/store.go index 7b40dff30..cca548d98 100644 --- a/macaroons/store.go +++ b/macaroons/store.go @@ -54,6 +54,10 @@ var ( // ErrEncKeyNotFound specifies that there was no encryption key found // even if one was expected to be generated. ErrEncKeyNotFound = fmt.Errorf("macaroon encryption key not found") + + // ErrDefaultRootKeyNotFound is returned when the default root key is + // not found in the DB when it is expected to be. + ErrDefaultRootKeyNotFound = fmt.Errorf("default root key not found") ) // RootKeyStorage implements the bakery.RootKeyStorage interface. @@ -140,8 +144,8 @@ func (r *RootKeyStorage) CreateUnlock(password *[]byte) error { }, func() {}) } -// ChangePassword decrypts the macaroon root key with the old password and then -// encrypts it again with the new password. +// ChangePassword decrypts all the macaroon root keys with the old password and +// then encrypts them again with the new password. func (r *RootKeyStorage) ChangePassword(oldPw, newPw []byte) error { // We need the store to already be unlocked. With this we can make sure // that there already is a key in the DB. @@ -159,19 +163,18 @@ func (r *RootKeyStorage) ChangePassword(oldPw, newPw []byte) error { if bucket == nil { return ErrRootKeyBucketNotFound } - encKeyDb := bucket.Get(encryptionKeyID) - rootKeyDb := bucket.Get(DefaultRootKeyID) - // Both the encryption key and the root key must be present - // otherwise we are in the wrong state to change the password. - if len(encKeyDb) == 0 || len(rootKeyDb) == 0 { + // The encryption key must be present, otherwise we are in the + // wrong state to change the password. + encKeyDB := bucket.Get(encryptionKeyID) + if len(encKeyDB) == 0 { return ErrEncKeyNotFound } // Unmarshal parameters for old encryption key and derive the // old key with them. encKeyOld := &snacl.SecretKey{} - err := encKeyOld.Unmarshal(encKeyDb) + err := encKeyOld.Unmarshal(encKeyDB) if err != nil { return err } @@ -188,21 +191,42 @@ func (r *RootKeyStorage) ChangePassword(oldPw, newPw []byte) error { return err } - // Now try to decrypt the root key with the old encryption key, - // encrypt it with the new one and then store it in the DB. - decryptedKey, err := encKeyOld.Decrypt(rootKeyDb) + // foundDefaultRootKey is used to keep track of if we have + // found and re-encrypted the default root key so that we can + // return an error if it is not found. + var foundDefaultRootKey bool + err = bucket.ForEach(func(k, v []byte) error { + // Skip the key if it is the encryption key ID since + // we do not want to re-encrypt this. + if bytes.Equal(k, encryptionKeyID) { + return nil + } + + if bytes.Equal(k, DefaultRootKeyID) { + foundDefaultRootKey = true + } + + // Now try to decrypt the root key with the old + // encryption key, encrypt it with the new one and then + // store it in the DB. + decryptedKey, err := encKeyOld.Decrypt(v) + if err != nil { + return err + } + + encryptedKey, err := encKeyNew.Encrypt(decryptedKey) + if err != nil { + return err + } + + return bucket.Put(k, encryptedKey) + }) if err != nil { return err } - rootKey := make([]byte, len(decryptedKey)) - copy(rootKey, decryptedKey) - encryptedKey, err := encKeyNew.Encrypt(rootKey) - if err != nil { - return err - } - err = bucket.Put(DefaultRootKeyID, encryptedKey) - if err != nil { - return err + + if !foundDefaultRootKey { + return ErrDefaultRootKeyNotFound } // Finally, store the new encryption key parameters in the DB diff --git a/macaroons/store_test.go b/macaroons/store_test.go index 39cff7273..ace1e764a 100644 --- a/macaroons/store_test.go +++ b/macaroons/store_test.go @@ -209,10 +209,7 @@ func TestStoreSetRootKey(t *testing.T) { } // TestStoreChangePassword tests that the password for the store can be changed -// without changing the root key. The test also demonstrates that currently, -// this change is only applied to the root key at the default root key ID -// location and not to other root keys. This will be fixed in an upcoming -// commit. +// without changing the root keys. func TestStoreChangePassword(t *testing.T) { tempDir, store := newTestStore(t) @@ -222,8 +219,7 @@ func TestStoreChangePassword(t *testing.T) { // Unlock the DB and read the current default root key and one other // non-default root key. Both of these should stay the same after - // changing the password but currently only the default root key is - // re-encrypted correclty. + // changing the password for the test to succeed. pw := []byte("weks") err = store.CreateUnlock(&pw) require.NoError(t, err) @@ -231,7 +227,7 @@ func TestStoreChangePassword(t *testing.T) { rootKey1, _, err := store.RootKey(defaultRootKeyIDContext) require.NoError(t, err) - _, _, err = store.RootKey(nonDefaultRootKeyIDContext) + rootKey2, _, err := store.RootKey(nonDefaultRootKeyIDContext) require.NoError(t, err) // Both passwords must be set. @@ -266,12 +262,12 @@ func TestStoreChangePassword(t *testing.T) { require.NoError(t, err) // Finally, read the root keys from the DB using the new password and - // make sure the default root key stayed the same but that the - // non-default root key could not be decrypted. - rootKeyDb, _, err := store.RootKey(defaultRootKeyIDContext) + // make sure that both root keys stayed the same. + rootKeyDB1, _, err := store.RootKey(defaultRootKeyIDContext) require.NoError(t, err) - require.Equal(t, rootKey1, rootKeyDb) + require.Equal(t, rootKey1, rootKeyDB1) - _, _, err = store.RootKey(nonDefaultRootKeyIDContext) - require.ErrorContains(t, err, "unable to decrypt") + rootKeyDB2, _, err := store.RootKey(nonDefaultRootKeyIDContext) + require.NoError(t, err) + require.Equal(t, rootKey2, rootKeyDB2) }