Refactor collate (#7)

* refactor collate, add services

* bump version and jinja2

* with logging, sns, opensearch, and better error handling

* updated FAQ

README.md

@@ -30,6 +30,7 @@ Access Analyzer falls short because:
 
 4. Does not identify AWS Service principals. This is mainly important because of [Wiz's AWSConfig, et al vulnverabilities](http://i.blackhat.com/USA21/Wednesday-Handouts/us-21-Breaking-The-Isolation-Cross-Account-AWS-Vulnerabilities.pdf)
 
+
 ## AWS IAM Access Analyzer comparison 
 
 Comparison based on AWS Documentation [1](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_aws-services-that-work-with-iam.html) and [2](https://docs.aws.amazon.com/IAM/latest/UserGuide/what-is-access-analyzer.html#what-is-access-analyzer-resource-identification), including services or resources outside of docs, e.g. VPC endpoints.
@@ -46,30 +47,30 @@ Comparison based on AWS Documentation [1](https://docs.aws.amazon.com/IAM/latest
 | KMS |  :white_check_mark: |  :white_check_mark: |
 | Secrets Manager |  :white_check_mark: |  :white_check_mark: |
 | Lambda | :white_check_mark: |  :white_check_mark: |
-| SNS | :x: |  :white_check_mark: |
+| SNS | :white_check_mark: |  :white_check_mark: |
 | SQS | :white_check_mark: |  :white_check_mark: |
 | RDS Snapshots | :x: |  :white_check_mark: |
 | RDS Cluster Snapshots | :x: |  :white_check_mark: |
 | ECR | :x: |  :white_check_mark: |
 | EFS | :white_check_mark: |  :white_check_mark: |
-| DynamoDB streams | :x: |  :white_check_mark: |
-| DynamoDB tables | :x: |  :white_check_mark: |
+| DynamoDB streams |  :white_check_mark: |  :white_check_mark: |
+| DynamoDB tables |  :white_check_mark: |  :white_check_mark: |
 | EBS Snapshots | :x: |  :white_check_mark: |
 | EventBridge | :white_check_mark: | :x: |
 | EventBridge Schema | :x: | :x: |
 | Mediastore | :x: | :x: |
 | Glue | :x: | :x: |
-| Kinesis Data Streams | :x: | :x: |
+| Kinesis Data Streams | :white_check_mark: | :x: |
 | Lex v2 | :x: | :x: |
 | Migration Hub Orchestrator | :x: | :x: |
-| OpenSearch | :x: | :x: |
+| OpenSearch | :white_check_mark: | :x: |
 | AWS PCA | :x: | :x: |
 | Redshift Serverless | :x: | :x: |
 | Serverless Application Repository | :x: | :x: |
 | SES v2 | :x: | :x: |
 | Incident Manager | :x: | :x: |
 | Incident Manager Contacts | :x: | :x: |
-| VPC endpoints | :x: | :x: |
+| VPC endpoints | :white_check_mark: | :x: |
 
 ## How to run
 
@@ -110,30 +111,41 @@ html_summary = r.HTML_report()
 
 ### IAM Permissions
 
-Permissions required.
+Permissions required to scan all services.
 
 ```json
 {
   "Version": "2012-10-17",
   "Statement": [
     {
       "Action": [
+        "dynamodb:GetResourcePolicy",
+        "dynamodb:ListStreams",
+        "dynamodb:ListTables",
+        "ec2:DescribeVpcEndpoints",
         "elasticfilesystem:DescribeFileSystemPolicy",
         "elasticfilesystem:DescribeFileSystems",
+        "es:DescribeDomains",
+        "es:ListDomainNames",
         "events:ListEventBuses",
         "glacier:GetVaultAccessPolicy",
         "glacier:ListVaults",
         "iam:ListRoles",
-        "organizations:ListAccounts",
+        "kinesis:GetResourcePolicy",
+        "kinesis:ListStreams", 
         "kms:GetKeyPolicy",
         "kms:ListKeys",
         "lambda:GetPolicy",
         "lambda:ListFunctions",
+        "organizations:DescribeOrganization",
+        "organizations:ListAccounts",
         "s3:GetBucketAcl",
         "s3:GetBucketPolicy",
         "s3:ListAllMyBuckets",
         "secretsmanager:GetResourcePolicy",
         "secretsmanager:ListSecrets",
+        "sns:GetTopicAttributes",
+        "sns:ListTopics",
         "sqs:GetQueueAttributes",
         "sqs:ListQueues"
       ],
@@ -161,7 +173,7 @@ Example:
 ```python
 class S3(Service):
 
-    def fetch(self, accounts: DefaultDict[str, Set] ) -> Findings:
+    def fetch(self, accounts: Accounts ) -> Findings:
         self._buckets = self.list_account_buckets()
         self.policies = self.get_bucket_policies()
         return super().collate(accounts, self.policies)
@@ -170,16 +182,21 @@ class S3(Service):
 
 ---
 
-:warning: AWSXenos currently assesses access based on [https://github.com/Netflix-Skunkworks/policyuniverse](https://github.com/Netflix-Skunkworks/policyuniverse).
-There are cases where IAM `conditions`, will _not_ be taken into account, therefore resulting in false positives. 
-This could be fairly common in KMS Customer Managed Keys created by AWS Services.
-AWSXenos findings are per IAM statement on an IAM policy.
+## FAQ
+
+### Are there false positives?
+Yes. AWSXenos doesn't take into consideration Identity or SCP. It assumes that everything else other than the resource or trust policy _has_ access. 
 
-## I want to add more known accounts
-Create a PR or raise an issue. Contributions are welcome.
+### Is this using an SMT Solver or automated reasoning ?
+No. AWSXenos only takes into account resource and IAM trust policies. Maybe in the next project or iteration.
 
+### Why not use [CheckAccessNotGranted](https://docs.aws.amazon.com/access-analyzer/latest/APIReference/API_CheckAccessNotGranted.html) ?
+We don't know the set of accounts that shouldn't access the resource or role.
 
+### How does it work ?
+AWSXenos currently assesses access based on [https://github.com/Netflix-Skunkworks/policyuniverse](https://github.com/Netflix-Skunkworks/policyuniverse).
 
 ## Features
 - [x] Use as library
-- [x] HTML and JSON output 
+- [x] HTML and JSON output 
+- [x] Multi-threaded querying of each service 

awsxenos/config.yaml

@@ -1,21 +1,33 @@
 plugins:
+  - module: dynamodb
+    class: DynamoDBTable
+  - module: dynamodb
+    class: DynamoDBStreams
+  - module: efs
+    class: EFSResource
+  - module: eventbridge
+    class: EventBus
+  - module: iam
+    class: IAM
+  - module: kinesis
+    class: Kinesis
+  - module: kms
+    class: KMS
+  - module: lambda
+    class: LambdaResource
+  - module: opensearch
+    class: OpenSearch
   - module: s3
     class: S3
   - module: s3
     class: S3ACL
   - module: s3
     class: S3Glacier
-  - module: iam
-    class: IAM
-  - module: kms
-    class: KMS
   - module: secretsmanager
     class: SecretsManager
-  - module: eventbridge
-    class: EventBus
-  - module: lambda
-    class: LambdaResource
+  - module: sns
+    class: SNS
   - module: sqs
     class: SQS
-  - module: efs
-    class: EFSResource
+  - module: vpc
+    class: VPCEndpoint

awsxenos/finding.py

@@ -1,11 +1,11 @@
 import abc
 from collections import defaultdict, UserDict
 from dataclasses import dataclass, field
-from typing import Any, DefaultDict, List, Set
+from typing import Any, DefaultDict, List
 
 from policyuniverse.arn import ARN  # type: ignore
 from policyuniverse.policy import Policy  # type: ignore
-from policyuniverse.statement import ConditionTuple  # type: ignore
+from policyuniverse.statement import ConditionTuple, Statement  # type: ignore
 
 
 @dataclass
@@ -26,104 +26,123 @@ class Accounts:
     known_accounts: List[Finding] = field(default_factory=list)
     unknown_accounts: List[Finding] = field(default_factory=list)
     aws_services: List[Finding] = field(default_factory=list)
+    org_id: str = ""
 
     def __getitem__(self, key):
         return super().__getattribute__(key)
 
+    def __contains__(self, key):
+        return getattr(self, key)
+
+
+"""Container for Finding"""
+
 
 class Findings(UserDict):
     def __missing__(self, key):
         self[key] = Accounts()
         return self[key]
 
 
+""" Container for any resource, e.g. IAM roles returned by a service
+    Expects a key of arn and a value of the policy
+"""
+
+
 class Resources(UserDict):
     def __missing__(self, key):
         self[key] = defaultdict()
         return self[key]
 
 
+"""Main class to derive from when implementing other services"""
+
+
 class Service(metaclass=abc.ABCMeta):
     @abc.abstractmethod
-    def fetch(self, accounts: DefaultDict[str, Set], **kwargs) -> Findings:
+    def fetch(self, accounts: Accounts, **kwargs) -> Findings:
         raise NotImplementedError
 
-    def collate(self, accounts: DefaultDict[str, Set], resources: Resources) -> Findings:
-        """Combine all accounts with all the resources to classify findings. Try custom_collate first and fallback to this.
+    def _get_account_type(self, account: str, accounts: Accounts) -> str:
+        account_types = ["org_accounts", "known_accounts"]
+        for account_type in account_types:
+            if account in accounts[account_type]:
+                return account_type
+        return "unknown_accounts"
+
+    def collate(self, accounts: Accounts, resources: Resources) -> Findings:
+        """Combine all accounts with all the resources to classify findings.
+            This is the default collation function called by Service
 
         Args:
-            accounts (DefaultDict[str, Set]): Key of account type. Value account ids
-            resources (DefaultDict[str, Dict[Any, Any]]): Key ResourceIdentifier. Value Dict PolicyDocument
+            accounts (Accounts): Key of account type. Value account ids
+            resources (Resources): Key ResourceIdentifier. Value Dict PolicyDocument
 
         Returns:
             DefaultDict[str, AccountType]: Key of ARN, Value of AccountType
         """
 
         findings = Findings()
-        for resource, policy_document in resources.items():
+
+        for resource, policy_document in resources.items():  # TODO: extract the IAM trust policy logic
             try:
                 policy = Policy(policy_document)
             except:
-                continue
-            for unparsed_principal in policy.whos_allowed():
-                try:
-                    principal = ARN(unparsed_principal.value)  # type: Any
-                except Exception as e:
-                    print(e)
-                    findings[resource].known_accounts.append(Finding(principal=unparsed_principal, external_id=True))
+                continue  # TODO: Don't fail silently
+            for st in policy.statements:
+                if st.effect != "Allow":
                     continue
-                # Check if Principal is an AWS Service
-                if principal.service:
-                    findings[resource].aws_services.append(Finding(principal=principal.arn, external_id=True))
-                # Check against org_accounts
-                elif principal.account_number in accounts["org_accounts"]:
-                    findings[resource].org_accounts.append(Finding(principal=principal.arn, external_id=True))
-                # Check against known external accounts
-                elif (
-                    principal.account_number in accounts["known_accounts"]
-                    or ConditionTuple(category="saml-endpoint", value="https://signin.aws.amazon.com/saml")
-                    in policy.whos_allowed()
-                ):
-                    sts_set = False
-                    for pstate in policy.statements:
-                        if "sts" in pstate.action_summary():
-                            try:
-                                conditions = [
-                                    k.lower() for k in list(pstate.statement["Condition"]["StringEquals"].keys())
-                                ]
-                                if "sts:externalid" in conditions:
-                                    findings[resource].known_accounts.append(
-                                        Finding(principal=principal.arn, external_id=True)
-                                    )
-                            except:
-                                findings[resource].known_accounts.append(
-                                    Finding(principal=principal.arn, external_id=False)
+                for unparsed_principal in st.principals:  # There is always a principal - including "*"
+                    principal = ARN(unparsed_principal)
+                    if st.condition_accounts:  # If condition exists on account, it's an account
+                        for account in st.condition_accounts:
+                            findings[resource][self._get_account_type(account, accounts)].append(
+                                Finding(principal=account, external_id=True)
+                            )
+                    elif st.condition_orgids:  # If condition exists on orgid, it's the orgid
+                        for org_id in st.condition_orgids:
+                            if accounts.org_id == org_id:
+                                findings[resource]["org_accounts"].append(
+                                    Finding(principal=principal.arn, external_id=True)  # type: ignore
+                                )
+                            else:
+                                findings[resource]["unknown_accounts"].append(
+                                    Finding(principal=principal.arn, external_id=True)  # type: ignore
                                 )
-                            finally:
-                                sts_set = True
-                                break
-                    if not sts_set:
-                        findings[resource].known_accounts.append(Finding(principal=principal.arn, external_id=False))
-
-                # Unknown Account
-                else:
-                    sts_set = False
-                    for pstate in policy.statements:
-                        if "sts" in pstate.action_summary():
+                    elif principal.account_number:  # if there are no conditions
+                        if "sts" in st.action_summary():  # IAM Assume Role
                             try:
-                                conditions = [
-                                    k.lower() for k in list(pstate.statement["Condition"]["StringEquals"].keys())
-                                ]
+                                conditions = [k.lower() for k in list(st.statement["Condition"]["StringEquals"].keys())]
                                 if "sts:externalid" in conditions:
-                                    findings[resource].unknown_accounts.append(
-                                        Finding(principal=principal.arn, external_id=True)
+                                    findings[resource][
+                                        self._get_account_type(principal.account_number, accounts)
+                                    ].append(
+                                        Finding(principal=principal.arn, external_id=True)  # type: ignore
+                                    )
+                                else:
+                                    findings[resource][
+                                        self._get_account_type(principal.account_number, accounts)
+                                    ].append(
+                                        Finding(principal=principal.arn, external_id=False)  # type: ignore
                                     )
                             except:
-                                findings[resource].unknown_accounts.append(
-                                    Finding(principal=principal.arn, external_id=False)
+                                findings[resource][self._get_account_type(principal.account_number, accounts)].append(
+                                    Finding(principal=principal.arn, external_id=False)  # type: ignore
                                 )
-                            finally:
-                                break
-                    if not sts_set:
-                        findings[resource].unknown_accounts.append(Finding(principal=principal.arn, external_id=False))
+                        else:
+                            findings[resource][self._get_account_type(principal.account_number, accounts)].append(
+                                Finding(principal=principal.arn, external_id=True)  # type: ignore
+                            )
+                    elif not principal.account_number and principal.service:  # It's an aws service
+                        findings[resource].aws_services.append(
+                            Finding(principal=principal.arn, external_id=True)  # type: ignore
+                        )
+                    elif not principal.account_number and not principal.service:  # It's anonymous
+                        findings[resource].unknown_accounts.append(
+                            Finding(principal=principal.arn, external_id=True)  # type: ignore
+                        )
+                    else:  # Catch-all
+                        findings[resource].unknown_accounts.append(
+                            Finding(principal=principal, external_id=True)  # type: ignore
+                        )
         return findings