-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(oci): add identity_storage_service_level_admins_scoped check for CIS 3.1 1.15 compliance
#10568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(oci): add identity_storage_service_level_admins_scoped check for CIS 3.1 1.15 compliance
#10568
Changes from 1 commit
bf3feb4
df471f6
cf9d4b6
564cfa2
076860e
fe6d42a
bd29bcf
d9727ae
3779c86
7eb049f
0821a02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| { | ||
| "Provider": "oraclecloud", | ||
| "CheckID": "identity_storage_service_level_admins_scoped", | ||
| "CheckTitle": "Identity policy does not grant delete to storage service level admins", | ||
| "CheckType": [], | ||
| "ServiceName": "identity", | ||
| "SubServiceName": "", | ||
| "ResourceIdTemplate": "", | ||
| "Severity": "high", | ||
| "ResourceType": "Policy", | ||
| "ResourceGroup": "IAM", | ||
| "Description": "**OCI IAM policies** are reviewed for **delete permissions**, specifically statements granting `manage` to storage services without scoping to avoid deletion. Only **active policies** are considered; the default tenant admin policy is excluded.", | ||
| "Risk": "Creating service-level administrators without the ability to delete the resource they are managing helps in tightly controlling access to Oracle Cloud Infrastructure (OCI) services by implementing the separation of duties security principle.", | ||
| "RelatedUrl": "", | ||
| "AdditionalURLs": [ | ||
| "https://docs.oracle.com/en-us/iaas/Content/Identity/Concepts/policygetstarted.htm", | ||
| "https://docs.oracle.com/en-us/iaas/Content/Identity/getstarted/identity-domains.htm" | ||
| ], | ||
| "Remediation": { | ||
| "Code": { | ||
| "CLI": "oci iam policy update --policy-id <policy-ocid> --statements '[\"Allow group <GroupName> to manage objects where request.permission!='OBJECT_DELETE'\"]'", | ||
| "NativeIaC": "", | ||
| "Other": "1. Login to OCI console.\n2. Go to Identity -> Policies, In the compartment dropdown, choose the compartment.\n3. Open each policy to view the policy statements.\n4. Verify the policies to ensure that the policy statements that grant access to storage service-level administrators have a condition that excludes access to delete the service they are the administrator for.", | ||
| "Terraform": "```hcl\nresource \"oci_identity_policy\" \"<example_resource_name>\" {\n compartment_id = \"<example_resource_id>\"\n name = \"<example_resource_name>\"\n description = \"<policy-description>\"\n\n # Critical: restrict storage service family to avoid broad deleting'\n statements = [\n \"Allow group <GroupName> to manage volumes in compartment <CompartmentName> where request.PERMISSIONS!='VOLUME_DELETE'\"\n ]\n}\n```" | ||
| }, | ||
| "Recommendation": { | ||
| "Text": "Add the appropriate where condition to any policy statement that allows the storage service-level admins to manage the storage service.", | ||
| "Url": "" | ||
| } | ||
| }, | ||
| "Categories": [ | ||
| "identity-access" | ||
| ], | ||
| "DependsOn": [], | ||
| "RelatedTo": [], | ||
| "Notes": "" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| from prowler.lib.check.models import Check, Check_Report_OCI | ||
| from prowler.providers.oraclecloud.services.identity.identity_client import ( | ||
| identity_client, | ||
| ) | ||
|
|
||
| class identity_storage_service_level_admins_scoped(Check): | ||
| """Ensure storage service-level admins cannot delete resources they manage (CIS 1.15)""" | ||
|
|
||
| def execute(self): | ||
| """Ensure service-level administrators can only manage resources of a specific service but not delete resources. | ||
|
|
||
| This check ensures that policies don't grant delete permission on storage like "manage volumes in tenancy" | ||
| without restricting delete permissions. | ||
| """ | ||
| findings = [] | ||
|
|
||
| storage_policies = { | ||
| "FILE-FAMILY": [ | ||
| "FILE-SYSTEMS", | ||
| "MOUNT-TARGETS", | ||
| "EXPORT-SETS" | ||
| ], | ||
| "OBJECT-FAMILY": [ | ||
| "BUCKETS", | ||
| "OBJECTS" | ||
| ], | ||
| "VOLUME-FAMILY": [ | ||
| "VOLUMES", | ||
| "VOLUME-ATTACHMENTS", | ||
| "VOLUME-BACKUPS" | ||
| ] | ||
| } | ||
| all_base_policies = [item for sublist in storage_policies.values() for item in sublist] | ||
|
|
||
| # Check for policies that violate least privilege by granting manage all-resources | ||
| for policy in identity_client.policies: | ||
| # Skip non-active policies | ||
| if policy.lifecycle_state != "ACTIVE": | ||
| continue | ||
|
|
||
| # Skip default tenant admin policy | ||
| if policy.name.upper() == "TENANT ADMIN POLICY": | ||
| continue | ||
|
|
||
| region = policy.region if hasattr(policy, "region") else "global" | ||
|
|
||
| has_violation = False | ||
| offending_statement = None | ||
| for statement in policy.statements: | ||
| statement_upper = statement.upper() | ||
| statement_words = statement_upper.split(" ") | ||
Check noticeCode scanning / CodeQL Unused local variable Note
Variable statement_words is not used.
|
||
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show fixed
Hide fixed
|
||
| # Only check groups | ||
| if not statement_upper.startswith("ALLOW GROUP"): | ||
| continue | ||
|
|
||
| # Check for "allow group ... to manage file service resources without restriction" (not specific to service/compartment) | ||
| if any(f"MANAGE {global_storage_policy}" in statement_upper for global_storage_policy in storage_policies): | ||
| if "WHERE" not in statement_upper: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic is still too permissive for CIS 3.1 control 1.15. The control does not require any Please update the check so it only treats a statement as compliant when it explicitly excludes the relevant delete permission for that resource, for example
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally implemented this behavior, but looking at the CIS 1.15 examples it seems service level admins refer to unrestricted manage access to storage services. Other CIS benchmarks also seem to flag it as such. I would think that These are the example policies adjusted to fit within the control within its definition document: Example policies for global/tenant level for block volume service-administrators: Example policies for global/tenant level for file storage system service-administrators: Example policies for global/tenant level for object storage system service-administrators: Each of these allows manage for a file service storage, but doesn't restrict the permissions at all without the request.permission!='X_DELETE'. In addition, the reference here https://docs.oracle.com/en/solutions/oci-best-practices/protect-data-rest1.html#GUID-EBE9212C-78B0-4795-9FB5-59DB0E91E106 linked in the control says that the permissions should be give "to only the users who need these privileges," which implies that some users should still be able to have delete permissions. I interpreted this as meaning that the CIS benchmark was to limit unrestricted delete permissions given to admins over all storage services. |
||
| has_violation = True | ||
| offending_statement = statement | ||
| break | ||
| if any(f"MANAGE {base_storage_policy}" in statement_upper for base_storage_policy in all_base_policies): | ||
| if "WHERE" not in statement_upper: | ||
| has_violation = True | ||
| offending_statement = statement | ||
| break | ||
| if "MANAGE ALL-RESOURCES" in statement_upper: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would also revisit the For CIS 1.15, broad statements should not pass just because they contain a |
||
| if "WHERE" not in statement_upper: | ||
| has_violation = True | ||
| offending_statement = statement | ||
| break | ||
|
|
||
| report = Check_Report_OCI( | ||
| metadata=self.metadata(), | ||
| resource=policy, | ||
| region=region, | ||
| resource_id=policy.id, | ||
| resource_name=policy.name, | ||
| compartment_id=policy.compartment_id, | ||
| ) | ||
|
|
||
| if has_violation: | ||
| report.status = "FAIL" | ||
| report.status_extended = f"Policy '{policy.name}' grants 'manage' permissions with delete. Service-level storage administrators should be created with delete permissions.\n{offending_statement}" | ||
| else: | ||
| report.status = "PASS" | ||
| report.status_extended = f"Policy '{policy.name}' does not grant storage service level admins delete permissions." | ||
|
|
||
| findings.append(report) | ||
|
|
||
| # If no policies found, that's also a finding | ||
| if not findings: | ||
| region = ( | ||
| identity_client.audited_regions[0].key | ||
| if identity_client.audited_regions | ||
| else "global" | ||
| ) | ||
| report = Check_Report_OCI( | ||
| metadata=self.metadata(), | ||
| resource={}, | ||
| region=region, | ||
| resource_id=identity_client.audited_tenancy, | ||
| resource_name="Tenancy", | ||
| compartment_id=identity_client.audited_tenancy, | ||
| ) | ||
| report.status = "PASS" | ||
| report.status_extended = ( | ||
| "No active storage-level admin policies found with delete permissions." | ||
| ) | ||
| findings.append(report) | ||
|
|
||
| return findings | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new check still needs real unit test coverage. At the moment the test file is empty, so we are not validating the CIS-specific behavior of the parser. Please add tests for at least:
This is especially important here because the correctness of the check depends on parsing OCI policy statements exactly as required by CIS 1.15. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please align the metadata remediation examples with the CIS 1.15 wording and examples.
A couple of things to fix here:
request.permissionshould be used consistentlyrequest.PERMISSIONS, which does not match the benchmark examplesVOLUME_DELETE,FILE_SYSTEM_DELETE,OBJECT_DELETE, etc.), not just a single generic exampleSince this metadata is what users will read for remediation, it should map directly to the benchmark language.