Add cross account trust rule for ES and OpenSearch domains (#200)

* upgrade to pycfmodel 0.13.0 in order to have ES and OpenSearch domains * add cross account trust rules for ES and OpenSearch domains * add tests and test files * bump version and update changelog Co-authored-by: Ramon <[email protected]>
Skyscanner · Jan 17, 2022 · 19dfee4 · 19dfee4
1 parent 5c388f8
commit 19dfee4
Show file tree

Hide file tree

Showing 13 changed files with 371 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,11 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [1.3.0] - 2022-1-17
+### Improvements
+- Add `ElasticsearchDomainCrossAccountTrustRule` and `OpenSearchDomainCrossAccountTrustRule`
+- Bump `pycfmodel` to `0.13.0`
+
 ## [1.2.2] - 2022-1-07
 ### Improvements
 - Bump `pycfmodel` to `0.11.1`

diff --git a/cfripper/__version__.py b/cfripper/__version__.py
@@ -1,3 +1,3 @@
-VERSION = (1, 2, 2)
+VERSION = (1, 3, 0)
 
 __version__ = ".".join(map(str, VERSION))
diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py
@@ -4,7 +4,9 @@
 from cfripper.rules.cross_account_trust import (
     CrossAccountCheckingRule,
     CrossAccountTrustRule,
+    ElasticsearchDomainCrossAccountTrustRule,
     KMSKeyCrossAccountTrustRule,
+    OpenSearchDomainCrossAccountTrustRule,
     S3CrossAccountTrustRule,
 )
 from cfripper.rules.ebs_volume_has_sse import EBSVolumeHasSSERule
@@ -51,6 +53,7 @@
         EC2SecurityGroupIngressOpenToWorldRule,
         EC2SecurityGroupMissingEgressRule,
         EC2SecurityGroupOpenToWorldRule,
+        ElasticsearchDomainCrossAccountTrustRule,
         FullWildcardPrincipalRule,
         HardcodedRDSPasswordRule,
         IAMRolesOverprivilegedRule,
@@ -59,6 +62,7 @@
         KMSKeyWildcardPrincipalRule,
         KMSKeyEnabledKeyRotation,
         ManagedPolicyOnUserRule,
+        OpenSearchDomainCrossAccountTrustRule,
         PartialWildcardPrincipalRule,
         PolicyOnUserRule,
         PrivilegeEscalationRule,

diff --git a/cfripper/rules/cross_account_trust.py b/cfripper/rules/cross_account_trust.py
@@ -1,7 +1,9 @@
 __all__ = [
     "CrossAccountCheckingRule",
     "CrossAccountTrustRule",
+    "ElasticsearchDomainCrossAccountTrustRule",
     "KMSKeyCrossAccountTrustRule",
+    "OpenSearchDomainCrossAccountTrustRule",
     "S3CrossAccountTrustRule",
 ]
 
@@ -10,8 +12,10 @@
 from typing import Dict, Optional, Set
 
 from pycfmodel.model.cf_model import CFModel
+from pycfmodel.model.resources.es_domain import ESDomain
 from pycfmodel.model.resources.iam_role import IAMRole
 from pycfmodel.model.resources.kms_key import KMSKey
+from pycfmodel.model.resources.opensearch_domain import OpenSearchDomain
 from pycfmodel.model.resources.properties.statement import Statement
 from pycfmodel.model.resources.resource import Resource
 from pycfmodel.model.resources.s3_bucket_policy import S3BucketPolicy
@@ -185,3 +189,59 @@ class KMSKeyCrossAccountTrustRule(CrossAccountCheckingRule):
     REASON = "{} has forbidden cross-account policy allow with {} for an KMS Key Policy"
     RESOURCE_TYPE = KMSKey
     PROPERTY_WITH_POLICYDOCUMENT = "KeyPolicy"
+
+
+class ElasticsearchDomainCrossAccountTrustRule(CrossAccountCheckingRule):
+    """
+    Checks for Elasticsearch domains that allow cross-account principals to get access.
+
+    Risk:
+        It might allow other AWS identities to read/modify data.
+
+    Fix:
+        If cross account permissions are required for ES domains, the stack should be added to the allowlist for this rule.
+        Otherwise, the access should be removed from the CloudFormation definition.
+
+    Filters context:
+        | Parameter   | Type        | Description                                                    |
+        |:-----------:|:-----------:|:--------------------------------------------------------------:|
+        |`config`     | str         | `config` variable available inside the rule                    |
+        |`extras`     | str         | `extras` variable available inside the rule                    |
+        |`logical_id` | str         | ID used in Cloudformation to refer the resource being analysed |
+        |`resource`   | `ESDomain`  | Resource that is being addressed                               |
+        |`statement`  | `Statement` | Statement being checked found in the Resource                  |
+        |`principal`  | `str`       | AWS Principal being checked found in the statement             |
+        |`account_id` | `str`       | Account ID found in the principal                              |
+    """
+
+    REASON = "{} has forbidden cross-account policy allow with {} for an ES domain policy."
+    RESOURCE_TYPE = ESDomain
+    PROPERTY_WITH_POLICYDOCUMENT = "AccessPolicies"
+
+
+class OpenSearchDomainCrossAccountTrustRule(CrossAccountCheckingRule):
+    """
+    Checks for OpenSearch domains that allow cross-account principals to get access.
+
+    Risk:
+        It might allow other AWS identities to read/modify data.
+
+    Fix:
+        If cross account permissions are required for OpenSearch domains, the stack should be added to the allowlist for this rule.
+        Otherwise, the access should be removed from the CloudFormation definition.
+
+    Filters context:
+        | Parameter   | Type               | Description                                                    |
+        |:-----------:|:------------------:|:--------------------------------------------------------------:|
+        |`config`     | str                | `config` variable available inside the rule                    |
+        |`extras`     | str                | `extras` variable available inside the rule                    |
+        |`logical_id` | str                | ID used in Cloudformation to refer the resource being analysed |
+        |`resource`   | `OpenSearchDomain` | Resource that is being addressed                               |
+        |`statement`  | `Statement`        | Statement being checked found in the Resource                  |
+        |`principal`  | `str`              | AWS Principal being checked found in the statement             |
+        |`account_id` | `str`              | Account ID found in the principal                              |
+    """
+
+    REASON = "{} has forbidden cross-account policy allow with {} for an OpenSearch domain policy."
+    RESOURCE_TYPE = OpenSearchDomain
+    PROPERTY_WITH_POLICYDOCUMENT = "AccessPolicies"
diff --git a/requirements.txt b/requirements.txt
@@ -10,7 +10,7 @@ cfn-flip==1.2.3
 click==7.1.2
 jmespath==0.10.0
 pluggy==0.13.1
-pycfmodel==0.11.1
+pycfmodel==0.13.0
 pydantic==1.8.2
 pydash==4.7.6
 python-dateutil==2.8.1

diff --git a/setup.py b/setup.py
@@ -11,7 +11,7 @@
     "cfn_flip>=1.2.0",
     "click~=7.1.1",
     "pluggy~=0.13.1",
-    "pycfmodel>=0.11.1",
+    "pycfmodel>=0.13.0",
     "pydash~=4.7.6",
     "PyYAML>=4.2b1",
 ]

diff --git a/tests/rules/test_CrossAccountTrustRule.py b/tests/rules/test_CrossAccountTrustRule.py
@@ -5,8 +5,13 @@
 from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk
 from cfripper.model.result import Failure
 from cfripper.rule_processor import RuleProcessor
-from cfripper.rules import DEFAULT_RULES, KMSKeyCrossAccountTrustRule
-from cfripper.rules.cross_account_trust import CrossAccountTrustRule
+from cfripper.rules import DEFAULT_RULES
+from cfripper.rules.cross_account_trust import (
+    CrossAccountTrustRule,
+    ElasticsearchDomainCrossAccountTrustRule,
+    KMSKeyCrossAccountTrustRule,
+    OpenSearchDomainCrossAccountTrustRule,
+)
 from tests.utils import compare_lists_of_failures, get_cfmodel_from
 
 
@@ -35,11 +40,31 @@ def template_valid_with_sts():
     return get_cfmodel_from("rules/CrossAccountTrustRule/valid_with_sts.yml").resolve()
 
 
+@pytest.fixture()
+def template_valid_with_sts_es_domain():
+    return get_cfmodel_from("rules/CrossAccountTrustRule/valid_with_sts_es_domain.yml").resolve()
+
+
+@pytest.fixture()
+def template_valid_with_sts_opensearch_domain():
+    return get_cfmodel_from("rules/CrossAccountTrustRule/valid_with_sts_opensearch_domain.yml").resolve()
+
+
 @pytest.fixture()
 def template_invalid_with_sts():
     return get_cfmodel_from("rules/CrossAccountTrustRule/invalid_with_sts.yml").resolve()
 
 
+@pytest.fixture()
+def template_invalid_with_sts_es_domain():
+    return get_cfmodel_from("rules/CrossAccountTrustRule/invalid_with_sts_es_domain.yml").resolve()
+
+
+@pytest.fixture()
+def template_invalid_with_sts_opensearch_domain():
+    return get_cfmodel_from("rules/CrossAccountTrustRule/invalid_with_sts_opensearch_domain.yml").resolve()
+
+
 @pytest.fixture()
 def expected_result_two_roles():
     return [
@@ -263,3 +288,157 @@ def test_sts_failure(template_invalid_with_sts):
             )
         ],
     )
+
+
+@pytest.mark.parametrize(
+    "principal",
+    [
+        "arn:aws:iam::999999999:root",
+        "arn:aws:iam::*:root",
+        "arn:aws:iam::*:*",
+        "arn:aws:iam::*:root*",
+        "arn:aws:iam::*:not-root*",
+        "*",
+    ],
+)
+def test_es_domain_cross_account_failure(principal):
+    rule = ElasticsearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    model = get_cfmodel_from("rules/CrossAccountTrustRule/es_domain_basic.yml").resolve(
+        extra_params={"Principal": principal}
+    )
+    result = rule.invoke(model)
+    assert not result.valid
+    assert compare_lists_of_failures(
+        result.failures,
+        [
+            Failure(
+                granularity=RuleGranularity.RESOURCE,
+                reason=f"TestDomain has forbidden cross-account policy allow with {principal} for an ES domain policy.",
+                risk_value=RuleRisk.MEDIUM,
+                rule="ElasticsearchDomainCrossAccountTrustRule",
+                rule_mode=RuleMode.BLOCKING,
+                actions=None,
+                resource_ids={"TestDomain"},
+            )
+        ],
+    )
+
+
+@pytest.mark.parametrize(
+    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+)
+def test_es_domain_cross_account_success(principal):
+    rule = ElasticsearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    model = get_cfmodel_from("rules/CrossAccountTrustRule/es_domain_basic.yml").resolve(
+        extra_params={"Principal": principal}
+    )
+    result = rule.invoke(model)
+
+    assert result.valid
+    assert compare_lists_of_failures(result.failures, [])
+
+
+def test_sts_valid_es_domain(template_valid_with_sts_es_domain):
+    rule = ElasticsearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    result = rule.invoke(template_valid_with_sts_es_domain)
+
+    assert result.valid
+    assert compare_lists_of_failures(result.failures, [])
+
+
+def test_sts_failure_es_domain(template_invalid_with_sts_es_domain):
+    rule = ElasticsearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    result = rule.invoke(template_invalid_with_sts_es_domain)
+
+    assert not result.valid
+    assert compare_lists_of_failures(
+        result.failures,
+        [
+            Failure(
+                granularity=RuleGranularity.RESOURCE,
+                reason="TestDomain has forbidden cross-account policy allow with arn:aws:sts::999999999:assumed-role/test-role/session for an ES domain policy.",
+                risk_value=RuleRisk.MEDIUM,
+                rule="ElasticsearchDomainCrossAccountTrustRule",
+                rule_mode=RuleMode.BLOCKING,
+                actions=None,
+                resource_ids={"TestDomain"},
+            )
+        ],
+    )
+
+
+@pytest.mark.parametrize(
+    "principal",
+    [
+        "arn:aws:iam::999999999:root",
+        "arn:aws:iam::*:root",
+        "arn:aws:iam::*:*",
+        "arn:aws:iam::*:root*",
+        "arn:aws:iam::*:not-root*",
+        "*",
+    ],
+)
+def test_opensearch_domain_cross_account_failure(principal):
+    rule = OpenSearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    model = get_cfmodel_from("rules/CrossAccountTrustRule/opensearch_domain_basic.yml").resolve(
+        extra_params={"Principal": principal}
+    )
+    result = rule.invoke(model)
+    assert not result.valid
+    assert compare_lists_of_failures(
+        result.failures,
+        [
+            Failure(
+                granularity=RuleGranularity.RESOURCE,
+                reason=f"TestDomain has forbidden cross-account policy allow with {principal} for an OpenSearch domain policy.",
+                risk_value=RuleRisk.MEDIUM,
+                rule="OpenSearchDomainCrossAccountTrustRule",
+                rule_mode=RuleMode.BLOCKING,
+                actions=None,
+                resource_ids={"TestDomain"},
+            )
+        ],
+    )
+
+
+@pytest.mark.parametrize(
+    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+)
+def test_opensearch_domain_cross_account_success(principal):
+    rule = OpenSearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    model = get_cfmodel_from("rules/CrossAccountTrustRule/opensearch_domain_basic.yml").resolve(
+        extra_params={"Principal": principal}
+    )
+    result = rule.invoke(model)
+
+    assert result.valid
+    assert compare_lists_of_failures(result.failures, [])
+
+
+def test_sts_valid_opensearch_domain(template_valid_with_sts_opensearch_domain):
+    rule = OpenSearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    result = rule.invoke(template_valid_with_sts_opensearch_domain)
+
+    assert result.valid
+    assert compare_lists_of_failures(result.failures, [])
+
+
+def test_sts_failure_opensearch_domain(template_invalid_with_sts_opensearch_domain):
+    rule = OpenSearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
+    result = rule.invoke(template_invalid_with_sts_opensearch_domain)
+
+    assert not result.valid
+    assert compare_lists_of_failures(
+        result.failures,
+        [
+            Failure(
+                granularity=RuleGranularity.RESOURCE,
+                reason="TestDomain has forbidden cross-account policy allow with arn:aws:sts::999999999:assumed-role/test-role/session for an OpenSearch domain policy.",
+                risk_value=RuleRisk.MEDIUM,
+                rule="OpenSearchDomainCrossAccountTrustRule",
+                rule_mode=RuleMode.BLOCKING,
+                actions=None,
+                resource_ids={"TestDomain"},
+            )
+        ],
+    )
diff --git a/tests/test_templates/rules/CrossAccountTrustRule/es_domain_basic.yml b/tests/test_templates/rules/CrossAccountTrustRule/es_domain_basic.yml
@@ -0,0 +1,21 @@
+AWSTemplateFormatVersion: "2010-09-09"
+Description: Testing ES Domain
+
+Parameters:
+  Principal:
+    Type: String
+
+Resources:
+  TestDomain:
+    Type: AWS::Elasticsearch::Domain
+    Properties:
+      DomainName: "test"
+      AccessPolicies:
+        Version: "2012-10-17"
+        Statement:
+          - Sid: "Allow full access of the domain"
+            Effect: "Allow"
+            Principal:
+              AWS: !Ref Principal
+            Action: "es:*"
+            Resource: "arn:aws:es:*:123456789012:domain/test/*"
diff --git a/tests/test_templates/rules/CrossAccountTrustRule/invalid_with_sts_es_domain.yml b/tests/test_templates/rules/CrossAccountTrustRule/invalid_with_sts_es_domain.yml
@@ -0,0 +1,19 @@
+AWSTemplateFormatVersion: "2010-09-09"
+Description: Testing ES Domain
+
+Resources:
+  TestDomain:
+    Type: AWS::Elasticsearch::Domain
+    Properties:
+      DomainName: "test"
+      AccessPolicies:
+        Version: "2012-10-17"
+        Statement:
+          - Sid: "Allow full access of the domain"
+            Effect: "Allow"
+            Principal:
+              AWS:
+                - "arn:aws:iam::123456789:role/test-role"
+                - "arn:aws:sts::999999999:assumed-role/test-role/session"
+            Action: "es:*"
+            Resource: "arn:aws:es:*:123456789012:domain/test/*"