From 8f50083706484878b0ff9c86a3edf098db5ea22d Mon Sep 17 00:00:00 2001 From: Long Zhang Date: Tue, 17 Dec 2024 11:41:59 +0100 Subject: [PATCH 1/4] fix obj variable referenced before assignment error Signed-off-by: Long Zhang --- services/elasticache/Elasticache.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/elasticache/Elasticache.py b/services/elasticache/Elasticache.py index ed24644..47b55ce 100644 --- a/services/elasticache/Elasticache.py +++ b/services/elasticache/Elasticache.py @@ -189,6 +189,7 @@ def advise(self): return objs for cluster in self.cluster_info: + obj = None if cluster.get('Engine') == 'memcached': obj = ElasticacheMemcached( cluster, self.elasticacheClient, self.driverInfo) From 57febbc07eeff5516cde6efdaae31afd2d73749e Mon Sep 17 00:00:00 2001 From: Sarika Subramaniam Date: Wed, 18 Dec 2024 12:13:35 +0000 Subject: [PATCH 2/4] Fix comments --- services/s3/drivers/S3Bucket.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/s3/drivers/S3Bucket.py b/services/s3/drivers/S3Bucket.py index eb9730b..072b2bc 100644 --- a/services/s3/drivers/S3Bucket.py +++ b/services/s3/drivers/S3Bucket.py @@ -18,7 +18,6 @@ def __init__(self, bucket, s3Client): self.init() def policyAllowsPublicRead (self, policy_document): - # Check if the policy allows public read access # Check if the policy allows public read access response = None policy_allows_public_read = False @@ -39,8 +38,7 @@ def policyAllowsPublicRead (self, policy_document): return policy_allows_public_read def policyAllowsPublicWrite (self, policy_document): - # Check if the policy allows public read access - # Check if the policy allows public read access + # Check if the policy allows write read access response = None policy_allows_public_write = True if policy_document: From 11b6dfceb4973ad78f83cb790b9f46d1f98cf860 Mon Sep 17 00:00:00 2001 From: Sarika Subramaniam Date: Wed, 18 Dec 2024 14:36:46 +0000 Subject: [PATCH 3/4] Fix issue #167 --- services/s3/drivers/S3Bucket.py | 104 ++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 44 deletions(-) diff --git a/services/s3/drivers/S3Bucket.py b/services/s3/drivers/S3Bucket.py index 072b2bc..03086ad 100644 --- a/services/s3/drivers/S3Bucket.py +++ b/services/s3/drivers/S3Bucket.py @@ -17,46 +17,67 @@ def __init__(self, bucket, s3Client): self.init() - def policyAllowsPublicRead (self, policy_document): - # Check if the policy allows public read access - response = None - policy_allows_public_read = False - if policy_document: - iam = boto3.client('iam') - response = iam.simulate_principal_policy( - PolicySourceArn=f"arn:aws:s3:::{self.bucket}", - PolicyInputList=[policy_document], - ActionNames=['s3:GetObject', 's3:ListBucket'], - ResourceArns=[f"arn:aws:s3:::{self.bucket}", f"arn:aws:s3:::{self.bucket}/*"], - PermissionsBoundarySet=[], - ContextEntries=[], - ResourceHandlingOption='SINGLE_SERVICE_MULTI_RESOURCE' - ) - - if response and response['EvaluationResults'][0]['EvalDecision'] == 'allowed': - policy_allows_public_read = True - return policy_allows_public_read - - def policyAllowsPublicWrite (self, policy_document): - # Check if the policy allows write read access - response = None - policy_allows_public_write = True - if policy_document: - iam = boto3.client('iam') - response = iam.simulate_principal_policy( - PolicySourceArn=f"arn:aws:s3:::{self.bucket}", - PolicyInputList=[policy_document], - ActionNames=['s3:PutObject'], - ResourceArns=[f"arn:aws:s3:::{self.bucket}", f"arn:aws:s3:::{self.bucket}/*"], - PermissionsBoundarySet=[], - ContextEntries=[], - ResourceHandlingOption='SINGLE_SERVICE_MULTI_RESOURCE' - ) - - if response and response['EvaluationResults'][0]['EvalDecision'] == 'allowed': - policy_allows_public_write = True - return policy_allows_public_write + def policyAllowsPublicRead(self, policy_document): + """ + Check if the policy allows public read access + + Args: + policy_document (str): The bucket policy as a string + + Returns: + bool: True if policy allows public read access, False otherwise + """ + if not policy_document: + return False + + try: + policy = json.loads(policy_document) + + # Check for public read in policy + for statement in policy['Statement']: + principal = statement.get('Principal', {}) + if (principal == '*' or principal.get('AWS') == '*') and \ + statement['Effect'] == 'Allow' and \ + any(action in ['s3:GetObject', 's3:GetObjectVersion', 's3:ListBucket'] + for action in (statement.get('Action', []) if isinstance(statement.get('Action', []), list) + else [statement.get('Action', [])])): + return True + + return False + + except (json.JSONDecodeError, KeyError): + return False + def policyAllowsPublicWrite(self, policy_document): + """ + Check if the policy allows public write access + + Args: + policy_document (str): The bucket policy as a string + + Returns: + bool: True if policy allows public write access, False otherwise + """ + if not policy_document: + return False + + try: + policy = json.loads(policy_document) + + # Check for public write in policy + for statement in policy['Statement']: + principal = statement.get('Principal', {}) + if (principal == '*' or principal.get('AWS') == '*') and \ + statement['Effect'] == 'Allow' and \ + any(action in ['s3:PutObject', 's3:DeleteObject'] + for action in (statement.get('Action', []) if isinstance(statement.get('Action', []), list) + else [statement.get('Action', [])])): + return True + + return False + + except (json.JSONDecodeError, KeyError): + return False def getBucketPolicy(self): try: @@ -131,12 +152,10 @@ def _checkAccess(self): except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'NoSuchAcl': return None - # check if S3 bucket has prohibited public reads policy = self.getBucketPolicy() - if (public_policy_restricted or not self.policyAllowsPublicRead(policy)) and (public_acl_restricted or not self.aclAllowsPublicRead(bucket_acl)): self.results['PublicReadAccessBlock'] = [1, 'Prohibited'] else: @@ -148,7 +167,6 @@ def _checkAccess(self): else: self.results['PublicWriteAccessBlock'] = [-1, 'NotProhibited'] - def _checkMfaDeleteAndVersioning(self): self.results['MFADelete'] = [-1, 'Off'] self.results['BucketVersioning'] = [-1, 'Off'] @@ -195,8 +213,6 @@ def _checkBucketReplication(self): if e.response['Error']['Code'] == 'ReplicationConfigurationNotFoundError': self.results['BucketReplication'] = [-1, 'Off'] - - def _checkLifecycle(self): self.results['BucketLifecycle'] = [1, 'On'] try: From f4cb6e0ae09971cb74524d14ba7511fd4d3749d0 Mon Sep 17 00:00:00 2001 From: KuetTai Date: Thu, 23 Jan 2025 15:50:22 +0800 Subject: [PATCH 4/4] Update README.md Include STS Token permision for crossAccount --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 8d7bdfa..90461c9 100755 --- a/README.md +++ b/README.md @@ -26,6 +26,8 @@ Running this tool is free as it is covered under the AWS Free Tier. If you have a. AWSCloudShellFullAccess b. cloudformation:CreateStack c. cloudformation:DeleteStack +4. (Optional) If you need to run crossAccounts, additional permission is needed + a. iam:SetSecurityTokenServicePreferences ## Installing service-screener V2 1. [Log in to your AWS account](https://docs.aws.amazon.com/cloudshell/latest/userguide/getting-started.html#start-session) using the IAM User with sufficient permissions described above.