prowler icon indicating copy to clipboard operation
prowler copied to clipboard

remove public_ports from SecurityGroup to make code more concise

Open kagahd opened this issue 1 year ago • 1 comments

Context

Make the AWS ec2 service and around around 15 prowler checks, that verify whether ports are open to the Internet, more concise.

Description

Each of the around 15 prowler checks that verify whether ports are open to the Internet had a condition to check all ingress rules of the security group if the property security_group.public_ports was False. The only exception to this was the prowler check ec2_securitygroup_allow_ingress_from_internet_to_all_ports. However, the value of security_group.public_ports was set by the method __describe_security_groups__ of the ec2_service.py which is run before the prowler checks are executed in order to gather information about the security groups of the ec2 service. The value security_group.public_ports is only set to True if self.audited_checks contained the above mentioned check ec2_securitygroup_allow_ingress_from_internet_to_all_ports AND if the method check_security_group returned True.

I found it strange that the generic method __describe_security_groups__ refers to a specific prowler check, here ec2_securitygroup_allow_ingress_from_internet_to_all_ports. Secondly, each of the 15 prowler checks evaluated the variable security_group.public_ports which however changed its value only in the above mentioned, very specific, condition and called then again the method check_security_group that was already executed beforehand by the method __describe_security_groups__ of the ec2_service.py.

I find it more concise to:

  • remove the property public_ports from the class SecurityGroup
  • remove the code from the method __describe_security_groups__ of the ec2_service.py that may change the property security_group.public_ports from False to True only for one specific condition
  • remove the condition statement if not security_group.public_ports in all of the 15 prowler checks

License

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

kagahd avatar May 11 '24 21:05 kagahd

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.44%. Comparing base (45ccd7e) to head (a606921).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3979      +/-   ##
==========================================
- Coverage   86.51%   86.44%   -0.08%     
==========================================
  Files         768      768              
  Lines       23899    23882      -17     
==========================================
- Hits        20677    20644      -33     
- Misses       3222     3238      +16     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 11 '24 21:05 codecov[bot]