Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deny_nonsecure_transport grants read-write access to all principals #30

Open
1 task done
davejbax opened this issue Apr 24, 2024 · 9 comments
Open
1 task done

Comments

@davejbax
Copy link

Description

The policy generated by deny_nonsecure_transport grants access to all AWS principals. This makes the use of IAM to control access to the filesystem impossible when this boolean is set, and is an extremely significant side-effect of the boolean (in contrast to having it set to false and using policy_statements) that is not clear in documentation.

The problematic policy was added in #21, in an attempt to fix #20 and #11. In particular, I think the policy given in #20 is the incorrect policy to fix the ECS issue because it only works because it grants access to all principals -- which is far too broad of a policy, and probably not the intention.

#20 mentions that the web console generated the policy. However, I believe the policies generated by the web console are intended to be used where IAM is not used to control access to the filesystem: all of them generate a similar policy granting access to all principals, with specific denies; this is because the 'default' EFS policy is to allow access to all principals, and use firewall rules to control access.

I think this module should support using IAM to selectively control access to the EFS filesystem, instead of firewall rules alone. At the very least, it should be made more explicit that deny_nonsecure_transport precludes the use of IAM. I would suggest creating a new boolean that makes it very explicit as to whether a 'allow all principals' policy will be attached; then, this could be set to false to facilitate the use of IAM to control access.

  • ✋ I have searched the open/closed issues and my issue is not listed.

Versions

  • Module version [Required]: v1.6.2

  • Terraform version: v1.6.3

  • Provider version(s): provider registry.terraform.io/hashicorp/aws v5.40.0

Reproduction Code [Required]

module "efs" {
  source = "terraform-aws-modules/efs/aws"

  # File system
  name           = "example"
  creation_token = "example-token"
  encrypted      = true
  kms_key_arn    = "arn:aws:kms:eu-west-1:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab"

  lifecycle_policy = {
    transition_to_ia = "AFTER_30_DAYS"
  }

  # File system policy
  attach_policy                      = true
  bypass_policy_lockout_safety_check = false
  deny_nonsecure_transport = true
  policy_statements = [
  # XXX: This policy has no effect, because of the 'grant all' policy added by deny_nonsecure_transport!
    {
      sid     = "Example"
      actions = ["elasticfilesystem:ClientMount"]
      principals = [
        {
          type        = "AWS"
          identifiers = ["arn:aws:iam::111122223333:role/EfsReadOnly"]
        }
      ]
    }
  ]

  # Mount targets / security group
  mount_targets = {
    "eu-west-1a" = {
      subnet_id = "subnet-abcde012"
    }
    "eu-west-1b" = {
      subnet_id = "subnet-bcde012a"
    }
    "eu-west-1c" = {
      subnet_id = "subnet-fghi345a"
    }
  }
  security_group_description = "Example EFS security group"
  security_group_vpc_id      = "vpc-1234556abcdef"
  security_group_rules = {
    vpc = {
      # relying on the defaults provdied for EFS/NFS (2049/TCP + ingress)
      description = "NFS ingress from VPC private subnets"
      cidr_blocks = ["10.99.3.0/24", "10.99.4.0/24", "10.99.5.0/24"]
    }
  }
}

Steps to reproduce the behavior:

  1. Create an EFS filesystem using the module that:
    • Has deny_nonsecure_transport set to true
    • Uses policy_statements to attempt to grant some form of access to a specific IAM principal
  2. Attempt to mount the EFS filesystem with an IAM principal that you did not grant explicit access to
  3. Notice that the EFS filesystem is mounted successfully

Expected behavior

The EFS filesystem should not be able to be mounted by any IAM principal when deny_nonsecure_transport is true

Actual behavior

The EFS filesystem is able to be mounted by any IAM principal when deny_nonsecure_transport is true, regardless of any allows in policy_statements.

@lorengordon
Copy link

I'd add that, checking EFS docs, the statement in question has nothing to do with nonsecure transport, and so gating it with deny_nonsecure_transport is rather misleading. The statement is instead just one approach AWS suggests to consider the file system non-public...

Copy link

github-actions bot commented Jun 6, 2024

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Jun 6, 2024
@davejbax
Copy link
Author

davejbax commented Jun 6, 2024

Not stale

@github-actions github-actions bot removed the stale label Jun 7, 2024
Copy link

github-actions bot commented Jul 7, 2024

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Jul 7, 2024
@lorengordon
Copy link

Still not stale

@github-actions github-actions bot removed the stale label Jul 8, 2024
Copy link

github-actions bot commented Aug 7, 2024

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Aug 7, 2024
@gmeligio
Copy link

gmeligio commented Aug 7, 2024

I hit this issue when setting the least privileged access to a new EFS file system. I was following this EFS guide and using deny_nonsecure_transport to leave that secure transport policy to the module and trying to add the rest of secure policies, but, as mentioned in this issue, deny_nonsecure_transport adds too broad permissions. I will try without deny_nonsecure_transport, but it's less ideal to repeat the permissions logic that this module already has.

@github-actions github-actions bot removed the stale label Aug 8, 2024
Copy link

github-actions bot commented Sep 7, 2024

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Sep 7, 2024
@davejbax
Copy link
Author

davejbax commented Sep 7, 2024

Not stale: issue still exists as far as I can tell

@github-actions github-actions bot removed the stale label Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants