aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.41k stars 3.8k forks source link

aws_efs (Python): Incorrect File System Policy defaults #27374

Open Kirizan opened 10 months ago

Kirizan commented 10 months ago

Describe the bug

When creating an EFS FileSystem in Python, a default file system policy is created regardless of the value passed to file_system_policy. If you specify a specific file system property, then it adds the custom policy to the default policy instead of replacing it. This all works as expected when deploying an EFS filesystem in TypeScript. I do not know if other languages also have the same issue.

Expected Behavior

I would expect the file_system_policy to reflect what was specified in the code.

Current Behavior

This policy is always applied or added to an EFS FileSystem:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "*"
            },
            "Action": [
                "elasticfilesystem:ClientRootAccess",
                "elasticfilesystem:ClientWrite"
            ],
            "Resource": "arn:aws:elasticfilesystem:us-east-1:<ACCN>:file-system/fs-0e8468d3b11778c73",
            "Condition": {
                "Bool": {
                    "elasticfilesystem:AccessedViaMountTarget": "true"
                }
            }
        }
    ]
}

Reproduction Steps

Deploy the following python CDK code, Browse to the console and you will see that there is an additional policy beyond the policy that was specified.

import aws_cdk as cdk
from aws_cdk import (
  Stack,
  aws_ec2 as ec2,
  aws_efs as efs,
  aws_iam as iam,
  RemovalPolicy
)
from constructs import Construct
class EFSConstruct(Construct):
  def __init__(
          self, scope: Construct, id: str,
          **kwargs) -> None:
    super().__init__(scope, id, **kwargs)
    self.file_system_name = "TestFileSystem"
    self.fs_policy = iam.PolicyDocument(
      statements=[
        iam.PolicyStatement(
          actions=["elasticfilesystem:*"],
          principals=[iam.AccountRootPrincipal()],
          resources=["*"]
        )
      ]
    )
    self.file_system = efs.FileSystem(
      self, "test_file_system",
      file_system_name=self.file_system_name,
      vpc=ec2.Vpc(self, "efs_test_vpc"),
      encrypted=True,
      performance_mode=efs.PerformanceMode.GENERAL_PURPOSE,
      enable_automatic_backups=True,
      file_system_policy=self.fs_policy,
      removal_policy=RemovalPolicy.DESTROY
    )
class CdkEfsTestStack(Stack):
  def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
    super().__init__(scope, construct_id, **kwargs)
    self.efs = EFSConstruct(
        self, "efs"
    )
app = cdk.App()
CdkEfsTestStack(app, "CdkEfsTestStack")
app.synth()

Deploy the following python CDK code, Browse to the console and you will see that there is a default policy, even though the file_system_policy was set to None (which according to the documentation, is the default value).

import aws_cdk as cdk
from aws_cdk import (
  Stack,
  aws_ec2 as ec2,
  aws_efs as efs,
  aws_iam as iam,
  RemovalPolicy
)
from constructs import Construct
class EFSConstruct(Construct):
  def __init__(
          self, scope: Construct, id: str,
          **kwargs) -> None:
    super().__init__(scope, id, **kwargs)
    self.file_system_name = "TestFileSystem"
    self.file_system = efs.FileSystem(
      self, "test_file_system",
      file_system_name=self.file_system_name,
      vpc=ec2.Vpc(self, "efs_test_vpc"),
      encrypted=True,
      performance_mode=efs.PerformanceMode.GENERAL_PURPOSE,
      enable_automatic_backups=True,
      file_system_policy=None,
      removal_policy=RemovalPolicy.DESTROY
    )
class CdkEfsTestStack(Stack):
  def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
    super().__init__(scope, construct_id, **kwargs)
    self.efs = EFSConstruct(
        self, "efs"
    )
app = cdk.App()
CdkEfsTestStack(app, "CdkEfsTestStack")
app.synth()

Possible Solution

No response

Additional Information/Context

I discovered this bug while trying to create an EFS backed ECS Fargate container. The default policy doesn't allow containers to access the file system

CDK CLI Version

2.99.0 (build 0aa1096)

Framework Version

No response

Node.js Version

v20.7.0

OS

MacOS

Language

Python

Language Version

Python 3.11.5

Other information

No response

indrora commented 9 months ago

Can you demonstrate that this does not happen in another language (e.g. Go/Java/etc) and is limited only to the Python bindings?

Kirizan commented 9 months ago

@indrora Yes, if you deploy this CDK project (https://github.com/richardneililagan/vaultwarden-ecs-fargate/tree/main), you will see that the file system has no File System Policy. I discovered the issue translating that project from TypeScript to Python (translating between languages helps me understand tools like CDK better).

peterwoodworth commented 9 months ago

This PR introduced a feature flag which adjusts how the policy creation works. You're likely seeing a difference due to whether this flag exists in your code or not - not because of a difference in language

You can either use the feature flag, or use allowAnonymousAccess instead

We should update the documentation on the fileSystemPolicy prop to reflect these changes

Kirizan commented 9 months ago

Neither the python code in my post, nor the code in the TypeScript project I shared (link to the relevant file and line) have that flag set. If it was just because of that flag, it needing to be configured in one language but not another is a difference in the languages.

Kirizan commented 9 months ago

This is not a documentation issue. Even if it is related to that feature flag, both environments were fresh, default, environments. Deploying an EFS filesystem using default settings in TypeScript results in a different configuration then deploying an EFS filesystem using default settings in Python. The Python default settings are not in line with how EFS defaults work. If you deploy EFS with default settings in the console, it matches the TypeScript deployment. The only place (that I've testing) with the incorrect defaults is when deploying with Python.

Kirizan commented 9 months ago

Just to double check, I went and deployed the python code using the allow_anonymous_access=True with the EFS. It still deploys something different then the TypeScript and Console defaults.

This policy is now applied when allowing anonymous access:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::<ACCN>:root"
            },
            "Action": "elasticfilesystem:*",
            "Resource": "*"
        }
    ]
}

In both TypeScript and the Console, the default file system policy is empty.

Kirizan commented 9 months ago

@indrora @peterwoodworth Can we get the documentation tag removed, since this is not a documentation issue?

kavehamazon commented 8 months ago

Also running into this issue.

nikvin15 commented 7 months ago

We are also getting the same!

Leonid-tup commented 7 months ago

I had the same issue. Setting the flag allow_anonymous_access to True solved that for me. The policy is now empty

IllarionovDimitri commented 3 weeks ago

i have generated a new project with cdk init and there is this cdk.json which now (with cdk version 147) generates much rules as before. one of them was indeed "@aws-cdk/aws-efs:denyAnonymousAccess": true, which as said in the docs "Default: false when using grantRead, grantWrite, grantRootAccess or set @aws-cdk/aws-efs:denyAnonymousAccess feature flag, otherwise true" might redefine the settings in the code. I set "@aws-cdk/aws-efs:denyAnonymousAccess": false and everything worked again, it means no policy was generated on EFS