Skip to content

Commit

Permalink
feat(azure): Check related with roles and vm access with MFA (#3638)
Browse files Browse the repository at this point in the history
Co-authored-by: Hugo Gálvez Ureña <[email protected]>
Co-authored-by: Sergio <[email protected]>
  • Loading branch information
3 people authored Apr 2, 2024
1 parent 5839d8c commit be19ec5
Show file tree
Hide file tree
Showing 14 changed files with 391 additions and 34 deletions.
12 changes: 12 additions & 0 deletions prowler/providers/azure/config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# General built-in roles
CONTRIBUTOR_ROLE_ID = "b24988ac-6180-42a0-ab88-20f7382dd24c"
OWNER_ROLE_ID = "8e3af657-a8ff-443c-a75c-2fe8c4bcb635"

# Compute roles
VIRTUAL_MACHINE_CONTRIBUTOR_ROLE_ID = "9980e02c-c2be-4d73-94e8-173b1dc7cf3c"
VIRTUAL_MACHINE_ADMINISTRATOR_LOGIN_ROLE_ID = "1c0163c0-47e6-4577-8991-ea5c82e286e4"
VIRTUAL_MACHINE_USER_LOGIN_ROLE_ID = "fb879df8-f326-4884-b1cf-06f3ad86be52"
VIRTUAL_MACHINE_LOCAL_USER_LOGIN_ROLE_ID = "602da2ba-a5c2-41da-b01d-5360126ab525"
WINDOWS_ADMIN_CENTER_ADMINISTRATOR_LOGIN_ROLE_ID = (
"a6333a3e-0164-44c3-b281-7a577aff287f"
)
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"Provider": "azure",
"CheckID": "entra_users_less_than_five_global_admins",
"CheckID": "entra_global_admin_in_less_than_five_users",
"CheckTitle": "Ensure fewer than 5 users have global administrator assignment",
"CheckType": [],
"ServiceName": "entra",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from prowler.providers.azure.services.entra.entra_client import entra_client


class entra_users_less_than_five_global_admins(Check):
class entra_global_admin_in_less_than_five_users(Check):
def execute(self) -> Check_Report_Azure:
findings = []

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ def execute(self) -> Check_Report_Azure:
report.resource_name = user_domain_name
report.resource_id = user.id
report.status_extended = (
f"User '{user.name}' does not have MFA enabled."
f"Non-privileged user {user.name} does not have MFA."
)

if len(user.authentication_methods) > 1:
report.status = "PASS"
report.status_extended = f"User '{user.name}' has MFA enabled."
report.status_extended = (
f"Non-privileged user {user.name} has MFA."
)

findings.append(report)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ def execute(self) -> Check_Report_Azure:
report.resource_name = user_domain_name
report.resource_id = user.id
report.status_extended = (
f"User '{user.name}' does not have MFA enabled."
f"Privileged user {user.name} does not have MFA."
)

if len(user.authentication_methods) > 1:
report.status = "PASS"
report.status_extended = f"User '{user.name}' has MFA enabled."
report.status_extended = f"Privileged user {user.name} has MFA."

findings.append(report)

Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"Provider": "azure",
"CheckID": "entra_user_with_vm_access_has_mfa",
"CheckTitle": "Ensure only MFA enabled identities can access privileged Virtual Machine",
"CheckType": [],
"ServiceName": "iam",
"SubServiceName": "",
"ResourceIdTemplate": "",
"Severity": "medium",
"ResourceType": "#microsoft.graph.users",
"Description": "Verify identities without MFA that can log in to a privileged virtual machine using separate login credentials. An adversary can leverage the access to move laterally and perform actions with the virtual machine's managed identity. Make sure the virtual machine only has necessary permissions, and revoke the admin-level permissions according to the least privileges principal",
"Risk": "Managed disks are by default encrypted on the underlying hardware, so no additional encryption is required for basic protection. It is available if additional encryption is required. Managed disks are by design more resilient that storage accounts. For ARM-deployed Virtual Machines, Azure Adviser will at some point recommend moving VHDs to managed disks both from a security and cost management perspective.",
"RelatedUrl": "",
"Remediation": {
"Code": {
"CLI": "",
"NativeIaC": "",
"Other": "",
"Terraform": ""
},
"Recommendation": {
"Text": "1. Log in to the Azure portal. Reducing access of managed identities attached to virtual machines. 2. This can be remediated by enabling MFA for user, Removing user access or • Case I : Enable MFA for users having access on virtual machines. 1. Navigate to Azure AD from the left pane and select Users from the Manage section. 2. Click on Per-User MFA from the top menu options and select each user with MULTI-FACTOR AUTH STATUS as Disabled and can login to virtual machines:  From quick steps on the right side select enable.  Click on enable multi-factor auth and share the link with the user to setup MFA as required. • Case II : Removing user access on a virtual machine. 1. Select the Subscription, then click on Access control (IAM). 2. Select Role assignments and search for Virtual Machine Administrator Login or Virtual Machine User Login or any role that provides access to log into virtual machines. 3. Click on Role Name, Select Assignments, and remove identities with no MFA configured. • Case III : Reducing access of managed identities attached to virtual machines. 1. Select the Subscription, then click on Access control (IAM). 2. Select Role Assignments from the top menu and apply filters on Assignment type as Privileged administrator roles and Type as Virtual Machines. 3. Click on Role Name, Select Assignments, and remove identities access make sure this follows the least privileges principal.",
"Url": ""
}
},
"Categories": [],
"DependsOn": [],
"RelatedTo": [],
"Notes": "This recommendation requires an Azure AD P2 License to implement. Ensure that identities that are provisioned to a virtual machine utilizes an RBAC/ABAC group and is allocated a role using Azure PIM, and the Role settings require MFA or use another PAM solution (like CyberArk) for accessing Virtual Machines."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from prowler.lib.check.models import Check, Check_Report_Azure
from prowler.providers.azure.config import (
CONTRIBUTOR_ROLE_ID,
OWNER_ROLE_ID,
VIRTUAL_MACHINE_ADMINISTRATOR_LOGIN_ROLE_ID,
VIRTUAL_MACHINE_CONTRIBUTOR_ROLE_ID,
VIRTUAL_MACHINE_LOCAL_USER_LOGIN_ROLE_ID,
VIRTUAL_MACHINE_USER_LOGIN_ROLE_ID,
WINDOWS_ADMIN_CENTER_ADMINISTRATOR_LOGIN_ROLE_ID,
)
from prowler.providers.azure.services.entra.entra_client import entra_client
from prowler.providers.azure.services.iam.iam_client import iam_client


class entra_user_with_vm_access_has_mfa(Check):
def execute(self) -> Check_Report_Azure:
findings = []

for users in entra_client.users.values():
for user_domain_name, user in users.items():
for (
subscription_name,
role_assigns,
) in iam_client.role_assignments.items():
for assignment in role_assigns.values():
if (
assignment.agent_type == "User"
and assignment.role_id
in [
CONTRIBUTOR_ROLE_ID,
OWNER_ROLE_ID,
VIRTUAL_MACHINE_CONTRIBUTOR_ROLE_ID,
VIRTUAL_MACHINE_ADMINISTRATOR_LOGIN_ROLE_ID,
VIRTUAL_MACHINE_USER_LOGIN_ROLE_ID,
VIRTUAL_MACHINE_LOCAL_USER_LOGIN_ROLE_ID,
WINDOWS_ADMIN_CENTER_ADMINISTRATOR_LOGIN_ROLE_ID,
]
and assignment.agent_id == user.id
):
report = Check_Report_Azure(self.metadata())
report.status = "FAIL"
report.status_extended = f"User {user.name} without MFA can access VMs in subscription {subscription_name}"
report.subscription = subscription_name
report.resource_name = user_domain_name
report.resource_id = user.id

if len(user.authentication_methods) > 1:
report.status = "PASS"
report.status_extended = f"User {user.name} can access VMs in subscription {subscription_name} but it has MFA."

findings.append(report)

return findings
36 changes: 36 additions & 0 deletions prowler/providers/azure/services/iam/iam_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class IAM(AzureService):
def __init__(self, audit_info):
super().__init__(AuthorizationManagementClient, audit_info)
self.roles, self.custom_roles = self.__get_roles__()
self.role_assignments = self.__get_role_assignments__()

def __get_roles__(self):
logger.info("IAM - Getting roles...")
Expand Down Expand Up @@ -52,6 +53,34 @@ def __get_roles__(self):
)
return builtin_roles, custom_roles

def __get_role_assignments__(self):
logger.info("IAM - Getting role assignments...")
role_assignments = {}
for subscription, client in self.clients.items():
try:
role_assignments.update({subscription: {}})
all_role_assignments = client.role_assignments.list_for_subscription(
filter="atScope()"
)
for role_assignment in all_role_assignments:
role_assignments[subscription].update(
{
role_assignment.id: RoleAssignment(
agent_id=role_assignment.principal_id,
agent_type=role_assignment.principal_type,
role_id=role_assignment.role_definition_id.split("/")[
-1
],
)
}
)
except Exception as error:
logger.error(f"Subscription name: {subscription}")
logger.error(
f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
)
return role_assignments


@dataclass
class Role:
Expand All @@ -60,3 +89,10 @@ class Role:
type: str
assignable_scopes: list[str]
permissions: list[Permission]


@dataclass
class RoleAssignment:
agent_id: str
agent_type: str
role_id: str
Original file line number Diff line number Diff line change
Expand Up @@ -4,55 +4,55 @@
from tests.providers.azure.azure_fixtures import DOMAIN


class Test_entra_users_less_than_five_global_admins:
class Test_entra_global_admin_in_less_than_five_users:
def test_entra_no_tenants(self):
entra_client = mock.MagicMock

with mock.patch(
"prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins.entra_client",
"prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users.entra_client",
new=entra_client,
):
from prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins import (
entra_users_less_than_five_global_admins,
from prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users import (
entra_global_admin_in_less_than_five_users,
)

entra_client.directory_roles = {}

check = entra_users_less_than_five_global_admins()
check = entra_global_admin_in_less_than_five_users()
result = check.execute()
assert len(result) == 0

def test_entra_tenant_empty(self):
entra_client = mock.MagicMock

with mock.patch(
"prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins.entra_client",
"prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users.entra_client",
new=entra_client,
):
from prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins import (
entra_users_less_than_five_global_admins,
from prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users import (
entra_global_admin_in_less_than_five_users,
)

entra_client.directory_roles = {DOMAIN: {}}

check = entra_users_less_than_five_global_admins()
check = entra_global_admin_in_less_than_five_users()
result = check.execute()
assert len(result) == 0

def test_entra_less_than_five_global_admins(self):
entra_client = mock.MagicMock

with mock.patch(
"prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins.entra_client",
"prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users.entra_client",
new=entra_client,
):
from prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users import (
entra_global_admin_in_less_than_five_users,
)
from prowler.providers.azure.services.entra.entra_service import (
DirectoryRole,
User,
)
from prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins import (
entra_users_less_than_five_global_admins,
)

id = str(uuid4())
id_user1 = str(uuid4())
Expand All @@ -70,7 +70,7 @@ def test_entra_less_than_five_global_admins(self):
}
}

check = entra_users_less_than_five_global_admins()
check = entra_global_admin_in_less_than_five_users()
result = check.execute()
assert len(result) == 1
assert result[0].status == "PASS"
Expand All @@ -83,16 +83,16 @@ def test_entra_more_than_five_global_admins(self):
entra_client = mock.MagicMock

with mock.patch(
"prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins.entra_client",
"prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users.entra_client",
new=entra_client,
):
from prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users import (
entra_global_admin_in_less_than_five_users,
)
from prowler.providers.azure.services.entra.entra_service import (
DirectoryRole,
User,
)
from prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins import (
entra_users_less_than_five_global_admins,
)

id = str(uuid4())
id_user1 = str(uuid4())
Expand All @@ -118,7 +118,7 @@ def test_entra_more_than_five_global_admins(self):
}
}

check = entra_users_less_than_five_global_admins()
check = entra_global_admin_in_less_than_five_users()
result = check.execute()
assert len(result) == 1
assert result[0].status == "FAIL"
Expand All @@ -134,16 +134,16 @@ def test_entra_exactly_five_global_admins(self):
entra_client = mock.MagicMock

with mock.patch(
"prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins.entra_client",
"prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users.entra_client",
new=entra_client,
):
from prowler.providers.azure.services.entra.entra_global_admin_in_less_than_five_users.entra_global_admin_in_less_than_five_users import (
entra_global_admin_in_less_than_five_users,
)
from prowler.providers.azure.services.entra.entra_service import (
DirectoryRole,
User,
)
from prowler.providers.azure.services.entra.entra_users_less_than_five_global_admins.entra_users_less_than_five_global_admins import (
entra_users_less_than_five_global_admins,
)

id = str(uuid4())
id_user1 = str(uuid4())
Expand All @@ -167,7 +167,7 @@ def test_entra_exactly_five_global_admins(self):
}
}

check = entra_users_less_than_five_global_admins()
check = entra_global_admin_in_less_than_five_users()
result = check.execute()
assert len(result) == 1
assert result[0].status == "FAIL"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ def test_entra_user_no_privileged_no_mfa(self):
result = check.execute()
assert len(result) == 1
assert result[0].status == "FAIL"
assert result[0].status_extended == "User 'foo' does not have MFA enabled."
assert (
result[0].status_extended
== "Non-privileged user foo does not have MFA."
)
assert result[0].resource_name == f"foo@{DOMAIN}"
assert result[0].resource_id == user_id
assert result[0].subscription == f"Tenant: {DOMAIN}"
Expand Down Expand Up @@ -102,7 +105,7 @@ def test_entra_user_no_privileged_mfa(self):
result = check.execute()
assert len(result) == 1
assert result[0].status == "PASS"
assert result[0].status_extended == "User 'foo' has MFA enabled."
assert result[0].status_extended == "Non-privileged user foo has MFA."
assert result[0].resource_name == f"foo@{DOMAIN}"
assert result[0].resource_id == user_id
assert result[0].subscription == f"Tenant: {DOMAIN}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def test_entra_user_privileged_no_mfa(self):
result = check.execute()
assert len(result) == 1
assert result[0].status == "FAIL"
assert result[0].status_extended == "User 'foo' does not have MFA enabled."
assert result[0].status_extended == "Privileged user foo does not have MFA."
assert result[0].resource_name == f"foo@{DOMAIN}"
assert result[0].resource_id == user_id
assert result[0].subscription == f"Tenant: {DOMAIN}"
Expand Down Expand Up @@ -164,7 +164,7 @@ def test_entra_user_privileged_mfa(self):
result = check.execute()
assert len(result) == 1
assert result[0].status == "PASS"
assert result[0].status_extended == "User 'foo' has MFA enabled."
assert result[0].status_extended == "Privileged user foo has MFA."
assert result[0].resource_name == f"foo@{DOMAIN}"
assert result[0].resource_id == user_id
assert result[0].subscription == f"Tenant: {DOMAIN}"
Loading

0 comments on commit be19ec5

Please sign in to comment.