-
Notifications
You must be signed in to change notification settings - Fork 352
feat(query): 6 new Beta queries and fixes for "Azure Instance Using Basic Authentication" - terraform/azure #7868
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
Open
cx-andre-pereira
wants to merge
15
commits into
master
Choose a base branch
from
AST-120855_Fixes_for_azure_instance_using_basic_authentication-terraform/azure
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
…yption On Managed Disk Disabled
…y azurerm_virtual_machine_scale_set for azure_instance_using_basic_authentication
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.








Reason for Proposed Changes
The current implementation of the "Azure Instance Using Basic Authentication" has a few issues:
Four new queries were asked to be implemented, all related to VM resources. The list of target resources is as follows:
The legacy "azurerm_virtual_machine" resource did not, from my research, apply to any of the queries. My findings will be documented below on a per query basis.
Finally 2 other queries targeting the "azurerm_managed_disk" disk encryption fields and the "azurerm_key_vault_key" "key_type" field, respectively, were implemented.
Proposed Changes
New queries :
1 Beta - VM Without Admin SSH Public Key Set
This query targets the "admin_ssh_key.public_key" field of the "
azurerm_linux_virtual_machine" and "azurerm_linux_virtual_machine_scale_set" resources. It will flag if there isn't at least onepublic_keyset inside aadmin_ssh_keyblock. The query takes into account the scenario of theadmin_ssh_keyblock(s) not declaring apublic_keyeven though the field is technically required.The legacy "
azurerm_virtual_machine" has the "os_profile_linux_config.ssh_keys" field which seems analog to this but, since there is already a query to enforce "os_profile_linux_config.disable_password_authentication" to be set to true, consequently making the "ssh_keys" field required, i did not find this to be worth including in the query. (said query is the one fixed by this PR - "Azure Instance Using Basic Authentication")2 Beta - VM With Extension Operations Enabled
This query was found to be applicable to all 4 VM resources, but the target field has different names depending on the resource.
For "
azurerm_windows_virtual_machine" and "azurerm_linux_virtual_machine" the target field is named "allow_extension_operations". For the other 2 "scale_set" resources the target field is "extension_operations_enabled".We must ensure the field is explicitly set to
falsesince enabling extension operations is a security liability; it can lead to processes executing with unnecessary administrative privileges on the vm.Note that this query will likely become outdated relatively soon if a single naming convention is adopted for the 4 resources rather than keeping the 2 distinct field names.
3 Beta - VM With Automatic Updates Disabled
This query is applicable only to windows based vm resources. It targets the "enable_automatic_updates" field and the "automatic_updates_enabled" field; the latter is the new name that replaces "enable_automatic_updates" for the "
azurerm_windows_virtual_machine". This change was extremely recent as of writing (less than 2 months ago - last version to support old field was 4.48), the "azurerm_windows_virtual_machine_scale_set" still uses the "enable_automatic_updates" field but it wouldn't be surprising if it changed to the new naming scheme soon.Both fields are supported by the implementation. Additionally if the scale-set resource goes through the change the query will still work regardless ensuring future proofing for the query.
NOTE - the query targets automatic "updates", not to be confused with the automatic "upgrade" which exists in all resources including legacy ones.
4 Beta - Disk Encryption On Managed Disk Disabled
azurerm_managed_disk" resources. This is done to ensure improved overall security for managed disks's data through extra encryption.5 Beta - Key Vault Without HSM Protection
azurerm_key_vault_key" resources to ensure it is set to a value that sets up encryption dependent on a HSM for overall stronger cryptography.6 Beta - VM Without Encryption At Host
The sixth and final query applies to all 4 VM resources once again. For this one all resources set the exact same field: "encryption_at_host_enabled".
Although not explicitly stated it seems the default value for the field is false; with this the query ensures that the
encryption_at_host_enabledfield is set to true. Like other queries in this PR this is meant to improve encryption standards by ensuring all files are encrypted before they leave the physical host machine.I submit this contribution under the Apache-2.0 license.