Secrets/Decrypt: Extract DecryptAuthorizer into its own interface (#104255)

* Decrypt: Create DecryptAuthorizer and implement it + noop impl

* DecryptStore: Replace authorization checks with DecryptAuthorizer + wire
pull/104220/head
Matheus Macabu 1 month ago committed by GitHub
parent faf1969ac8
commit b714f142c1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 5
      pkg/registry/apis/secret/contracts/decrypt.go
  2. 91
      pkg/registry/apis/secret/decrypt/authorizer.go
  3. 142
      pkg/registry/apis/secret/decrypt/authorizer_test.go
  4. 16
      pkg/registry/apis/secret/decrypt/noop_authorizer.go
  5. 1
      pkg/server/wire.go
  6. 81
      pkg/storage/secret/metadata/decrypt_store.go
  7. 5
      pkg/storage/secret/metadata/decrypt_store_test.go

@ -19,5 +19,10 @@ type DecryptStorage interface {
Decrypt(ctx context.Context, namespace xkube.Namespace, name string) (secretv0alpha1.ExposedSecureValue, error)
}
// DecryptAuthorizer is the interface for authorizing decryption requests.
type DecryptAuthorizer interface {
Authorize(ctx context.Context, secureValueDecrypters []string) (identity string, allowed bool)
}
// TEMPORARY: Needed to pass it with wire.
type DecryptAllowList map[string]struct{}

@ -0,0 +1,91 @@
package decrypt
import (
"context"
"strings"
claims "github.com/grafana/authlib/types"
secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
)
// decryptAuthorizer is the authorizer implementation for decrypt operations.
type decryptAuthorizer struct {
allowList contracts.DecryptAllowList
}
func ProvideDecryptAuthorizer(allowList contracts.DecryptAllowList) contracts.DecryptAuthorizer {
return &decryptAuthorizer{
allowList: allowList,
}
}
// authorize checks whether the auth info token has the right permissions to decrypt the secure value.
func (a *decryptAuthorizer) Authorize(ctx context.Context, secureValueDecrypters []string) (string, bool) {
authInfo, ok := claims.AuthInfoFrom(ctx)
if !ok {
return "", false
}
tokenPermissions := authInfo.GetTokenPermissions()
tokenActors := make(map[string]struct{}, 0)
for _, permission := range tokenPermissions {
// Will look like `secret.grafana.app/securevalues/<actor>:decrypt` for now.
gr, verb, found := strings.Cut(permission, ":")
if !found {
continue
}
// If it isn't decrypt, then we don't care to check.
if verb != "decrypt" {
continue
}
parts := strings.Split(gr, "/")
if len(parts) != 3 {
continue
}
group, resource, actor := parts[0], parts[1], parts[2]
if group != secretv0alpha1.GROUP || resource != secretv0alpha1.SecureValuesResourceInfo.GetName() || actor == "" {
continue
}
// TEMPORARY: while we can't onboard every app into secrets, we can block them from decrypting
// securevalues preemptively here before even reaching out to the database.
// This check can be removed once we open the gates for any service to use secrets.
if _, exists := a.allowList[actor]; !exists {
continue
}
tokenActors[actor] = struct{}{}
}
// If we arrived here and the token actors is empty, it means the permissions either have an invalid format,
// or it didn't pass the allow list, meaning no allowed decryptor.
if len(tokenActors) == 0 {
return "", false
}
// TEMPORARY: while we still need to mix permission and identity, we can use this
// to decide whether the SecureValue can be decrypted or not.
// Once we have an `actor` field in the JWT claims, we can have a properly formatted permission,
// like `secret.grafana.app/securevalues{/<name>}:decrypt` and do regular access control eval,
// and for the `decrypters` part here, we can just check it against the `actor` field, which at
// that point will have a different format, depending on how the `actor` will be formatted.
// Check whether at least one of declared token actors matches the allowed decrypters from the SecureValue.
allowed := false
var identity string
for _, decrypter := range secureValueDecrypters {
if _, exists := tokenActors[decrypter]; exists {
allowed = true
identity = decrypter
break
}
}
return identity, allowed
}

@ -0,0 +1,142 @@
package decrypt
import (
"context"
"testing"
"github.com/grafana/authlib/authn"
"github.com/grafana/authlib/types"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/apimachinery/identity"
)
func TestDecryptAuthorizer(t *testing.T) {
t.Run("when no auth info is present, it returns false", func(t *testing.T) {
ctx := context.Background()
authorizer := ProvideDecryptAuthorizer(nil)
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when token permissions are empty, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{})
authorizer := ProvideDecryptAuthorizer(nil)
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when permission format is malformed (missing verb), it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1"})
authorizer := ProvideDecryptAuthorizer(nil)
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when permission verb is not exactly `decrypt`, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1:something"})
authorizer := ProvideDecryptAuthorizer(nil)
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when permission does not have 3 parts, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues:decrypt"})
authorizer := ProvideDecryptAuthorizer(nil)
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when permission has group that is not `secret.grafana.app`, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"wrong.group/securevalues/invalid:decrypt"})
authorizer := ProvideDecryptAuthorizer(nil)
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when permission has resource that is not `securevalues`, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/invalid-resource/invalid:decrypt"})
authorizer := ProvideDecryptAuthorizer(nil)
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when the actor is not in the allow list, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/allowed2:decrypt"})
authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"allowed1": {}})
identity, allowed := authorizer.Authorize(ctx, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when the actor doesn't match any allowed decrypters, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1:decrypt"})
authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}})
identity, allowed := authorizer.Authorize(ctx, []string{"group2"})
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when the actor matches an allowed decrypter, it returns true", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1:decrypt"})
authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}})
identity, allowed := authorizer.Authorize(ctx, []string{"group1"})
require.True(t, allowed)
require.Equal(t, "group1", identity)
})
t.Run("when there are multiple permissions, some invalid, only valid ones are considered", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{
"secret.grafana.app/securevalues/group1:decrypt",
"secret.grafana.app/securevalues/invalid:read",
"wrong.group/securevalues/group2:decrypt",
"secret.grafana.app/securevalues/group2:decrypt",
})
authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}, "group2": {}})
identity, allowed := authorizer.Authorize(ctx, []string{"group2", "group3"})
require.True(t, allowed)
require.Equal(t, "group2", identity)
})
t.Run("when multiple valid actors match decrypters, the first match already returns true", func(t *testing.T) {
ctx := createAuthContext(context.Background(), []string{
"secret.grafana.app/securevalues/group1:decrypt",
"secret.grafana.app/securevalues/group2:decrypt",
})
authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}, "group2": {}})
identity, allowed := authorizer.Authorize(ctx, []string{"group2", "group1"})
require.True(t, allowed)
require.Equal(t, "group2", identity)
})
}
func createAuthContext(ctx context.Context, permissions []string) context.Context {
requester := &identity.StaticRequester{
AccessTokenClaims: &authn.Claims[authn.AccessTokenClaims]{
Rest: authn.AccessTokenClaims{
Permissions: permissions,
},
},
}
return types.WithAuthInfo(ctx, requester)
}

@ -0,0 +1,16 @@
package decrypt
import (
"context"
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
)
// NoopAlwaysAllowedAuthorizer is a no-op implementation of the DecryptAuthorizer which always returns `allowed=true`.
type NoopAlwaysAllowedAuthorizer struct{}
var _ contracts.DecryptAuthorizer = &NoopAlwaysAllowedAuthorizer{}
func (a *NoopAlwaysAllowedAuthorizer) Authorize(ctx context.Context, secureValueDecrypters []string) (identity string, allowed bool) {
return "", true
}

@ -423,6 +423,7 @@ var wireBasicSet = wire.NewSet(
secretmetadata.ProvideSecureValueMetadataStorage,
secretmetadata.ProvideKeeperMetadataStorage,
secretmetadata.ProvideDecryptStorage,
secretdecrypt.ProvideDecryptAuthorizer,
secretdecrypt.ProvideDecryptAllowList,
secretencryption.ProvideDataKeyStorage,
secretencryption.ProvideEncryptedValueStorage,

@ -3,7 +3,6 @@ package metadata
import (
"context"
"fmt"
"strings"
claims "github.com/grafana/authlib/types"
@ -21,7 +20,7 @@ func ProvideDecryptStorage(
keeperService secretkeeper.Service,
keeperMetadataStorage contracts.KeeperMetadataStorage,
secureValueMetadataStorage contracts.SecureValueMetadataStorage,
allowList contracts.DecryptAllowList,
decryptAuthorizer contracts.DecryptAuthorizer,
) (contracts.DecryptStorage, error) {
if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) ||
!features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) {
@ -33,11 +32,15 @@ func ProvideDecryptStorage(
return nil, fmt.Errorf("failed to get keepers: %w", err)
}
if decryptAuthorizer == nil {
return nil, fmt.Errorf("a decrypt authorizer is required")
}
return &decryptStorage{
keeperMetadataStorage: keeperMetadataStorage,
keepers: keepers,
secureValueMetadataStorage: secureValueMetadataStorage,
allowList: allowList,
decryptAuthorizer: decryptAuthorizer,
}, nil
}
@ -46,13 +49,13 @@ type decryptStorage struct {
keeperMetadataStorage contracts.KeeperMetadataStorage
keepers map[contracts.KeeperType]contracts.Keeper
secureValueMetadataStorage contracts.SecureValueMetadataStorage
allowList contracts.DecryptAllowList
decryptAuthorizer contracts.DecryptAuthorizer
}
// Decrypt decrypts a secure value from the keeper.
func (s *decryptStorage) Decrypt(ctx context.Context, namespace xkube.Namespace, name string) (secretv0alpha1.ExposedSecureValue, error) {
authInfo, ok := claims.AuthInfoFrom(ctx)
if !ok {
// Basic authn check before reading a secure value metadata, it is here on purpose.
if _, ok := claims.AuthInfoFrom(ctx); !ok {
return "", contracts.ErrDecryptNotAuthorized
}
@ -64,7 +67,7 @@ func (s *decryptStorage) Decrypt(ctx context.Context, namespace xkube.Namespace,
return "", contracts.ErrDecryptNotFound
}
identity, authorized := s.authorize(authInfo, sv.Spec.Decrypters)
identity, authorized := s.decryptAuthorizer.Authorize(ctx, sv.Spec.Decrypters)
if !authorized {
return "", contracts.ErrDecryptNotAuthorized
}
@ -89,67 +92,3 @@ func (s *decryptStorage) Decrypt(ctx context.Context, namespace xkube.Namespace,
return exposedValue, nil
}
// authorize checks whether the auth info token has the right permissions to decrypt the secure value.
func (s *decryptStorage) authorize(authInfo claims.AuthInfo, svDecrypters []string) (string, bool) {
tokenPermissions := authInfo.GetTokenPermissions()
tokenActors := make(map[string]struct{}, 0)
for _, permission := range tokenPermissions {
// Will look like `secret.grafana.app/securevalues/<actor>:decrypt` for now.
gr, verb, found := strings.Cut(permission, ":")
if !found {
continue
}
// If it isn't decrypt, then we don't care to check.
if verb != "decrypt" {
continue
}
parts := strings.Split(gr, "/")
if len(parts) != 3 {
continue
}
group, resource, actor := parts[0], parts[1], parts[2]
if group != secretv0alpha1.GROUP || resource != secretv0alpha1.SecureValuesResourceInfo.GetName() || actor == "" {
continue
}
// TEMPORARY: while we can't onboard every app into secrets, we can block them from decrypting
// securevalues preemptively here before even reaching out to the database.
// This check can be removed once we open the gates for any service to use secrets.
if _, exists := s.allowList[actor]; !exists {
continue
}
tokenActors[actor] = struct{}{}
}
// If we arrived here and the token actors is empty, it means the permissions either have an invalid format,
// or it didn't pass the allow list, meaning no allowed decryptor.
if len(tokenActors) == 0 {
return "", false
}
// TEMPORARY: while we still need to mix permission and identity, we can use this
// to decide whether the SecureValue can be decrypted or not.
// Once we have an `actor` field in the JWT claims, we can have a properly formatted permission,
// like `secret.grafana.app/securevalues{/<name>}:decrypt` and do regular access control eval,
// and for the `decrypters` part here, we can just check it against the `actor` field, which at
// that point will have a different format, depending on how the `actor` will be formatted.
// Check whether at least one of declared token actors matches the allowed decrypters from the SecureValue.
allowed := false
var identity string
for _, decrypter := range svDecrypters {
if _, exists := tokenActors[decrypter]; exists {
allowed = true
identity = decrypter
break
}
}
return identity, allowed
}

@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/infra/usagestats"
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
"github.com/grafana/grafana/pkg/registry/apis/secret/decrypt"
"github.com/grafana/grafana/pkg/registry/apis/secret/encryption"
"github.com/grafana/grafana/pkg/registry/apis/secret/encryption/cipher"
encryptionmanager "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/manager"
@ -310,8 +311,10 @@ func setupDecryptTestService(t *testing.T, allowList map[string]struct{}) (*decr
secureValueMetadataStorage, err := ProvideSecureValueMetadataStorage(db, features, accessClient, keeperMetadataStorage, keeperService)
require.NoError(t, err)
decryptAuthorizer := decrypt.ProvideDecryptAuthorizer(allowList)
// Initialize the decrypt storage
decryptSvc, err := ProvideDecryptStorage(features, keeperService, keeperMetadataStorage, secureValueMetadataStorage, allowList)
decryptSvc, err := ProvideDecryptStorage(features, keeperService, keeperMetadataStorage, secureValueMetadataStorage, decryptAuthorizer)
require.NoError(t, err)
return decryptSvc.(*decryptStorage), secureValueMetadataStorage

Loading…
Cancel
Save