Encryption: Fix multiple data keys migration (#49848)

* Add migration

* Migrator: Extend support to rename columns

* Fix getting current key

* Fix column name in migration

* Fix deks reencryption

* Fix caching

* Add back separate caches for byName and byPrefix

* Do not concatenate prefix with uid

* Rename DataKey struc fields

* SQLStore: Add deprecation comments for breaking migrations

* Add comment

* Minor corrections

Co-authored-by: Joan López de la Franca Beltran <joanjan14@gmail.com>

pkg/services/datasources/datasources.go

@@ -40,15 +40,15 @@ type DataSourceService interface {
 	DecryptedValues(ctx context.Context, ds *models.DataSource) (map[string]string, error)
 
 	// DecryptedValue decrypts the encrypted datasource secureJSONData identified by key
-	// and returns the decryped value.
+	// and returns the decrypted value.
 	DecryptedValue(ctx context.Context, ds *models.DataSource, key string) (string, bool, error)
 
 	// DecryptedBasicAuthPassword decrypts the encrypted datasource basic authentication
-	// password and returns the decryped value.
+	// password and returns the decrypted value.
 	DecryptedBasicAuthPassword(ctx context.Context, ds *models.DataSource) (string, error)
 
 	// DecryptedPassword decrypts the encrypted datasource password and returns the
-	// decryped value.
+	// decrypted value.
 	DecryptedPassword(ctx context.Context, ds *models.DataSource) (string, error)
 }
 

pkg/services/secrets/database/database.go

@@ -33,7 +33,7 @@ func (ss *SecretsStoreImpl) GetDataKey(ctx context.Context, id string) (*secrets
 	err := ss.sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
 		var err error
 		exists, err = sess.Table(dataKeysTable).
-			Where("id = ?", id).
+			Where("name = ?", id).
 			Get(dataKey)
 		return err
 	})
@@ -49,14 +49,14 @@ func (ss *SecretsStoreImpl) GetDataKey(ctx context.Context, id string) (*secrets
 	return dataKey, nil
 }
 
-func (ss *SecretsStoreImpl) GetCurrentDataKey(ctx context.Context, name string) (*secrets.DataKey, error) {
+func (ss *SecretsStoreImpl) GetCurrentDataKey(ctx context.Context, label string) (*secrets.DataKey, error) {
 	dataKey := &secrets.DataKey{}
 	var exists bool
 
 	err := ss.sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
 		var err error
 		exists, err = sess.Table(dataKeysTable).
-			Where("name = ? AND active = ?", name, ss.sqlStore.Dialect.BooleanStr(true)).
+			Where("label = ? AND active = ?", label, ss.sqlStore.Dialect.BooleanStr(true)).
 			Get(dataKey)
 		return err
 	})
@@ -66,7 +66,7 @@ func (ss *SecretsStoreImpl) GetCurrentDataKey(ctx context.Context, name string)
 	}
 
 	if err != nil {
-		return nil, fmt.Errorf("failed getting data key: %w", err)
+		return nil, fmt.Errorf("failed getting current data key: %w", err)
 	}
 
 	return dataKey, nil
@@ -137,7 +137,7 @@ func (ss *SecretsStoreImpl) ReEncryptDataKeys(
 				ss.log.Warn(
 					"Could not find provider to re-encrypt data encryption key",
 					"id", k.Id,
-					"name", k.Name,
+					"label", k.Label,
 					"provider", k.Provider,
 				)
 				return nil
@@ -148,7 +148,7 @@ func (ss *SecretsStoreImpl) ReEncryptDataKeys(
 				ss.log.Warn(
 					"Error while decrypting data encryption key to re-encrypt it",
 					"id", k.Id,
-					"name", k.Name,
+					"label", k.Label,
 					"provider", k.Provider,
 					"err", err,
 				)
@@ -158,25 +158,25 @@ func (ss *SecretsStoreImpl) ReEncryptDataKeys(
 			// Updating current data key by re-encrypting it with current provider.
 			// Accessing the current provider within providers map should be safe.
 			k.Provider = currProvider
-			k.Name = secrets.KeyName(k.Scope, currProvider)
+			k.Label = secrets.KeyLabel(k.Scope, currProvider)
 			k.Updated = time.Now()
 			k.EncryptedData, err = providers[currProvider].Encrypt(ctx, decrypted)
 			if err != nil {
 				ss.log.Warn(
 					"Error while re-encrypting data encryption key",
 					"id", k.Id,
-					"name", k.Name,
+					"label", k.Label,
 					"provider", k.Provider,
 					"err", err,
 				)
 				return nil
 			}
 
-			if _, err := sess.Table(dataKeysTable).Where("id = ?", k.Id).Update(k); err != nil {
+			if _, err := sess.Table(dataKeysTable).Where("name = ?", k.Id).Update(k); err != nil {
 				ss.log.Warn(
 					"Error while re-encrypting data encryption key",
 					"id", k.Id,
-					"name", k.Name,
+					"label", k.Label,
 					"provider", k.Provider,
 					"err", err,
 				)

pkg/services/secrets/fakes/fake_store.go

@@ -24,9 +24,9 @@ func (f FakeSecretsStore) GetDataKey(_ context.Context, id string) (*secrets.Dat
 	return key, nil
 }
 
-func (f FakeSecretsStore) GetCurrentDataKey(_ context.Context, name string) (*secrets.DataKey, error) {
+func (f FakeSecretsStore) GetCurrentDataKey(_ context.Context, label string) (*secrets.DataKey, error) {
 	for _, key := range f.store {
-		if key.Name == name && key.Active {
+		if key.Label == label && key.Active {
 			return key, nil
 		}
 	}

pkg/services/secrets/manager/cache.go

@@ -14,8 +14,9 @@ var (
 
 type dataKeyCacheEntry struct {
 	id         string
-	name       string
+	label      string
 	dataKey    []byte
+	active     bool
 	expiration time.Time
 }
 
@@ -26,14 +27,14 @@ func (e dataKeyCacheEntry) expired() bool {
 type dataKeyCache struct {
 	mtx      sync.RWMutex
 	byId     map[string]*dataKeyCacheEntry
-	byName   map[string]*dataKeyCacheEntry
+	byLabel  map[string]*dataKeyCacheEntry
 	cacheTTL time.Duration
 }
 
 func newDataKeyCache(ttl time.Duration) *dataKeyCache {
 	return &dataKeyCache{
 		byId:     make(map[string]*dataKeyCacheEntry),
-		byName:   make(map[string]*dataKeyCacheEntry),
+		byLabel:  make(map[string]*dataKeyCacheEntry),
 		cacheTTL: ttl,
 	}
 }
@@ -56,15 +57,15 @@ func (c *dataKeyCache) getById(id string) (*dataKeyCacheEntry, bool) {
 	return entry, true
 }
 
-func (c *dataKeyCache) getByName(name string) (*dataKeyCacheEntry, bool) {
+func (c *dataKeyCache) getByLabel(label string) (*dataKeyCacheEntry, bool) {
 	c.mtx.RLock()
 	defer c.mtx.RUnlock()
 
-	entry, exists := c.byName[name]
+	entry, exists := c.byLabel[label]
 
 	cacheReadsCounter.With(prometheus.Labels{
 		"hit":    strconv.FormatBool(exists),
-		"method": "byName",
+		"method": "byLabel",
 	}).Inc()
 
 	if !exists || entry.expired() {
@@ -81,7 +82,7 @@ func (c *dataKeyCache) add(entry *dataKeyCacheEntry) {
 	entry.expiration = now().Add(c.cacheTTL)
 
 	c.byId[entry.id] = entry
-	c.byName[entry.name] = entry
+	c.byLabel[entry.label] = entry
 }
 
 func (c *dataKeyCache) removeExpired() {
@@ -94,9 +95,9 @@ func (c *dataKeyCache) removeExpired() {
 		}
 	}
 
-	for name, entry := range c.byName {
+	for label, entry := range c.byLabel {
 		if entry.expired() {
-			delete(c.byName, name)
+			delete(c.byLabel, label)
 		}
 	}
 }
@@ -104,6 +105,6 @@ func (c *dataKeyCache) removeExpired() {
 func (c *dataKeyCache) flush() {
 	c.mtx.Lock()
 	c.byId = make(map[string]*dataKeyCacheEntry)
-	c.byName = make(map[string]*dataKeyCacheEntry)
+	c.byLabel = make(map[string]*dataKeyCacheEntry)
 	c.mtx.Unlock()
 }

pkg/services/secrets/manager/manager.go

@@ -146,11 +146,13 @@ func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byt
 
 	// If encryption featuremgmt.FlagEnvelopeEncryption toggle is on, use envelope encryption
 	scope := opt()
-	name := secrets.KeyName(scope, s.currentProviderID)
+	label := secrets.KeyLabel(scope, s.currentProviderID)
 
-	id, dataKey, err := s.currentDataKey(ctx, name, scope, sess)
+	var id string
+	var dataKey []byte
+	id, dataKey, err = s.currentDataKey(ctx, label, scope, sess)
 	if err != nil {
-		s.log.Error("Failed to get current data key", "error", err, "name", name)
+		s.log.Error("Failed to get current data key", "error", err, "label", label)
 		return nil, err
 	}
 
@@ -176,21 +178,21 @@ func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byt
 // currentDataKey looks up for current data key in cache or database by name, and decrypts it.
 // If there's no current data key in cache nor in database it generates a new random data key,
 // and stores it into both the in-memory cache and database (encrypted by the encryption provider).
-func (s *SecretsService) currentDataKey(ctx context.Context, name string, scope string, sess *xorm.Session) (string, []byte, error) {
+func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope string, sess *xorm.Session) (string, []byte, error) {
 	// We want only one request fetching current data key at time to
 	// avoid the creation of multiple ones in case there's no one existing.
 	s.mtx.Lock()
 	defer s.mtx.Unlock()
 
 	// We try to fetch the data key, either from cache or database
-	id, dataKey, err := s.dataKeyByName(ctx, name)
+	id, dataKey, err := s.dataKeyByLabel(ctx, label)
 	if err != nil {
 		return "", nil, err
 	}
 
 	// If no existing data key was found, create a new one
 	if dataKey == nil {
-		id, dataKey, err = s.newDataKey(ctx, name, scope, sess)
+		id, dataKey, err = s.newDataKey(ctx, label, scope, sess)
 		if err != nil {
 			return "", nil, err
 		}
@@ -199,16 +201,16 @@ func (s *SecretsService) currentDataKey(ctx context.Context, name string, scope
 	return id, dataKey, nil
 }
 
-// dataKeyByName looks up for data key in cache.
+// dataKeyByLabel looks up for data key in cache.
 // Otherwise, it fetches it from database, decrypts it and caches it decrypted.
-func (s *SecretsService) dataKeyByName(ctx context.Context, name string) (string, []byte, error) {
+func (s *SecretsService) dataKeyByLabel(ctx context.Context, label string) (string, []byte, error) {
 	// 0. Get data key from in-memory cache.
-	if entry, exists := s.dataKeyCache.getByName(name); exists {
+	if entry, exists := s.dataKeyCache.getByLabel(label); exists && entry.active {
 		return entry.id, entry.dataKey, nil
 	}
 
 	// 1. Get data key from database.
-	dataKey, err := s.store.GetCurrentDataKey(ctx, name)
+	dataKey, err := s.store.GetCurrentDataKey(ctx, label)
 	if err != nil {
 		if errors.Is(err, secrets.ErrDataKeyNotFound) {
 			return "", nil, nil
@@ -229,13 +231,18 @@ func (s *SecretsService) dataKeyByName(ctx context.Context, name string) (string
 	}
 
 	// 3. Store the decrypted data key into the in-memory cache.
-	s.dataKeyCache.add(&dataKeyCacheEntry{id: dataKey.Id, name: dataKey.Name, dataKey: decrypted})
+	s.dataKeyCache.add(&dataKeyCacheEntry{
+		id:      dataKey.Id,
+		label:   dataKey.Label,
+		dataKey: decrypted,
+		active:  dataKey.Active,
+	})
 
 	return dataKey.Id, decrypted, nil
 }
 
 // newDataKey creates a new random data key, encrypts it and stores it into the database and cache.
-func (s *SecretsService) newDataKey(ctx context.Context, name string, scope string, sess *xorm.Session) (string, []byte, error) {
+func (s *SecretsService) newDataKey(ctx context.Context, label string, scope string, sess *xorm.Session) (string, []byte, error) {
 	// 1. Create new data key.
 	dataKey, err := newRandomDataKey()
 	if err != nil {
@@ -257,11 +264,11 @@ func (s *SecretsService) newDataKey(ctx context.Context, name string, scope stri
 	// 3. Store its encrypted value into the DB.
 	id := util.GenerateShortUID()
 	dbDataKey := secrets.DataKey{
-		Id:            id,
 		Active:        true,
-		Name:          name,
+		Id:            id,
 		Provider:      s.currentProviderID,
 		EncryptedData: encrypted,
+		Label:         label,
 		Scope:         scope,
 	}
 
@@ -276,7 +283,12 @@ func (s *SecretsService) newDataKey(ctx context.Context, name string, scope stri
 	}
 
 	// 4. Store the decrypted data key into the in-memory cache.
-	s.dataKeyCache.add(&dataKeyCacheEntry{id: id, name: name, dataKey: dataKey})
+	s.dataKeyCache.add(&dataKeyCacheEntry{
+		id:      id,
+		label:   label,
+		dataKey: dataKey,
+		active:  true,
+	})
 
 	return id, dataKey, nil
 }
@@ -419,7 +431,12 @@ func (s *SecretsService) dataKeyById(ctx context.Context, id string) ([]byte, er
 	}
 
 	// 3. Store the decrypted data key into the in-memory cache.
-	s.dataKeyCache.add(&dataKeyCacheEntry{id: id, name: dataKey.Name, dataKey: decrypted})
+	s.dataKeyCache.add(&dataKeyCacheEntry{
+		id:      dataKey.Id,
+		label:   dataKey.Label,
+		dataKey: decrypted,
+		active:  dataKey.Active,
+	})
 
 	return decrypted, nil
 }

pkg/services/secrets/manager/manager_test.go

@@ -100,8 +100,8 @@ func TestSecretsService_DataKeys(t *testing.T) {
 
 	dataKey := &secrets.DataKey{
 		Id:            util.GenerateShortUID(),
+		Label:         "test1",
 		Active:        true,
-		Name:          "test1",
 		Provider:      "test",
 		EncryptedData: []byte{0x62, 0xAF, 0xA1, 0x1A},
 	}
@@ -120,15 +120,15 @@ func TestSecretsService_DataKeys(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, dataKey.EncryptedData, res.EncryptedData)
 		assert.Equal(t, dataKey.Provider, res.Provider)
-		assert.Equal(t, dataKey.Name, res.Name)
+		assert.Equal(t, dataKey.Label, res.Label)
 		assert.Equal(t, dataKey.Id, res.Id)
 		assert.True(t, dataKey.Active)
 
-		current, err := store.GetCurrentDataKey(ctx, dataKey.Name)
+		current, err := store.GetCurrentDataKey(ctx, dataKey.Label)
 		require.NoError(t, err)
 		assert.Equal(t, dataKey.EncryptedData, current.EncryptedData)
 		assert.Equal(t, dataKey.Provider, current.Provider)
-		assert.Equal(t, dataKey.Name, current.Name)
+		assert.Equal(t, dataKey.Label, current.Label)
 		assert.Equal(t, dataKey.Id, current.Id)
 		assert.True(t, current.Active)
 	})
@@ -137,7 +137,7 @@ func TestSecretsService_DataKeys(t *testing.T) {
 		k := &secrets.DataKey{
 			Id:            util.GenerateShortUID(),
 			Active:        false,
-			Name:          "test2",
+			Label:         "test2",
 			Provider:      "test",
 			EncryptedData: []byte{0x62, 0xAF, 0xA1, 0x1A},
 		}
@@ -145,7 +145,7 @@ func TestSecretsService_DataKeys(t *testing.T) {
 		err := store.CreateDataKey(ctx, k)
 		require.Error(t, err)
 
-		res, err := store.GetDataKey(ctx, k.Name)
+		res, err := store.GetDataKey(ctx, k.Id)
 		assert.Equal(t, secrets.ErrDataKeyNotFound, err)
 		assert.Nil(t, res)
 	})
@@ -287,7 +287,7 @@ func TestSecretsService_Run(t *testing.T) {
 
 		// Data encryption key cache should contain one element
 		require.Len(t, svc.dataKeyCache.byId, 1)
-		require.Len(t, svc.dataKeyCache.byName, 1)
+		require.Len(t, svc.dataKeyCache.byLabel, 1)
 
 		t.Cleanup(func() { now = time.Now })
 		now = func() time.Time { return time.Now().Add(10 * time.Minute) }
@@ -302,7 +302,7 @@ func TestSecretsService_Run(t *testing.T) {
 		// the cleanup process should have happened,
 		// therefore the cache should be empty.
 		require.Len(t, svc.dataKeyCache.byId, 0)
-		require.Len(t, svc.dataKeyCache.byName, 0)
+		require.Len(t, svc.dataKeyCache.byLabel, 0)
 	})
 }
 
@@ -337,12 +337,12 @@ func TestSecretsService_ReEncryptDataKeys(t *testing.T) {
 		_, err := svc.Decrypt(ctx, ciphertext)
 		require.NoError(t, err)
 		require.NotEmpty(t, svc.dataKeyCache.byId)
-		require.NotEmpty(t, svc.dataKeyCache.byName)
+		require.NotEmpty(t, svc.dataKeyCache.byLabel)
 
 		err = svc.ReEncryptDataKeys(ctx)
 		require.NoError(t, err)
 
 		assert.Empty(t, svc.dataKeyCache.byId)
-		assert.Empty(t, svc.dataKeyCache.byName)
+		assert.Empty(t, svc.dataKeyCache.byLabel)
 	})
 }

pkg/services/secrets/secrets.go

@@ -33,7 +33,7 @@ type Service interface {
 // Store defines methods to interact with secrets storage
 type Store interface {
 	GetDataKey(ctx context.Context, id string) (*DataKey, error)
-	GetCurrentDataKey(ctx context.Context, name string) (*DataKey, error)
+	GetCurrentDataKey(ctx context.Context, label string) (*DataKey, error)
 	GetAllDataKeys(ctx context.Context) ([]*DataKey, error)
 	CreateDataKey(ctx context.Context, dataKey *DataKey) error
 	CreateDataKeyWithDBSession(ctx context.Context, dataKey *DataKey, sess *xorm.Session) error
@@ -61,7 +61,7 @@ func (id ProviderID) Kind() (string, error) {
 	return parts[0], nil
 }
 
-func KeyName(scope string, providerID ProviderID) string {
+func KeyLabel(scope string, providerID ProviderID) string {
 	return fmt.Sprintf("%s/%s@%s", time.Now().Format("2006-01-02"), scope, providerID)
 }
 

pkg/services/secrets/types.go

@@ -9,8 +9,8 @@ var ErrDataKeyNotFound = errors.New("data key not found")
 
 type DataKey struct {
 	Active        bool
-	Id            string
-	Name          string
+	Id            string `xorm:"name"` // renaming the col in the db itself would break backward compatibility with 8.5.x
+	Label         string
 	Scope         string
 	Provider      ProviderID
 	EncryptedData []byte

pkg/services/sqlstore/migrations/secrets_mig.go

@@ -61,4 +61,16 @@ func addSecretsMigration(mg *migrator.Migrator) {
 	mg.AddMigration("copy data_keys id column values into name", migrator.NewRawSQLMigration(
 		fmt.Sprintf("UPDATE %s SET %s = %s", dataKeysV1.Name, "name", "id"),
 	))
+	// ------- This is done for backward compatibility with versions > v8.3.x
+	mg.AddMigration("rename data_keys name column to label", migrator.NewRenameColumnMigration(
+		dataKeysV1, dataKeysV1.Columns[0], "label",
+	))
+
+	mg.AddMigration("rename data_keys id column back to name", migrator.NewRenameColumnMigration(
+		dataKeysV1,
+		&migrator.Column{Name: "id", Type: migrator.DB_NVarchar, Length: 100, IsPrimaryKey: true},
+		"name",
+	))
+
+	// --------------------
 }