8000 Use Azure ADE status to prevent false positives by ncc-akis · Pull Request #1547 · nccgroup/ScoutSuite · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use Azure ADE status to prevent false positives #1547

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

Merged

Conversation

ncc-akis
Copy link
Contributor

Description

In certain circumstances, ScoutSuite can generate false positive reports regarding the rules:

  • OS and Data Disks Not Encrypted with CMK
  • Unattached Disks Not Encrypted with CMK

This can occur when the VMs are configured to use Azure Disk Encryption (ADE). In this case, Server Side Encryption (SSE) is still enabled, but cannot be used in conjunction with CMK. Because ADE is a suitable alternative which provides stronger protection than SSE, it is not correct to flag the lack of CMK as an issue when ADE is enabled.

The changes in this PR ensure that the presence of ADE is detected. The simplest way I found to do this was to look for the encryption_settings_collection attribute in the raw_disk object. Based on my testing it seems that this attribute is only present when ADE is enabled, and it is set to None otherwise. Furthermore, the encryption_settings_version property must be set to 1.0 or 1.1 (see https://learn.microsoft.com/en-us/python/api/azure-mgmt-compute/azure.mgmt.compute.v2022_07_02.models.encryptionsettingscollection?view=azure-python). Practically it might be 1.1 only but 1.0 is mentioned in the doc as related to "Azure Disk Encryption with AAD app" so I thought I'd include it.

Because ADE requires the presence of extensions named "AzureDiskEncryption" or "AzureDiskEncryptionForLinux", I decided also to modify the "Virtual Machine Extensions Installed" rule, to prevent it triggering in the presence of those extensions.

Type of change

Select the relevant option(s):

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works (optional)
  • New and existing unit tests pass locally with my changes

Copy link
Collaborator
@fernando-gallego fernando-gallego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@fernando-gallego fernando-gallego merged commit 7125290 into nccgroup:develop Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0