From b08de452315aab5d6bcef59c0a8514fd7c49b9c4 Mon Sep 17 00:00:00 2001 From: Robert Jacob Date: Wed, 21 Feb 2024 19:14:49 +0100 Subject: [PATCH] feat(operator): Extend Azure secret validation (#12007) Co-authored-by: Periklis Tsirakidis --- operator/CHANGELOG.md | 1 + .../handlers/internal/storage/secrets.go | 33 ++++++++++++++-- .../handlers/internal/storage/secrets_test.go | 39 ++++++++++++------- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/operator/CHANGELOG.md b/operator/CHANGELOG.md index 8dae6eced0bfa..e6aaec29b99c7 100644 --- a/operator/CHANGELOG.md +++ b/operator/CHANGELOG.md @@ -1,5 +1,6 @@ ## Main +- [12007](https://github.com/grafana/loki/pull/12007) **xperimental**: Extend Azure secret validation - [12008](https://github.com/grafana/loki/pull/12008) **xperimental**: Support using multiple buckets with AWS STS - [11964](https://github.com/grafana/loki/pull/11964) **xperimental**: Provide Azure region for managed credentials using environment variable - [11920](https://github.com/grafana/loki/pull/11920) **xperimental**: Refactor handling of credentials in managed-auth mode diff --git a/operator/internal/handlers/internal/storage/secrets.go b/operator/internal/handlers/internal/storage/secrets.go index 2492eea4d4191..80dde97b61367 100644 --- a/operator/internal/handlers/internal/storage/secrets.go +++ b/operator/internal/handlers/internal/storage/secrets.go @@ -1,11 +1,14 @@ package storage import ( + "bytes" "context" "crypto/sha1" + "encoding/base64" "encoding/json" "errors" "fmt" + "io" "sort" corev1 "k8s.io/api/core/v1" @@ -33,9 +36,18 @@ var ( errAzureNoCredentials = errors.New("azure storage secret does contain neither account_key or client_id") errAzureMixedCredentials = errors.New("azure storage secret can not contain both account_key and client_id") errAzureManagedIdentityNoOverride = errors.New("when in managed mode, storage secret can not contain credentials") + errAzureInvalidEnvironment = errors.New("azure environment invalid (valid values: AzureGlobal, AzureChinaCloud, AzureGermanCloud, AzureUSGovernment)") + errAzureInvalidAccountKey = errors.New("azure account key is not valid base64") errGCPParseCredentialsFile = errors.New("gcp storage secret cannot be parsed from JSON content") errGCPWrongCredentialSourceFile = errors.New("credential source in secret needs to point to token file") + + azureValidEnvironments = map[string]bool{ + "AzureGlobal": true, + "AzureChinaCloud": true, + "AzureGermanCloud": true, + "AzureUSGovernment": true, + } ) const gcpAccountTypeExternal = "external_account" @@ -159,11 +171,15 @@ func hashSecretData(s *corev1.Secret) (string, error) { func extractAzureConfigSecret(s *corev1.Secret, fg configv1.FeatureGates) (*storage.AzureStorageConfig, error) { // Extract and validate mandatory fields - env := s.Data[storage.KeyAzureEnvironmentName] - if len(env) == 0 { + env := string(s.Data[storage.KeyAzureEnvironmentName]) + if env == "" { return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureEnvironmentName) } + if !azureValidEnvironments[env] { + return nil, fmt.Errorf("%w: %s", errAzureInvalidEnvironment, env) + } + accountName := s.Data[storage.KeyAzureStorageAccountName] if len(accountName) == 0 { return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageAccountName) @@ -188,7 +204,7 @@ func extractAzureConfigSecret(s *corev1.Secret, fg configv1.FeatureGates) (*stor } return &storage.AzureStorageConfig{ - Env: string(env), + Env: env, Container: string(container), EndpointSuffix: string(endpointSuffix), Audience: string(audience), @@ -219,6 +235,10 @@ func validateAzureCredentials(s *corev1.Secret, fg configv1.FeatureGates) (workl } if len(accountKey) > 0 { + if err := validateBase64(accountKey); err != nil { + return false, errAzureInvalidAccountKey + } + // have both account_name and account_key -> no workload identity federation return false, nil } @@ -235,6 +255,13 @@ func validateAzureCredentials(s *corev1.Secret, fg configv1.FeatureGates) (workl return true, nil } +func validateBase64(data []byte) error { + buf := bytes.NewBuffer(data) + reader := base64.NewDecoder(base64.StdEncoding, buf) + _, err := io.ReadAll(reader) + return err +} + func extractGCSConfigSecret(s *corev1.Secret) (*storage.GCSStorageConfig, error) { // Extract and validate mandatory fields bucket := s.Data[storage.KeyGCPStorageBucketName] diff --git a/operator/internal/handlers/internal/storage/secrets_test.go b/operator/internal/handlers/internal/storage/secrets_test.go index ca3623b718c1b..647de5632b4bf 100644 --- a/operator/internal/handlers/internal/storage/secrets_test.go +++ b/operator/internal/handlers/internal/storage/secrets_test.go @@ -84,11 +84,20 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{}, wantError: "missing secret field: environment", }, + { + name: "invalid environment", + secret: &corev1.Secret{ + Data: map[string][]byte{ + "environment": []byte("invalid-environment"), + }, + }, + wantError: "azure environment invalid (valid values: AzureGlobal, AzureChinaCloud, AzureGermanCloud, AzureUSGovernment): invalid-environment", + }, { name: "missing account_name", secret: &corev1.Secret{ Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), }, }, wantError: "missing secret field: account_name", @@ -97,7 +106,7 @@ func TestAzureExtract(t *testing.T) { name: "missing container", secret: &corev1.Secret{ Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "account_name": []byte("id"), }, }, @@ -108,7 +117,7 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("id"), }, @@ -120,7 +129,7 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("test-account-name"), "account_key": []byte("test-account-key"), @@ -134,7 +143,7 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("test-account-name"), "client_id": []byte("test-client-id"), @@ -147,7 +156,7 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("test-account-name"), "client_id": []byte("test-client-id"), @@ -161,7 +170,7 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "account_name": []byte("test-account-name"), "container": []byte("this,that"), "region": []byte("test-region"), @@ -184,10 +193,10 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("id"), - "account_key": []byte("secret"), + "account_key": []byte("dGVzdC1hY2NvdW50LWtleQ=="), // test-account-key "audience": []byte("test-audience"), }, }, @@ -198,10 +207,10 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("id"), - "account_key": []byte("secret"), + "account_key": []byte("dGVzdC1hY2NvdW50LWtleQ=="), // test-account-key }, }, wantCredentialMode: lokiv1.CredentialModeStatic, @@ -211,7 +220,7 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("test-account-name"), "client_id": []byte("test-client-id"), @@ -227,7 +236,7 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "account_name": []byte("test-account-name"), "container": []byte("this,that"), "region": []byte("test-region"), @@ -256,10 +265,10 @@ func TestAzureExtract(t *testing.T) { secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Data: map[string][]byte{ - "environment": []byte("here"), + "environment": []byte("AzureGlobal"), "container": []byte("this,that"), "account_name": []byte("id"), - "account_key": []byte("secret"), + "account_key": []byte("dGVzdC1hY2NvdW50LWtleQ=="), // test-account-key "endpoint_suffix": []byte("suffix"), }, },