Skip to content

Commit 724721e

Browse files
committed
fix: avoid sending empty ShieldedInstanceInitialState on image create
CreateShieldedVMStateConfig has been returning a non-nil pointer to a zero-value compute.InitialStateConfig{} whenever no signature inputs (image_platform_key, image_key_exchange_key, image_signatures_db, image_forbidden_signatures_db) were configured. Because Go's omitempty tag only drops nil pointers (not pointers to empty structs), the image insert request body included: "shieldedInstanceInitialState": {} GCP treats an explicit (even empty) shieldedInstanceInitialState as *the* initial state for the new image and therefore replaces the PK / KEKs / db / dbx that would otherwise be inherited from the source disk. With no signature databases, UEFI Secure Boot has nothing to validate the bootloader against, and any VM launched from such an image with shielded-vm secure boot enabled fails to boot: BdsDxe: failed to load Boot0001 "UEFI Google PersistentDisk" ... Status: Security Violation. This regression was introduced in #318 (released in v1.2.5). Subsequent work in #333 removed the UEFI_COMPATIBLE gate but kept the empty-struct return path, so v1.2.6 is still affected. Fix: return nil from CreateShieldedVMStateConfig when no signature inputs are configured, so the caller leaves compute.Image.ShieldedInstanceInitialState unset and GCP inherits the initial state from the source disk. Adds: - Unit tests for CreateShieldedVMStateConfig covering both the no-inputs (nil return) and inputs-provided (non-nil return) paths. - Step-level regression test asserting that the image insert spec has no ShieldedInstanceInitialState when no signature inputs are configured.
1 parent 6254a5d commit 724721e

3 files changed

Lines changed: 96 additions & 0 deletions

File tree

builder/googlecompute/step_create_image_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,36 @@ func TestStepCreateImage_setsDeprecationFields(t *testing.T) {
9494
assert.Contains(t, []string{"DEPRECATED", "ACTIVE"}, d.DeprecatedImageStatus.State, "State should be DEPRECATED or ACTIVE")
9595
}
9696

97+
// Regression test: when no Secure Boot signature inputs are configured, the
98+
// image insert request must not include a shieldedInstanceInitialState field.
99+
// Sending an explicit (even empty) InitialStateConfig replaces the
100+
// PK/KEKs/db/dbx that would otherwise be inherited from the source disk and
101+
// causes Secure Boot to fail on VMs launched from the resulting image
102+
// (UEFI: "Status: Security Violation").
103+
func TestStepCreateImage_noSignatureInputsLeavesInitialStateNil(t *testing.T) {
104+
state := testState(t)
105+
step := new(StepCreateImage)
106+
defer step.Cleanup(state)
107+
108+
c := state.Get("config").(*Config)
109+
d := state.Get("driver").(*common.DriverMock)
110+
111+
// Clear all signature inputs supplied by the default testConfig.
112+
c.ImagePlatformKey = ""
113+
c.ImageKeyExchangeKey = nil
114+
c.ImageSignaturesDB = nil
115+
c.ImageForbiddenSignaturesDB = nil
116+
117+
action := step.Run(context.Background(), state)
118+
assert.Equal(t, multistep.ActionContinue, action, "Step did not pass.")
119+
120+
assert.Nil(
121+
t,
122+
d.CreateImageSpec.ShieldedInstanceInitialState,
123+
"shieldedInstanceInitialState must be nil so the source disk's initial state is inherited",
124+
)
125+
}
126+
97127
func TestStepCreateImageNonUEFI_image(t *testing.T) {
98128
state := testState(t)
99129
step := new(StepCreateImage)

lib/common/shielded_vms.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ func FillFileContentBuffer(certOrKeyFile string) (*compute.FileContentBuffer, er
3434
}
3535

3636
func CreateShieldedVMStateConfig(imagePlatformKey string, imageKeyExchangeKey []string, imageSignaturesDB []string, imageForbiddenSignaturesDB []string) (*compute.InitialStateConfig, error) {
37+
// When no Secure Boot signature inputs are configured, return nil so the
38+
// caller leaves ShieldedInstanceInitialState unset on the image payload.
39+
// Sending an explicit (even empty) InitialStateConfig replaces the
40+
// PK/KEKs/db/dbx that would otherwise be inherited from the source disk,
41+
// which causes Secure Boot to fail on VMs launched from the resulting
42+
// image (UEFI: "Status: Security Violation").
43+
if imagePlatformKey == "" && len(imageKeyExchangeKey) == 0 && len(imageSignaturesDB) == 0 && len(imageForbiddenSignaturesDB) == 0 {
44+
return nil, nil
45+
}
46+
3747
shieldedVMStateConfig := &compute.InitialStateConfig{}
3848
if imagePlatformKey != "" {
3949
shieldedData, err := FillFileContentBuffer(imagePlatformKey)

lib/common/shielded_vms_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright IBM Corp. 2013, 2025
2+
// SPDX-License-Identifier: MPL-2.0
3+
4+
package common
5+
6+
import (
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
// When no Secure Boot signature inputs are configured, the helper must return
15+
// a nil *InitialStateConfig so the caller does not attach an empty
16+
// shieldedInstanceInitialState to the image insert request. Attaching an
17+
// empty value overrides the PK/KEKs/db/dbx that would otherwise be inherited
18+
// from the source disk and breaks Secure Boot on the resulting image.
19+
func TestCreateShieldedVMStateConfig_NoInputsReturnsNil(t *testing.T) {
20+
cfg, err := CreateShieldedVMStateConfig("", nil, nil, nil)
21+
assert.NoError(t, err)
22+
assert.Nil(t, cfg, "expected nil config when no signature inputs are configured")
23+
24+
cfg, err = CreateShieldedVMStateConfig("", []string{}, []string{}, []string{})
25+
assert.NoError(t, err)
26+
assert.Nil(t, cfg, "expected nil config when signature inputs are empty slices")
27+
}
28+
29+
func TestCreateShieldedVMStateConfig_PopulatesFieldsWhenInputsProvided(t *testing.T) {
30+
dir := t.TempDir()
31+
keyPath := filepath.Join(dir, "fake-key")
32+
if err := os.WriteFile(keyPath, []byte("fake key data"), 0600); err != nil {
33+
t.Fatalf("failed to write fake key: %v", err)
34+
}
35+
36+
tests := []struct {
37+
name string
38+
pk string
39+
keks []string
40+
dbs []string
41+
dbxs []string
42+
}{
43+
{name: "platform key only", pk: keyPath},
44+
{name: "kek only", keks: []string{keyPath}},
45+
{name: "db only", dbs: []string{keyPath}},
46+
{name: "dbx only", dbxs: []string{keyPath}},
47+
}
48+
49+
for _, tc := range tests {
50+
t.Run(tc.name, func(t *testing.T) {
51+
cfg, err := CreateShieldedVMStateConfig(tc.pk, tc.keks, tc.dbs, tc.dbxs)
52+
assert.NoError(t, err)
53+
assert.NotNil(t, cfg, "expected non-nil config when at least one signature input is provided")
54+
})
55+
}
56+
}

0 commit comments

Comments
 (0)