From e80c0209cc7c11b6be57b16eb8fbeab874c7955a Mon Sep 17 00:00:00 2001 From: Rajdeep Singh Chauhan Date: Thu, 6 Jun 2024 13:19:26 -0400 Subject: [PATCH] ARO-4376 add dynamic validation for platformworkloadidentityprofile --- pkg/api/error.go | 2 + pkg/util/mocks/dynamic/dynamic.go | 15 ++ pkg/util/rbac/rbac.go | 19 +-- pkg/validate/dynamic/dynamic.go | 37 ++++- .../platformworkloadidentityprofile.go | 134 ++++++++++++++++++ .../openshiftcluster_validatedynamic.go | 14 ++ 6 files changed, 207 insertions(+), 14 deletions(-) create mode 100644 pkg/validate/dynamic/platformworkloadidentityprofile.go diff --git a/pkg/api/error.go b/pkg/api/error.go index 6d007fc6dd4..a6b91bee585 100644 --- a/pkg/api/error.go +++ b/pkg/api/error.go @@ -104,6 +104,8 @@ const ( CloudErrorCodeRequestDisallowedByPolicy = "RequestDisallowedByPolicy" CloudErrorCodeInvalidNetworkAddress = "InvalidNetworkAddress" CloudErrorCodeThrottlingLimitExceeded = "ThrottlingLimitExceeded" + CloudErrorCodePlatformWorkloadIdentityMismatch = "CloudErrorCodePlatformWorkloadIdentityMismatch" + CloudErrorCodeInvalidClusterMSICount = "CloudErrorCodeInvalidClusterMSICount" ) // NewCloudError returns a new CloudError diff --git a/pkg/util/mocks/dynamic/dynamic.go b/pkg/util/mocks/dynamic/dynamic.go index f413a3f81a8..a9a2f718839 100644 --- a/pkg/util/mocks/dynamic/dynamic.go +++ b/pkg/util/mocks/dynamic/dynamic.go @@ -12,6 +12,7 @@ import ( gomock "github.com/golang/mock/gomock" api "github.com/Azure/ARO-RP/pkg/api" + database "github.com/Azure/ARO-RP/pkg/database" dynamic "github.com/Azure/ARO-RP/pkg/validate/dynamic" ) @@ -117,6 +118,20 @@ func (mr *MockDynamicMockRecorder) ValidateLoadBalancerProfile(ctx, oc interface return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateLoadBalancerProfile", reflect.TypeOf((*MockDynamic)(nil).ValidateLoadBalancerProfile), ctx, oc) } +// ValidatePlatformWorkloadIdentityProfile mocks base method. +func (m *MockDynamic) ValidatePlatformWorkloadIdentityProfile(ctx context.Context, oc *api.OpenShiftCluster, dbPlatformWorkloadIdentityRoleSets database.PlatformWorkloadIdentityRoleSets) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ValidatePlatformWorkloadIdentityProfile", ctx, oc, dbPlatformWorkloadIdentityRoleSets) + ret0, _ := ret[0].(error) + return ret0 +} + +// ValidatePlatformWorkloadIdentityProfile indicates an expected call of ValidatePlatformWorkloadIdentityProfile. +func (mr *MockDynamicMockRecorder) ValidatePlatformWorkloadIdentityProfile(ctx, oc, dbPlatformWorkloadIdentityRoleSets interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidatePlatformWorkloadIdentityProfile", reflect.TypeOf((*MockDynamic)(nil).ValidatePlatformWorkloadIdentityProfile), ctx, oc, dbPlatformWorkloadIdentityRoleSets) +} + // ValidatePreConfiguredNSGs mocks base method. func (m *MockDynamic) ValidatePreConfiguredNSGs(ctx context.Context, oc *api.OpenShiftCluster, subnets []dynamic.Subnet) error { m.ctrl.T.Helper() diff --git a/pkg/util/rbac/rbac.go b/pkg/util/rbac/rbac.go index 314908053a1..775ae77e6f0 100644 --- a/pkg/util/rbac/rbac.go +++ b/pkg/util/rbac/rbac.go @@ -12,15 +12,16 @@ import ( ) const ( - RoleACRPull = "7f951dda-4ed3-4680-a7ca-43fe172d538d" - RoleContributor = "b24988ac-6180-42a0-ab88-20f7382dd24c" - RoleDocumentDBAccountContributor = "5bd9cd88-fe45-4216-938b-f97437e15450" - RoleDocumentDBDataContributor = "00000000-0000-0000-0000-000000000002" - RoleDNSZoneContributor = "befefa01-2a29-4197-83a8-272ff33ce314" - RoleNetworkContributor = "4d97b98b-1d4f-4787-a291-c67834d212e7" - RoleOwner = "8e3af657-a8ff-443c-a75c-2fe8c4bcb635" - RoleReader = "acdd72a7-3385-48ef-bd42-f606fba81ae7" - RoleStorageBlobDataContributor = "ba92f5b4-2d11-453d-a403-e96b0029c9fe" + RoleACRPull = "7f951dda-4ed3-4680-a7ca-43fe172d538d" + RoleContributor = "b24988ac-6180-42a0-ab88-20f7382dd24c" + RoleDocumentDBAccountContributor = "5bd9cd88-fe45-4216-938b-f97437e15450" + RoleDocumentDBDataContributor = "00000000-0000-0000-0000-000000000002" + RoleDNSZoneContributor = "befefa01-2a29-4197-83a8-272ff33ce314" + RoleNetworkContributor = "4d97b98b-1d4f-4787-a291-c67834d212e7" + RoleOwner = "8e3af657-a8ff-443c-a75c-2fe8c4bcb635" + RoleReader = "acdd72a7-3385-48ef-bd42-f606fba81ae7" + RoleStorageBlobDataContributor = "ba92f5b4-2d11-453d-a403-e96b0029c9fe" + RoleAzureRedHatOpenShiftFederatedCredentialRole = "ef318e2a-8334-4a05-9e4a-295a196c6a6e" ) // ResourceRoleAssignment returns a Resource granting roleID on the resource of diff --git a/pkg/validate/dynamic/dynamic.go b/pkg/validate/dynamic/dynamic.go index ba87cfa16ee..d5cb56ffd57 100644 --- a/pkg/validate/dynamic/dynamic.go +++ b/pkg/validate/dynamic/dynamic.go @@ -22,9 +22,12 @@ import ( "github.com/Azure/ARO-RP/pkg/api" apisubnet "github.com/Azure/ARO-RP/pkg/api/util/subnet" + "github.com/Azure/ARO-RP/pkg/database" "github.com/Azure/ARO-RP/pkg/env" + "github.com/Azure/ARO-RP/pkg/portal/middleware" "github.com/Azure/ARO-RP/pkg/util/azureclient" "github.com/Azure/ARO-RP/pkg/util/azureclient/authz/remotepdp" + "github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization" "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute" "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/network" "github.com/Azure/ARO-RP/pkg/util/token" @@ -73,6 +76,7 @@ type Dynamic interface { ValidateEncryptionAtHost(ctx context.Context, oc *api.OpenShiftCluster) error ValidateLoadBalancerProfile(ctx context.Context, oc *api.OpenShiftCluster) error ValidatePreConfiguredNSGs(ctx context.Context, oc *api.OpenShiftCluster, subnets []Subnet) error + ValidatePlatformWorkloadIdentityProfile(ctx context.Context, oc *api.OpenShiftCluster, dbPlatformWorkloadIdentityRoleSets database.PlatformWorkloadIdentityRoleSets) error } type dynamic struct { @@ -80,9 +84,11 @@ type dynamic struct { appID string // for use when reporting an error authorizerType AuthorizerType // This represents the Subject for CheckAccess. Could be either FP or SP. - checkAccessSubjectInfoCred azcore.TokenCredential - env env.Interface - azEnv *azureclient.AROEnvironment + checkAccessSubjectInfoCred azcore.TokenCredential + env env.Interface + azEnv *azureclient.AROEnvironment + platformIdentities []api.PlatformWorkloadIdentity + platformIdentitiesActionsMap map[string][]string virtualNetworks virtualNetworksGetClient diskEncryptionSets compute.DiskEncryptionSetsClient @@ -91,6 +97,7 @@ type dynamic struct { spNetworkUsage network.UsageClient loadBalancerBackendAddressPoolsClient network.LoadBalancerBackendAddressPoolsClient pdpClient remotepdp.RemotePDPClient + roleDefinitions armauthorization.RoleDefinitionsClient } type AuthorizerType string @@ -110,6 +117,7 @@ func NewValidator( authorizerType AuthorizerType, cred azcore.TokenCredential, pdpClient remotepdp.RemotePDPClient, + roleDefinitions armauthorization.RoleDefinitionsClient, ) Dynamic { return &dynamic{ log: log, @@ -128,6 +136,7 @@ func NewValidator( resourceSkusClient: compute.NewResourceSkusClient(azEnv, subscriptionID, authorizer), pdpClient: pdpClient, loadBalancerBackendAddressPoolsClient: network.NewLoadBalancerBackendAddressPoolsClient(azEnv, subscriptionID, authorizer), + roleDefinitions: roleDefinitions, } } @@ -386,12 +395,12 @@ func (dv *dynamic) validateNatGatewayPermissions(ctx context.Context, s Subnet) return err } -func (dv *dynamic) validateActions(ctx context.Context, r *azure.Resource, actions []string) error { +func (dv *dynamic) validateActionsByOID(ctx context.Context, r *azure.Resource, actions []string, oid *string) error { // ARM has a 5 minute cache around role assignment creation, so wait one minute longer timeoutCtx, cancel := context.WithTimeout(ctx, 6*time.Minute) defer cancel() - c := closure{dv: dv, ctx: ctx, resource: r, actions: actions} + c := closure{dv: dv, ctx: ctx, resource: r, actions: actions, oid: oid} return wait.PollImmediateUntil(30*time.Second, c.usingCheckAccessV2, timeoutCtx.Done()) } @@ -780,6 +789,24 @@ func (dv *dynamic) ValidatePreConfiguredNSGs(ctx context.Context, oc *api.OpenSh return nil } +func (dv *dynamic) validateActions(ctx context.Context, r *azure.Resource, actions []string) error { + if dv.platformIdentities != nil { + for _, platformIdentity := range dv.platformIdentities { + actionsToValidate := middleware.GroupsIntersect(actions, dv.platformIdentitiesActionsMap[platformIdentity.OperatorName]) + if len(actionsToValidate) > 0 { + if err := dv.validateActionsByOID(ctx, r, actionsToValidate, &platformIdentity.ObjectID); err != nil { + return err + } + } + } + } else { + if err := dv.validateActionsByOID(ctx, r, actions, nil); err != nil { + return err + } + } + return nil +} + func (dv *dynamic) validateNSGPermissions(ctx context.Context, nsgID string) error { nsg, err := azure.ParseResourceID(nsgID) if err != nil { diff --git a/pkg/validate/dynamic/platformworkloadidentityprofile.go b/pkg/validate/dynamic/platformworkloadidentityprofile.go new file mode 100644 index 00000000000..b9d53c7d5be --- /dev/null +++ b/pkg/validate/dynamic/platformworkloadidentityprofile.go @@ -0,0 +1,134 @@ +package dynamic + +import ( + "context" + "net/http" + + sdkauthorization "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v3" + "github.com/Azure/go-autorest/autorest/azure" + "github.com/stretchr/testify/assert" + + "github.com/Azure/ARO-RP/pkg/api" + "github.com/Azure/ARO-RP/pkg/database" + "github.com/Azure/ARO-RP/pkg/util/rbac" +) + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +type fakeT struct{} + +func (t fakeT) Errorf(string, ...interface{}) {} + +func (dv *dynamic) ValidatePlatformWorkloadIdentityProfile(ctx context.Context, oc *api.OpenShiftCluster, dbPlatformWorkloadIdentityRoleSets database.PlatformWorkloadIdentityRoleSets) error { + dv.log.Print("ValidatePlatformWorkloadIdentityProfile") + + if oc.Properties.PlatformWorkloadIdentityProfile == nil || oc.Properties.ServicePrincipalProfile != nil { + return nil + } + + requestedInstallVersion := oc.Properties.ClusterProfile.Version + platformIdentitiesActionsMap := map[string][]string{} + var roles []api.PlatformWorkloadIdentityRole + + docs, err := dbPlatformWorkloadIdentityRoleSets.ListAll(ctx) + if err != nil { + return err + } + + for _, doc := range docs.PlatformWorkloadIdentityRoleSetDocuments { + if requestedInstallVersion == doc.PlatformWorkloadIdentityRoleSet.Properties.OpenShiftVersion { + roles = doc.PlatformWorkloadIdentityRoleSet.Properties.PlatformWorkloadIdentityRoles + } + } + + requiredOperatorIdentities := []string{} + recievedOperatorIdentities := []string{} + for _, role := range roles { + requiredOperatorIdentities = append(requiredOperatorIdentities, role.OperatorName) + // platformIdentitiesRoleMap[role.OperatorName] = role + } + for _, role := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities { + recievedOperatorIdentities = append(recievedOperatorIdentities, role.OperatorName) + } + ok := assert.ElementsMatch(fakeT{}, requiredOperatorIdentities, recievedOperatorIdentities) + if !ok { + return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePlatformWorkloadIdentityMismatch, + "properties.ValidatePlatformWorkloadIdentityProfile.PlatformWorkloadIdentities", "There's a mismatch between the required and expected set of platform workload identities for the requested OpenShift version '%s'.", requestedInstallVersion) + } + + err = dv.validateClusterMSI(ctx, oc) + if err != nil { + return err + } + + for _, role := range roles { + definition, err := dv.roleDefinitions.GetByID(ctx, role.RoleDefinitionID, &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}) + if err != nil { + return err + } + + if len(definition.Properties.Permissions) <= 0 { + return api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInvalidClusterMSICount, + "dynamic.ValidatePlatformWorkloadIdentityProfile", "No Permissions exists for the role %s", role.RoleDefinitionID) + } + + actions := []string{} + for _, action := range definition.Properties.Permissions[0].Actions { + actions = append(actions, *action) + } + platformIdentitiesActionsMap[role.OperatorName] = actions + } + + dv.platformIdentities = oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities + dv.platformIdentitiesActionsMap = platformIdentitiesActionsMap + return nil +} + +func (dv *dynamic) validateClusterMSI(ctx context.Context, oc *api.OpenShiftCluster) error { + if len(oc.Identity.UserAssignedIdentities) <= 0 { + return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidClusterMSICount, + "identity.userAssignedIdentities", "No OpenShift Cluster associated User Assigned Identity is provided for the Workload Identity OpenShift cluster creation") + } + + if len(oc.Identity.UserAssignedIdentities) > 1 { + return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidClusterMSICount, + "identity.userAssignedIdentities", "More than one OpenShift Cluster associated User Assigned Identity are provided for the Workload Identity OpenShift cluster creation") + } + + for _, identity := range oc.Identity.UserAssignedIdentities { + return dv.validateClusterMSIPermissions(ctx, identity.PrincipalID, oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities) + } + + return nil +} + +func (dv *dynamic) validateClusterMSIPermissions(ctx context.Context, oid string, platformIdentities []api.PlatformWorkloadIdentity) error { + definition, err := dv.roleDefinitions.GetByID(ctx, rbac.RoleAzureRedHatOpenShiftFederatedCredentialRole, &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}) + if err != nil { + return err + } + + if len(definition.Properties.Permissions) <= 0 { + return api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInvalidClusterMSICount, + "dynamic.validateClusterMSIPermissions", "No Permissions exists for the role %s", rbac.RoleAzureRedHatOpenShiftFederatedCredentialRole) + } + + actions := []string{} + for _, action := range definition.Properties.Permissions[0].Actions { + actions = append(actions, *action) + } + + for _, platformIdentity := range platformIdentities { + pid, err := azure.ParseResourceID(platformIdentity.ResourceID) + if err != nil { + return err + } + + err = dv.validateActionsByOID(ctx, &pid, actions, &oid) + if err != nil { + return err + } + } + return nil +} diff --git a/pkg/validate/openshiftcluster_validatedynamic.go b/pkg/validate/openshiftcluster_validatedynamic.go index b7669797b21..f49e3bce295 100644 --- a/pkg/validate/openshiftcluster_validatedynamic.go +++ b/pkg/validate/openshiftcluster_validatedynamic.go @@ -20,6 +20,7 @@ import ( "github.com/Azure/ARO-RP/pkg/database" "github.com/Azure/ARO-RP/pkg/env" "github.com/Azure/ARO-RP/pkg/util/azureclient/authz/remotepdp" + "github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization" "github.com/Azure/ARO-RP/pkg/validate/dynamic" ) @@ -151,6 +152,11 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error { } spAuthorizer := azidext.NewTokenCredentialAdapter(spClientCred, scopes) + roleDefinitions, err := armauthorization.NewRoleDefinitionsClient(dv.env.Environment(), fpClientCred) + if err != nil { + return err + } + spDynamic := dynamic.NewValidator( dv.log, dv.env, @@ -161,6 +167,7 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error { dynamic.AuthorizerClusterServicePrincipal, spClientCred, pdpClient, + roleDefinitions, ) // SP validation @@ -169,6 +176,12 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error { return err } + // PlatformWorkloadIdentity and ClusterMSIIdentity Validation + err = spDynamic.ValidatePlatformWorkloadIdentityProfile(ctx, dv.oc, dv.dbPlatformWorkloadIdentityRoleSets) + if err != nil { + return err + } + err = spDynamic.ValidateVnet( ctx, dv.oc.Location, @@ -216,6 +229,7 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error { dynamic.AuthorizerFirstParty, fpClientCred, pdpClient, + nil, ) err = fpDynamic.ValidateVnet(