feat(ec2): Amazon EC2 Instances Should Not Use Multiple ENIs #4935

MarioRgzLpz · 2024-09-05T10:24:12Z

Context

Using multiple ENIs can lead to increased network security complexity and unintended network paths, particularly in instances with multiple subnets. I added a new check that helps ensure that instances are not overly complex or exposed to additional network risks.

Checks whether an Amazon EC2 instance uses multiple Elastic Network Interfaces (ENIs) or Elastic Fabric Adapters (EFAs). The control passes if the instance is using a single network adapter.

Description

Change ec2_service to add a new attribute network interfaces to Instance model saving ENIs ids to check type. Added the check logic in ec2_instance_multiple_eni. Added tests for service and check.

Checklist

Are there new checks included in this PR? Yes
- If so, do we need to update permissions for the provider? No
Review if the code is being covered by tests.
Review if code is being documented following this specification https://github.com/google/styleguide/blob/gh-pages/pyguide.md#38-comments-and-docstrings
Review if backport is needed.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

… chose what we save for each launch template version, adjust the service test to the changes

…nterfaces

…bute public_ip_addresses with ipaddress IPs to the NetworkInterface model with tests, added tags to LaunchTemplate model and list with network interfaces to TemplateData model

…erfaces to adjust to the change from list to dict and add new attribute public_ip_addresses to one ec2 test

…he-data-saved-in-template-data-in-launch-template-version

…blic_ip_addresses

…l, network_interfaces, and adjust service tests

…t-use-multiple-en-is

MrCloudSec · 2024-09-10T18:28:28Z

prowler/providers/aws/services/ec2/ec2_service.py

@@ -72,6 +72,11 @@ def _describe_instances(self, regional_client):
                        if not self.audit_resources or (
                            is_resource_filtered(arn, self.audit_resources)
                        ):
+                            enis = []
+                            for eni in instance.get("NetworkInterfaces"):


Suggested change

for eni in instance.get("NetworkInterfaces"):

for eni in instance.get("NetworkInterfaces, []"):

MrCloudSec · 2024-09-10T18:34:18Z

prowler/providers/aws/services/ec2/ec2_instance_multiple_eni/ec2_instance_multiple_eni.py

+                        )
+
+                message_status_extended = ""
+                if len(eni_types["efa"]) + len(eni_types["interface"]) > 1:


Why not trunk here?

SecurityHub recommendation is to check only for multiple EFAs or interfaces. Thats because a trunk ENI is used to increase the number of ENIs you can attach to a instance. For more information look at the first paragraph here https://docs.aws.amazon.com/AmazonECS/latest/developerguide/container-instance-eni.html and at the AWSVPC trunking section here https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-account-settings.html. I don't know if there should be a limit to trunk ENIs maybe we have to discuss that.

Can we then ignore trunk ENIs in the check? So the user do not get confused when seeing the status extended since he is not going to be able to delete those trunk ENIs.

Why aren't you able to delete trunk ENIs? Looked for it in the docs but didn't find it. Still I think is a good idea to ignore trunk ENIs since they only add noise and no real value to the provided information.

We saw that trunks are normal interfaces with a specific option so I would count it as an ENI too.

MrCloudSec · 2024-09-10T18:35:00Z

...providers/aws/services/ec2/ec2_instance_multiple_eni/ec2_instance_multiple_eni.metadata.json

@@ -0,0 +1,34 @@
+{
+  "Provider": "aws",
+  "CheckID": "ec2_instance_multiple_eni",


Suggested change

"CheckID": "ec2_instance_multiple_eni",

"CheckID": "ec2_instance_uses_single_eni",

MrCloudSec · 2024-09-10T18:55:55Z

tests/providers/aws/services/ec2/ec2_instance_multiple_eni/ec2_instance_multiple_eni_test.py

+            assert result[0].resource_id == "i-0123456789abcdef0"
+            assert (
+                result[0].status_extended
+                == "EC2 Instance i-0123456789abcdef0 uses multiple ENIs: "


Can you prettify this message please?

What do you mean by prettify? Should I add global attributes for the instance id and enis ids?

Something like:

"EC2 Instance i-0123456789abcdef0 uses only one ENI: (EFAs: ['eni-2'], Trunks: ['eni-1', 'eni-3'].)"

codecov · 2024-09-10T18:59:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.98%. Comparing base (c0c5996) to head (b6fce74).
Report is 1206 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4935      +/-   ##
==========================================
+ Coverage   88.94%   88.98%   +0.04%     
==========================================
  Files         951      955       +4     
  Lines       29182    29295     +113     
==========================================
+ Hits        25956    26069     +113     
  Misses       3226     3226

Components	Coverage Δ
prowler	`88.98% <100.00%> (+0.04%)`	⬆️
api	`∅ <ø> (∅)`

…t-use-multiple-en-is

…rettify status extended

MarioRgzLpz and others added 11 commits August 23, 2024 10:41

feat(ec2): Add new TemplateData model to the service so we can better…

b7222cc

… chose what we save for each launch template version, adjust the service test to the changes

feat(ec2): Adjust old secret check and test to the model changes

3c7adc3

chore(ec2): Delete some comments and add a default value for NetworkI…

2abc66e

…nterfaces

feat(ec2): Change ec2 network interfaces to a dict, added a new attri…

476dce8

…bute public_ip_addresses with ipaddress IPs to the NetworkInterface model with tests, added tags to LaunchTemplate model and list with network interfaces to TemplateData model

refactor(ec2/route53): Change one route53 check that uses network int…

cddb66b

…erfaces to adjust to the change from list to dict and add new attribute public_ip_addresses to one ec2 test

chore(ec2): Add tags to LaunchTemplate

241d558

Merge branch 'master' into PRWLR-4617-change-ec-2-service-to-adjust-t…

bd7c344

…he-data-saved-in-template-data-in-launch-template-version

fix(ec2): Fix flake8 error

def4332

fix(ec2): Fix type error in ec2 service NetworkInterface attribute pu…

caf21ea

…blic_ip_addresses

feat(ec2): Modify the service to add a new attribute to Instance mode…

ba69921

…l, network_interfaces, and adjust service tests

feat(ec2): Add check logic with respective unit test and metadata

1550379

MarioRgzLpz requested review from a team as code owners September 5, 2024 10:24

Base automatically changed from PRWLR-4617-change-ec-2-service-to-adjust-the-data-saved-in-template-data-in-launch-template-version to master September 9, 2024 16:32

Merge branch 'master' into PRWLR-4489-amazon-ec-2-instances-should-no…

c365698

…t-use-multiple-en-is

github-actions bot added the provider/aws Issues/PRs related with the AWS provider label Sep 10, 2024

MrCloudSec reviewed Sep 10, 2024

View reviewed changes

MarioRgzLpz and others added 3 commits September 11, 2024 11:48

chore(ec2): Rename check, add default value to NetworkInterface

8f5017f

Merge branch 'master' into PRWLR-4489-amazon-ec-2-instances-should-no…

191388f

…t-use-multiple-en-is

chore(ec2): Change check logic to FAIL if there are trunk ENIs too, p…

b6fce74

…rettify status extended

MrCloudSec self-requested a review September 12, 2024 17:36

MrCloudSec approved these changes Sep 12, 2024

View reviewed changes

MrCloudSec merged commit 92b6e72 into master Sep 12, 2024
11 checks passed

MrCloudSec deleted the PRWLR-4489-amazon-ec-2-instances-should-not-use-multiple-en-is branch September 12, 2024 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ec2): Amazon EC2 Instances Should Not Use Multiple ENIs #4935

feat(ec2): Amazon EC2 Instances Should Not Use Multiple ENIs #4935

MarioRgzLpz commented Sep 5, 2024

MrCloudSec Sep 10, 2024

MrCloudSec Sep 10, 2024

MarioRgzLpz Sep 11, 2024

MrCloudSec Sep 11, 2024

MarioRgzLpz Sep 12, 2024

MrCloudSec Sep 12, 2024

MrCloudSec Sep 10, 2024

MrCloudSec Sep 10, 2024

MarioRgzLpz Sep 11, 2024

MrCloudSec Sep 11, 2024

codecov bot commented Sep 10, 2024 •

edited

Loading

	for eni in instance.get("NetworkInterfaces"):
	for eni in instance.get("NetworkInterfaces, []"):

	"CheckID": "ec2_instance_multiple_eni",
	"CheckID": "ec2_instance_uses_single_eni",

feat(ec2): Amazon EC2 Instances Should Not Use Multiple ENIs #4935

feat(ec2): Amazon EC2 Instances Should Not Use Multiple ENIs #4935

Conversation

MarioRgzLpz commented Sep 5, 2024

Context

Description

Checklist

License

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 10, 2024 • edited Loading

Codecov Report

codecov bot commented Sep 10, 2024 •

edited

Loading