-
Notifications
You must be signed in to change notification settings - Fork 244
feat: add Ubuntu 22.04 FIPS VHDs #7721
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
base: main
Are you sure you want to change the base?
Conversation
Update vhd-scanning.sh Update vhd-scanning.sh Revert "try another sku with lower core counts" This reverts commit a5094f0.
|
|
||
| if [ "$FIPS_FEATURE_STATE" != "Registered" ]; then | ||
| echo "Registering FIPS 140-3 compliance feature..." | ||
| az feature register --namespace Microsoft.Compute --name OptInToFips1403Compliance |
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.
what is we get that the feature is not available ? we will poll and retry ?
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.
Currently it will try for 5 min, time out, and echo a warning before continuing.
This means it will attempt to create the vm without the feature, which will fail.
|
|
||
| # Wait for VM to be ready | ||
| echo "Waiting for VM to be ready..." | ||
| az vm wait --created --name $SCAN_VM_NAME --resource-group $RESOURCE_GROUP_NAME |
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.
is there a timeout for this wait command ? could we make is explicit on the CLI so we know what it is form code inspection ?
Also, should we catch timeout cases, and return an error code and error message that that VM was never created ?
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.
- default timeout is 1h and can be explicitly set. Would a shorter time ~30min be better?
- the existing vm creation pattern in vhd-scanning.sh only attempts to create the vm and does not have any error handling. I can look into catching the timeout case for the fips vm though.
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.
Pull request overview
This PR enables the creation of Ubuntu 22.04 FIPS VHDs by implementing support for FIPS 140-3 encryption through Azure REST API calls. Previously blocked due to the need for FIPS 140-3 encryption (which requires subscription feature registration), this implementation follows the Microsoft documentation for Azure VM extensions with FIPS support.
Changes:
- Added new
fips-helper.shscript with functions to register the FIPS 140-3 compliance feature and create VMs via REST API with FIPS encryption enabled - Modified
vhd-scanning.shto detect Ubuntu 22.04 FIPS scenarios and route VM creation through the new REST API approach instead of standardaz vm create - Updated Go datamodel to use the dynamic
LinuxSIGImageVersionfor Ubuntu 22.04 FIPS images instead of hardcoded placeholder versions - Enabled Ubuntu 22.04 FIPS builds in the release pipeline by setting default values to
true
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vhdbuilder/packer/fips-helper.sh | New helper script containing functions for FIPS 140-3 feature registration and REST API-based VM creation |
| vhdbuilder/packer/vhd-scanning.sh | Modified to detect Ubuntu 22.04 FIPS scenario and invoke the new FIPS helper functions; refactored VM_SIZE into a variable for reusability |
| pkg/agent/datamodel/sig_config.go | Updated version field from hardcoded placeholder to dynamic LinuxSIGImageVersion for both 2204 FIPS image templates |
| pkg/agent/datamodel/sig_config_test.go | Added test expectations for the two new Ubuntu 22.04 FIPS distros |
| .pipelines/.vsts-vhd-builder-release.yaml | Enabled Ubuntu 22.04 FIPS builds by changing defaults from false to true |
| --method put \ | ||
| --url "https://management.azure.com/subscriptions/${SUBSCRIPTION_ID}/resourceGroups/${RESOURCE_GROUP_NAME}/providers/Microsoft.Compute/virtualMachines/${SCAN_VM_NAME}?api-version=2024-11-01" \ | ||
| --body "$VM_BODY" | ||
|
|
Copilot
AI
Jan 23, 2026
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.
The az rest command at line 117-120 lacks error handling. If the REST API call fails, the script will continue to the az vm wait command which will likely fail or hang indefinitely. Consider adding error checking after the az rest call to verify the VM was created successfully, or add || exit 1 to ensure the script exits if the API call fails. This is especially important since this is using a REST API that might have different error behavior than the standard az vm create command.
| local az_rest_exit_code=$? | |
| if [ "$az_rest_exit_code" -ne 0 ]; then | |
| echo "Error: Failed to create VM with FIPS 140-3 encryption via REST API (exit code: $az_rest_exit_code)" >&2 | |
| return "$az_rest_exit_code" | |
| fi |
| az vm create --resource-group $RESOURCE_GROUP_NAME \ | ||
| --name $SCAN_VM_NAME \ | ||
| --image $VHD_IMAGE \ | ||
| --nics $SCANNING_NIC_ID \ | ||
| --admin-username $SCAN_VM_ADMIN_USERNAME \ | ||
| --admin-password $SCAN_VM_ADMIN_PASSWORD \ | ||
| --os-disk-size-gb 50 \ |
Copilot
AI
Jan 23, 2026
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.
The az vm create call passes SCAN_VM_ADMIN_PASSWORD directly via --admin-password while the script is running under set -x, so the full command line including the plaintext password will be emitted to build logs and any attached consoles. An attacker or insider with access to CI logs could recover this password and log into the scanning VM while it is active. Disable shell tracing around this command or use a mechanism that avoids putting the password on the command line so the credential never appears in logs.
| local VM_BODY=$(build_fips_vm_body \ | ||
| "$PACKER_BUILD_LOCATION" \ | ||
| "$SCAN_VM_NAME" \ | ||
| "$SCAN_VM_ADMIN_USERNAME" \ | ||
| "$SCAN_VM_ADMIN_PASSWORD" \ | ||
| "$VHD_IMAGE" \ | ||
| "$SCANNING_NIC_ID" \ | ||
| "$UMSI_RESOURCE_ID" \ | ||
| "$vm_size") | ||
|
|
||
| # Create the VM using REST API | ||
| az rest \ | ||
| --method put \ | ||
| --url "https://management.azure.com/subscriptions/${SUBSCRIPTION_ID}/resourceGroups/${RESOURCE_GROUP_NAME}/providers/Microsoft.Compute/virtualMachines/${SCAN_VM_NAME}?api-version=2024-11-01" \ | ||
| --body "$VM_BODY" |
Copilot
AI
Jan 23, 2026
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.
In the FIPS creation path, build_fips_vm_body embeds SCAN_VM_ADMIN_PASSWORD as adminPassword in the JSON body and create_fips_vm passes that JSON to az rest via --body while the caller script uses set -x, causing the full JSON (including the plaintext password) and REST invocation to be printed into logs. Anyone with access to these logs can recover the admin password for the FIPS-enabled scanning VM and potentially access it while it is running. Ensure the VM body construction and az rest call execute with shell tracing disabled or use a secret-safe mechanism so the password value is never written to stdout/stderr or persisted in logs.
What this PR does / why we need it:
Enable creation of Ubuntu 22.04 FIPS VHD. This was blocked by the need for FIPS 140-3 encryption, which is only supported through Azure REST API calls.
Implements instructions listed here https://learn.microsoft.com/en-us/azure/virtual-machines/extensions/agent-linux-fips
Which issue(s) this PR fixes:
Fixes #