fix(query): adding support for CloudFormation queries missing ingress/egress resources - Part 1

[cx-andre-pereira]

Closes #7303

Reason for Proposed Changes

• Part 2 | Part 3

• Currently some CloudFormation queries do not support the newer "AWS::EC2::SecurityGroupIngress" and "AWS::EC2::SecurityGroupEgress" resources, only working for samples with the legacy "AWS::EC2::SecurityGroup" resource.

• This Pull Request will only handle the first 5 queries to simplify the review processes to come.

Query List

Query Name	Query ID	Query Logic
ELB With Security Group Without Outbound Rules	01d5a458-a6c4-452a-ac50-054d59275b7c	query 1*
DB Security Group With Public Scope	9564406d-e761-4e61-b8d7-5926e3ab8e79	query 2*
Default Security Groups With Unrestricted Traffic	ea33fcf7-394b-4d11-a228-985c5d08f205	query 3*
EC2 Sensitive Port Is Publicly Exposed	494b03d3-bf40-4464-8524-7c56ad0700ed	query 4*
ELB Sensitive Port Is Exposed To Entire Network	78055456-f670-4d2e-94d5-392d1cf4f5e4	query 5*
HTTP Port Open To Internet	ddfc4eaa-af23-409f-b96c-bf5c45dc4daa	query 6
Remote Desktop Port Open To Internet	c9846969-d066-431f-9b34-8c4abafe422a	query 7
Security Groups Allows Unrestricted Outbound Traffic	66f2d8f9-a911-4ced-ae27-34f09690bb2c	query 8
Security Group Unrestricted Access To RDP	3ae83918-7ec7-4cb8-80db-b91ef0f94002	query 9
Security Groups With Exposed Admin Ports	cdbb0467-2957-4a77-9992-7b55b29df7b7	query 10
Security Groups With Meta IP	adcd0082-e90b-4b63-862b-21899f6e6a48	query 11
Security Group With Unrestricted Access To SSH	6e856af2-62d7-4ba2-adc1-73b62cef9cc1	query 12
Unknown Port Exposed To Internet	829ce3b8-065c-41a3-ad57-e0accfea82d2	query 13
DB Security Group Open To Large Scope	0104165b-02d5-426f-abc9-91fb48189899	query 14

• During initial development, the "ELB With Security Group Without Inbound Rules" query stood out right away. My goal was to use this query as a template for the outbound rules analog since it seemingly already supports the "AWS::EC2::SecurityGroupIngress" resources. This "fix" was done 5 years ago and was faulty on an already defective query. Currently:
  • The query uses its auxiliary function "withoutOutboundRules" incorrectly since it will flag in case the target "SecurityGroups" do not have a not null "SecurityGroupIngress" array OR do not have standalone "AWS::EC2::SecurityGroupIngress" resources associated with them, when the actual goal is both conditions simultaneously.
  • Additionally the auxiliary function searches all documents from the input which is a critical error that can easily lead to False Positives, if we scan 100 samples with a group named "sample_group" and a single one meets the conditions to flag, the "sample_group" would flag in ALL files.
  • Finally the query's metadata and auxiliary function name mention "outbound" as warned in bug(cloudformation): elb_with_security_group_without_inbound_rules fails when other SG use AWS::EC2::SecurityGroupIngress #7303.

Proposed Changes

• Firstly i fixed the issues mentioned for the ✅"ELB With Security Group Without Inbound Rules" query, the new implementation now works as intended, additionally new negative tests were added to show "AWS::EC2::SecurityGroupIngress" resources prevent flagging and new positive tests to show that the "GroupId" must have the correct reference name, lastly some of these new tests have an "elb" of type "AWS::ElasticLoadBalancingV2::LoadBalancer" to ensure that said type is also supported. Note that the changes on this query affected 4 E2E tests because line 14 was wrongfully flagged on positive.yaml, the security grup has standalone ingress rules but still flagged because it did not declare a "SecurityGroupIngress" array.

• ✅ELB With Security Group Without Outbound Rules

  • Having fixed the analog "Inbound" rule query this was a simple copy paste and update of resource names along with the tests, the implementations are near identical. All mentions of "ingress" and related resources were replaced with "egress".

• ✅DB Security Group With Public Scope

  • For this query i started by simplifying the overall query logic so that there is a single CxPolicy in use instead of 3 very similar ones.
  • I added all the relevant resources to a "types" array to maintain the single CxPolicy while also adding a new auxiliary function : "get_ingress_list". This new function is similar to the one made for the original terraform fixs and it allows handling both array and non array ingress instances through a single policy by making sure the main auxiliary function always receives an array and just adjusts the "search" values for each scenario.
  • Speaking of the "search" values these were improved for the case of inline Ingress arrays since now the query will point to the specific entry on the array, this will affect simId values and the "transition" file was adjusted accordingly.
  • I added a new condition to the "is_public_dbinstance" (previously "check_public") function so that it flags in case the "PubliclyAccessible" resource is undefined because the default value for it varies as per the documentation : "The default behavior value depends on your VPC setup and the database subnet group." , so we cannot assume that it defaults to "false".
  • Added "searchLine" values and made it so ipv6 is checked for all valid forms of "::/0". To do this i moved the "unrestricted_ipv6" list of valid forms from the terraform library to the common library since it is relevant for any platform handling ipv6 addresses. With this change a terraform query that used the array had to be updated.
  • Finally i added all the missing relevant tests and included comments for them and the query logic.

• ✅Default Security Groups With Unrestricted Traffic

  • Here to add support for the new resources i created a new 'CxPolicy' to handle all standalone ingress/egress resources. The flagging of standalone ingress/egress resources will point to the "GroupId" that associates the "default" group getting flagged with said rule.
  • I added "searchLine" values and new tests, also the existing tests were shortened to focus on the query's target resource and the keyActual/Expected values were also updated to better describe the reasoning for the query flagging.
  • To top it off a new negative test was added to show that a security group that is not named "default" does not flag.

Note : In the query the "searchValue" for the current implementation to the whole "Properties" field of the flagged "AWS::EC2::SecurityGroup" field when it could point to the "Properties.SecurityGroupIngress" or "Properties.SecurityGroupEgress" fields, this time i left said value unchanged; it does not seem warranted given the impact on the simID.

• ✅EC2 Sensitive Port Is Publicly Exposed and ✅ELB Sensitive Port Is Exposed To Entire Network
  • Some changes to the "get_ingress_list" function were done, i simplified the implementation and transferred it to the cloudformation library since it was of use for this query along with the one i made it for originally (DB Security Group With Public Scope) which was updated accordingly. After some changes the query uses its own get_ingress_list_if_exists function and the search_for_standalone_ingress function to get all ingresses available; additionally the "IpProtocol" set to "-1" scenario is properly handles by the get_sensitive_ports function.
  • I removed the "udpPortsMap" array that was used on this query and the last query on this PR. This map was a smaller version of the common library's tcpPortsMap that only included ports that are typically used with UDP; from my research and based on all analog terraform queries we should check all vulnerable ports for the "udp" protocol as well not just the ones that are "expected" to be used through TCP only; with this change the related queries were simplified and are more thorough overall.
  • I simplified and structured the testing for both queries to be very similar and streamlined whilst testing all relevant scenarios. Additionally these tests include ipv6 since, now, the CidrIpv6 field is being checked for. It was not "missing" per say before since only the newly added standalone ingress resource supports ipv6 in the first place.
  • Finally for the "search" values i made sure they always pointed to the relevant ingress through the "get_search_values" auxiliary function.

Note : Initially i found the "EC2" query's implementation to be particularly odd; it did not flag all "AWS::EC2::SecurityGroup" that were exposed, only the ones associated with an "AWS::EC2::Instance". This is inline with the query naming but goes against the way most other queries work and how the analog terraform query works. The "ELB" analog has the same behavior for ELB resources. I believe it could be argued the analog terraform query's approach is better by, with a single query, flagging all exposed security groups regardless of being in use or not; but since deleting a query is not something i can do out of the blue i decided to simply fall back to focus on the original intent of both these queries with focus on the support for the missing resource, the "AWS::EC2::SecurityGroupIngress".

• After completing all the queries initial implementations i decided to adjust the logic for the DB Security Group With Public Scope query since the code for the main auxiliary function was significantly bigger than necessary. (a5fd335)

• Additionally i checked if it was possible to include the legacy "[AWS::RDS::DBSecurityGroup]
  (https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/aws-resource-rds-dbsecuritygroup.html)" resource whenever possible but it is only possible for the query that already checked for it (Default Security Groups With Unrestricted Traffic), all the other queries rely on referencing of security groups through either the an "ELB" or "EC2" instance resource which exclusively support "AWS::EC2::SecurityGroup" VPC based security groups, they are incompatible with the "EC2-Classic" based "AWS::RDS::DBSecurityGroup" which is mostly unused nowadays. The one query that could apply "Default Security Groups With Unrestricted Traffic" is also incompatible since it relies on checking that the "GroupName" field is set to default, this field is exclusive to the newer security group resource.

Final Notes:

• These queries update is related to the terraform platform's fixes in #7646 .
• The cloudFormation_aws.yaml simID transition file contains all the queries affected not only in this part but also for parts 2 and 3 to facilitate merger.
• I should point that any relocation of auxiliary functions to the common or cloudformation libraries will be done only after this first part and part2 are merged in order to facilitate cross referencing between all altered queries and ensure only those used in 3+ queries or that can be refactored into more generic functions are relocated/refactored.

I submit this contribution under the Apache-2.0 license.

[github-actions]

KICS version: v2.1.13

Category Results CRITICAL 0 HIGH 0 MEDIUM 0 LOW 0 INFO 0 TRACE 0 TOTAL 0	Metric Values Files scanned 1 Files parsed 1 Files failed to scan 0 Total executed queries 47 Queries failed to execute 0 Execution time 0