From ce23f4f2de2d818cd18807d93b7780fd9e7d15af Mon Sep 17 00:00:00 2001 From: Vault Automation Date: Fri, 19 Dec 2025 11:57:08 -0700 Subject: [PATCH] VAULT-41425: AWS secrets engine observations (#11395) (#11514) * add observations for the aws secrets engine * add mock recorder * add tests to verify observations are created * fix comment * update godoc and switch to require * fix type assertion, add test Co-authored-by: miagilepner --- builtin/logical/aws/observation_consts.go | 58 +++++++++++++++++ builtin/logical/aws/path_config_lease.go | 9 +++ builtin/logical/aws/path_config_root.go | 14 ++++ builtin/logical/aws/path_config_root_test.go | 50 +++++++++++++-- .../logical/aws/path_config_rotate_root.go | 7 ++ builtin/logical/aws/path_roles.go | 19 +++++- builtin/logical/aws/path_roles_test.go | 64 ++++++++++++++++++- builtin/logical/aws/path_static_creds.go | 4 ++ builtin/logical/aws/path_static_roles.go | 23 ++++++- builtin/logical/aws/path_user.go | 17 ++++- builtin/logical/aws/rotation.go | 5 ++ builtin/logical/aws/rotation_test.go | 8 ++- builtin/logical/aws/secret_access_keys.go | 38 +++++++++++ sdk/framework/backend.go | 35 ++++++++++ .../testhelpers/observations/recorder.go | 64 +++++++++++++++++++ 15 files changed, 402 insertions(+), 13 deletions(-) create mode 100644 builtin/logical/aws/observation_consts.go create mode 100644 sdk/helper/testhelpers/observations/recorder.go diff --git a/builtin/logical/aws/observation_consts.go b/builtin/logical/aws/observation_consts.go new file mode 100644 index 0000000000..6152be4406 --- /dev/null +++ b/builtin/logical/aws/observation_consts.go @@ -0,0 +1,58 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package aws + +const ( + + // Root config observations + // These observations have rotation_period, rotation_schedule, + // rotation_window, and disable_automatic_rotation metadata + + ObservationTypeAWSRootConfigWrite = "aws/config/root/write" + ObservationTypeAWSRootConfigRead = "aws/config/root/read" + ObservationTypeAWSRootConfigRotate = "aws/config/root/rotate" + + // Lease config observations + // These observations have lease and lease_max durations + + ObservationTypeAWSLeaseConfigWrite = "aws/config/lease/write" + ObservationTypeAWSLeaseConfigRead = "aws/config/lease/read" + + // Role related observations + // These observations have role_name and credentials_type metadata + + ObservationTypeAWSRoleWrite = "aws/role/write" + ObservationTypeAWSRoleRead = "aws/role/read" + + // Role delete observation + // This observation only has role_name metadata to avoid doing a storage read + + ObservationTypeAWSRoleDelete = "aws/role/delete" + + // Static role related observations + // These observations have role_name and rotation_period metadata + + ObservationTypeAWSStaticRoleWrite = "aws/static-role/write" + ObservationTypeAWSStaticRoleRead = "aws/static-role/read" + ObservationTypeAWSStaticRoleDelete = "aws/static-role/delete" + + // Credential related observations + // These observations have role_name, credentials_type, ttl, max_ttl, and is_sts metadata + + ObservationTypeAWSCredentialCreateSuccess = "aws/credential/create/success" + ObservationTypeAWSCredentialCreateFail = "aws/credential/create/fail" + + // Secret lifecycle observations + // These observations don't have access to the role_name + // They only have is_sts and credentials_type metadata + + ObservationTypeAWSCredentialRenew = "aws/credential/renew" + ObservationTypeAWSCredentialRevoke = "aws/credential/revoke" + + // Static credential related observations + // These observations have role_name + + ObservationTypeAWSStaticCredentialRead = "aws/static-credential/read" + ObservationTypeAWSStaticCredentialRotate = "aws/static-credential/rotate" +) diff --git a/builtin/logical/aws/path_config_lease.go b/builtin/logical/aws/path_config_lease.go index e27cb9cc12..dda2d885f9 100644 --- a/builtin/logical/aws/path_config_lease.go +++ b/builtin/logical/aws/path_config_lease.go @@ -106,6 +106,10 @@ func (b *backend) pathLeaseWrite(ctx context.Context, req *logical.Request, d *f return nil, err } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSLeaseConfigWrite, map[string]interface{}{ + "lease": lease.String(), + "lease_max": leaseMax.String(), + }) return nil, nil } @@ -118,6 +122,11 @@ func (b *backend) pathLeaseRead(ctx context.Context, req *logical.Request, data return nil, nil } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSLeaseConfigRead, map[string]interface{}{ + "lease": lease.Lease.String(), + "lease_max": lease.LeaseMax.String(), + }) + return &logical.Response{ Data: map[string]interface{}{ "lease": lease.Lease.String(), diff --git a/builtin/logical/aws/path_config_root.go b/builtin/logical/aws/path_config_root.go index 0f58a3511b..fa525db8d6 100644 --- a/builtin/logical/aws/path_config_root.go +++ b/builtin/logical/aws/path_config_root.go @@ -157,6 +157,13 @@ func (b *backend) pathConfigRootRead(ctx context.Context, req *logical.Request, config.PopulatePluginIdentityTokenData(configData) config.PopulateAutomatedRotationData(configData) + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSRootConfigRead, map[string]interface{}{ + "rotation_period": config.RotationPeriod.String(), + "rotation_schedule": config.RotationSchedule, + "rotation_window": config.RotationWindow.String(), + "disable_automatic_rotation": config.DisableAutomatedRotation, + }) + return &logical.Response{ Data: configData, }, nil @@ -316,6 +323,13 @@ func (b *backend) pathConfigRootWrite(ctx context.Context, req *logical.Request, return nil, wrappedError } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSRootConfigWrite, map[string]interface{}{ + "rotation_period": rc.RotationPeriod.String(), + "rotation_schedule": rc.RotationSchedule, + "rotation_window": rc.RotationWindow.String(), + "disable_automatic_rotation": rc.DisableAutomatedRotation, + }) + // clear possible cached IAM / STS clients after successfully updating // config/root b.iamClient = nil diff --git a/builtin/logical/aws/path_config_root_test.go b/builtin/logical/aws/path_config_root_test.go index b82a63d349..242322973c 100644 --- a/builtin/logical/aws/path_config_root_test.go +++ b/builtin/logical/aws/path_config_root_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/vault/sdk/helper/automatedrotationutil" "github.com/hashicorp/vault/sdk/helper/pluginidentityutil" "github.com/hashicorp/vault/sdk/helper/pluginutil" + "github.com/hashicorp/vault/sdk/helper/testhelpers/observations" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/rotation" "github.com/stretchr/testify/assert" @@ -23,7 +24,8 @@ func TestBackend_PathConfigRoot(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} - + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { t.Fatal(err) @@ -60,6 +62,7 @@ func TestBackend_PathConfigRoot(t *testing.T) { t.Fatalf("bad: config writing failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, Storage: config.StorageView, @@ -68,6 +71,7 @@ func TestBackend_PathConfigRoot(t *testing.T) { if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("bad: config reading failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigRead)) // Ensure default values are enforced configData["max_retries"] = -1 @@ -111,6 +115,7 @@ func TestBackend_PathConfigRoot(t *testing.T) { if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("bad: config writing failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 2, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, @@ -120,6 +125,7 @@ func TestBackend_PathConfigRoot(t *testing.T) { if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("bad: config reading failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 2, or.NumObservationsByType(ObservationTypeAWSRootConfigRead)) delete(configData, "secret_key") require.Equal(t, configData, resp.Data) @@ -131,8 +137,10 @@ func TestBackend_PathConfigRoot(t *testing.T) { // TestBackend_PathConfigRoot_STSFallback tests valid versions of STS fallback parameters - slice and csv func TestBackend_PathConfigRoot_STSFallback(t *testing.T) { config := logical.TestBackendConfig() + or := observations.NewTestObservationRecorder() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -170,6 +178,7 @@ func TestBackend_PathConfigRoot_STSFallback(t *testing.T) { t.Fatalf("bad: config writing failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, Storage: config.StorageView, @@ -178,6 +187,7 @@ func TestBackend_PathConfigRoot_STSFallback(t *testing.T) { if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("bad: config reading failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigRead)) delete(configData, "secret_key") // remove rotation_period from response for comparison with original config @@ -219,6 +229,7 @@ func TestBackend_PathConfigRoot_STSFallback(t *testing.T) { t.Fatalf("bad: config writing failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 2, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, Storage: config.StorageView, @@ -228,6 +239,8 @@ func TestBackend_PathConfigRoot_STSFallback(t *testing.T) { t.Fatalf("bad: config reading failed: resp:%#v\n err: %v", resp, err) } + require.Equal(t, 2, or.NumObservationsByType(ObservationTypeAWSRootConfigRead)) + delete(configData, "secret_key") // remove rotation_period from response for comparison with original config delete(resp.Data, "rotation_period") @@ -240,18 +253,20 @@ func TestBackend_PathConfigRoot_STSFallback(t *testing.T) { } // TestBackend_PathConfigRoot_STSFallback_mismatchedfallback ensures configuration writing will fail if the -// region/endpoint entries are different lengths +// sts fallback regions and sts fallback endpoints entries are different lengths func TestBackend_PathConfigRoot_STSFallback_mismatchedfallback(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { t.Fatal(err) } - // test we can handle comma separated strings, per CommaStringSlice + // sts fallback endpoints has 2 entries, regions has 1 configData := map[string]interface{}{ "access_key": "AKIAEXAMPLE", "secret_key": "RandomData", @@ -260,7 +275,7 @@ func TestBackend_PathConfigRoot_STSFallback_mismatchedfallback(t *testing.T) { "sts_endpoint": "https://sts.us-west-2.amazonaws.com", "sts_region": "", "sts_fallback_endpoints": "1.1.1.1,8.8.8.8", - "sts_fallback_regions": "zone-1,zone-2", + "sts_fallback_regions": "zone-1", "max_retries": 10, "username_template": defaultUserNameTemplate, "role_arn": "", @@ -279,9 +294,9 @@ func TestBackend_PathConfigRoot_STSFallback_mismatchedfallback(t *testing.T) { if err != nil { t.Fatalf("bad: config writing failed: err: %v", err) } - if resp != nil && !resp.IsError() { - t.Fatalf("expected an error, but it successfully wrote") - } + require.NotNil(t, resp) + require.True(t, resp.IsError()) + require.Equal(t, 0, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) } // TestBackend_PathConfigRoot_STSFallback_defaultEndpointRegion ensures that if no endpoints are specified, we can @@ -290,6 +305,8 @@ func TestBackend_PathConfigRoot_STSFallback_defaultEndpointRegion(t *testing.T) config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -330,6 +347,7 @@ func TestBackend_PathConfigRoot_STSFallback_defaultEndpointRegion(t *testing.T) t.Fatalf("region and endpoint didn't match: %s vs. %s", *(cfg.Region), *(cfg.Endpoint)) } } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) } // TestBackend_PathConfigRoot_IAM_specifiedRegion ensures that if a region is set, we get a good config (with a blank @@ -338,6 +356,8 @@ func TestBackend_PathConfigRoot_IAM_specifiedRegion(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -369,6 +389,7 @@ func TestBackend_PathConfigRoot_IAM_specifiedRegion(t *testing.T) { t.Fatalf("bad: config writing failed: err: %v", err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) cfg, err := b.getRootIAMConfig(context.Background(), config.StorageView, b.Logger()) if err != nil { t.Fatalf("couldn't get IAM configs with default region/endpoints: %v", err) @@ -385,8 +406,10 @@ func TestBackend_PathConfigRoot_IAM_specifiedRegion(t *testing.T) { // good config func TestBackend_PathConfigRoot_IAM_specifiedRegionAndEndpoint(t *testing.T) { config := logical.TestBackendConfig() + or := observations.NewTestObservationRecorder() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -420,6 +443,7 @@ func TestBackend_PathConfigRoot_IAM_specifiedRegionAndEndpoint(t *testing.T) { t.Fatalf("bad: config writing failed: err: %v", err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) cfg, err := b.getRootIAMConfig(context.Background(), config.StorageView, b.Logger()) if err != nil { t.Fatalf("couldn't get IAM configs with default region/endpoints: %v", err) @@ -439,6 +463,8 @@ func TestBackend_PathConfigRoot_IAM_defaultEndpointRegion(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -467,6 +493,7 @@ func TestBackend_PathConfigRoot_IAM_defaultEndpointRegion(t *testing.T) { t.Fatalf("bad: config writing failed: err: %v", err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) cfg, err := b.getRootIAMConfig(context.Background(), config.StorageView, b.Logger()) if err != nil { t.Fatalf("couldn't get IAM configs with default region/endpoints: %v", err) @@ -483,6 +510,8 @@ func TestBackend_PathConfigRoot_STSIAM_SetEverything(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -520,6 +549,7 @@ func TestBackend_PathConfigRoot_STSIAM_SetEverything(t *testing.T) { t.Fatalf("bad: config writing failed: err: %v", err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) // get IAM cfg, err := b.getRootIAMConfig(context.Background(), config.StorageView, b.Logger()) if err != nil { @@ -549,6 +579,8 @@ func TestBackend_PathConfigRoot_PluginIdentityToken(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -572,6 +604,7 @@ func TestBackend_PathConfigRoot_PluginIdentityToken(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, resp) assert.ErrorContains(t, resp.Error(), pluginidentityutil.ErrPluginWorkloadIdentityUnsupported.Error()) + require.Equal(t, 0, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) } // TestBackend_PathConfigRoot_RegisterRootRotation tests that configuration @@ -580,6 +613,8 @@ func TestBackend_PathConfigRoot_RegisterRootRotation(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} config.System = &testSystemView{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or nsCtx := namespace.ContextWithNamespace(context.Background(), namespace.RootNamespace) @@ -606,6 +641,7 @@ func TestBackend_PathConfigRoot_RegisterRootRotation(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, resp) assert.ErrorContains(t, resp.Error(), automatedrotationutil.ErrRotationManagerUnsupported.Error()) + require.Equal(t, 0, or.NumObservationsByType(ObservationTypeAWSRootConfigWrite)) } type testSystemView struct { diff --git a/builtin/logical/aws/path_config_rotate_root.go b/builtin/logical/aws/path_config_rotate_root.go index 7da2e53f79..381fa25709 100644 --- a/builtin/logical/aws/path_config_rotate_root.go +++ b/builtin/logical/aws/path_config_rotate_root.go @@ -123,6 +123,13 @@ func (b *backend) rotateRoot(ctx context.Context, req *logical.Request) (*logica return nil, fmt.Errorf("error deleting old access key: %w", err) } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSRootConfigRotate, map[string]interface{}{ + "rotation_period": config.RotationPeriod.String(), + "rotation_schedule": config.RotationSchedule, + "rotation_window": config.RotationWindow.String(), + "disable_automatic_rotation": config.DisableAutomatedRotation, + }) + return &logical.Response{ Data: map[string]interface{}{ "access_key": config.AccessKey, diff --git a/builtin/logical/aws/path_roles.go b/builtin/logical/aws/path_roles.go index b2153c6b44..43f8887b8b 100644 --- a/builtin/logical/aws/path_roles.go +++ b/builtin/logical/aws/path_roles.go @@ -215,18 +215,24 @@ func (b *backend) pathRoleList(ctx context.Context, req *logical.Request, d *fra } func (b *backend) pathRolesDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + roleName := d.Get("name").(string) for _, prefix := range []string{"policy/", "role/"} { - err := req.Storage.Delete(ctx, prefix+d.Get("name").(string)) + err := req.Storage.Delete(ctx, prefix+roleName) if err != nil { return nil, err } } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSRoleDelete, map[string]interface{}{ + "role_name": roleName, + }) + return nil, nil } func (b *backend) pathRolesRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - entry, err := b.roleRead(ctx, req.Storage, d.Get("name").(string), true) + roleName := d.Get("name").(string) + entry, err := b.roleRead(ctx, req.Storage, roleName, true) if err != nil { return nil, err } @@ -234,6 +240,10 @@ func (b *backend) pathRolesRead(ctx context.Context, req *logical.Request, d *fr return nil, nil } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSRoleRead, map[string]interface{}{ + "credential_types": entry.CredentialTypes, + "role_name": roleName, + }) return &logical.Response{ Data: entry.toResponseData(), }, nil @@ -385,6 +395,11 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f return nil, err } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSRoleWrite, map[string]interface{}{ + "credential_types": roleEntry.CredentialTypes, + "role_name": roleName, + }) + if len(resp.Warnings) == 0 { return nil, nil } diff --git a/builtin/logical/aws/path_roles_test.go b/builtin/logical/aws/path_roles_test.go index 7ba0c5e70c..6daaf8863e 100644 --- a/builtin/logical/aws/path_roles_test.go +++ b/builtin/logical/aws/path_roles_test.go @@ -12,7 +12,9 @@ import ( "testing" "github.com/hashicorp/go-multierror" + "github.com/hashicorp/vault/sdk/helper/testhelpers/observations" "github.com/hashicorp/vault/sdk/logical" + "github.com/stretchr/testify/require" ) const adminAccessPolicyARN = "arn:aws:iam::aws:policy/AdministratorAccess" @@ -22,6 +24,8 @@ func TestBackend_PathListRoles(t *testing.T) { var err error config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -49,6 +53,8 @@ func TestBackend_PathListRoles(t *testing.T) { } } + require.Equal(t, 10, or.NumObservationsByType(ObservationTypeAWSRoleWrite)) + resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ListOperation, Path: "roles", @@ -225,7 +231,8 @@ func TestRoleCRUDWithPermissionsBoundary(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} - + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { t.Fatal(err) @@ -249,6 +256,8 @@ func TestRoleCRUDWithPermissionsBoundary(t *testing.T) { t.Fatalf("bad: role creation failed. resp:%#v\nerr:%v", resp, err) } + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRoleWrite)) + request = &logical.Request{ Operation: logical.ReadOperation, Path: "roles/" + roleName, @@ -258,6 +267,9 @@ func TestRoleCRUDWithPermissionsBoundary(t *testing.T) { if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("bad: reading role failed. resp:%#v\nerr:%v", resp, err) } + + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRoleRead)) + if resp.Data["credential_type"] != iamUserCred { t.Errorf("bad: expected credential_type of %s, got %s instead", iamUserCred, resp.Data["credential_type"]) } @@ -269,6 +281,8 @@ func TestRoleCRUDWithPermissionsBoundary(t *testing.T) { func TestRoleWithPermissionsBoundaryValidation(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) if err := b.Setup(context.Background(), config); err != nil { @@ -301,6 +315,8 @@ func TestRoleWithPermissionsBoundaryValidation(t *testing.T) { if err == nil && (resp == nil || !resp.IsError()) { t.Fatalf("bad: expected role creation to fail due to malformed permissions_boundary_arn, but it didn't. resp:%#v\nerr:%v", resp, err) } + + require.Equal(t, 0, or.NumObservationsByType(ObservationTypeAWSRoleWrite)) } func TestValidateAWSManagedPolicy(t *testing.T) { @@ -508,3 +524,49 @@ func TestRoleEntryValidationFederationTokenCred(t *testing.T) { t.Errorf("bad: invalid roleEntry with unrecognized PermissionsBoundary %#v passed validation", roleEntry) } } + +// TestRoleWriteObservationMetadata verifies that when a role is created, an observation +// is recorded with the correct metadata. +func TestRoleWriteObservationMetadata(t *testing.T) { + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or + + b := Backend(config) + if err := b.Setup(context.Background(), config); err != nil { + t.Fatal(err) + } + + roleName := "test_observation_role" + roleData := map[string]interface{}{ + "credential_type": assumedRoleCred, + "role_arns": []string{"arn:aws:iam::123456789012:role/TestRole"}, + "default_sts_ttl": 3600, + "max_sts_ttl": 7200, + } + + request := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "roles/" + roleName, + Storage: config.StorageView, + Data: roleData, + } + + resp, err := b.HandleRequest(context.Background(), request) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: role creation failed. resp:%#v\nerr:%v", resp, err) + } + + // Verify observation was recorded + require.Equal(t, 1, or.NumObservationsByType(ObservationTypeAWSRoleWrite)) + + // Get the observation and verify metadata + observations := or.ObservationsByType(ObservationTypeAWSRoleWrite) + require.Len(t, observations, 1) + + observation := observations[0] + require.NotNil(t, observation.Data) + require.Equal(t, roleName, observation.Data["role_name"]) + require.Equal(t, []string{assumedRoleCred}, observation.Data["credential_types"]) +} diff --git a/builtin/logical/aws/path_static_creds.go b/builtin/logical/aws/path_static_creds.go index 24622d0b1d..fd2ac65748 100644 --- a/builtin/logical/aws/path_static_creds.go +++ b/builtin/logical/aws/path_static_creds.go @@ -82,6 +82,10 @@ func (b *backend) pathStaticCredsRead(ctx context.Context, req *logical.Request, return nil, fmt.Errorf("failed to decode credentials: %w", err) } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSStaticCredentialRead, map[string]interface{}{ + "role_name": roleName.(string), + }) + return &logical.Response{ Data: structs.New(credentials).Map(), }, nil diff --git a/builtin/logical/aws/path_static_roles.go b/builtin/logical/aws/path_static_roles.go index 21b4cff93e..90804552dc 100644 --- a/builtin/logical/aws/path_static_roles.go +++ b/builtin/logical/aws/path_static_roles.go @@ -129,6 +129,11 @@ func (b *backend) pathStaticRolesRead(ctx context.Context, req *logical.Request, return nil, fmt.Errorf("failed to decode configuration for static role %q: %w", roleName, err) } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSStaticRoleRead, map[string]interface{}{ + "role_name": roleName.(string), + "rotation_period": config.RotationPeriod.String(), + }) + return &logical.Response{ Data: formatResponse(config), }, nil @@ -260,6 +265,11 @@ func (b *backend) pathStaticRolesWrite(ctx context.Context, req *logical.Request } } + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSStaticRoleWrite, map[string]interface{}{ + "role_name": config.Name, + "rotation_period": config.RotationPeriod.String(), + }) + return &logical.Response{ Data: formatResponse(config), }, nil @@ -299,7 +309,18 @@ func (b *backend) pathStaticRolesDelete(ctx context.Context, req *logical.Reques return nil, fmt.Errorf("couldn't delete key from queue: %w", err) } - return nil, req.Storage.Delete(ctx, formatRoleStoragePath(roleName.(string))) + err = req.Storage.Delete(ctx, formatRoleStoragePath(roleName.(string))) + if err != nil { + return nil, fmt.Errorf("failed to delete role %q from storage: %w", roleName.(string), err) + } + + b.TryRecordObservationWithRequest(ctx, req, ObservationTypeAWSStaticRoleDelete, map[string]interface{}{ + "role_name": roleName.(string), + "rotation_period": cfg.RotationPeriod.String(), + }) + + // ignore-nil-nil-function-check + return nil, nil } func (b *backend) pathStaticRolesList(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { diff --git a/builtin/logical/aws/path_user.go b/builtin/logical/aws/path_user.go index d5d43190a3..116dd160e4 100644 --- a/builtin/logical/aws/path_user.go +++ b/builtin/logical/aws/path_user.go @@ -74,7 +74,7 @@ func pathUser(b *backend) *framework.Path { } } -func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { +func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (response *logical.Response, rerr error) { roleName := d.Get("name").(string) // Read the policy @@ -144,6 +144,21 @@ func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *fr } } + defer func() { + obsType := ObservationTypeAWSCredentialCreateSuccess + if rerr != nil { + obsType = ObservationTypeAWSCredentialCreateFail + } + + b.TryRecordObservationWithRequest(ctx, req, obsType, map[string]interface{}{ + "role_name": roleName, + "credential_types": role.CredentialTypes, + "ttl": ttl, + "max_ttl": maxTTL, + "is_sts": credentialType != iamUserCred, + }) + }() + switch credentialType { case iamUserCred: return b.secretAccessKeysCreate(ctx, req.Storage, req.DisplayName, roleName, role) diff --git a/builtin/logical/aws/rotation.go b/builtin/logical/aws/rotation.go index bd3c37db6d..50a7563e4e 100644 --- a/builtin/logical/aws/rotation.go +++ b/builtin/logical/aws/rotation.go @@ -82,6 +82,11 @@ func (b *backend) rotateCredential(ctx context.Context, storage logical.Storage) return true, fmt.Errorf("failed to add item into the rotation queue for role %q: %w", cfg.Name, err) } + b.TryRecordObservationWithRequest(ctx, nil, ObservationTypeAWSStaticCredentialRotate, map[string]interface{}{ + "role_name": cfg.Name, + "rotation_period": cfg.RotationPeriod.String(), + }) + return true, nil } diff --git a/builtin/logical/aws/rotation_test.go b/builtin/logical/aws/rotation_test.go index 5fb90736ab..71989b9cd9 100644 --- a/builtin/logical/aws/rotation_test.go +++ b/builtin/logical/aws/rotation_test.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/helper/testhelpers" vaulthttp "github.com/hashicorp/vault/http" + "github.com/hashicorp/vault/sdk/helper/testhelpers/observations" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/queue" "github.com/hashicorp/vault/vault" @@ -114,9 +115,11 @@ func TestRotation(t *testing.T) { t.Run(c.name, func(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} + or := observations.NewTestObservationRecorder() + config.ObservationRecorder = or b := Backend(config) - + require.NoError(t, b.Setup(context.Background(), config)) expirations := make([]*time.Time, len(c.creds)) // insert all our creds for i, cred := range c.creds { @@ -214,6 +217,7 @@ func TestRotation(t *testing.T) { t.Fatalf("got an error rotating credentials: %s", err) } + numChanged := 0 // check our credentials for i, cred := range c.creds { entry, err := config.StorageView.Get(bgCTX, formatCredsStoragePath(cred.config.Name)) @@ -229,11 +233,13 @@ func TestRotation(t *testing.T) { if cred.changed { require.Equal(t, out.SecretAccessKey, newSecret, "expected the key for cred %d to have changed, but it hasn't", i) require.NotEqual(t, out.Expiration.UTC(), expirations[i].UTC(), "expected the expiration for cred %d to have changed, but it hasn't", i) + numChanged++ } else { require.Equal(t, out.SecretAccessKey, oldSecret, "expected the key for cred %d to have stayed the same, but it changed", i) require.Equal(t, out.Expiration.UTC(), expirations[i].UTC(), "expected the expiration for cred %d to have changed, but it hasn't", i) } } + require.Equal(t, numChanged, or.NumObservationsByType(ObservationTypeAWSStaticCredentialRotate)) }) } } diff --git a/builtin/logical/aws/secret_access_keys.go b/builtin/logical/aws/secret_access_keys.go index 1e075a4d14..e2bf3fee6c 100644 --- a/builtin/logical/aws/secret_access_keys.go +++ b/builtin/logical/aws/secret_access_keys.go @@ -514,6 +514,11 @@ func (b *backend) secretAccessKeysRenew(ctx context.Context, req *logical.Reques isSTS, ok := isSTSRaw.(bool) if ok { if isSTS { + // We don't have any access to the role name, so there's not + // anything else we can add to the observation + b.TryRecordObservationWithRequest(ctx, nil, ObservationTypeAWSCredentialRenew, map[string]interface{}{ + "is_sts": true, + }) return nil, nil } } @@ -530,6 +535,22 @@ func (b *backend) secretAccessKeysRenew(ctx context.Context, req *logical.Reques resp := &logical.Response{Secret: req.Secret} resp.Secret.TTL = lease.Lease resp.Secret.MaxTTL = lease.LeaseMax + + policyRaw, ok := req.Secret.InternalData["policy"] + var credentialTypes []string + if ok { + policy, ok := policyRaw.(*awsRoleEntry) + if ok { + credentialTypes = policy.CredentialTypes + } + } + + // The role name isn't present, so we can't add it to the observation + b.TryRecordObservationWithRequest(ctx, nil, ObservationTypeAWSCredentialRenew, map[string]interface{}{ + "is_sts": false, + "credential_types": credentialTypes, + }) + return resp, nil } @@ -542,6 +563,9 @@ func (b *backend) secretAccessKeysRevoke(ctx context.Context, req *logical.Reque isSTS, ok := isSTSRaw.(bool) if ok { if isSTS { + b.TryRecordObservationWithRequest(ctx, nil, ObservationTypeAWSCredentialRevoke, map[string]interface{}{ + "is_sts": true, + }) return nil, nil } } else { @@ -567,6 +591,20 @@ func (b *backend) secretAccessKeysRevoke(ctx context.Context, req *logical.Reque return nil, err } + policyRaw, ok := req.Secret.InternalData["policy"] + var credentialTypes []string + if ok { + policy, ok := policyRaw.(*awsRoleEntry) + if ok { + credentialTypes = policy.CredentialTypes + } + } + + b.TryRecordObservationWithRequest(ctx, nil, ObservationTypeAWSCredentialRevoke, map[string]interface{}{ + "is_sts": false, + "credential_types": credentialTypes, + }) + return nil, nil } diff --git a/sdk/framework/backend.go b/sdk/framework/backend.go index 4803c59bdb..e46ab8b75f 100644 --- a/sdk/framework/backend.go +++ b/sdk/framework/backend.go @@ -823,6 +823,41 @@ func (b *Backend) RecordObservation(ctx context.Context, observationType string, return b.observations.RecordObservationFromPlugin(ctx, observationType, data) } +// RecordObservationWithRequest is used to record observations through the +// plugin's observation system. It attaches information from the request to the +// observation data. The request path, client ID, entity ID, and request ID are +// included. +// This method returns ErrNoObservations if the observation system has not been +// configured or enabled. +func (b *Backend) RecordObservationWithRequest(ctx context.Context, req *logical.Request, observationType string, data map[string]interface{}) error { + if b.observations == nil { + return ErrNoObservations + } + + // Attach request information to the observation data + if req != nil { + if data == nil { + data = make(map[string]interface{}) + } + data["path"] = req.Path + data["client_id"] = req.ClientID + data["entity_id"] = req.EntityID + data["request_id"] = req.ID + } + + return b.observations.RecordObservationFromPlugin(ctx, observationType, data) +} + +// TryRecordObservationWithRequest is like RecordObservationWithRequest but +// logs a warning instead of returning an error if recording the observation fails +// for reasons other than the observation system not being configured/enabled +func (b *Backend) TryRecordObservationWithRequest(ctx context.Context, req *logical.Request, observationType string, data map[string]interface{}) { + err := b.RecordObservationWithRequest(ctx, req, observationType, data) + if err != nil && !errors.Is(err, ErrNoObservations) { + b.Logger().Warn("failed to record observation", "error", err, "observation_type", observationType) + } +} + // FieldSchema is a basic schema to describe the format of a path field. type FieldSchema struct { Type FieldType diff --git a/sdk/helper/testhelpers/observations/recorder.go b/sdk/helper/testhelpers/observations/recorder.go new file mode 100644 index 0000000000..03468317ae --- /dev/null +++ b/sdk/helper/testhelpers/observations/recorder.go @@ -0,0 +1,64 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: MPL-2.0 + +package observations + +import ( + "context" + "sync" +) + +// TestObservation represents a recorded observation for testing purposes. +// It contains the type of the observation and associated data. +type TestObservation struct { + Type string + Data map[string]interface{} +} + +// TestObservationRecorder is an implementation of an observation recorder +// that stores observations in memory for testing purposes. +type TestObservationRecorder struct { + l sync.RWMutex + observationsByType map[string][]*TestObservation +} + +// NewTestObservationRecorder creates a new instance of TestObservationRecorder. +func NewTestObservationRecorder() *TestObservationRecorder { + return &TestObservationRecorder{ + observationsByType: make(map[string][]*TestObservation), + } +} + +func (t *TestObservationRecorder) RecordObservationFromPlugin(_ context.Context, observationType string, data map[string]interface{}) error { + t.l.Lock() + defer t.l.Unlock() + o := &TestObservation{ + Type: observationType, + Data: data, + } + + t.observationsByType[observationType] = append(t.observationsByType[observationType], o) + return nil +} + +// NumObservationsByType returns the number of observations recorded of the given type. +func (t *TestObservationRecorder) NumObservationsByType(observationType string) int { + t.l.RLock() + defer t.l.RUnlock() + + return len(t.observationsByType[observationType]) +} + +// ObservationsByType returns all observations recorded of the given type. +// It returns a copy of the slice. If you only need the count, use NumObservationsByType. +func (t *TestObservationRecorder) ObservationsByType(observationType string) []*TestObservation { + t.l.RLock() + defer t.l.RUnlock() + + ofType := t.observationsByType[observationType] + toReturn := make([]*TestObservation, 0, len(ofType)) + for _, o := range ofType { + toReturn = append(toReturn, o) + } + return toReturn +}