s3 bucket policy with not_principal always changes due to ordering #13144

craiggenner · 2020-05-04T15:35:48Z

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

$ terraform -v
Terraform v0.12.24

provider.aws v2.60.0
provider.template v2.1.2

Affected Resource(s)

aws_s3_bucket_policy
aws_iam_policy_document
aws_s3_bucket

Terraform Configuration Files

resource "aws_s3_bucket" "av" {
  bucket = "app-storage"
  acl    = "private"
}

data "aws_iam_policy_document" "s3_av" {
  statement {
    sid       = "DenyInfected"
    effect    = "Deny"
    actions   = ["s3:GetObject", "s3:PutObjectTagging"]
    resources = ["arn:aws:s3:::${var.s3_bucket}/*"]

    not_principals {
      type        = "AWS"
      identifiers = [
        aws_iam_role.antivirus.arn,
        "arn:aws:sts::${var.account_id}:assumed-role/${aws_iam_role.antivirus.name}/${aws_lambda_function.antivirus.function_name}"
      ]
    }

    # Deny where 'av-status' tag is 'INFECTED'
    condition {
      test     = "StringEquals"
      variable = "s3:ExistingObjectTag/av-status"
      values   = ["INFECTED"]
    }
  }
}

resource "aws_s3_bucket_policy" "av" {
  bucket = aws_s3_bucket.av.id
  policy = data.aws_iam_policy_document.s3_av.json
}

Debug Output

https://gist.github.com/craiggenner/b315384e6ca2cead486b14665840807e

Panic Output

None

Expected Behavior

Terraform shouldn't expect to make any changes

Actual Behavior

Terraform always wants to replace the policy of the s3 bucket

Steps to Reproduce

Create terraform code as above
terraform apply
terraform apply

Important Factoids

Running this against eu-west-1 region.

The issue appears to be related to the use of the sts assumed-role in the not_principals, a regular iam role entry doesn't produce this. With multiple entries the order of the not_principals block is moved around within the json policy

References

Terraform aws s3 bucket policy are planned for change every time terraform#4948

The text was updated successfully, but these errors were encountered:

roccomuscaritolo-okta · 2020-11-25T23:11:49Z

I can confirm this issue is still occurring in AWS hashicorp/aws version v3.18.0.

Even adding a sort on the principals directly does not change this diff behavior.

principals {
      type        = "AWS"
      identifiers = sort([for role in data.aws_iam_role.decrypt_allowed_roles : role.arn])
    }

While the end result makes no difference, at a much larger scale this puts an undo burden on the reviewer to manually parse the output. I personally have had to review about 9 prinicple changes for someone simply adding 1 new role to a list.

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # aws_s3_bucket_policy.main will be updated in-place
  ~ resource "aws_s3_bucket_policy" "main" {
        bucket = "rocco-testing"
        id     = "rocco-testing"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                  ~ {
                        Action    = "s3:*"
                        Effect    = "Allow"
                      ~ Principal = {
                          ~ AWS = [
                                "arn:aws:iam::123456789:role/test",
                              + "arn:aws:iam::123456789:role/rundeck-test",
                                "arn:aws:iam::123456789:role/kallentest",
                                "arn:aws:iam::123456789:role/career-test",
                              + "arn:aws:iam::123456789:role/career-test123",
                                "arn:aws:iam::123456789:root",
                              - "arn:aws:iam::123456789:role/rundeck-test",
                            ]
                        }
                        Resource  = [
                            "arn:aws:s3:::rocco-testing/*",
                            "arn:aws:s3:::rocco-testing",
                        ]
                        Sid       = "Allow test/root access"
                    },
                ]
                Version   = "2012-10-17"
            }
        )
    }

Plan: 0 to add, 1 to change, 0 to destroy.

------------------------------------------------------------------------

iainelder · 2021-05-24T18:37:43Z

@craiggenner I can confirm what @roccomuscaritolo-okta sees. It happens in the normal principal field as well.

The crazy thing is that if you test an expression such as [for role in data.aws_iam_role.decrypt_allowed_roles : role.arn] in terraform console then it always returns the same order of elements.

It looks like the bucket policy resource is jumbling them up at some point.

iainelder · 2021-05-24T19:34:15Z

The Principals and NotPrincipals elements of IAM policy statements appear to be handled in exactly the same way, so that would explain why the behavior is observed in both places.

terraform-provider-aws/aws/iam_policy_model.go

Lines 22 to 23 in 3b05635

    
           Principals    IAMPolicyStatementPrincipalSet `json:"Principal,omitempty"` 
        
           NotPrincipals IAMPolicyStatementPrincipalSet `json:"NotPrincipal,omitempty"`

The problem appears to occur in the unmarshalling of the policy document. That's a guess because it only happens when the result of encodejson has been set on the bucket policy resource.

I'm unsure if a change is required in UnmarshalJSON. It looks like it just iterates over whatever its given without reodering anything.

terraform-provider-aws/aws/iam_policy_model.go

Lines 127 to 159 in e9a98cf

    
           func (ps *IAMPolicyStatementPrincipalSet) UnmarshalJSON(b []byte) error { 
        
           	var out IAMPolicyStatementPrincipalSet 
        
           	var data interface{} 
        
           	if err := json.Unmarshal(b, &data); err != nil { 
        
           		return err 
        
           	} 
        
           	switch t := data.(type) { 
        
           	case string: 
        
           		out = append(out, IAMPolicyStatementPrincipal{Type: "*", Identifiers: []string{"*"}}) 
        
           	case map[string]interface{}: 
        
           		for key, value := range data.(map[string]interface{}) { 
        
           			switch vt := value.(type) { 
        
           			case string: 
        
           				out = append(out, IAMPolicyStatementPrincipal{Type: key, Identifiers: value.(string)}) 
        
           			case []interface{}: 
        
           				values := []string{} 
        
           				for _, v := range value.([]interface{}) { 
        
           					values = append(values, v.(string)) 
        
           				} 
        
           				out = append(out, IAMPolicyStatementPrincipal{Type: key, Identifiers: values}) 
        
           			default: 
        
           				return fmt.Errorf("Unsupported data type %T for IAMPolicyStatementPrincipalSet.Identifiers", vt) 
        
           			} 
        
           		} 
        
           	default: 
        
           		return fmt.Errorf("Unsupported data type %T for IAMPolicyStatementPrincipalSet", t) 
        
           	} 
        
           	*ps = out 
        
           	return nil 
        
           }

The schema for the principal policy element says that identifiers are a set, and so unordered. As it turns out, the order matters for diffs. Instead of using schema.TypeSet, should it instead be using schema.TypeList?

terraform-provider-aws/aws/data_source_aws_iam_policy_document.go

Lines 344 to 364 in ac06ced

    
           func dataSourceAwsIamPolicyPrincipalSchema() *schema.Schema { 
        
           	return &schema.Schema{ 
        
           		Type:     schema.TypeSet, 
        
           		Optional: true, 
        
           		Elem: &schema.Resource{ 
        
           			Schema: map[string]*schema.Schema{ 
        
           				"type": { 
        
           					Type:     schema.TypeString, 
        
           					Required: true, 
        
           				}, 
        
           				"identifiers": { 
        
           					Type:     schema.TypeSet, 
        
           					Required: true, 
        
           					Elem: &schema.Schema{ 
        
           						Type: schema.TypeString, 
        
           					}, 
        
           				}, 
        
           			}, 
        
           		}, 
        
           	} 
        
           }

A similar change was applied to preserve the order of condition values. The type of the values in the condition schema was changes from TypeSet to TypeList.

e9a98cf

#17556

@YakDriver you were here recently (it was your PR that fixed the condition ordering!) Do you think changing TypeSet to TypeList here would be enough to fix this?

iainelder · 2021-05-25T08:34:13Z

See below for a small Terraform configuration that reproduces the issue. Change to a temporary directory and copy the configuration to a file in the directory called main.tf.

Before running the Terraform commands, first set your environment with credentials and a region.

export AWS_PROFILE=...
export AWS_DEFAULT_REGION=...

Output after first apply shows that the principal elements in the bucket policy are ordered according to the configuration.

terraform apply
terraform output --raw test | jq

{
  "Statement": [
    {
      "Action": "*",
      "Effect": "Deny",
      "Principal": {
        "AWS": [
          "arn:aws:iam::111111111111:role/role-0",
          "arn:aws:iam::111111111111:role/role-1",
          "arn:aws:iam::111111111111:role/role-2",
          "arn:aws:iam::111111111111:role/role-3",
          "arn:aws:iam::111111111111:role/role-4",
          "arn:aws:iam::111111111111:role/role-5",
          "arn:aws:iam::111111111111:role/role-6",
          "arn:aws:iam::111111111111:role/role-7",
          "arn:aws:iam::111111111111:role/role-8",
          "arn:aws:iam::111111111111:role/role-9"
        ]
      },
      "Resource": "arn:aws:s3:::tftest20210525081545966100000001"
    }
  ],
  "Version": "2012-10-17"
}

Second apply shows a change to the output. Without changing the effective permissions of the bucket policy, the principals are reordered.

terraform apply
terraform output --raw test | jq

Changes to Outputs:
  ~ test = jsonencode(
      ~ {
          ~ Statement = [
              ~ {
                  ~ Principal = {
                      ~ AWS = [
                          - "arn:aws:iam::111111111111:role/role-0",
                          - "arn:aws:iam::111111111111:role/role-1",
                          - "arn:aws:iam::111111111111:role/role-2",
                          - "arn:aws:iam::111111111111:role/role-3",
                            "arn:aws:iam::111111111111:role/role-4",
                            "arn:aws:iam::111111111111:role/role-5",
                          - "arn:aws:iam::111111111111:role/role-6",
                          - "arn:aws:iam::111111111111:role/role-7",
                          + "arn:aws:iam::111111111111:role/role-2",
                          + "arn:aws:iam::111111111111:role/role-3",
                          + "arn:aws:iam::111111111111:role/role-0",
                          + "arn:aws:iam::111111111111:role/role-1",
                            "arn:aws:iam::111111111111:role/role-8",
                            "arn:aws:iam::111111111111:role/role-9",
                          + "arn:aws:iam::111111111111:role/role-6",
                          + "arn:aws:iam::111111111111:role/role-7",
                        ]
                    }
                    # (3 unchanged elements hidden)
                },
            ]
            # (1 unchanged element hidden)
        }
    )

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Deny",
      "Principal": {
        "AWS": [
          "arn:aws:iam::111111111111:role/role-4",
          "arn:aws:iam::111111111111:role/role-5",
          "arn:aws:iam::111111111111:role/role-2",
          "arn:aws:iam::111111111111:role/role-3",
          "arn:aws:iam::111111111111:role/role-0",
          "arn:aws:iam::111111111111:role/role-1",
          "arn:aws:iam::111111111111:role/role-8",
          "arn:aws:iam::111111111111:role/role-9",
          "arn:aws:iam::111111111111:role/role-6",
          "arn:aws:iam::111111111111:role/role-7"
        ]
      },
      "Action": "*",
      "Resource": "arn:aws:s3:::tftest20210525081545966100000001"
    }
  ]
}

The order of the principals changes arbitrarily with each successive apply.

The Terraform configuration:

output "test" {
  value = aws_s3_bucket_policy.test.policy
}

resource "aws_s3_bucket" "test" {
  bucket_prefix = "tftest"
}

resource "aws_s3_bucket_policy" "test" {
  bucket = aws_s3_bucket.test.id

  policy = jsonencode({
      Statement = [
        {
          Principal = {
            AWS = [for r in aws_iam_role.test: r.arn]
          }
          Resource = "${aws_s3_bucket.test.arn}"
          Action = "*"
          Effect = "Deny"
        }
      ]
      Version = "2012-10-17"
  })
}

resource "aws_iam_role" "test" {
  count = 10
    
  name = "role-${count.index}"

  assume_role_policy = jsonencode({
    Version = "2012-10-17"
    Statement = [
      {
        Effect = "Deny"
        Action = "sts:AssumeRole"
        Principal = {
          AWS = "*"
        }
      }
    ]
  })
}

ewbankkit · 2021-07-08T20:44:46Z

Potentially related: jen20/awspolicyequivalence#10.

crawforde · 2021-08-05T21:34:09Z

I can confirm I'm having this same problem for both Principal and NotPrincipal blocks using terraform v1.0.0 and provider version v3.52.0. I recall this used to be an issue and then I believe it was corrected and I hadn't see it for a while. I was surprised to see it rearing its ugly head again. Is this a regression?

virgofx · 2021-10-09T03:22:44Z

Also observing this issue with hashicorp/aws 3.60.0 and Terraform 1.0.8.

github-actions · 2021-12-02T08:05:00Z

This functionality has been released in v3.68.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-05-27T02:33:33Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

ghost added service/iam Issues and PRs that pertain to the iam service. service/s3 Issues and PRs that pertain to the s3 service. labels May 4, 2020

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label May 4, 2020

iainelder mentioned this issue May 24, 2021

Audit log bucket policy always shows a diff nozaq/terraform-aws-secure-baseline#172

Closed

ewbankkit added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 5, 2021

NiklasWagner mentioned this issue Aug 26, 2021

DiffSuppressFunc should be used for "Objects have changed outside of Terraform" logic hashicorp/terraform-plugin-sdk#801

Closed

ewbankkit mentioned this issue Oct 14, 2021

Order is lost for data aws_iam_policy_document when applied to S3 buckets, iam roles, kms keys, etc #11801

Closed

This was referenced Nov 30, 2021

[Tracking Issue] Order is lost in JSON/policies when applied #21968

Closed

s3: Preserve order in JSON/policy arrays #21997

Merged

YakDriver closed this as completed in #21997 Dec 2, 2021

github-actions bot added this to the v3.68.0 milestone Dec 2, 2021

YakDriver mentioned this issue Feb 18, 2022

Perpetual diffs / Objects have changed outside of Terraform (v3.70.0+) #23288

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

s3 bucket policy with not_principal always changes due to ordering #13144

s3 bucket policy with not_principal always changes due to ordering #13144

craiggenner commented May 4, 2020

roccomuscaritolo-okta commented Nov 25, 2020

iainelder commented May 24, 2021

iainelder commented May 24, 2021

iainelder commented May 25, 2021 •

edited

Loading

ewbankkit commented Jul 8, 2021

crawforde commented Aug 5, 2021 •

edited

Loading

virgofx commented Oct 9, 2021

github-actions bot commented Dec 2, 2021

github-actions bot commented May 27, 2022

s3 bucket policy with not_principal always changes due to ordering #13144

s3 bucket policy with not_principal always changes due to ordering #13144

Comments

craiggenner commented May 4, 2020

Community Note

Terraform Version

Affected Resource(s)

Terraform Configuration Files

Debug Output

Panic Output

Expected Behavior

Actual Behavior

Steps to Reproduce

Important Factoids

References

roccomuscaritolo-okta commented Nov 25, 2020

iainelder commented May 24, 2021

iainelder commented May 24, 2021

iainelder commented May 25, 2021 • edited Loading

ewbankkit commented Jul 8, 2021

crawforde commented Aug 5, 2021 • edited Loading

virgofx commented Oct 9, 2021

github-actions bot commented Dec 2, 2021

github-actions bot commented May 27, 2022

iainelder commented May 25, 2021 •

edited

Loading

crawforde commented Aug 5, 2021 •

edited

Loading