docs: Update statement about IAM policies for DynamoDB tables

[tdmalone]

The S3 backend documentation currently states that:

It is not possible to apply such fine-grained access control to the DynamoDB table used for locking, so it is possible for any user with Terraform access to lock any workspace state, even if they do not have access to read or write that state.

This is no longer correct: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/specifying-conditions.html

Probably semi-related: #13378

[apparentlymart]

Thanks for pointing that out, @tdmalone!

I think to complete this (and also fix #13378 at the same time) we'll need to come up with some real example policies that match the way the backend generates object keys, and any necessary configuration for the table, etc.

[tdmalone]

I'm happy to provide what I've got (ideally I would just put this into a PR to update the docs... I'm just totally outta time so if no-one else manages to do it, I might be able to find time eventually!):

resource "aws_dynamodb_table" "state" {
  name           = "terraform-state"
  hash_key       = "LockID"
  read_capacity  = 2
  write_capacity = 2

  attribute {
    name = "LockID"
    type = "S"
  }
}

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "AllowStateLocking",
      "Effect": "Allow",
      "Action": [
        "dynamodb:DeleteItem",
        "dynamodb:GetItem",
        "dynamodb:PutItem"
      ],
      "Resource": "arn:aws:dynamodb:${region}:${account_id}:table/${table_name}",
      "Condition": {
        "ForAllValues:StringEquals": {
          "dynamodb:LeadingKeys": [
            "${table_name}/${state_key}",
            "${table_name}/${state_key}-md5"
          ]
        }
      }
    }
  ]
}

[davidgatti]

I'm confused, how can you provide an array for dynamodb:LeadingKeys? And what would be the result of such Policy?

[tdmalone]

@davidgatti See Creating a Condition That Tests Multiple Key Values (Set Operations) in the IAM documentation:

ForAllValues – The condition returns true if there's a match between every one of the specified key values in the request and at least one value in the policy.

Along with the DynamoDB IAM conditions docs, essentially this means that if the partition key (LockID) value is "my-state-table/my-state-name" OR "my-state-table/my-state-name-md5", the request will be allowed. If it's "my-state-table/someone-elses-state-name", it will be denied.

There's also ForAnyValues. I don't think I fully tested this at the time but my understanding of that from the documentation is that it could allow keys through that I would like to deny as long as an allowable key is also present in the request (i.e. a bulk insert).

[davidgatti]

Hmm, respect in this case, I would not have inferred from the docs that you can pass an array to a dynamodb:LeadingKeys key. Thank you for this insight :)

[Sk8rMarc]

I'm confused about using :StringLike with LeadingKeys in a Deny clause to protect certain rows in a table. The documentation for dynamodb:LeadingKeys says to always use ForAllValues. vs ForAnyValue.

Anyone know whether ForAllValues:StringLike with dynamodb:LeadingKeys in a deny statement can be used to defend/protect more than one row in a given table ?

[Sk8rMarc]

I did some testing.

Given: :StringLike with two or more wildcard values for dynamodb:LeadingKeys

• DeleteItem: Didn't matter if ForAllValues or ForAnyValue, deny statement with this condition worked.
• BatchWriteItem/DeleteRequest: If the batch intersected with deny statement condition values, all items in batch were deleted with ForAllValues, including the items we were trying to protect. I presume because the batch set wasn't contained within the deny statement condition values. With ForAnyValue, the batch deletion fails if any item in the batch matches any one of the deny statement condition values.

[tdmalone]

@Sk8rMarc If I’m following correctly (and I’m on my phone on the couch, so I might not be!), that sounds about right to me - my initial suggestion to use ForAllValues was for an Allow statement. It sounds like a wise idea to use ForAnyValue for a Deny statement due to the switched logic.

I think the original scope of this request would probably center around writing a valid policy for Allow requests - to give the necessary permissions in the first instance - but if the docs weren’t gonna get too long/complicated then I suppose an ‘allow all’ and ‘deny some’ example might be helpful to some!

[mildwonkey]

I am going to close this issue due to inactivity. There have been many changes to our documentation and the s3 backend itself since this was initially opened which should cover this request: https://www.terraform.io/docs/backends/types/s3.html

If there is still a question, I recommend the the community forum, where there are far more people available to help. If there is a bug or you would like to make a feature request, please open a new issue and fill out the template. Thanks!

[tdmalone]

Hi @mildwonkey, this wasn't a support thread, it was a request for a doc update - and questions regarding the topic ended up being asked here because the docs are incorrect.

The initial request still stands - the docs were once correct, but just haven't been since additional functionality was added to IAM for DynamoDB.

So, this should probably be re-opened.

From the initial post in this issue:

The S3 backend documentation currently states that:

It is not possible to apply such fine-grained access control to the DynamoDB table used for locking, so it is possible for any user with Terraform access to lock any workspace state, even if they do not have access to read or write that state.

This is no longer correct: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/specifying-conditions.html

This paragraph quoted above is still in place on the current live docs, so the changes to the documentation since this was initially opened don't appear to have covered this request.

[mildwonkey]

Thanks for pointing that details out @tdmalone , I'm sorry! I closed this in error.

[duncm]

This was fixed by #30423 \o/

[laurapacilio]

Hooray! Thank you for flagging this @duncm - I'm going to close this out then.

[github-actions[bot]]

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.