Encryption: Refactor secrets service (#41771)

* Refactor kmsproviders pkg

* Update tests

* Fix linting

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

pkg/expr/service_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/encryption/ossencryption"
+	"github.com/grafana/grafana/pkg/services/kmsproviders/osskmsproviders"
 	"github.com/grafana/grafana/pkg/services/secrets/fakes"
 	"github.com/grafana/grafana/pkg/services/secrets/manager"
 	"github.com/grafana/grafana/pkg/setting"
@@ -30,13 +31,16 @@ func TestService(t *testing.T) {
 	}
 
 	cfg := setting.NewCfg()
+	encr := ossencryption.ProvideService()
+	settings := setting.ProvideProvider(cfg)
 
-	secretsService := manager.ProvideSecretsService(
+	secretsService, err := manager.ProvideSecretsService(
 		fakes.NewFakeSecretsStore(),
-		bus.GetBus(),
+		osskmsproviders.ProvideService(encr, settings),
-		ossencryption.ProvideService(),
+		encr,
-		setting.ProvideProvider(cfg),
+		settings,
 	)
+	require.NoError(t, err)
 
 	s := Service{
 		cfg:            cfg,

pkg/server/wire.go

@@ -160,7 +160,6 @@ var wireBasicSet = wire.NewSet(
 	elasticsearch.ProvideService,
 	secretsManager.ProvideSecretsService,
 	wire.Bind(new(secrets.Service), new(*secretsManager.SecretsService)),
-	wire.Bind(new(secrets.ProvidersRegistrar), new(*secretsManager.SecretsService)),
 	secretsDatabase.ProvideSecretsStore,
 	wire.Bind(new(secrets.Store), new(*secretsDatabase.SecretsStoreImpl)),
 	grafanads.ProvideService,

pkg/server/wireexts_oss.go

@@ -17,6 +17,8 @@ import (
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/encryption"
 	"github.com/grafana/grafana/pkg/services/encryption/ossencryption"
+	"github.com/grafana/grafana/pkg/services/kmsproviders"
+	"github.com/grafana/grafana/pkg/services/kmsproviders/osskmsproviders"
 	"github.com/grafana/grafana/pkg/services/licensing"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/login/authinfoservice"
@@ -62,6 +64,8 @@ var wireExtsBasicSet = wire.NewSet(
 	acdb.ProvideService,
 	wire.Bind(new(accesscontrol.ResourceStore), new(*acdb.AccessControlStore)),
 	wire.Bind(new(accesscontrol.PermissionsProvider), new(*acdb.AccessControlStore)),
+	osskmsproviders.ProvideService,
+	wire.Bind(new(kmsproviders.Service), new(osskmsproviders.Service)),
 )
 
 var wireExtsSet = wire.NewSet(

pkg/services/kmsproviders/kmsproviders.go

@@ -0,0 +1,11 @@
+package kmsproviders
+
+import "github.com/grafana/grafana/pkg/services/secrets"
+
+const (
+	Default = "secretKey"
+)
+
+type Service interface {
+	Provide() (map[string]secrets.Provider, error)
+}

pkg/services/kmsproviders/osskmsproviders/osskmsproviders.go

@@ -0,0 +1,31 @@
+package osskmsproviders
+
+import (
+	"github.com/grafana/grafana/pkg/services/encryption"
+	"github.com/grafana/grafana/pkg/services/kmsproviders"
+	"github.com/grafana/grafana/pkg/services/secrets"
+	grafana "github.com/grafana/grafana/pkg/services/secrets/defaultprovider"
+	"github.com/grafana/grafana/pkg/setting"
+)
+
+type Service struct {
+	enc      encryption.Internal
+	settings setting.Provider
+}
+
+func ProvideService(enc encryption.Internal, settings setting.Provider) Service {
+	return Service{
+		enc:      enc,
+		settings: settings,
+	}
+}
+
+func (s Service) Provide() (map[string]secrets.Provider, error) {
+	if !s.settings.IsFeatureToggleEnabled(secrets.EnvelopeEncryptionFeatureToggle) {
+		return nil, nil
+	}
+
+	return map[string]secrets.Provider{
+		kmsproviders.Default: grafana.New(s.settings, s.enc),
+	}, nil
+}

pkg/services/secrets/manager/helpers.go

@@ -3,7 +3,8 @@ package manager
 import (
 	"testing"
 
-	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/services/kmsproviders/osskmsproviders"
+
 	"github.com/grafana/grafana/pkg/services/encryption/ossencryption"
 	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/setting"
@@ -24,15 +25,19 @@ func SetupTestService(tb testing.TB, store secrets.Store) *SecretsService {
 		secret_key = ` + defaultKey))
 	require.NoError(tb, err)
 	cfg := &setting.Cfg{Raw: raw}
-	cfg.FeatureToggles = map[string]bool{envelopeEncryptionFeatureToggle: true}
+	cfg.FeatureToggles = map[string]bool{secrets.EnvelopeEncryptionFeatureToggle: true}
 
 	settings := &setting.OSSImpl{Cfg: cfg}
-	assert.True(tb, settings.IsFeatureToggleEnabled(envelopeEncryptionFeatureToggle))
+	assert.True(tb, settings.IsFeatureToggleEnabled(secrets.EnvelopeEncryptionFeatureToggle))
 
-	return ProvideSecretsService(
+	encryption := ossencryption.ProvideService()
+	secretsService, err := ProvideSecretsService(
 		store,
-		bus.New(),
+		osskmsproviders.ProvideService(encryption, settings),
-		ossencryption.ProvideService(),
+		encryption,
 		settings,
 	)
+	require.NoError(tb, err)
+
+	return secretsService
 }

pkg/services/secrets/manager/manager.go

@@ -9,22 +9,15 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/services/encryption"
+	"github.com/grafana/grafana/pkg/services/kmsproviders"
 	"github.com/grafana/grafana/pkg/services/secrets"
-	grafana "github.com/grafana/grafana/pkg/services/secrets/defaultprovider"
 	"github.com/grafana/grafana/pkg/setting"
 	"xorm.io/xorm"
 )
 
-const (
-	defaultProvider                 = "secretKey"
-	envelopeEncryptionFeatureToggle = "envelopeEncryption"
-)
-
 type SecretsService struct {
 	store    secrets.Store
-	bus      bus.Bus
 	enc      encryption.Internal
 	settings setting.Provider
 
@@ -33,15 +26,21 @@ type SecretsService struct {
 	dataKeyCache    map[string]dataKeyCacheItem
 }
 
-func ProvideSecretsService(store secrets.Store, bus bus.Bus, enc encryption.Internal, settings setting.Provider) *SecretsService {
+func ProvideSecretsService(
-	providers := map[string]secrets.Provider{
+	store secrets.Store,
-		defaultProvider: grafana.New(settings, enc),
+	kmsProvidersService kmsproviders.Service,
+	enc encryption.Internal,
+	settings setting.Provider,
+) (*SecretsService, error) {
+	providers, err := kmsProvidersService.Provide()
+	if err != nil {
+		return nil, err
 	}
-	currentProvider := settings.KeyValue("security", "encryption_provider").MustString(defaultProvider)
+
+	currentProvider := settings.KeyValue("security", "encryption_provider").MustString(kmsproviders.Default)
 
 	s := &SecretsService{
 		store:           store,
-		bus:             bus,
 		enc:             enc,
 		settings:        settings,
 		providers:       providers,
@@ -49,7 +48,7 @@ func ProvideSecretsService(store secrets.Store, bus bus.Bus, enc encryption.Inte
 		dataKeyCache:    make(map[string]dataKeyCacheItem),
 	}
 
-	return s
+	return s, nil
 }
 
 type dataKeyCacheItem struct {
@@ -65,11 +64,11 @@ func (s *SecretsService) Encrypt(ctx context.Context, payload []byte, opt secret
 
 func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byte, opt secrets.EncryptionOptions, sess *xorm.Session) ([]byte, error) {
 	// Use legacy encryption service if envelopeEncryptionFeatureToggle toggle is off
-	if !s.settings.IsFeatureToggleEnabled(envelopeEncryptionFeatureToggle) {
+	if !s.settings.IsFeatureToggleEnabled(secrets.EnvelopeEncryptionFeatureToggle) {
 		return s.enc.Encrypt(ctx, payload, setting.SecretKey)
 	}
 
-	// If encryption envelopeEncryptionFeatureToggle toggle is on, use envelope encryption
+	// If encryption secrets.EnvelopeEncryptionFeatureToggle toggle is on, use envelope encryption
 	scope := opt()
 	keyName := fmt.Sprintf("%s/%s@%s", time.Now().Format("2006-01-02"), scope, s.currentProvider)
 
@@ -103,12 +102,12 @@ func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byt
 }
 
 func (s *SecretsService) Decrypt(ctx context.Context, payload []byte) ([]byte, error) {
-	// Use legacy encryption service if envelopeEncryptionFeatureToggle toggle is off
+	// Use legacy encryption service if secrets.EnvelopeEncryptionFeatureToggle toggle is off
-	if !s.settings.IsFeatureToggleEnabled(envelopeEncryptionFeatureToggle) {
+	if !s.settings.IsFeatureToggleEnabled(secrets.EnvelopeEncryptionFeatureToggle) {
 		return s.enc.Decrypt(ctx, payload, setting.SecretKey)
 	}
 
-	// If encryption envelopeEncryptionFeatureToggle toggle is on, use envelope encryption
+	// If encryption secrets.EnvelopeEncryptionFeatureToggle toggle is on, use envelope encryption
 	if len(payload) == 0 {
 		return nil, fmt.Errorf("unable to decrypt empty payload")
 	}

pkg/services/secrets/manager/manager_test.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/services/encryption/ossencryption"
+	"github.com/grafana/grafana/pkg/services/kmsproviders/osskmsproviders"
 	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/services/secrets/database"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
@@ -160,15 +160,17 @@ func TestSecretsService_UseCurrentProvider(t *testing.T) {
 		raw, err := ini.Load([]byte(rawCfg))
 		require.NoError(t, err)
 
-		cfg := &setting.Cfg{Raw: raw, FeatureToggles: map[string]bool{envelopeEncryptionFeatureToggle: true}}
+		cfg := &setting.Cfg{Raw: raw, FeatureToggles: map[string]bool{secrets.EnvelopeEncryptionFeatureToggle: true}}
 		settings := &setting.OSSImpl{Cfg: cfg}
 
-		svc := ProvideSecretsService(
+		encr := ossencryption.ProvideService()
+		svc, err := ProvideSecretsService(
 			database.ProvideSecretsStore(sqlstore.InitTestDB(t)),
-			bus.New(),
+			osskmsproviders.ProvideService(encr, settings),
-			ossencryption.ProvideService(),
+			encr,
 			settings,
 		)
+		require.NoError(t, err)
 
 		assert.Equal(t, "awskms.second_key", svc.currentProvider)
 	})
@@ -185,19 +187,21 @@ func TestSecretsService_UseCurrentProvider(t *testing.T) {
 		raw, err := ini.Load([]byte(rawCfg))
 		require.NoError(t, err)
 
-		cfg := &setting.Cfg{Raw: raw, FeatureToggles: map[string]bool{envelopeEncryptionFeatureToggle: true}}
+		cfg := &setting.Cfg{Raw: raw, FeatureToggles: map[string]bool{secrets.EnvelopeEncryptionFeatureToggle: true}}
 		settings := &setting.OSSImpl{Cfg: cfg}
 
 		secretStore := database.ProvideSecretsStore(sqlstore.InitTestDB(t))
 		fake := fakeProvider{}
 		providerID := "fake-provider.some-key"
 
-		svcEncrypt := ProvideSecretsService(
+		encr := ossencryption.ProvideService()
+		svcEncrypt, err := ProvideSecretsService(
 			secretStore,
-			bus.New(),
+			osskmsproviders.ProvideService(encr, settings),
-			ossencryption.ProvideService(),
+			encr,
 			settings,
 		)
+		require.NoError(t, err)
 
 		svcEncrypt.RegisterProvider(providerID, &fake)
 		require.NoError(t, err)
@@ -208,12 +212,13 @@ func TestSecretsService_UseCurrentProvider(t *testing.T) {
 
 		// secret service tries to find a DEK in a cache first before calling provider's decrypt
 		// to bypass the cache, we set up one more secrets service to test decrypting
-		svcDecrypt := ProvideSecretsService(
+		svcDecrypt, err := ProvideSecretsService(
 			secretStore,
-			bus.New(),
+			osskmsproviders.ProvideService(encr, settings),
-			ossencryption.ProvideService(),
+			encr,
 			settings,
 		)
+		require.NoError(t, err)
 		svcDecrypt.RegisterProvider(providerID, &fake)
 		_, _ = svcDecrypt.Decrypt(context.Background(), encrypted)
 		assert.True(t, fake.decryptCalled, "fake provider's decrypt should be called")

pkg/services/secrets/secrets.go

@@ -6,6 +6,10 @@ import (
 	"xorm.io/xorm"
 )
 
+const (
+	EnvelopeEncryptionFeatureToggle = "envelopeEncryption"
+)
+
 // Service is an envelope encryption service in charge of encrypting/decrypting secrets.
 // It is a replacement for encryption.Service
 type Service interface {
@@ -24,12 +28,6 @@ type Service interface {
 	GetDecryptedValue(ctx context.Context, sjd map[string][]byte, key, fallback string) string
 }
 
-type ProvidersRegistrar interface {
-	CurrentProviderID() string
-	GetProviders() map[string]Provider
-	RegisterProvider(providerID string, provider Provider)
-}
-
 // Store defines methods to interact with secrets storage
 type Store interface {
 	GetDataKey(ctx context.Context, name string) (*DataKey, error)