From b1b9cc43a83a2a55b71afaf0ebc0dba1d8fc7701 Mon Sep 17 00:00:00 2001 From: Dana Axinte <53751979+dana-axinte@users.noreply.github.com> Date: Fri, 25 Jul 2025 17:11:17 +0100 Subject: [PATCH] SecretsManager: Adding ability to disable all DEKs (#108444) * Adding dek deactivation and rename list dek * disable data keys from manager * separate interface and don't use in encryption manager --- .../apis/secret/contracts/data_key.go | 7 +- .../secret/encryption/manager/manager_test.go | 8 +-- .../encryption/data/data_key_disable_all.sql | 7 ++ .../secret/encryption/data_key_store.go | 68 +++++++++++++++++-- .../secret/encryption/data_key_store_test.go | 15 +++- pkg/storage/secret/encryption/metrics.go | 41 +++++++++-- pkg/storage/secret/encryption/query.go | 8 +++ pkg/storage/secret/encryption/query_test.go | 9 +++ .../mysql--data_key_disable_all-disable.sql | 7 ++ ...postgres--data_key_disable_all-disable.sql | 7 ++ .../sqlite--data_key_disable_all-disable.sql | 7 ++ 11 files changed, 167 insertions(+), 17 deletions(-) create mode 100644 pkg/storage/secret/encryption/data/data_key_disable_all.sql create mode 100755 pkg/storage/secret/encryption/testdata/mysql--data_key_disable_all-disable.sql create mode 100755 pkg/storage/secret/encryption/testdata/postgres--data_key_disable_all-disable.sql create mode 100755 pkg/storage/secret/encryption/testdata/sqlite--data_key_disable_all-disable.sql diff --git a/pkg/registry/apis/secret/contracts/data_key.go b/pkg/registry/apis/secret/contracts/data_key.go index bac21460d18..5ee4bead843 100644 --- a/pkg/registry/apis/secret/contracts/data_key.go +++ b/pkg/registry/apis/secret/contracts/data_key.go @@ -29,7 +29,12 @@ type DataKeyStorage interface { CreateDataKey(ctx context.Context, dataKey *SecretDataKey) error GetDataKey(ctx context.Context, namespace, uid string) (*SecretDataKey, error) GetCurrentDataKey(ctx context.Context, namespace, label string) (*SecretDataKey, error) - GetAllDataKeys(ctx context.Context, namespace string) ([]*SecretDataKey, error) + ListDataKeys(ctx context.Context, namespace string) ([]*SecretDataKey, error) DisableDataKeys(ctx context.Context, namespace string) error DeleteDataKey(ctx context.Context, namespace, uid string) error } + +// GlobalDataKeyStorage is an interface for namespace unbounded operations. +type GlobalDataKeyStorage interface { + DisableAllDataKeys(ctx context.Context) error +} diff --git a/pkg/registry/apis/secret/encryption/manager/manager_test.go b/pkg/registry/apis/secret/encryption/manager/manager_test.go index 97166c52267..ede903c06d6 100644 --- a/pkg/registry/apis/secret/encryption/manager/manager_test.go +++ b/pkg/registry/apis/secret/encryption/manager/manager_test.go @@ -45,7 +45,7 @@ func TestEncryptionService_EnvelopeEncryption(t *testing.T) { require.NoError(t, err) assert.Equal(t, plaintext, decrypted) - keys, err := svc.store.GetAllDataKeys(ctx, namespace) + keys, err := svc.store.ListDataKeys(ctx, namespace) require.NoError(t, err) assert.Equal(t, len(keys), 1) }) @@ -60,7 +60,7 @@ func TestEncryptionService_EnvelopeEncryption(t *testing.T) { require.NoError(t, err) assert.Equal(t, plaintext, decrypted) - keys, err := svc.store.GetAllDataKeys(ctx, namespace) + keys, err := svc.store.ListDataKeys(ctx, namespace) require.NoError(t, err) assert.Equal(t, len(keys), 1) }) @@ -139,12 +139,12 @@ func TestEncryptionService_DataKeys(t *testing.T) { }) t.Run("deleting DEK when no id provided must fail", func(t *testing.T) { - beforeDelete, err := store.GetAllDataKeys(ctx, namespace) + beforeDelete, err := store.ListDataKeys(ctx, namespace) require.NoError(t, err) err = store.DeleteDataKey(ctx, namespace, "") require.Error(t, err) - afterDelete, err := store.GetAllDataKeys(ctx, namespace) + afterDelete, err := store.ListDataKeys(ctx, namespace) require.NoError(t, err) assert.Equal(t, beforeDelete, afterDelete) }) diff --git a/pkg/storage/secret/encryption/data/data_key_disable_all.sql b/pkg/storage/secret/encryption/data/data_key_disable_all.sql new file mode 100644 index 00000000000..92a2bd6a5c6 --- /dev/null +++ b/pkg/storage/secret/encryption/data/data_key_disable_all.sql @@ -0,0 +1,7 @@ +UPDATE + {{ .Ident "secret_data_key" }} +SET + {{ .Ident "active" }} = false, + {{ .Ident "updated" }} = {{ .Arg .Updated }} +WHERE {{ .Ident "active" }} = true +; diff --git a/pkg/storage/secret/encryption/data_key_store.go b/pkg/storage/secret/encryption/data_key_store.go index b80da516acd..6d38d117901 100644 --- a/pkg/storage/secret/encryption/data_key_store.go +++ b/pkg/storage/secret/encryption/data_key_store.go @@ -9,6 +9,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" ) @@ -161,14 +162,14 @@ func (ss *encryptionStoreImpl) GetCurrentDataKey(ctx context.Context, namespace, }, nil } -func (ss *encryptionStoreImpl) GetAllDataKeys(ctx context.Context, namespace string) ([]*contracts.SecretDataKey, error) { +func (ss *encryptionStoreImpl) ListDataKeys(ctx context.Context, namespace string) ([]*contracts.SecretDataKey, error) { start := time.Now() - ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.GetAllDataKeys", trace.WithAttributes( + ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.ListDataKeys", trace.WithAttributes( attribute.String("namespace", namespace), )) defer func() { span.End() - ss.metrics.GetAllDataKeysDuration.Observe(float64(time.Since(start))) + ss.metrics.ListDataKeysDuration.Observe(float64(time.Since(start))) }() req := listDataKeys{ @@ -299,8 +300,8 @@ func (ss *encryptionStoreImpl) DisableDataKeys(ctx context.Context, namespace st return fmt.Errorf("getting rows affected: %w", err) } - if rowsAffected != 1 { - return fmt.Errorf("expected 1 row affected, but affected %d", rowsAffected) + if rowsAffected == 0 { + logging.FromContext(ctx).Info("Disable all data keys: no keys were disabled for namespace", "namespace", namespace) } return nil @@ -348,3 +349,60 @@ func (ss *encryptionStoreImpl) DeleteDataKey(ctx context.Context, namespace, uid return nil } + +type globalEncryptionStoreImpl struct { + db contracts.Database + dialect sqltemplate.Dialect + tracer trace.Tracer + metrics *GlobalDataKeyMetrics +} + +func ProvideGlobalDataKeyStorage( + db contracts.Database, + tracer trace.Tracer, + registerer prometheus.Registerer, +) (contracts.GlobalDataKeyStorage, error) { + store := &globalEncryptionStoreImpl{ + db: db, + dialect: sqltemplate.DialectForDriver(db.DriverName()), + tracer: tracer, + metrics: NewGlobalDataKeyMetrics(registerer), + } + + return store, nil +} + +func (ss *globalEncryptionStoreImpl) DisableAllDataKeys(ctx context.Context) error { + start := time.Now() + ctx, span := ss.tracer.Start(ctx, "GlobalDataKeyStorage.DisableAllDataKeys") + defer func() { + span.End() + ss.metrics.DisableAllDataKeysDuration.Observe(float64(time.Since(start))) + }() + + req := disableAllDataKeys{ + SQLTemplate: sqltemplate.New(ss.dialect), + Updated: time.Now(), + } + + query, err := sqltemplate.Execute(sqlDataKeyDisableAll, req) + if err != nil { + return fmt.Errorf("execute template %q: %w", sqlDataKeyDisableAll.Name(), err) + } + + result, err := ss.db.ExecContext(ctx, query, req.GetArgs()...) + if err != nil { + return fmt.Errorf("updating data keys: %w", err) + } + + rowsAffected, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("getting rows affected: %w", err) + } + + if rowsAffected == 0 { + logging.FromContext(ctx).Info("Disable all data keys: no keys were disabled") + } + + return nil +} diff --git a/pkg/storage/secret/encryption/data_key_store_test.go b/pkg/storage/secret/encryption/data_key_store_test.go index 134be881231..18081997ad5 100644 --- a/pkg/storage/secret/encryption/data_key_store_test.go +++ b/pkg/storage/secret/encryption/data_key_store_test.go @@ -31,6 +31,8 @@ func TestEncryptionStoreImpl_DataKeyLifecycle(t *testing.T) { tracer := noop.NewTracerProvider().Tracer("test") store, err := ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, nil) require.NoError(t, err) + globalStore, err := ProvideGlobalDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, nil) + require.NoError(t, err) ctx := context.Background() @@ -71,8 +73,8 @@ func TestEncryptionStoreImpl_DataKeyLifecycle(t *testing.T) { require.Equal(t, dataKey.UID, currentKey.UID) require.Equal(t, dataKey.Namespace, currentKey.Namespace) - // Test GetAllDataKeys - allKeys, err := store.GetAllDataKeys(ctx, "test-namespace") + // Test ListDataKeys + allKeys, err := store.ListDataKeys(ctx, "test-namespace") require.NoError(t, err) require.Len(t, allKeys, 1) require.Equal(t, dataKey.UID, allKeys[0].UID) @@ -101,6 +103,15 @@ func TestEncryptionStoreImpl_DataKeyLifecycle(t *testing.T) { require.Equal(t, unchangingDataKey.UID, staticKey.UID) require.Equal(t, unchangingDataKey.Namespace, staticKey.Namespace) require.True(t, staticKey.Active) + + // Test DisableAllDataKeys + err = globalStore.DisableAllDataKeys(ctx) + require.NoError(t, err) + + // Verify that remaining data keys are disabled + disabledKey, err = store.GetDataKey(ctx, "static-namespace", "static-uid") + require.NoError(t, err) + require.False(t, disabledKey.Active) } type PassThroughEncryptionProvider struct{} diff --git a/pkg/storage/secret/encryption/metrics.go b/pkg/storage/secret/encryption/metrics.go index 5a18d400764..5262139cdd5 100644 --- a/pkg/storage/secret/encryption/metrics.go +++ b/pkg/storage/secret/encryption/metrics.go @@ -14,7 +14,7 @@ type DataKeyMetrics struct { CreateDataKeyDuration prometheus.Histogram GetDataKeyDuration prometheus.Histogram GetCurrentDataKeyDuration prometheus.Histogram - GetAllDataKeysDuration prometheus.Histogram + ListDataKeysDuration prometheus.Histogram DisableDataKeysDuration prometheus.Histogram DeleteDataKeyDuration prometheus.Histogram } @@ -42,11 +42,11 @@ func newDataKeyMetrics() *DataKeyMetrics { Help: "Duration of get current data key operations", Buckets: prometheus.DefBuckets, }), - GetAllDataKeysDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + ListDataKeysDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ Namespace: namespace, Subsystem: subsystem, - Name: "get_all_data_keys_duration_seconds", - Help: "Duration of get all data keys operations", + Name: "list_data_keys_duration_seconds", + Help: "Duration of list data keys operations", Buckets: prometheus.DefBuckets, }), DisableDataKeysDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ @@ -76,7 +76,7 @@ func NewDataKeyMetrics(reg prometheus.Registerer) *DataKeyMetrics { m.CreateDataKeyDuration, m.GetDataKeyDuration, m.GetCurrentDataKeyDuration, - m.GetAllDataKeysDuration, + m.ListDataKeysDuration, m.DisableDataKeysDuration, m.DeleteDataKeyDuration, ) @@ -84,3 +84,34 @@ func NewDataKeyMetrics(reg prometheus.Registerer) *DataKeyMetrics { return m } + +type GlobalDataKeyMetrics struct { + DisableAllDataKeysDuration prometheus.Histogram +} + +func newGlobalDataKeyMetrics() *GlobalDataKeyMetrics { + return &GlobalDataKeyMetrics{ + + DisableAllDataKeysDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "disable_all_data_keys_duration_seconds", + Help: "Duration of disable all data keys operations", + Buckets: prometheus.DefBuckets, + }), + } +} + +// NewGlobalDataKeyMetrics returns an instance of the GlobalDataKeyMetrics +// struct containing registered metrics if [reg] is not nil. +func NewGlobalDataKeyMetrics(reg prometheus.Registerer) *GlobalDataKeyMetrics { + m := newGlobalDataKeyMetrics() + + if reg != nil { + reg.MustRegister( + m.DisableAllDataKeysDuration, + ) + } + + return m +} diff --git a/pkg/storage/secret/encryption/query.go b/pkg/storage/secret/encryption/query.go index 25f21cb77ba..573e295dcaa 100644 --- a/pkg/storage/secret/encryption/query.go +++ b/pkg/storage/secret/encryption/query.go @@ -28,6 +28,7 @@ var ( sqlDataKeyList = mustTemplate("data_key_list.sql") sqlDataKeyDisable = mustTemplate("data_key_disable.sql") sqlDataKeyDelete = mustTemplate("data_key_delete.sql") + sqlDataKeyDisableAll = mustTemplate("data_key_disable_all.sql") ) // TODO: Move this to a common place so that all stores can use @@ -140,3 +141,10 @@ type deleteDataKey struct { } func (r deleteDataKey) Validate() error { return nil } + +type disableAllDataKeys struct { + sqltemplate.SQLTemplate + Updated time.Time +} + +func (r disableAllDataKeys) Validate() error { return nil } diff --git a/pkg/storage/secret/encryption/query_test.go b/pkg/storage/secret/encryption/query_test.go index 51e078d8e25..c2ebf7a9635 100644 --- a/pkg/storage/secret/encryption/query_test.go +++ b/pkg/storage/secret/encryption/query_test.go @@ -159,6 +159,15 @@ func TestDataKeyQueries(t *testing.T) { }, }, }, + sqlDataKeyDisableAll: { + { + Name: "disable", + Data: &disableAllDataKeys{ + SQLTemplate: mocks.NewTestingSQLTemplate(), + Updated: time.Unix(1735689600, 0).UTC(), + }, + }, + }, }, }) } diff --git a/pkg/storage/secret/encryption/testdata/mysql--data_key_disable_all-disable.sql b/pkg/storage/secret/encryption/testdata/mysql--data_key_disable_all-disable.sql new file mode 100755 index 00000000000..49972ef5026 --- /dev/null +++ b/pkg/storage/secret/encryption/testdata/mysql--data_key_disable_all-disable.sql @@ -0,0 +1,7 @@ +UPDATE + `secret_data_key` +SET + `active` = false, + `updated` = '2025-01-01 00:00:00 +0000 UTC' +WHERE `active` = true +; diff --git a/pkg/storage/secret/encryption/testdata/postgres--data_key_disable_all-disable.sql b/pkg/storage/secret/encryption/testdata/postgres--data_key_disable_all-disable.sql new file mode 100755 index 00000000000..8b72ac271fc --- /dev/null +++ b/pkg/storage/secret/encryption/testdata/postgres--data_key_disable_all-disable.sql @@ -0,0 +1,7 @@ +UPDATE + "secret_data_key" +SET + "active" = false, + "updated" = '2025-01-01 00:00:00 +0000 UTC' +WHERE "active" = true +; diff --git a/pkg/storage/secret/encryption/testdata/sqlite--data_key_disable_all-disable.sql b/pkg/storage/secret/encryption/testdata/sqlite--data_key_disable_all-disable.sql new file mode 100755 index 00000000000..8b72ac271fc --- /dev/null +++ b/pkg/storage/secret/encryption/testdata/sqlite--data_key_disable_all-disable.sql @@ -0,0 +1,7 @@ +UPDATE + "secret_data_key" +SET + "active" = false, + "updated" = '2025-01-01 00:00:00 +0000 UTC' +WHERE "active" = true +;