Merge pull request #341 from andyroyle/fix/empty-shielded-initial-state-overrides-source

fix: avoid sending empty ShieldedInstanceInitialState on image create

Author: tanmay-hc

builder/googlecompute/step_create_image_test.go

@@ -94,6 +94,32 @@ func TestStepCreateImage_setsDeprecationFields(t *testing.T) {
 	assert.Contains(t, []string{"DEPRECATED", "ACTIVE"}, d.DeprecatedImageStatus.State, "State should be DEPRECATED or ACTIVE")
 }
 
+// Regression test: when no Secure Boot signature inputs are configured, the
+// image insert request must not include a shieldedInstanceInitialState field.
+// Sending an explicit (even empty) InitialStateConfig replaces the
+// PK/KEKs/db/dbx that would otherwise be inherited from the source disk and
+// causes Secure Boot to fail on VMs launched from the resulting image
+// (UEFI: "Status: Security Violation").
+func TestStepCreateImage_noSignatureInputsLeavesInitialStateNil(t *testing.T) {
+	state := testState(t)
+	step := new(StepCreateImage)
+	defer step.Cleanup(state)
+
+	c := state.Get("config").(*Config)
+	d := state.Get("driver").(*common.DriverMock)
+
+	// Clear all signature inputs supplied by the default testConfig.
+	c.ImagePlatformKey = ""
+	c.ImageKeyExchangeKey = nil
+	c.ImageSignaturesDB = nil
+	c.ImageForbiddenSignaturesDB = nil
+
+	action := step.Run(context.Background(), state)
+	assert.Equal(t, multistep.ActionContinue, action, "Step did not pass.")
+
+	assert.Nil(t, d.CreateImageSpec.ShieldedInstanceInitialState, "shieldedInstanceInitialState must be nil so the source disk's initial state is inherited")
+}
+
 func TestStepCreateImageNonUEFI_image(t *testing.T) {
 	state := testState(t)
 	step := new(StepCreateImage)

lib/common/shielded_vms.go

@@ -36,6 +36,16 @@ func FillFileContentBuffer(certOrKeyFile string) (*compute.FileContentBuffer, er
 }
 
 func CreateShieldedVMStateConfig(imagePlatformKey string, imageKeyExchangeKey []string, imageSignaturesDB []string, imageForbiddenSignaturesDB []string) (*compute.InitialStateConfig, error) {
+	// When no Secure Boot signature inputs are configured, return nil so the
+	// caller leaves ShieldedInstanceInitialState unset on the image payload.
+	// Sending an explicit (even empty) InitialStateConfig replaces the
+	// PK/KEKs/db/dbx that would otherwise be inherited from the source disk,
+	// which causes Secure Boot to fail on VMs launched from the resulting
+	// image (UEFI: "Status: Security Violation").
+	if imagePlatformKey == "" && len(imageKeyExchangeKey) == 0 && len(imageSignaturesDB) == 0 && len(imageForbiddenSignaturesDB) == 0 {
+		return nil, nil
+	}
+
 	shieldedVMStateConfig := &compute.InitialStateConfig{}
 	if imagePlatformKey != "" {
 		shieldedData, err := FillFileContentBuffer(imagePlatformKey)

lib/common/shielded_vms_test.go

@@ -0,0 +1,61 @@
+// Copyright IBM Corp. 2013, 2025
+// SPDX-License-Identifier: MPL-2.0
+
+package common
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// When no Secure Boot signature inputs are configured, the helper must return
+// a nil *InitialStateConfig so the caller does not attach an empty
+// shieldedInstanceInitialState to the image insert request. Attaching an
+// empty value overrides the PK/KEKs/db/dbx that would otherwise be inherited
+// from the source disk and breaks Secure Boot on the resulting image.
+func TestCreateShieldedVMStateConfig_NoInputsReturnsNil(t *testing.T) {
+	cfg, err := CreateShieldedVMStateConfig("", nil, nil, nil)
+	assert.NoError(t, err)
+	assert.Nil(t, cfg, "expected nil config when no signature inputs are configured")
+
+	cfg, err = CreateShieldedVMStateConfig("", []string{}, []string{}, []string{})
+	assert.NoError(t, err)
+	assert.Nil(t, cfg, "expected nil config when signature inputs are empty slices")
+}
+
+func TestCreateShieldedVMStateConfig_PopulatesFieldsWhenInputsProvided(t *testing.T) {
+	dir := t.TempDir()
+	keyPath := filepath.Join(dir, "fake-key")
+	if err := os.WriteFile(keyPath, []byte("fake key data"), 0600); err != nil {
+		t.Fatalf("failed to write fake key: %v", err)
+	}
+
+	testcases := []struct {
+		name string
+		pk   string
+		keks []string
+		dbs  []string
+		dbxs []string
+	}{
+		{name: "platform key only", pk: keyPath},
+		{name: "kek only", keks: []string{keyPath}},
+		{name: "db only", dbs: []string{keyPath}},
+		{name: "dbx only", dbxs: []string{keyPath}},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg, err := CreateShieldedVMStateConfig(tt.pk, tt.keks, tt.dbs, tt.dbxs)
+			assert.NoError(t, err)
+			assert.NotNil(t, cfg)
+
+			assert.Equal(t, tt.pk != "", cfg.Pk != nil, "Pk presence should match input")
+			assert.Len(t, cfg.Keks, len(tt.keks))
+			assert.Len(t, cfg.Dbs, len(tt.dbs))
+			assert.Len(t, cfg.Dbxs, len(tt.dbxs))
+		})
+	}
+}