diff --git a/internal/builder/vm/lcow/sandbox_options.go b/internal/builder/vm/lcow/sandbox_options.go index b6cc22f94b..b7d939c826 100644 --- a/internal/builder/vm/lcow/sandbox_options.go +++ b/internal/builder/vm/lcow/sandbox_options.go @@ -22,6 +22,11 @@ type SandboxOptions struct { // FullyPhysicallyBacked indicates all memory allocations are backed by physical memory. FullyPhysicallyBacked bool + // NoSecurityHardware indicates that SNP hardware is not available. When true, + // the security policy is still plumbed to the GCS but the HCS document uses the + // standard (non-SNP) format. + NoSecurityHardware bool + // ConfidentialConfig carries confidential computing fields that are not // part of the HCS document but are needed for confidential VM setup. ConfidentialConfig *ConfidentialConfig diff --git a/internal/builder/vm/lcow/specs.go b/internal/builder/vm/lcow/specs.go index 10e6b9363d..e607a4990c 100644 --- a/internal/builder/vm/lcow/specs.go +++ b/internal/builder/vm/lcow/specs.go @@ -65,6 +65,11 @@ func BuildSandboxConfig( return nil, nil, fmt.Errorf("failed to parse sandbox options: %w", err) } + // isConfidentialSNP is true when we have a security policy AND real SNP hardware. + // This gates SNP-specific HCS document construction (schema V25, confidential boot, etc.). + // When no-security-hardware is set, we still plumb the policy but use the standard HCS doc. + isConfidentialSNP := sandboxOptions.ConfidentialConfig != nil && !sandboxOptions.NoSecurityHardware + // ================== Parse Topology (CPU, Memory, NUMA) options ================= // =============================================================================== @@ -89,7 +94,7 @@ func BuildSandboxConfig( // Parse NUMA settings only for non-confidential VMs. var numa *hcsschema.Numa var numaProcessors *hcsschema.NumaProcessors - if sandboxOptions.ConfidentialConfig == nil { + if !isConfidentialSNP { numa, numaProcessors, err = parseNUMAOptions( ctx, spec.Annotations, @@ -121,11 +126,11 @@ func BuildSandboxConfig( // ================== Parse Boot options ========================================= // =============================================================================== - // For confidential VMs, we don't use the standard boot options - the UEFI secure boot + // For SNP confidential VMs, we don't use the standard boot options - the UEFI secure boot // settings will be set by parseConfidentialOptions. bootOptions := &hcsschema.Chipset{} var rootFsFullPath string - if sandboxOptions.ConfidentialConfig == nil { + if !isConfidentialSNP { bootOptions, rootFsFullPath, err = parseBootOptions(ctx, opts, spec.Annotations) if err != nil { return nil, nil, fmt.Errorf("failed to parse boot options: %w", err) @@ -141,9 +146,9 @@ func BuildSandboxConfig( spec.Annotations, spec.Devices, rootFsFullPath, - numa != nil && numaProcessors != nil, // isNumaEnabled - sandboxOptions.FullyPhysicallyBacked, // isFullyPhysicallyBacked - sandboxOptions.ConfidentialConfig != nil, // isConfidential + numa != nil && numaProcessors != nil, // isNumaEnabled + sandboxOptions.FullyPhysicallyBacked, // isFullyPhysicallyBacked + isConfidentialSNP, // isConfidential ) if err != nil { return nil, nil, fmt.Errorf("failed to parse device options: %w", err) @@ -156,7 +161,7 @@ func BuildSandboxConfig( hvSocketConfig, comPorts, err := setAdditionalOptions( ctx, spec.Annotations, - sandboxOptions.ConfidentialConfig != nil, // isConfidential + isConfidentialSNP, // isConfidential ) if err != nil { return nil, nil, fmt.Errorf("failed to parse additional parameters: %w", err) @@ -169,7 +174,7 @@ func BuildSandboxConfig( var securitySettings *hcsschema.SecuritySettings var guestState *hcsschema.GuestState var filesToCleanOnError []string - if sandboxOptions.ConfidentialConfig != nil { + if isConfidentialSNP { bootOptions, securitySettings, guestState, @@ -204,9 +209,9 @@ func BuildSandboxConfig( // =============================================================================== // Build the kernel command line after all options are parsed. - // For confidential VMs (SNP mode), kernel args are embedded in VMGS file, so skip this. + // For SNP confidential VMs, kernel args are embedded in VMGS file, so skip this. var kernelArgs string - if sandboxOptions.ConfidentialConfig == nil { + if !isConfidentialSNP { kernelArgs, err = buildKernelArgs( ctx, opts, @@ -238,7 +243,7 @@ func BuildSandboxConfig( // Use Schema V21 for non-confidential cases. // Use Schema V25 for confidential cases. schema := schemaversion.SchemaV21() - if sandboxOptions.ConfidentialConfig != nil { + if isConfidentialSNP { schema = schemaversion.SchemaV25() } @@ -332,12 +337,13 @@ func parseSandboxOptions(ctx context.Context, platform string, annotations map[s // Determine if this is a confidential VM early, as it affects boot options parsing securityPolicy := oci.ParseAnnotationsString(annotations, shimannotations.LCOWSecurityPolicy, "") noSecurityHardware := oci.ParseAnnotationsBool(ctx, annotations, shimannotations.NoSecurityHardware, false) - if securityPolicy != "" && !noSecurityHardware { + if len(securityPolicy) > 0 { sandboxOptions.ConfidentialConfig = &ConfidentialConfig{ SecurityPolicy: securityPolicy, - SecurityPolicyEnforcer: oci.ParseAnnotationsString(annotations, shimannotations.LCOWSecurityPolicyEnforcer, ""), + SecurityPolicyEnforcer: oci.ParseAnnotationsString(annotations, shimannotations.LCOWSecurityPolicyEnforcer, "rego"), UvmReferenceInfoFile: oci.ParseAnnotationsString(annotations, shimannotations.LCOWReferenceInfoFile, vmutils.DefaultUVMReferenceInfoFile), } + sandboxOptions.NoSecurityHardware = noSecurityHardware log.G(ctx).WithFields(logrus.Fields{ "securityPolicy": securityPolicy, diff --git a/internal/builder/vm/lcow/specs_test.go b/internal/builder/vm/lcow/specs_test.go index e02b7b48ed..7747ee242d 100644 --- a/internal/builder/vm/lcow/specs_test.go +++ b/internal/builder/vm/lcow/specs_test.go @@ -1111,7 +1111,7 @@ func TestBuildSandboxConfig_SecurityPolicyInteractions(t *testing.T) { errContains: "v2 shims do not support vPMem devices", }, { - name: "scratch encryption defaults to false when security hardware is bypassed", + name: "scratch encryption defaults to true when security hardware is bypassed", spec: &vm.Spec{ Annotations: map[string]string{ shimannotations.LCOWSecurityPolicy: "eyJ0ZXN0IjoidGVzdCJ9", @@ -1120,10 +1120,10 @@ func TestBuildSandboxConfig_SecurityPolicyInteractions(t *testing.T) { }, validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) { t.Helper() - // When NoSecurityHardware is true, isConfidential is false, - // so EnableScratchEncryption defaults to false - if sandboxOpts.EnableScratchEncryption != false { - t.Error("expected scratch encryption disabled by default when security hardware is bypassed") + // When NoSecurityHardware is true but a security policy is present, + // ConfidentialConfig is still set, so EnableScratchEncryption defaults to true + if sandboxOpts.EnableScratchEncryption != true { + t.Error("expected scratch encryption enabled by default when security policy is present") } }, }, @@ -1152,6 +1152,45 @@ func TestBuildSandboxConfig_SecurityPolicyInteractions(t *testing.T) { } }, }, + { + name: "no-security-hardware sets ConfidentialConfig but uses standard HCS doc", + spec: &vm.Spec{ + Annotations: map[string]string{ + shimannotations.LCOWSecurityPolicy: "eyJ0ZXN0IjoidGVzdCJ9", + shimannotations.LCOWSecurityPolicyEnforcer: "rego", + shimannotations.NoSecurityHardware: "true", + }, + }, + validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) { + t.Helper() + // ConfidentialConfig should be set for policy plumbing + if sandboxOpts.ConfidentialConfig == nil { + t.Fatal("expected ConfidentialConfig to be set even with no-security-hardware") + } + if sandboxOpts.ConfidentialConfig.SecurityPolicy != "eyJ0ZXN0IjoidGVzdCJ9" { + t.Errorf("expected security policy to be set, got %q", sandboxOpts.ConfidentialConfig.SecurityPolicy) + } + if sandboxOpts.ConfidentialConfig.SecurityPolicyEnforcer != "rego" { + t.Errorf("expected security policy enforcer 'rego', got %q", sandboxOpts.ConfidentialConfig.SecurityPolicyEnforcer) + } + if !sandboxOpts.NoSecurityHardware { + t.Error("expected NoSecurityHardware to be true") + } + // HCS doc should use standard schema (V21), not SNP schema (V25) + if doc.SchemaVersion.Major != 2 || doc.SchemaVersion.Minor != 1 { + t.Errorf("expected schema V2.1 for no-security-hardware, got V%d.%d", doc.SchemaVersion.Major, doc.SchemaVersion.Minor) + } + // GuestState should NOT be set (no SNP boot) + if doc.VirtualMachine.GuestState != nil { + t.Error("expected no GuestState in no-security-hardware mode") + } + // Standard boot options should be used (kernel direct or UEFI) + chipset := doc.VirtualMachine.Chipset + if chipset.LinuxKernelDirect == nil && chipset.Uefi == nil { + t.Error("expected standard boot options in no-security-hardware mode") + } + }, + }, } runTestCases(t, ctx, defaultOpts, tests) diff --git a/internal/oci/uvm.go b/internal/oci/uvm.go index 5bfa68f7f0..0273c1fa54 100644 --- a/internal/oci/uvm.go +++ b/internal/oci/uvm.go @@ -203,14 +203,17 @@ func handleLCOWSecurityPolicy(ctx context.Context, a map[string]string, lopts *u // this might change if the building of the vmgs file were to be done on demand but that is likely // much slower and noy very useful. We do respect the filename of the vmgs file so if it is necessary to // have different options then multiple files could be used. - if len(lopts.SecurityPolicy) > 0 && !noSecurityHardware { + if len(lopts.SecurityPolicy) > 0 { + lopts.SecurityPolicyEnabled = true + } + + if lopts.SecurityPolicyEnabled && !noSecurityHardware { // VPMem not supported by the enlightened kernel for SNP so set count to zero. lopts.VPMemDeviceCount = 0 // set the default GuestState filename. lopts.GuestStateFilePath = vmutils.DefaultGuestStateFile lopts.KernelBootOptions = "" lopts.AllowOvercommit = false - lopts.SecurityPolicyEnabled = true // There are two possible ways to boot SNP mode. Either kernelinitrd.vmgs which consists of kernel plus initrd.cpio.gz // Or a kernel.vmgs file (without an initrd) plus a separate vhd file which is dmverity protected via a hash tree @@ -224,7 +227,7 @@ func handleLCOWSecurityPolicy(ctx context.Context, a map[string]string, lopts *u lopts.DmVerityMode = true } - if len(lopts.SecurityPolicy) > 0 { + if lopts.SecurityPolicyEnabled { // will only be false if explicitly set false by the annotation. We will otherwise default to true when there is a security policy lopts.EnableScratchEncryption = ParseAnnotationsBool(ctx, a, annotations.LCOWEncryptedScratchDisk, true) } diff --git a/internal/oci/uvm_test.go b/internal/oci/uvm_test.go index 609cab5225..9d13271568 100644 --- a/internal/oci/uvm_test.go +++ b/internal/oci/uvm_test.go @@ -9,6 +9,7 @@ import ( runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "github.com/Microsoft/hcsshim/pkg/annotations" "github.com/google/go-cmp/cmp" "github.com/opencontainers/runtime-spec/specs-go" @@ -327,3 +328,97 @@ func Test_SpecToUVMCreateOptions_Common(t *testing.T) { }) } } + +func Test_SpecToUVMCreateOptions_LCOW_SecurityPolicy_SNP(t *testing.T) { + s := &specs.Spec{ + Linux: &specs.Linux{}, + Annotations: map[string]string{ + annotations.LCOWSecurityPolicy: "test-policy", + }, + } + + opts, err := SpecToUVMCreateOpts(context.Background(), s, t.Name(), "") + if err != nil { + t.Fatalf("unexpected error generating UVM opts: %v", err) + } + + lopts := (opts).(*uvm.OptionsLCOW) + if !lopts.SecurityPolicyEnabled { + t.Fatal("SecurityPolicyEnabled should be true when SecurityPolicy is set") + } + if lopts.SecurityPolicy != "test-policy" { + t.Fatalf("expected SecurityPolicy=%q, got %q", "test-policy", lopts.SecurityPolicy) + } + if lopts.VPMemDeviceCount != 0 { + t.Fatalf("expected VPMemDeviceCount=0 for SNP, got %d", lopts.VPMemDeviceCount) + } + if lopts.GuestStateFilePath != vmutils.DefaultGuestStateFile { + t.Fatalf("expected GuestStateFilePath=%q, got %q", vmutils.DefaultGuestStateFile, lopts.GuestStateFilePath) + } + if !lopts.DmVerityMode { + t.Fatal("DmVerityMode should be true for SNP") + } + if lopts.AllowOvercommit { + t.Fatal("AllowOvercommit should be false for SNP") + } + if !lopts.EnableScratchEncryption { + t.Fatal("EnableScratchEncryption should be true when SecurityPolicy is set") + } +} + +func Test_SpecToUVMCreateOptions_LCOW_SecurityPolicy_NoSecurityHardware(t *testing.T) { + s := &specs.Spec{ + Linux: &specs.Linux{}, + Annotations: map[string]string{ + annotations.LCOWSecurityPolicy: "test-policy", + annotations.NoSecurityHardware: "true", + }, + } + + opts, err := SpecToUVMCreateOpts(context.Background(), s, t.Name(), "") + if err != nil { + t.Fatalf("unexpected error generating UVM opts: %v", err) + } + + lopts := (opts).(*uvm.OptionsLCOW) + if !lopts.SecurityPolicyEnabled { + t.Fatal("SecurityPolicyEnabled should be true even with no-security-hardware") + } + if lopts.SecurityPolicy != "test-policy" { + t.Fatalf("expected SecurityPolicy=%q, got %q", "test-policy", lopts.SecurityPolicy) + } + // SNP-specific settings should NOT be applied + if lopts.VPMemDeviceCount == 0 { + t.Fatal("VPMemDeviceCount should not be zeroed in no-security-hardware mode") + } + if lopts.GuestStateFilePath != "" { + t.Fatalf("GuestStateFilePath should be empty in no-security-hardware mode, got %q", lopts.GuestStateFilePath) + } + if lopts.DmVerityMode { + t.Fatal("DmVerityMode should be false in no-security-hardware mode") + } + // Policy-dependent settings should still be applied + if !lopts.EnableScratchEncryption { + t.Fatal("EnableScratchEncryption should be true when SecurityPolicy is set, even with no-security-hardware") + } +} + +func Test_SpecToUVMCreateOptions_LCOW_NoSecurityPolicy(t *testing.T) { + s := &specs.Spec{ + Linux: &specs.Linux{}, + Annotations: make(map[string]string), + } + + opts, err := SpecToUVMCreateOpts(context.Background(), s, t.Name(), "") + if err != nil { + t.Fatalf("unexpected error generating UVM opts: %v", err) + } + + lopts := (opts).(*uvm.OptionsLCOW) + if lopts.SecurityPolicyEnabled { + t.Fatal("SecurityPolicyEnabled should be false when no SecurityPolicy is set") + } + if lopts.SecurityPolicy != "" { + t.Fatalf("expected empty SecurityPolicy, got %q", lopts.SecurityPolicy) + } +} diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index c6e9a52ffc..61ff4f98ae 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -954,7 +954,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error // HCS config for SNP isolated vm is quite different to the usual case var doc *hcsschema.ComputeSystem - if opts.SecurityPolicyEnabled { + if opts.GuestStateFilePath != "" { doc, err = makeLCOWSecurityDoc(ctx, opts, uvm) if logrus.IsLevelEnabled(logrus.TraceLevel) { log.G(ctx).WithFields(logrus.Fields{