Repository: lyft/cartography

Issue:
AWS Inspector findings timeout
*Title*: *AWS Inspector findings timeout*

*Description*:
> Cartography is crashing when attempting to retrieve Inspector finding from ap-northeast-3

*To Reproduce*:
> cartography --experimental-neo4j-4x-support --neo4j-uri bolt://127.0.0.1:7687  --aws-sync-all-profiles

*Logs*:
> INFO:cartography.intel.aws.inspector:Syncing AWS Inspector findings for account XXX and region ap-northeast-3
INFO:backoff:Backing off inner_function(...) for 1.0s (botocore.exceptions.ClientError: An error occurred (InternalServerErrorException) when calling the ListFindings operation (reached max retries: 4): Endpoint request timed out)
WARNING:cartography.util:Backing off 1.0 seconds after 1 tries. Calling function <function aws_handle_regions.<locals>.inner_function at 0x106b0e710>
INFO:backoff:Backing off inner_function(...) for 0.2s (botocore.exceptions.ClientError: An error occurred (InternalServerErrorException) when calling the ListFindings operation (reached max retries: 4): Endpoint request timed out)
WARNING:cartography.util:Backing off 0.2 seconds after 2 tries. Calling function <function aws_handle_regions.<locals>.inner_function at 0x106b0e710>
INFO:backoff:Backing off inner_function(...) for 0.2s (botocore.exceptions.ClientError: An error occurred (InternalServerErrorException) when calling the ListFindings operation (reached max retries: 4): Endpoint request timed out)
WARNING:cartography.util:Backing off 0.2 seconds after 3 tries. Calling function <function aws_handle_regions.<locals>.inner_function at 0x106b0e710>
INFO:backoff:Backing off inner_function(...) for 6.4s (botocore.exceptions.ClientError: An error occurred (InternalServerErrorException) when calling the ListFindings operation (reached max retries: 4): Endpoint request timed out)
WARNING:cartography.util:Backing off 6.4 seconds after 4 tries. Calling function <function aws_handle_regions.<locals>.inner_function at 0x106b0e710>
ERROR:backoff:Giving up inner_function(...) after 5 tries (botocore.exceptions.ClientError: An error occurred (InternalServerErrorException) when calling the ListFindings operation (reached max retries: 4): Endpoint request timed out)



*Please complete the following information:*:
>  - Cartography 0.65.0



*Additional context*:
> Sounds like inspector does not exist in ap-northeast-3, so maybe that has something to do with it?


Discussion:

Patch:
diff --git a/cartography/intel/aws/inspector.py b/cartography/intel/aws/inspector.py
index 023dc88e..0262fd15 100644
--- a/cartography/intel/aws/inspector.py
+++ b/cartography/intel/aws/inspector.py
@@ -8,6 +8,8 @@ import boto3
 import neo4j
 
 from cartography.util import aws_handle_regions
+from cartography.util import aws_paginate
+from cartography.util import batch
 from cartography.util import run_cleanup_job
 from cartography.util import timeit
 
@@ -17,13 +19,45 @@ logger = logging.getLogger(__name__)
 
 @timeit
 @aws_handle_regions
-def get_inspector_findings(session: boto3.session.Session, region: str) -> List[Dict]:
+def get_inspector_findings(
+    session: boto3.session.Session,
+    region: str,
+    current_aws_account_id: str,
+) -> List[Dict]:
+    '''
+    We must list_findings by filtering the request, otherwise the request could tiemout.
+    First, we filter by account_id. And since there may be millions of CLOSED findings that may never go away,
+    we will only fetch those in ACTIVE or SUPPRESSED statuses.
+    list_members will get us all the accounts that
+    have delegated access to the account specified by current_aws_account_id.
+    '''
     client = session.client('inspector2', region_name=region)
-    paginator = client.get_paginator('list_findings')
-    findings = []
 
-    for page in paginator.paginate():
-        findings.extend(page['findings'])
+    members = aws_paginate(client, 'list_members', 'members')
+    # the current host account may not be considered a "member", but we still fetch its findings
+    accounts = [current_aws_account_id] + [m['accountId'] for m in members]
+
+    findings = []
+    for account in accounts:
+        logger.info(f'Getting findings for member account {account} in region {region}')
+        findings.extend(
+            aws_paginate(
+                client, 'list_findings', 'findings', filterCriteria={
+                    'awsAccountId': [
+                        {
+                            'comparison': 'EQUALS',
+                            'value': account,
+                        },
+                    ],
+                    'findingStatus': [
+                        {
+                            'comparison': 'NOT_EQUALS',
+                            'value': 'CLOSED',
+                        },
+                    ],
+                },
+            ),
+        )
     return findings
 
 
@@ -43,7 +77,9 @@ def transform_inspector_findings(results: List[Dict]) -> Tuple[List, List]:
         finding['awsaccount'] = f['awsAccountId']
         finding['description'] = f['description']
         finding['type'] = f['type']
-        finding['cvssscore'] = f['inspectorScoreDetails']['adjustedCvss']['score']
+        finding['status'] = f['status']
+        if f.get('inspectorScoreDetails'):
+            finding['cvssscore'] = f['inspectorScoreDetails']['adjustedCvss']['score']
         if f['resources'][0]['type'] == "AWS_EC2_INSTANCE":
             finding['instanceid'] = f['resources'][0]['id']
         if f['resources'][0]['type'] == "AWS_ECR_CONTAINER_IMAGE":
@@ -148,6 +184,7 @@ def _load_findings_tx(
             finding.relatedvulnerabilities = new_finding.relatedvulnerabilities,
             finding.source = new_finding.source,
             finding.sourceurl = new_finding.sourceurl,
+            finding.status = new_finding.status,
             finding.vendorcreatedat = new_finding.vendorcreatedat,
             finding.vendorseverity = new_finding.vendorseverity,
             finding.vendorupdatedat = new_finding.vendorupdatedat,
@@ -188,12 +225,14 @@ def load_inspector_findings(
     neo4j_session: neo4j.Session, findings: List[Dict], region: str,
     aws_update_tag: int,
 ) -> None:
-    neo4j_session.write_transaction(
-        _load_findings_tx,
-        findings=findings,
-        region=region,
-        aws_update_tag=aws_update_tag,
-    )
+    for i, findings_batch in enumerate(batch(findings), start=1):
+        logger.info(f'Loading batch number {i}')
+        neo4j_session.write_transaction(
+            _load_findings_tx,
+            findings=findings_batch,
+            region=region,
+            aws_update_tag=aws_update_tag,
+        )
 
 
 def _load_packages_tx(
@@ -265,7 +304,7 @@ def sync(
 ) -> None:
     for region in regions:
         logger.info(f"Syncing AWS Inspector findings for account {current_aws_account_id} and region {region}")
-        findings = get_inspector_findings(boto3_session, region)
+        findings = get_inspector_findings(boto3_session, region, current_aws_account_id)
         finding_data, package_data = transform_inspector_findings(findings)
         logger.info(f"Loading {len(package_data)} packages")
         load_inspector_packages(neo4j_session, package_data, region, update_tag)
diff --git a/cartography/util.py b/cartography/util.py
index 15306620..dbca3d1a 100644
--- a/cartography/util.py
+++ b/cartography/util.py
@@ -15,6 +15,7 @@ from typing import TypeVar
 from typing import Union
 
 import backoff
+import boto3
 import botocore
 import neo4j
 
@@ -34,6 +35,7 @@ logger = logging.getLogger(__name__)
 STATUS_SUCCESS = 0
 STATUS_FAILURE = 1
 STATUS_KEYBOARD_INTERRUPT = 130
+DEFAULT_BATCH_SIZE = 1000
 
 
 def run_analysis_job(
@@ -133,6 +135,26 @@ def timeit(method: F) -> F:
     return cast(F, timed)
 
 
+def aws_paginate(
+    client: boto3.client,
+    method_name: str,
+    object_name: str,
+    **kwargs: Any,
+) -> List[Dict]:
+    '''
+    Helper method for boilerplate boto3 pagination
+    The **kwargs will be forwarded to the paginator
+    '''
+    paginator = client.get_paginator(method_name)
+    items = []
+    i = 0
+    for i, page in enumerate(paginator.paginate(**kwargs), start=1):
+        if i % 100 == 0:
+            logger.info(f'fetching page number {i}')
+        items.extend(page[object_name])
+    return items
+
+
 AWSGetFunc = TypeVar('AWSGetFunc', bound=Callable[..., List])
 
 # fix for AWS TooManyRequestsException
@@ -166,6 +188,7 @@ def aws_handle_regions(func: AWSGetFunc) -> AWSGetFunc:
         'AuthFailure',
         'InvalidClientTokenId',
         'UnrecognizedClientException',
+        'InternalServerErrorException',
     ]
 
     @wraps(func)
@@ -222,7 +245,7 @@ def camel_to_snake(name: str) -> str:
     return re.sub(r'(?<!^)(?=[A-Z])', '_', name).lower()
 
 
-def batch(items: Iterable, size: int = 1000) -> List[List]:
+def batch(items: Iterable, size: int = DEFAULT_BATCH_SIZE) -> List[List]:
     '''
     Takes an Iterable of items and returns a list of lists of the same items,
      batched into chunks of the provided `size`.
diff --git a/docs/root/modules/aws/config.md b/docs/root/modules/aws/config.md
index cadd8786..7a02c5e5 100644
--- a/docs/root/modules/aws/config.md
+++ b/docs/root/modules/aws/config.md
@@ -6,7 +6,7 @@ Follow these steps to analyze AWS assets with Cartography.
 
 ### Single AWS Account Setup
 
-1. Set up an AWS identity (user, group, or role) for Cartography to use.  Ensure that this identity has the built-in AWS [SecurityAudit policy](https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_job-functions.html#jf_security-auditor) (arn:aws:iam::aws:policy/SecurityAudit) attached.  This policy grants access to read security config metadata.
+1. Set up an AWS identity (user, group, or role) for Cartography to use.  Ensure that this identity has the built-in AWS [SecurityAudit policy](https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_job-functions.html#jf_security-auditor) (arn:aws:iam::aws:policy/SecurityAudit) attached.  This policy grants access to read security config metadata. The SecurityAudit policy does not yet containe permissions for `inspector2`, so you will also need the [AmazonInspector2ReadOnlyAccess policy](https://docs.aws.amazon.com/inspector/latest/user/security-iam-awsmanpol.html#security-iam-awsmanpol-AmazonInspector2ReadOnlyAccess).
 1. Set up AWS credentials to this identity on your server, using a `config` and `credential` file.  For details, see AWS' [official guide](https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html).
 1. [Optional] Configure AWS Retry settings using `AWS_MAX_ATTEMPTS` and `AWS_RETRY_MODE` environment variables. This helps in API Rate Limit throttling and TooManyRequestException related errors. For details, see AWS' [official guide](https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html#using-environment-variables).
 


Test patch:
diff --git a/tests/unit/cartography/intel/aws/test_inspector.py b/tests/unit/cartography/intel/aws/test_inspector.py
index 1fe9257b..dc0cf1a9 100644
--- a/tests/unit/cartography/intel/aws/test_inspector.py
+++ b/tests/unit/cartography/intel/aws/test_inspector.py
@@ -17,7 +17,7 @@ TEST_UPDATE_TAG = 123456789
 def test_get_inspector_findings():
     mock_boto = mock.MagicMock()
 
-    ret = get_inspector_findings(mock_boto, 'us-east-1')
+    ret = get_inspector_findings(mock_boto, 'us-east-1', '1234')
 
     assert ret == []
     mock_boto.client.assert_called_once_with('inspector2', region_name='us-east-1')
@@ -43,6 +43,7 @@ def test_transform_inspector_findings_network():
             'portrangeend': 124,
             'portrangebegin': 123,
             'type': 'NETWORK_REACHABILITY',
+            'status': 'ACTIVE',
         },
     ]
 
@@ -75,6 +76,7 @@ def test_transform_inspector_findings_package():
             'vendorupdatedat': None,
             'vendorseverity': 'Moderate',
             'sourceurl': 'https://access.redhat.com/security/cve/CVE-2017-9059',
+            'status': 'ACTIVE',
 
             'vulnerablepackageids': [
                 'kernel-tools|X86_64|4.9.17|6.29.amzn1|0',
@@ -117,14 +119,14 @@ def test_transform_inspector_findings_package():
 @mock.patch("cartography.intel.aws.inspector._load_findings_tx")
 def test_load_inspector_findings(load_mock):
     mock_neo4j = mock.MagicMock()
-    findings = []
+    findings = ['any']
     region = 'us-east-1'
     aws_update_tag = 0
 
     load_inspector_findings(mock_neo4j, findings, region, aws_update_tag)
 
     mock_neo4j.write_transaction.assert_called_once_with(
-        load_mock, findings=[], region='us-east-1', aws_update_tag=0,
+        load_mock, findings=['any'], region='us-east-1', aws_update_tag=0,
     )
 
 
@@ -177,8 +179,8 @@ def test_sync(
     sync(mock_neo4j, mock_boto3, regions, current_aws_account_id, update_tag, common_job_parameters)
 
     get_findings_mock.assert_has_calls([
-        mock.call(mock_boto3, 'us-east-1'),
-        mock.call(mock_boto3, 'us-east-2'),
+        mock.call(mock_boto3, 'us-east-1', current_aws_account_id),
+        mock.call(mock_boto3, 'us-east-2', current_aws_account_id),
     ])
     transform_mock.assert_has_calls([
         mock.call([]),