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

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

Open
Kirizan opened this issue Oct 2, 2023 · 11 comments
Open

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

Kirizan opened this issue Oct 2, 2023 · 11 comments
Labels
@aws-cdk/aws-efs Related to Amazon Elastic File System bug This issue is a bug. documentation This is a problem with documentation. p2

Comments

@Kirizan
Copy link

Kirizan commented Oct 2, 2023

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

@Kirizan Kirizan added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 2, 2023
@github-actions github-actions bot added the @aws-cdk/aws-efs Related to Amazon Elastic File System label Oct 2, 2023
@indrora
Copy link
Contributor

indrora commented Oct 3, 2023

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

@indrora indrora added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 3, 2023
@Kirizan
Copy link
Author

Kirizan commented Oct 4, 2023

@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).

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 4, 2023
@peterwoodworth
Copy link
Contributor

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

@peterwoodworth peterwoodworth added p2 documentation This is a problem with documentation. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 4, 2023
@peterwoodworth peterwoodworth changed the title aws_efs (Python): Incorrect File System Policy defaults aws_efs (Python): Incorrect File System Policy default in docs Oct 4, 2023
@peterwoodworth peterwoodworth changed the title aws_efs (Python): Incorrect File System Policy default in docs aws_efs: Incorrect File System Policy default in docs Oct 4, 2023
@Kirizan
Copy link
Author

Kirizan commented Oct 4, 2023

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 Kirizan changed the title aws_efs: Incorrect File System Policy default in docs aws_efs (Python): Incorrect File System Policy defaults Oct 6, 2023
@Kirizan
Copy link
Author

Kirizan commented Oct 6, 2023

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
Copy link
Author

Kirizan commented Oct 6, 2023

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
Copy link
Author

Kirizan commented Oct 23, 2023

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

@kavehamazon
Copy link

kavehamazon commented Nov 22, 2023

Also running into this issue.

@nikvin15
Copy link

We are also getting the same!

@Leonid-tup
Copy link

Leonid-tup commented Dec 25, 2023

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

@IllarionovDimitri
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-efs Related to Amazon Elastic File System bug This issue is a bug. documentation This is a problem with documentation. p2
Projects
None yet
Development

No branches or pull requests

7 participants