feat: add rule AZ-CMP-005 Trusted Launch (Secure Boot + vTPM) check - #273
Vishnu2707 merged 2 commits into
Conversation
AZ-CMP-005 collided with OWASP#273 (Trusted Launch check), opened a day before this PR and already claiming that rule ID. Renumbered the rule file, playbook, tests, and all four compliance framework entries to AZ-CMP-006. Also fixes a false-positive gap flagged in review: the rule only checked for an NSG on the VMSS network interface configuration itself, missing the case where the NSG is attached at the subnet level instead. A VMSS whose NIC has no NSG but deploys into a subnet that does have one was being incorrectly flagged. Now resolves each network interface configuration's subnet (via the existing get_virtual_networks() collector, no new collector needed) and treats either a NIC-level or subnet-level NSG as compliant, matching how AZ-NET-010 already reads subnet.network_security_group. Added SOC2 to the rule's own FRAMEWORKS dict (was previously only in soc2.json, inconsistent with how several other rules, e.g. AZ-KV-006, already include it directly). Addresses review feedback from TFT444 and m-khan-97 on OWASP#275. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com>
parthrohit22
left a comment
There was a problem hiding this comment.
nice work @shariqueahmad108-ship-it ,
Solid rule. I like that you didn't just flag every VM without Trusted Launch - you actually resolve Gen1 vs Gen2 via the OS disk and treat "can't determine" as NOT_APPLICABLE instead of guessing, which keeps this from throwing false positives on VMs that physically can't support it. Confidential VM carve-out is a nice touch too. Verified the SDK fields (security_type, uefi_settings, secure_boot_enabled, v_tpm_enabled, hyper_v_generation) against the real azure-mgmt-compute models, all match. 562 passed, clean lint. No conflict with #275's AZ-CMP-006. LGTM, approving.
Vishnu2707
left a comment
There was a problem hiding this comment.
One thing before merge though, the playbook uses RG=$1 and VM=$2 directly, with set -u that'll just crash instead of showing the usage message if someone runs it with no args. Same issue @TFT444 flagged on KV-006 before, just needs ${1:-} and ${2:-} like fix_az_net_016.sh already does. That should be a quick fix though.
| # Note: only supported on Gen2 VM sizes/images; the update requires a restart to take effect. | ||
|
|
||
| set -euo pipefail | ||
|
|
There was a problem hiding this comment.
please use ${1:-} and ${2:-} like how it is present in fix_az_net_016.sh to avoid further bugs
Use ${1:-}/${2:-} so running the playbook with missing arguments prints the
usage message and exits 1, instead of crashing on an unbound variable under
`set -euo pipefail`. Matches the convention in fix_az_net_016.sh.
Addresses review feedback from @Vishnu2707 on OWASP#273.
Signed-off-by: shariqueahmad108-ship-it <shariqueahmad108@gmail.com>
|
Good catch @Vishnu2707 — fixed in 3415de1. Switched to |
TFT444
left a comment
There was a problem hiding this comment.
Single blocking issue: fix_az_cmp_005.sh uses RG=\ and VM=\ with set -u at the top. Running the script without arguments crashes before the usage message is printed. Change both to \ and , matching the pattern in fix_az_net_016.sh. Everything else looks good.
|
Thanks @TFT444 — that one's already handled: |
|
@shariqueahmad108-ship-it branch have conflict solve it please |
Adds a Compute scan rule that flags Generation 2 VMs which do not have Trusted Launch fully enabled (security type TrustedLaunch with both Secure Boot and vTPM on). Without them, unsigned or malicious code can run during boot and persist beneath the OS, evading OS-level antimalware and EDR. Generation 1 VMs cannot use Trusted Launch and are treated as NOT_APPLICABLE: a VM's list_all() representation does not carry its Hyper-V generation, so the rule resolves the OS disk's hyper_v_generation to confirm a VM is Gen2 before flagging, and never raises a false finding against Gen1 hardware. A security type already declared as TrustedLaunch is itself Gen2-only, so a Secure-Boot/vTPM-off VM there is flagged without a disk lookup. Confidential VMs (which provide Secure Boot and vTPM by construction) are out of scope, and a VM whose generation cannot be confirmed is left unflagged rather than risking a Gen1 false positive. Includes the remediation playbook (playbooks/cli/fix_az_cmp_005.sh) and maps the rule across the four compliance frameworks: NIST CSF PR.DS-6, ISO 27001 A.12.5.1, and SOC 2 CC6.8. CIS uses the repository's existing N/A convention (as in AZ-KV-001) because CIS Azure Foundations 2.0.0 has no dedicated Trusted Launch recommendation. Adds six unit tests covering the compliant, non-compliant Gen2, TrustedLaunch-declared-but-vTPM-off, Gen1 NOT_APPLICABLE, unknown-generation, and Confidential-VM cases. Closes OWASP#269 Signed-off-by: shariqueahmad108-ship-it <shariqueahmad108@gmail.com>
3415de1 to
67d21d7
Compare
Use ${1:-}/${2:-} so running the playbook with missing arguments prints the
usage message and exits 1, instead of crashing on an unbound variable under
`set -euo pipefail`. Matches the convention in fix_az_net_016.sh.
Addresses review feedback from @Vishnu2707 on OWASP#273.
Signed-off-by: shariqueahmad108-ship-it <shariqueahmad108@gmail.com>
AZ-CMP-005 collided with OWASP#273 (Trusted Launch check), opened a day before this PR and already claiming that rule ID. Renumbered the rule file, playbook, tests, and all four compliance framework entries to AZ-CMP-006. Also fixes a false-positive gap flagged in review: the rule only checked for an NSG on the VMSS network interface configuration itself, missing the case where the NSG is attached at the subnet level instead. A VMSS whose NIC has no NSG but deploys into a subnet that does have one was being incorrectly flagged. Now resolves each network interface configuration's subnet (via the existing get_virtual_networks() collector, no new collector needed) and treats either a NIC-level or subnet-level NSG as compliant, matching how AZ-NET-010 already reads subnet.network_security_group. Added SOC2 to the rule's own FRAMEWORKS dict (was previously only in soc2.json, inconsistent with how several other rules, e.g. AZ-KV-006, already include it directly). Addresses review feedback from TFT444 and m-khan-97 on OWASP#275. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com>
|
Done — rebased onto the latest |
Use ${1:-}/${2:-} so running the playbook with missing arguments prints the
usage message and exits 1, instead of crashing on an unbound variable under
`set -euo pipefail`. Matches the convention in fix_az_net_016.sh.
Addresses review feedback from @Vishnu2707 on OWASP#273.
Signed-off-by: shariqueahmad108-ship-it <shariqueahmad108@gmail.com>
67d21d7 to
50f5930
Compare
|
Friendly ping on this one — it's green across all 20 checks and mergeable now that the rebase conflict and the ruff-format issue are both sorted. @TFT444 your original blocker (the |
TFT444
left a comment
There was a problem hiding this comment.
My prior blocking issue (fix_az_cmp_005.sh crashing on missing args under set -u) is resolved in commit 50f5930. The playbook now uses RG="${1:-}" / VM="${2:-}" with the subsequent null-guard, matching the fix_az_net_016.sh pattern exactly as requested.
Blocker resolved
playbooks/cli/fix_az_cmp_005.sh lines 9-10: RG="${1:-}" / VM="${2:-}" are present, followed by the if [ -z "$RG" ] || [ -z "$VM" ] usage guard on lines 12-15. Running the script with no arguments now prints the usage message and exits 1 instead of crashing on unbound variables.
Rule correctness — no new issues found
- SDK fields verified:
security_profile.security_type,security_profile.uefi_settings.secure_boot_enabled,uefi_settings.v_tpm_enabled,storage_profile.os_disk.managed_disk.hyper_v_generationall match the realazure-mgmt-computemodels. azure_client.get_virtual_machines(),azure_client.get_disk(), andazure_client.parse_resource_id()all exist inscanner/azure_client.py.- Indeterminate handling is correct:
security_profile is Nonecauses_trusted_launch_fully_enabledto returnFalseand_os_disk_generationto returnNone, and the rule skips with no false finding. Theuefivariable re-reads fromsecurity_profilebefore building finding metadata soNonepropagates cleanly. - The Gen2-without-security-profile case is sound:
security_type == "TrustedLaunch"is itself a Gen2 attestation, so no disk lookup is needed for TrustedLaunch-declared-but-vTPM-off.
RULE_ID collision with PR #275 — resolved
PR #275 originally claimed AZ-CMP-005. That was blocked in the #275 review and #275 was subsequently renamed to AZ-CMP-006. No collision exists.
Compliance mappings: All 4 framework JSONs include AZ-CMP-005. NIST CSF PR.DS-6, ISO 27001 A.12.5.1, SOC 2 CC6.8 are legitimate mappings. CIS uses N/A-CMP-005 with a clear rationale (no dedicated CIS Azure Foundations 2.0.0 recommendation), consistent with the existing AZ-KV-001 convention.
Test coverage: 6 tests cover compliant TrustedLaunch, non-compliant Gen2 with no security profile, TrustedLaunch-declared-but-vTPM-off, Gen1 NOT_APPLICABLE, unknown-generation skip, and Confidential VM skip. The intentional false-negative trade-off for unreadable-disk Gen2 VMs is documented and mirrors AZ-CMP-002's stance.
Playbook: bash -n passes. The az vm update invocation uses the correct flags (--security-type TrustedLaunch --enable-secure-boot true --enable-vtpm true).
Note for maintainers: Vishnu2707's CHANGES_REQUESTED review remains active in the GitHub UI (same fix, same commit). The fix commit directly addresses that feedback; a maintainer should ask Vishnu2707 to re-review before merging.
|
Thanks for the follow-up. I checked the current head (50f5930): the playbook now uses the safe argument defaults and a usage guard before any Azure command. Tanvir has already approved this revision; the remaining changes-requested review is from @Vishnu2707 on that same argument-handling issue. Vishnu, could you re-check the fix and update your review when you have a moment? @parthrohit22, your earlier approval was dismissed, so please refresh it against this head if you are still happy with the changes. Let us clear these through re-review before merging; I have not dismissed anyone’s review. |
parthrohit22
left a comment
There was a problem hiding this comment.
Re-reviewed 50f5930.
The rule is in good shape and matches the fail-safe design I liked on the first pass:
- Gen1 VMs and VMs whose generation can't be confirmed as
V2are skipped, never flagged. security_type == "TrustedLaunch"with Secure Boot / vTPM off is flagged directly (TrustedLaunch is Gen2-only, so no disk lookup needed); everything else resolves the Hyper-V generation from the managed OS disk and only flags a confirmedV2.ConfidentialVMis correctly out of scope._trusted_launch_fully_enabled()requiresis Truefor bothsecure_boot_enabledandv_tpm_enabled, so a missing/None value is not treated as compliant.
fix_az_cmp_005.sh now runs under set -euo pipefail with ${1:-} / ${2:-} and a usage guard before any az call — the arg-handling issue from the earlier reviews is resolved (TFT444 has since approved). Framework mappings follow the existing convention (N/A-CMP-005 for CIS, real control ids for NIST/ISO/SOC2). 6 AZ-CMP-005 tests pass locally, bash -n on the playbook is clean, and all PR checks are green.
Approving. The only outstanding change request is @Vishnu2707's from Aug 17, which predates the set -u fix.
AZ-CMP-005 collided with OWASP#273 (Trusted Launch check), opened a day before this PR and already claiming that rule ID. Renumbered the rule file, playbook, tests, and all four compliance framework entries to AZ-CMP-006. Also fixes a false-positive gap flagged in review: the rule only checked for an NSG on the VMSS network interface configuration itself, missing the case where the NSG is attached at the subnet level instead. A VMSS whose NIC has no NSG but deploys into a subnet that does have one was being incorrectly flagged. Now resolves each network interface configuration's subnet (via the existing get_virtual_networks() collector, no new collector needed) and treats either a NIC-level or subnet-level NSG as compliant, matching how AZ-NET-010 already reads subnet.network_security_group. Added SOC2 to the rule's own FRAMEWORKS dict (was previously only in soc2.json, inconsistent with how several other rules, e.g. AZ-KV-006, already include it directly). Addresses review feedback from TFT444 and m-khan-97 on OWASP#275. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com>
Vishnu2707
left a comment
There was a problem hiding this comment.
Necessary changes have been made, hence approving!
* feat: add VM Scale Set inventory collector and rule AZ-CMP-005 Adds AzureClient.get_virtual_machine_scale_sets() (list_all across the subscription, following the get_virtual_machines() pattern) and its MockAzureClient test double. Ships AZ-CMP-005 as the first rule to use it: flags VMSS network interface configurations that provision a public IP with no NSG attached, the VMSS-template equivalent of AZ-CMP-001's per-VM NIC check. Detection reads the network interface configuration template directly (network_interface_configurations[].ip_configurations[] / .network_security_group) rather than resolving separate NIC resources, since a VMSS profile embeds these settings inline. Includes a remediation playbook (az vmss update --set on the network profile, with an explicit warning about the required instance upgrade), collector and rule tests, and compliance framework mappings. CIS is mapped to N/A-CMP-005 following the repository's established convention for the same real control (7.1, owned by AZ-CMP-001) applied to a second resource type, since the one-CIS-ID-per-rule convention doesn't allow reusing 7.1 directly. Closes #271 Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com> * fix: renumber AZ-CMP-005 to AZ-CMP-006, fix subnet-NSG false positive AZ-CMP-005 collided with #273 (Trusted Launch check), opened a day before this PR and already claiming that rule ID. Renumbered the rule file, playbook, tests, and all four compliance framework entries to AZ-CMP-006. Also fixes a false-positive gap flagged in review: the rule only checked for an NSG on the VMSS network interface configuration itself, missing the case where the NSG is attached at the subnet level instead. A VMSS whose NIC has no NSG but deploys into a subnet that does have one was being incorrectly flagged. Now resolves each network interface configuration's subnet (via the existing get_virtual_networks() collector, no new collector needed) and treats either a NIC-level or subnet-level NSG as compliant, matching how AZ-NET-010 already reads subnet.network_security_group. Added SOC2 to the rule's own FRAMEWORKS dict (was previously only in soc2.json, inconsistent with how several other rules, e.g. AZ-KV-006, already include it directly). Addresses review feedback from TFT444 and m-khan-97 on #275. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com> * fix: distinguish unresolved subnet from confirmed no-NSG in AZ-CMP-006 subnet_nsgs.get(subnet_id, False) treated an unresolved subnet reference (VNet collection failure, missing permissions, or a subnet ID this scan never saw) identically to a resolved subnet confirmed to have no NSG, reintroducing the false-positive the previous commit was meant to fix. Now distinguishes three states per ip_configuration: resolved+has NSG (compliant), resolved+no NSG (confirmed non-compliant, HIGH), and unresolved (indeterminate, LOW), mirroring the confirmed/indeterminate pattern already established in az_cmp_002.py. Also normalizes subnet IDs to lowercase before comparison, since Azure resource IDs are case-insensitive and the two APIs involved (VMSS network profile vs. VNet subnets) aren't guaranteed to return matching casing. Adds regression tests for an unresolved subnet (VNet collection returns empty) and for a differently-cased subnet ID match. Addresses review feedback from ritiksah141 on #275. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com> * fix: resolve _subnet_id name collision from rebasing onto AZ-CMP-007 AZ-CMP-007 (merged into dev while this branch was in review) and this branch's subnet-NSG fix for AZ-CMP-006 each defined their own _subnet_id() helper with different signatures. Landing them in the same file after the rebase left two same-named functions, and the second definition silently shadowed the first, breaking the AZ-CMP-007 subnet-exposure test. Renamed this branch's helper to _vnet_subnet_id() to remove the collision. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com> * fix: three real gaps in AZ-CMP-006 flagged by review - Only ip_configurations that actually carry a public IP are checked against subnet NSG state. A non-primary ip_config with no public IP is not internet-reachable, so its subnet must not be able to force a finding on an otherwise-compliant net_config. - Removed the break after the first non-compliant net_config. A VMSS with several exposed configs now gets one finding per config instead of silently hiding every attack surface after the first. - Indeterminate findings now carry vnets_collected in metadata, so a persistent zero across many findings is visible as a VNet-collection problem instead of reading as an ordinary per-subnet indeterminate result. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com> * fix: require all public ip_configs protected, not any, in AZ-CMP-006 A net_config can carry several public ip_configs on different subnets. 'any(status is True for status in subnet_statuses)' treated the whole net_config as compliant if any one of them was subnet -protected, even when another public ip_config on the same net_config was still exposed on an unprotected subnet. Compliance now requires every public ip_config to be protected. A confirmed-unprotected ip_config makes the net_config non-compliant regardless of whether another ip_config on it is merely unresolved, matching the existing confirmed-beats-indeterminate severity model. Added a regression test: two public ip_configs on different subnets, one protected and one not, expecting one confirmed HIGH finding. Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com> --------- Signed-off-by: Dipesh Ray <dipesh.ray.g@gmail.com>
What
Implements Rule 1 of #269 —
AZ-CMP-005, flagging Generation 2 VMs that don't have Trusted Launch (Secure Boot and vTPM) fully enabled. (Per the issue's Coordination note, the Managed Identity rule was descoped, so this PR is the sole remaining scope.)Follows the
scanner/rules/one-file-per-rule template, with the matching remediation playbook and compliance-framework mappings.Detection logic
security_type == "TrustedLaunch"and bothuefi_settings.secure_boot_enabledandv_tpm_enabledareTrue.list_all()representation doesn't carry its Hyper-V generation, so the rule resolves the OS disk'shyper_v_generation(viaazure_client.get_disk(), the same patternAZ-CMP-002uses) to confirm a VM is Gen2 before flagging. A readable Gen1 disk reportsV1and is skipped — no false finding against hardware that can't run Trusted Launch.security_type == "TrustedLaunch"but Secure Boot / vTPM off is Gen2-only by definition, so it's flagged without needing a disk lookup.AZ-CMP-002). Trade-off: a Gen2 VM whose disk generation can't be read is a possible false negative; I erred toward zero Gen1 false positives, but happy to switch to surfacing it as a distinct low-severity/unknown result if you'd prefer theAZ-CMP-002treatment.Compliance mappings (all four framework JSONs)
PR.DS-6A.12.5.1CC6.8N/A-CMP-005AZ-KV-001) rather than a fabricated control id — glad to renumber if there's a preferred mapping.Tests
Six unit tests added to
tests/test_rules_compute.py(mock-based, no network):V2) without a security profile → one MEDIUM findingTrustedLaunchdeclared but vTPM off → flagged without a disk lookupV1) → NOT_APPLICABLE, no findingHonest testing note
I don't have an Azure subscription, so I could not run the issue's "tested against a real Azure free-trial subscription" step — the live checkboxes below are left unchecked for a maintainer/reviewer to confirm. The rule is fully covered by the unit tests above (compliant / non-compliant / Gen1 / unknown / confidential) and the fixtures mirror the real SDK attribute shapes (
SecurityProfile.security_type,UefiSettings.secure_boot_enabled/v_tpm_enabled,Disk.hyper_v_generation).Test plan (from the issue)
fix_az_cmp_005.sh)Closes #269