Bug 1750338: improve permissions simulation by adding region info

pkg/aws/actuator/actuator.go

@@ -149,7 +149,7 @@ func (a *AWSActuator) Exists(ctx context.Context, cr *minterv1.CredentialsReques
 
 // needsUpdate will return whether the current credentials satisfy what's being requested
 // in the CredentialsRequest
-func (a *AWSActuator) needsUpdate(ctx context.Context, cr *minterv1.CredentialsRequest, infraName string) (bool, error) {
+func (a *AWSActuator) needsUpdate(ctx context.Context, cr *minterv1.CredentialsRequest, infraName, region string) (bool, error) {
 	logger := a.getLogger(cr)
 	// If the secret simply doesn't exist, we definitely need an update
 	exists, err := a.Exists(ctx, cr)
@@ -231,7 +231,10 @@ func (a *AWSActuator) needsUpdate(ctx context.Context, cr *minterv1.CredentialsR
 
 	} else {
 		// for passthrough creds, just see if we have the permissions requested in the credentialsrequest
-		goodEnough, err := ccaws.CheckPermissionsUsingQueryClient(readAWSClient, awsClient, awsSpec.StatementEntries, logger)
+		simParams := &ccaws.SimulateParams{
+			Region: region,
+		}
+		goodEnough, err := ccaws.CheckPermissionsUsingQueryClient(readAWSClient, awsClient, awsSpec.StatementEntries, simParams, logger)
 		if err != nil {
 			return true, fmt.Errorf("error validating whether current creds are good enough: %v", err)
 		}
@@ -266,8 +269,13 @@ func (a *AWSActuator) sync(ctx context.Context, cr *minterv1.CredentialsRequest)
 		return err
 	}
 
+	region, err := utils.LoadInfrastructureRegion(a.Client, logger)
+	if err != nil {
+		return err
+	}
+
 	// Should we update anything
-	needsUpdate, err := a.needsUpdate(ctx, cr, infraName)
+	needsUpdate, err := a.needsUpdate(ctx, cr, infraName, region)
 	if err != nil {
 		logger.WithError(err).Error("error determining whether a credentials update is needed")
 		return &actuatoriface.ActuatorError{

pkg/aws/utils.go

@@ -106,9 +106,17 @@ func init() {
 	}
 }
 
+// SimulateParams captures any additional details that should be used
+// when simulating permissions.
+type SimulateParams struct {
+	Region string
+}
+
 // CheckCloudCredCreation will see whether we have enough permissions to create new sub-creds
 func CheckCloudCredCreation(awsClient Client, logger log.FieldLogger) (bool, error) {
-	return CheckPermissionsAgainstActions(awsClient, credMintingActions, logger)
+	// Empty SimulateParams{} b/c creating IAM users and assigning policies
+	// are all IAM API alls which are not region-specific
+	return CheckPermissionsAgainstActions(awsClient, credMintingActions, &SimulateParams{}, logger)
 }
 
 // getClientDetails will return the *iam.User associated with the provided client's credentials,
@@ -136,7 +144,8 @@ func getClientDetails(awsClient Client) (*iam.User, bool, error) {
 
 // CheckPermissionsUsingQueryClient will use queryClient to query whether the credentials in targetClient can perform the actions
 // listed in the statementEntries. queryClient will need iam:GetUser and iam:SimulatePrincipalPolicy
-func CheckPermissionsUsingQueryClient(queryClient, targetClient Client, statementEntries []minterv1.StatementEntry, logger log.FieldLogger) (bool, error) {
+func CheckPermissionsUsingQueryClient(queryClient, targetClient Client, statementEntries []minterv1.StatementEntry,
+	params *SimulateParams, logger log.FieldLogger) (bool, error) {
 	targetUser, isRoot, err := getClientDetails(targetClient)
 	if err != nil {
 		return false, fmt.Errorf("error gathering AWS credentials details: %v", err)
@@ -157,6 +166,17 @@ func CheckPermissionsUsingQueryClient(queryClient, targetClient Client, statemen
 	input := &iam.SimulatePrincipalPolicyInput{
 		PolicySourceArn: targetUser.Arn,
 		ActionNames:     allowList,
+		ContextEntries:  []*iam.ContextEntry{},
+	}
+
+	if params != nil {
+		if params.Region != "" {
+			input.ContextEntries = append(input.ContextEntries, &iam.ContextEntry{
+				ContextKeyName:   aws.String("aws:RequestedRegion"),
+				ContextKeyType:   aws.String("string"),
+				ContextKeyValues: []*string{aws.String(params.Region)},
+			})
+		}
 	}
 
 	// Either all actions are allowed and we'll return 'true', or it's a failure
@@ -189,14 +209,15 @@ func CheckPermissionsUsingQueryClient(queryClient, targetClient Client, statemen
 
 // CheckPermissionsAgainstStatementList will test to see whether the list of actions in the provided
 // list of StatementEntries can work with the credentials used by the passed-in awsClient
-func CheckPermissionsAgainstStatementList(awsClient Client, statementEntries []minterv1.StatementEntry, logger log.FieldLogger) (bool, error) {
-	return CheckPermissionsUsingQueryClient(awsClient, awsClient, statementEntries, logger)
+func CheckPermissionsAgainstStatementList(awsClient Client, statementEntries []minterv1.StatementEntry,
+	params *SimulateParams, logger log.FieldLogger) (bool, error) {
+	return CheckPermissionsUsingQueryClient(awsClient, awsClient, statementEntries, params, logger)
 }
 
 // CheckPermissionsAgainstActions will take the static list of Actions to check whether the provided
 // awsClient creds have sufficient permissions to perform the actions.
 // Will return true/false indicating whether the permissions are sufficient.
-func CheckPermissionsAgainstActions(awsClient Client, actionList []string, logger log.FieldLogger) (bool, error) {
+func CheckPermissionsAgainstActions(awsClient Client, actionList []string, params *SimulateParams, logger log.FieldLogger) (bool, error) {
 	statementList := []minterv1.StatementEntry{
 		{
 			Action:   actionList,
@@ -205,15 +226,15 @@ func CheckPermissionsAgainstActions(awsClient Client, actionList []string, logge
 		},
 	}
 
-	return CheckPermissionsAgainstStatementList(awsClient, statementList, logger)
+	return CheckPermissionsAgainstStatementList(awsClient, statementList, params, logger)
 }
 
 // CheckCloudCredPassthrough will see if the provided creds are good enough to pass through
 // to other components as-is based on the static list of permissions needed by the various
 // users of CredentialsRequests
 // TODO: move away from static list (to dynamic passthrough validation?)
-func CheckCloudCredPassthrough(awsClient Client, logger log.FieldLogger) (bool, error) {
-	return CheckPermissionsAgainstActions(awsClient, credPassthroughActions, logger)
+func CheckCloudCredPassthrough(awsClient Client, params *SimulateParams, logger log.FieldLogger) (bool, error) {
+	return CheckPermissionsAgainstActions(awsClient, credPassthroughActions, params, logger)
 }
 
 func readCredentialRequest(cr []byte) (*minterv1.CredentialsRequest, error) {

pkg/controller/credentialsrequest/credentialsrequest_controller_test.go

@@ -1453,6 +1453,11 @@ func testInfrastructure(infraName string) *configv1.Infrastructure {
 		Status: configv1.InfrastructureStatus{
 			Platform:           configv1.AWSPlatformType,
 			InfrastructureName: infraName,
+			PlatformStatus: &configv1.PlatformStatus{
+				AWS: &configv1.AWSPlatformStatus{
+					Region: "test-region-2",
+				},
+			},
 		},
 	}
 }

pkg/controller/secretannotator/aws/reconciler.go

@@ -156,7 +156,14 @@ func (r *ReconcileCloudCredSecret) validateCloudCredsSecret(secret *corev1.Secre
 	}
 
 	// Else, can we just pass through the current creds?
-	cloudCheckResult, err = ccaws.CheckCloudCredPassthrough(awsClient, r.Logger)
+	region, err := utils.LoadInfrastructureRegion(r.Client, r.Logger)
+	if err != nil {
+		return err
+	}
+	simParams := &ccaws.SimulateParams{
+		Region: region,
+	}
+	cloudCheckResult, err = ccaws.CheckCloudCredPassthrough(awsClient, simParams, r.Logger)
 	if err != nil {
 		r.updateSecretAnnotations(secret, constants.InsufficientAnnotation)
 		return fmt.Errorf("failed checking passthrough cloud creds: %v", err)

pkg/controller/secretannotator/aws/reconciler_test.go

@@ -56,6 +56,7 @@ const (
 	testAWSAccessKeyID     = "FAKEAWSACCESSKEYID"
 	testInfraName          = "testcluster-abc123"
 	testAWSSecretAccessKey = "KEEPITSECRET"
+	testAWSRegion          = "test-region-2"
 )
 
 var (
@@ -132,12 +133,27 @@ func TestSecretAnnotatorReconcile(t *testing.T) {
 				mockSimulatePrincipalPolicyCredMinterFail(mockAWSClient)
 
 				mockGetUser(mockAWSClient)
-				mockSimulatePrincipalPolicyCredPassthroughSuccess(mockAWSClient)
+				mockSimulatePrincipalPolicyCredPassthrough(mockAWSClient, testAWSRegion)
 
 				return mockAWSClient
 			},
 			validateAnnotationValue: constants.PassthroughAnnotation,
 		},
+		{
+			name:     "cred passthrough mode wrong region permission",
+			existing: []runtime.Object{testSecret()},
+			mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient {
+				mockAWSClient := mockaws.NewMockClient(mockCtrl)
+				mockGetUser(mockAWSClient)
+				mockSimulatePrincipalPolicyCredMinterFail(mockAWSClient)
+
+				mockGetUser(mockAWSClient)
+				mockSimulatePrincipalPolicyCredPassthrough(mockAWSClient, "expect-this-region")
+
+				return mockAWSClient
+			},
+			validateAnnotationValue: constants.InsufficientAnnotation,
+		},
 		{
 			name:     "useless creds",
 			existing: []runtime.Object{testSecret()},
@@ -185,6 +201,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) {
 				Status: configv1.InfrastructureStatus{
 					Platform:           configv1.AWSPlatformType,
 					InfrastructureName: testInfraName,
+					PlatformStatus: &configv1.PlatformStatus{
+						AWS: &configv1.AWSPlatformStatus{
+							Region: testAWSRegion,
+						},
+					},
 				},
 			}
 
@@ -286,13 +307,30 @@ func mockSimulatePrincipalPolicyCredMinterFail(mockAWSClient *mockaws.MockClient
 		})
 }
 
-func mockSimulatePrincipalPolicyCredPassthroughSuccess(mockAWSClient *mockaws.MockClient) {
+func mockSimulatePrincipalPolicyCredPassthrough(mockAWSClient *mockaws.MockClient, expectedRegion string) {
 	mockAWSClient.EXPECT().SimulatePrincipalPolicyPages(gomock.Any(), gomock.Any()).Return(nil).
 		Do(func(input *iam.SimulatePrincipalPolicyInput, f func(*iam.SimulatePolicyResponse, bool) bool) {
-			f(successfulSimulateResponse, true)
+			if checkRegionParamSet(input, expectedRegion) {
+				f(successfulSimulateResponse, true)
+			} else {
+				f(failedSimulationResponse, true)
+			}
 		})
 }
 
+func checkRegionParamSet(input *iam.SimulatePrincipalPolicyInput, expectedRegion string) bool {
+	for _, ctx := range input.ContextEntries {
+		if *ctx.ContextKeyName == "aws:RequestedRegion" {
+			for _, value := range ctx.ContextKeyValues {
+				if *value == expectedRegion {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
 func mockSimulatePrincipalPolicyCredPassthroughFail(mockAWSClient *mockaws.MockClient) {
 	mockAWSClient.EXPECT().SimulatePrincipalPolicyPages(gomock.Any(), gomock.Any()).Return(nil).
 		// Now in the Do() receive the lambda function f() so we can send it the failed result

pkg/controller/utils/utils.go

@@ -54,16 +54,31 @@ func LoadCredsFromSecret(kubeClient client.Client, namespace, secretName string)
 // LoadInfrastructureName loads the cluster Infrastructure config and returns the infra name
 // used to identify this cluster, and tag some cloud objects.
 func LoadInfrastructureName(c client.Client, logger log.FieldLogger) (string, error) {
-	infra := &configv1.Infrastructure{}
-	err := c.Get(context.Background(), types.NamespacedName{Name: "cluster"}, infra)
+	infra, err := getInfrastructure(c)
 	if err != nil {
 		logger.WithError(err).Error("error loading Infrastructure config 'cluster'")
 		return "", err
 	}
-
-	logger.Debugf("Loaded infrastructure name: %s", infra.Status.InfrastructureName)
+	logger.Debugf("Loading infrastructure name: %s", infra.Status.InfrastructureName)
 	return infra.Status.InfrastructureName, nil
+}
+
+// LoadInfrastructureRegion loads the AWS region the cluster is installed to.
+func LoadInfrastructureRegion(c client.Client, logger log.FieldLogger) (string, error) {
+	infra, err := getInfrastructure(c)
+	if err != nil {
+		logger.WithError(err).Error("error loading Infrastructure region")
+		return "", err
+	}
+	return infra.Status.PlatformStatus.AWS.Region, nil
+}
 
+func getInfrastructure(c client.Client) (*configv1.Infrastructure, error) {
+	infra := &configv1.Infrastructure{}
+	if err := c.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, infra); err != nil {
+		return nil, err
+	}
+	return infra, nil
 }
 
 // GetCredentialsRequestCloudType decodes a Spec.ProviderSpec and returns the kind