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.

Author: ellemouton

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)
-		if err != nil {
-			return err
-		}
-		rootKey := make([]byte, len(decryptedKey))
-		copy(rootKey, decryptedKey)
-		encryptedKey, err := encKeyNew.Encrypt(rootKey)
+		// 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
 		}
-		err = bucket.Put(DefaultRootKeyID, encryptedKey)
-		if err != nil {
-			return err
+
+		if !foundDefaultRootKey {
+			return ErrDefaultRootKeyNotFound
 		}
 
 		// Finally, store the new encryption key parameters in the DB

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,16 +219,15 @@ 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)
 
 	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)
 }