Encryption: Cache new DEKs (only) after a caution period (#60664)

* Encryption: Cache new DEKs (only) after commit

* Fix typo

* Update secrets manager tests with new failing case

* Update secrets manager tests with new clarifications (comments)

* Correct broken method calls

* Unify methods

* Cache data keys only after a caution period

* Caution period for data keys caching only for encrypt ops
This commit is contained in:
Joan López de la Franca Beltran
2023-01-26 10:54:31 +01:00
committed by GitHub
parent 3a442610d2
commit c4e067d49d
7 changed files with 301 additions and 114 deletions
+65 -41
View File
@@ -19,15 +19,21 @@ import (
"github.com/grafana/grafana/pkg/services/secrets"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/sync/errgroup"
"xorm.io/xorm"
)
const (
keyIdDelimiter = '#'
)
var (
// now is used for testing purposes,
// as a way to fake time.Now function.
now = time.Now
)
type SecretsService struct {
store secrets.Store
enc encryption.Internal
@@ -152,10 +158,6 @@ func (s *SecretsService) encryptedWithEnvelopeEncryption(payload []byte) bool {
var b64 = base64.RawStdEncoding
func (s *SecretsService) Encrypt(ctx context.Context, payload []byte, opt secrets.EncryptionOptions) ([]byte, error) {
return s.EncryptWithDBSession(ctx, payload, opt, nil)
}
func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byte, opt secrets.EncryptionOptions, sess *xorm.Session) ([]byte, error) {
// Use legacy encryption service if featuremgmt.FlagDisableEnvelopeEncryption toggle is on
if s.features.IsEnabled(featuremgmt.FlagDisableEnvelopeEncryption) {
return s.enc.Encrypt(ctx, payload, setting.SecretKey)
@@ -175,7 +177,7 @@ func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byt
var id string
var dataKey []byte
id, dataKey, err = s.currentDataKey(ctx, label, scope, sess)
id, dataKey, err = s.currentDataKey(ctx, label, scope)
if err != nil {
s.log.Error("Failed to get current data key", "error", err, "label", label)
return nil, err
@@ -203,7 +205,7 @@ 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, label string, scope string, sess *xorm.Session) (string, []byte, error) {
func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope string) (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()
@@ -217,7 +219,7 @@ func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope
// If no existing data key was found, create a new one
if dataKey == nil {
id, dataKey, err = s.newDataKey(ctx, label, scope, sess)
id, dataKey, err = s.newDataKey(ctx, label, scope)
if err != nil {
return "", nil, err
}
@@ -226,7 +228,7 @@ func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope
return id, dataKey, nil
}
// dataKeyByLabel looks up for data key in cache.
// dataKeyByLabel looks up for data key in cache by label.
// Otherwise, it fetches it from database, decrypts it and caches it decrypted.
func (s *SecretsService) dataKeyByLabel(ctx context.Context, label string) (string, []byte, error) {
// 0. Get data key from in-memory cache.
@@ -256,18 +258,13 @@ func (s *SecretsService) dataKeyByLabel(ctx context.Context, label string) (stri
}
// 3. Store the decrypted data key into the in-memory cache.
s.dataKeyCache.add(&dataKeyCacheEntry{
id: dataKey.Id,
label: dataKey.Label,
dataKey: decrypted,
active: dataKey.Active,
})
s.cacheDataKey(dataKey, decrypted)
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, label string, scope string, sess *xorm.Session) (string, []byte, error) {
func (s *SecretsService) newDataKey(ctx context.Context, label string, scope string) (string, []byte, error) {
// 1. Create new data key.
dataKey, err := newRandomDataKey()
if err != nil {
@@ -288,6 +285,7 @@ func (s *SecretsService) newDataKey(ctx context.Context, label string, scope str
// 3. Store its encrypted value into the DB.
id := util.GenerateShortUID()
dbDataKey := secrets.DataKey{
Active: true,
Id: id,
@@ -297,24 +295,11 @@ func (s *SecretsService) newDataKey(ctx context.Context, label string, scope str
Scope: scope,
}
if sess == nil {
err = s.store.CreateDataKey(ctx, &dbDataKey)
} else {
err = s.store.CreateDataKeyWithDBSession(ctx, &dbDataKey, sess)
}
err = s.store.CreateDataKey(ctx, &dbDataKey)
if err != nil {
return "", nil, err
}
// 4. Store the decrypted data key into the in-memory cache.
s.dataKeyCache.add(&dataKeyCacheEntry{
id: id,
label: label,
dataKey: dataKey,
active: true,
})
return id, dataKey, nil
}
@@ -388,13 +373,9 @@ func (s *SecretsService) Decrypt(ctx context.Context, payload []byte) ([]byte, e
}
func (s *SecretsService) EncryptJsonData(ctx context.Context, kv map[string]string, opt secrets.EncryptionOptions) (map[string][]byte, error) {
return s.EncryptJsonDataWithDBSession(ctx, kv, opt, nil)
}
func (s *SecretsService) EncryptJsonDataWithDBSession(ctx context.Context, kv map[string]string, opt secrets.EncryptionOptions, sess *xorm.Session) (map[string][]byte, error) {
encrypted := make(map[string][]byte)
for key, value := range kv {
encryptedData, err := s.EncryptWithDBSession(ctx, []byte(value), opt, sess)
encryptedData, err := s.Encrypt(ctx, []byte(value), opt)
if err != nil {
return nil, err
}
@@ -457,12 +438,7 @@ 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: dataKey.Id,
label: dataKey.Label,
dataKey: decrypted,
active: dataKey.Active,
})
s.cacheDataKey(dataKey, decrypted)
return decrypted, nil
}
@@ -547,3 +523,51 @@ func (s *SecretsService) Run(ctx context.Context) error {
}
}
}
// Caching a data key is tricky, because at SecretsService level we cannot guarantee
// that a newly created data key has actually been persisted, depending on the different
// use cases that rely on SecretsService encryption and different database engines that
// we have support for, because the data key creation may have happened within a DB TX,
// that may fail afterwards.
//
// Therefore, if we cache a data key that hasn't been persisted with success (and won't),
// and later that one is used for a encryption operation (aside from the DB TX that created
// it), we may end up with data encrypted by a non-persisted data key, which could end up
// in (unrecoverable) data corruption.
//
// So, we cache the data key by id and/or by label, depending on the data key's lifetime,
// assuming that a data key older than a "caution period" should have been persisted.
//
// Look at the comments inline for further details.
// You can also take a look at the issue below for more context:
// https://github.com/grafana/grafana-enterprise/issues/4252
func (s *SecretsService) cacheDataKey(dataKey *secrets.DataKey, decrypted []byte) {
// First, we cache the data key by id, because cache "by id" is
// only used by decrypt operations, so no risk of corrupting data.
entry := &dataKeyCacheEntry{
id: dataKey.Id,
label: dataKey.Label,
dataKey: decrypted,
active: dataKey.Active,
}
s.dataKeyCache.addById(entry)
// Then, we cache the data key by label, ONLY if data key's lifetime
// is longer than a certain "caution period", because cache "by label"
// is used (only) by encrypt operations, and we want to ensure that
// no data key is cached for encryption ops before being persisted.
const cautionPeriod = 10 * time.Minute
// We consider a "caution period" of 10m to be long enough for any database
// transaction that implied a data key creation to have finished successfully.
//
// Therefore, we consider that if we fetch a data key from the database,
// more than 10m later than its creation, it should have been actually
// persisted - i.e. the transaction that created it is no longer running.
nowMinusCautionPeriod := now().Add(-cautionPeriod)
if dataKey.Created.Before(nowMinusCautionPeriod) {
s.dataKeyCache.addByLabel(entry)
}
}