Encryption: Fix multiple data keys migration (#49848) (#50207)

* 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>
(cherry picked from commit 4f8111e24e)

Co-authored-by: Tania <yalyna.ts@gmail.com>
This commit is contained in:
Grot (@grafanabot)
2022-06-04 13:13:34 +02:00
committed by GitHub
co-authored by Tania
parent f404191ccb
commit 67cbd5015d
9 changed files with 85 additions and 55 deletions
+11 -10
View File
@@ -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()
}
+33 -16
View File
@@ -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
}
@@ -417,7 +429,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
}
+10 -10
View File
@@ -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)
})
}