From 0214399017550a748b3dd8f99d21a53199904a3d Mon Sep 17 00:00:00 2001 From: Hugo <10965479+hugoghx@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:18:35 +0000 Subject: [PATCH] feat(plugin): Add credential rotation primitives --- plugin/service/storage/rotation.go | 88 +++++++++++++++ plugin/service/storage/rotation_test.go | 137 ++++++++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 plugin/service/storage/rotation.go create mode 100644 plugin/service/storage/rotation_test.go diff --git a/plugin/service/storage/rotation.go b/plugin/service/storage/rotation.go new file mode 100644 index 0000000..2db5484 --- /dev/null +++ b/plugin/service/storage/rotation.go @@ -0,0 +1,88 @@ +package storage + +import ( + "context" + "encoding/json" + "fmt" + "sync" + "time" + + "github.com/minio/madmin-go/v3" +) + +const ( + defaultPolicyTemplate = `{ + "Statement": [ + { + "Action": [ + "s3:GetObject", + "s3:GetObjectAttributes", + "s3:PutObject", + "s3:DeleteObject" + ], + "Effect": "Allow", + "Resource": "arn:aws:s3:::%[1]s/*" + }, + { + "Action": "s3:ListBucket", + "Effect": "Allow", + "Resource": "arn:aws:s3:::%[1]s" + }, + { + "Action": [ + "admin:CreateServiceAccount", + "admin:RemoveServiceAccount" + ], + "Effect": "Allow" + } + ], + "Version": "2012-10-17" + }` +) + +type deleteInputCredsFn func() error + +// rotateCredentials uses the incoming service account credentials to generate a +// new service account. It returns a new StorageSecrets object with the new +// service account credentials and a callback function to delete the service +// account provided in the input. +func rotateCredentials(ctx context.Context, bucketName string, sa *StorageAttributes, inSec *StorageSecrets) (*StorageSecrets, deleteInputCredsFn, error) { + err := ensureServiceAccount(ctx, sa, inSec) + if err != nil { + return nil, nil, fmt.Errorf("failed to ensure minio service account credentials: %w", err) + } + + ac, err := newMadminClient(sa, inSec) + if err != nil { + return nil, nil, fmt.Errorf("failed to create minio admin client: %w", err) + } + + policy := json.RawMessage(fmt.Sprintf(defaultPolicyTemplate, bucketName)) + newCreds, err := ac.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ + Name: "Boundary MinIO Service Account", + Description: fmt.Sprintf("Boundary-managed MinIO service account for storage bucket '%s'", bucketName), + Policy: policy, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to create new minio service account: %w", err) + } + + newSec := &StorageSecrets{ + AccessKeyId: newCreds.AccessKey, + SecretAccessKey: newCreds.SecretKey, + LastRotatedTime: time.Now(), + } + + inCl, newCl := inSec.Clone(), newSec.Clone() + return newSec, sync.OnceValue(func() error { + ac, err := newMadminClient(sa, newCl) + if err != nil { + return fmt.Errorf("failed to create minio admin client: %w", err) + } + err = ac.DeleteServiceAccount(ctx, inCl.AccessKeyId) + if err != nil { + return fmt.Errorf("failed to delete minio service account: %w", err) + } + return nil + }), nil +} diff --git a/plugin/service/storage/rotation_test.go b/plugin/service/storage/rotation_test.go new file mode 100644 index 0000000..c1b724a --- /dev/null +++ b/plugin/service/storage/rotation_test.go @@ -0,0 +1,137 @@ +package storage + +import ( + "context" + "encoding/json" + "testing" + + internaltest "github.com/hashicorp/boundary-plugin-minio/internal/testing" + "github.com/minio/madmin-go/v3" + "github.com/minio/minio-go/v7" + "github.com/stretchr/testify/require" +) + +func TestRotateCredentials(t *testing.T) { + ctx := context.Background() + server := internaltest.NewMinioServer(t) + + bucketName := "test-bucket" + err := server.Client.MakeBucket(ctx, bucketName, minio.MakeBucketOptions{}) + require.NoError(t, err) + + tests := []struct { + name string + ss *StorageSecrets + expErrMsg string + credDelErr bool + }{ + { + name: "notUsingServiceAccount", + ss: &StorageSecrets{ + AccessKeyId: server.RootUsername, + SecretAccessKey: server.RootPassword, + }, + expErrMsg: "failed to ensure minio service account credentials: failed to obtain service account info: The specified service account is not found", + }, + { + name: "addServiceAccountFail", + ss: func() *StorageSecrets { + creds, err := server.AdminClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ + Policy: json.RawMessage(` + { + "Statement": [ + { + "Action": [ + "admin:CreateServiceAccount", + "admin:RemoveServiceAccount" + ], + "Effect": "Deny" + } + ], + "Version": "2012-10-17" + } + `), + }) + require.NoError(t, err) + + return &StorageSecrets{ + AccessKeyId: creds.AccessKey, + SecretAccessKey: creds.SecretKey, + } + }(), + expErrMsg: "failed to create new minio service account: Access Denied", + }, + { + name: "credentialDeletionFail", + ss: func() *StorageSecrets { + creds, err := server.AdminClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{}) + require.NoError(t, err) + + return &StorageSecrets{ + AccessKeyId: creds.AccessKey, + SecretAccessKey: creds.SecretKey, + } + }(), + credDelErr: true, + }, + { + name: "success", + ss: func() *StorageSecrets { + creds, err := server.AdminClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{}) + require.NoError(t, err) + + return &StorageSecrets{ + AccessKeyId: creds.AccessKey, + SecretAccessKey: creds.SecretKey, + } + }(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sa := &StorageAttributes{ + EndpointUrl: server.ApiAddr, + UseSSL: false, + } + + ss, delFn, err := rotateCredentials(context.Background(), bucketName, sa, tt.ss) + if tt.expErrMsg != "" { + require.ErrorContains(t, err, tt.expErrMsg) + require.Nil(t, ss) + require.Nil(t, delFn) + return + } + + require.NoError(t, err) + require.NotNil(t, ss) + require.NotNil(t, delFn) + require.NotEqual(t, ss.AccessKeyId, tt.ss.AccessKeyId) + require.NotEqual(t, ss.SecretAccessKey, tt.ss.SecretAccessKey) + require.NotEmpty(t, ss.LastRotatedTime) + + // Ensure old creds still exist since the callback hasn't been + // called. We then make the delete call and finally assert that the + // credential was removed if there was no error when attempting to + // delete. + _, err = server.AdminClient.InfoServiceAccount(ctx, tt.ss.AccessKeyId) + require.NoError(t, err) + + if tt.credDelErr { + sa.EndpointUrl = "localhost:1" + require.ErrorContains(t, delFn(), "failed to delete minio service account") + require.ErrorContains(t, delFn(), "failed to delete minio service account") // 2nd call should return the same state (due to sync.Once). + + _, err := server.AdminClient.InfoServiceAccount(ctx, tt.ss.AccessKeyId) + require.NoError(t, err) + return + } else { + require.NoError(t, delFn()) + require.NoError(t, delFn()) // 2nd call should return the same state (due to sync.Once). + } + + _, err = server.AdminClient.InfoServiceAccount(ctx, tt.ss.AccessKeyId) + require.ErrorContains(t, err, "service account is not found") + }) + } +}