From f6f3956952c482cfccbb2d06aead8088f06203ae Mon Sep 17 00:00:00 2001 From: Bryn Mailer Date: Mon, 15 Jun 2026 09:31:23 +1000 Subject: [PATCH] feat: enable viperblock at-rest encryption by default on fresh installs (#323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: enable viperblock at-rest encryption by default on fresh installs Fresh installs left nodes..viperblock.encryption_key_file empty, which viperblockd treats as legacy cleartext mode. The encryption machinery existed but the install path never generated a key or set the field. Provision a cluster-wide viperblock master key in all three fresh-install flows and render its path into spinifex.toml: - admin init (single node): generate the key locally. - admin init --multi-node (leader): generate once, save locally, and distribute via the formation server (mirrors IAM master key distribution). - admin join (joiner): receive the shared key from the leader rather than minting its own, so volumes sealed on any node open everywhere. Lenient on absence — an older leader returns no key, so the joiner warns and stays cleartext instead of failing the join. The key file is omitted from the template when unset, so upgrades with no existing key keep running cleartext. * fix: make viperblock encryption key group-readable for both readers The key is loaded by two services per node — viperblockd (spinifex-viperblock) and the awsgw EC2 handlers (spinifex-gw), both in the spinifex group — so it must be group-readable, not owner-only like the per-node predastore key. The previous commit generated the key but never registered its path with the deployment ownership machinery, leaving /etc/spinifex/viperblock as root:root 0700 (non-traversable) and the key as root:root 0600. The daemon failed at startup with "stat ... permission denied". Mirror master.key: setup.sh creates /etc/spinifex/viperblock (0750), and SetServiceOwnership chowns the dir to spinifex-viperblock and promotes the key to root:spinifex 0640. * fix: read viperblock config.json through StateBody for encrypted volumes The metadata layer parsed config.json with json.Unmarshal/NewDecoder directly, which fails on encrypted volumes/AMIs now that viperblock wraps VBState in a valid-JSON envelope ({"v":1,"payload":...,"authtag":...}). DescribeImages silently skipped encrypted AMIs (count=0 → "no AMIs available"), bricking the whole launch path. Route every config.json read through viperblock.StateBody, which unwraps the envelope (no key needed — the metadata is authenticated, not encrypted) and is a no-op for unencrypted volumes: - image: DescribeImages, getVolumeConfig, amiNameExists, GetAMIConfig - volume: getVolumeConfigAndEncryption, and mergeVolumeConfig (so the refuse-to-merge-encrypted guard sees the real VBState instead of clobbering the envelope with a tag-less blob) - admin: readAMIConfig, readVolumeConfig --- cmd/spinifex/cmd/admin.go | 99 +++++++++++++++++++- cmd/spinifex/cmd/admin_test.go | 68 ++++++++++++++ cmd/spinifex/cmd/templates/spinifex.toml | 3 + scripts/setup.sh | 8 ++ spinifex/admin/admin.go | 18 +++- spinifex/admin/images_remove.go | 4 +- spinifex/formation/formation.go | 13 +++ spinifex/formation/formation_test.go | 25 +++++ spinifex/handlers/ec2/image/service_impl.go | 27 ++++-- spinifex/handlers/ec2/volume/service_impl.go | 10 +- 10 files changed, 257 insertions(+), 18 deletions(-) diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index f2b1d537c..049fa1e83 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -1127,6 +1127,17 @@ func runAdminInit(cmd *cobra.Command, args []string) { fmt.Println("\n🔐 Generated predastore encryption key (per-node, never transmitted)") fmt.Printf(" Key: %s\n", predastoreKeyPath) + // Viperblock at-rest encryption key is cluster-wide; on a single-node init + // there are no joiners, so generate it locally and enable encryption by + // default for all volumes created on this install. + viperblockKeyPath, err := writeViperblockEncryptionKey(configDir) + if err != nil { + fmt.Fprintf(os.Stderr, "Error generating viperblock encryption key: %v\n", err) + os.Exit(1) + } + fmt.Println("\n🔐 Generated viperblock at-rest encryption key") + fmt.Printf(" Key: %s\n", viperblockKeyPath) + fmt.Printf("\n🔑 Generated admin credentials (save these — they won't be shown again):\n") fmt.Printf(" Access Key: %s\n", bootstrapResult.AdminAccessKey) fmt.Printf(" Secret Key: %s\n", bootstrapResult.AdminSecretKey) @@ -1314,6 +1325,8 @@ func runAdminInit(cmd *cobra.Command, args []string) { GPUPassthrough: gpuPassthrough, IPSecEnabled: ipsecEnabled, + + EncryptionKeyFile: viperblockKeyPath, } // Print external networking summary @@ -1422,6 +1435,22 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, fmt.Println("\n🔐 Generated predastore encryption key (per-node, never transmitted)") fmt.Printf(" Key: %s\n", predastoreKeyPath) + // Viperblock at-rest encryption key is cluster-wide and shared: the leader + // generates it once and distributes it to joiners via the formation server + // so a volume sealed on any node can be opened on any other. + viperblockKey, err := handlers_iam.GenerateMasterKey() + if err != nil { + fmt.Fprintf(os.Stderr, "❌ Error generating viperblock encryption key: %v\n", err) + os.Exit(1) + } + viperblockKeyPath, err := saveViperblockEncryptionKey(configDir, viperblockKey) + if err != nil { + fmt.Fprintf(os.Stderr, "❌ Error saving viperblock encryption key: %v\n", err) + os.Exit(1) + } + fmt.Println("\n🔐 Generated viperblock at-rest encryption key (cluster-wide, shared with joiners)") + fmt.Printf(" Key: %s\n", viperblockKeyPath) + // Read CA cert/key for distribution to joining nodes caCertData, err := os.ReadFile(filepath.Join(configDir, "ca.pem")) if err != nil { @@ -1450,6 +1479,9 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, // Include master key in formation server for distribution to joining nodes fs.SetMasterKey(base64.StdEncoding.EncodeToString(masterKey)) + // Distribute the cluster-wide viperblock encryption key to joiners + fs.SetViperblockKey(base64.StdEncoding.EncodeToString(viperblockKey)) + // Register self (init node) as the first node selfNode := formation.NodeInfo{ Name: node, @@ -1557,6 +1589,8 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, OperatorEmail: email, + EncryptionKeyFile: viperblockKeyPath, + // Init node runs ovn-central locally OVNNBAddr: "tcp:127.0.0.1:6641", OVNSBAddr: "tcp:127.0.0.1:6642", @@ -1902,6 +1936,28 @@ func runAdminJoin(cmd *cobra.Command, args []string) { } fmt.Printf("✅ Predastore encryption key generated: %s\n", predastoreKeyPath) + // Viperblock at-rest encryption key is cluster-wide: receive it from the + // leader rather than generating one, so this node can open volumes sealed + // elsewhere. Lenient on absence — an older leader that predates key + // distribution returns nothing; fall back to cleartext rather than fail + // the join. + var viperblockKeyPath string + if statusResp.ViperblockKey == "" { + fmt.Println("⚠️ Leader did not provide a viperblock encryption key; at-rest encryption disabled on this node") + } else { + viperblockKeyBytes, err := base64.StdEncoding.DecodeString(statusResp.ViperblockKey) + if err != nil { + fmt.Fprintf(os.Stderr, "❌ Error decoding viperblock encryption key: %v\n", err) + os.Exit(1) + } + viperblockKeyPath, err = saveViperblockEncryptionKey(configDir, viperblockKeyBytes) + if err != nil { + fmt.Fprintf(os.Stderr, "❌ Error saving viperblock encryption key: %v\n", err) + os.Exit(1) + } + fmt.Printf("✅ Viperblock encryption key received from leader: %s\n", viperblockKeyPath) + } + // Generate server cert signed by CA with this node's bind IP if err := admin.GenerateServerCertOnly(configDir, bindIP); err != nil { fmt.Fprintf(os.Stderr, "Error generating server certificate: %v\n", err) @@ -1993,6 +2049,8 @@ func runAdminJoin(cmd *cobra.Command, args []string) { OperatorEmail: email, + EncryptionKeyFile: viperblockKeyPath, + // Joining nodes connect to the init node's OVN NB/SB DB OVNNBAddr: fmt.Sprintf("tcp:%s:6641", leaderIP), OVNSBAddr: fmt.Sprintf("tcp:%s:6642", leaderIP), @@ -2278,6 +2336,7 @@ func runCertRenew(cmd *cobra.Command, _ []string) { type configDirs struct { AWSGW string Predastore string + Viperblock string NATS string Spinifex string } @@ -2287,10 +2346,11 @@ func createConfigSubdirs(configDir string) (configDirs, error) { dirs := configDirs{ AWSGW: filepath.Join(configDir, "awsgw"), Predastore: filepath.Join(configDir, "predastore"), + Viperblock: filepath.Join(configDir, "viperblock"), NATS: filepath.Join(configDir, "nats"), Spinifex: filepath.Join(configDir, "spinifex"), } - for _, dir := range []string{dirs.AWSGW, dirs.Predastore, dirs.NATS, dirs.Spinifex} { + for _, dir := range []string{dirs.AWSGW, dirs.Predastore, dirs.Viperblock, dirs.NATS, dirs.Spinifex} { if err := os.MkdirAll(dir, 0700); err != nil { return configDirs{}, fmt.Errorf("create directory %s: %w", dir, err) } @@ -2412,6 +2472,43 @@ func writePredastoreEncryptionKey(configDir string) (string, error) { return keyPath, nil } +// writeViperblockEncryptionKey generates a fresh 32-byte AES-256 master key for +// viperblock at-rest encryption and writes it to +// /viperblock/encryption.key with mode 0600. Unlike the predastore +// key, this key is cluster-wide and shared: the leader generates it once at init +// and distributes it to joiners via the formation server, so a volume sealed on +// any node can be opened on any other. Loaded at startup via masterkey.LoadShared. +func writeViperblockEncryptionKey(configDir string) (string, error) { + viperblockDir := filepath.Join(configDir, "viperblock") + if err := os.MkdirAll(viperblockDir, 0750); err != nil { + return "", fmt.Errorf("create viperblock config dir: %w", err) + } + key, err := handlers_iam.GenerateMasterKey() + if err != nil { + return "", fmt.Errorf("generate viperblock encryption key: %w", err) + } + keyPath := filepath.Join(viperblockDir, "encryption.key") + if err := handlers_iam.SaveMasterKey(keyPath, key); err != nil { + return "", fmt.Errorf("save viperblock encryption key: %w", err) + } + return keyPath, nil +} + +// saveViperblockEncryptionKey writes an already-generated 32-byte viperblock +// master key (received from the formation leader) to +// /viperblock/encryption.key with mode 0600. +func saveViperblockEncryptionKey(configDir string, key []byte) (string, error) { + viperblockDir := filepath.Join(configDir, "viperblock") + if err := os.MkdirAll(viperblockDir, 0750); err != nil { + return "", fmt.Errorf("create viperblock config dir: %w", err) + } + keyPath := filepath.Join(viperblockDir, "encryption.key") + if err := handlers_iam.SaveMasterKey(keyPath, key); err != nil { + return "", fmt.Errorf("save viperblock encryption key: %w", err) + } + return keyPath, nil +} + // writeSystemCredentials writes the system access key to a plaintext JSON file. // The daemon reads this at startup to inject credentials into ALB VM cloud-init // for SigV4-authenticated communication with the AWS gateway. diff --git a/cmd/spinifex/cmd/admin_test.go b/cmd/spinifex/cmd/admin_test.go index d194ebe7a..9c9979c4c 100644 --- a/cmd/spinifex/cmd/admin_test.go +++ b/cmd/spinifex/cmd/admin_test.go @@ -252,6 +252,74 @@ func TestWritePredastoreEncryptionKey(t *testing.T) { assert.NotEqual(t, key1, key2, "per-node keys must differ") } +// The viperblock key gates at-rest encryption for every volume on a fresh +// install. masterkey.LoadShared rejects anything that isn't exactly 32 bytes at +// 0640-or-stricter, so a regression in size/mode would silently fall back to +// cleartext (empty path) or fail service startup. +func TestWriteViperblockEncryptionKey(t *testing.T) { + configDir := t.TempDir() + + keyPath, err := writeViperblockEncryptionKey(configDir) + require.NoError(t, err) + assert.Equal(t, filepath.Join(configDir, "viperblock", "encryption.key"), keyPath) + + info, err := os.Stat(keyPath) + require.NoError(t, err) + assert.Equal(t, int64(32), info.Size(), "viperblock master key must be exactly 32 bytes") + assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "viperblock master key must be mode 0600") +} + +// Joiners persist the leader's shared key via saveViperblockEncryptionKey +// against a configDir with no viperblock/ subdir yet. The bytes round-trip +// verbatim — the whole cluster shares one key, so any mutation would orphan +// volumes sealed on other nodes. +func TestSaveViperblockEncryptionKey_RoundTrip(t *testing.T) { + configDir := t.TempDir() + key := make([]byte, 32) + for i := range key { + key[i] = byte(i) + } + + keyPath, err := saveViperblockEncryptionKey(configDir, key) + require.NoError(t, err) + assert.Equal(t, filepath.Join(configDir, "viperblock", "encryption.key"), keyPath) + + got, err := os.ReadFile(keyPath) + require.NoError(t, err) + assert.Equal(t, key, got, "shared key must be written verbatim") +} + +// A fresh install must render encryption_key_file so viperblockd enables +// at-rest encryption; an empty field (upgrade / legacy) must omit the line +// entirely so the volume stays in cleartext legacy mode. +func TestSpinifexTomlTemplate_EncryptionKeyFile(t *testing.T) { + base := admin.ConfigSettings{ + Node: "node1", Az: "ap-southeast-2a", Port: "4432", Region: "ap-southeast-2", + BindIP: "0.0.0.0", AccessKey: "AKIATEST", SecretKey: "SECRET", + AccountID: "123456789012", NatsToken: "token", + OVNNBAddr: "tcp:127.0.0.1:6641", OVNSBAddr: "tcp:127.0.0.1:6642", + } + + dir := t.TempDir() + withKey := base + withKey.ConfigDir = dir + withKey.EncryptionKeyFile = "/etc/spinifex/viperblock/encryption.key" + path := filepath.Join(dir, "spinifex.toml") + require.NoError(t, admin.GenerateConfigFile(path, spinifexTomlTemplate, withKey)) + data, err := os.ReadFile(path) + require.NoError(t, err) + assert.Contains(t, string(data), `encryption_key_file = "/etc/spinifex/viperblock/encryption.key"`) + + dir2 := t.TempDir() + noKey := base + noKey.ConfigDir = dir2 + path2 := filepath.Join(dir2, "spinifex.toml") + require.NoError(t, admin.GenerateConfigFile(path2, spinifexTomlTemplate, noKey)) + data2, err := os.ReadFile(path2) + require.NoError(t, err) + assert.NotContains(t, string(data2), "encryption_key_file", "blank key must omit the field") +} + // Joiners run `spx admin join` against a configDir that doesn't yet contain // a predastore/ subdir. The helper must create it (predastore subdir is // owned by spinifex-storage in prod, but the helper just needs the dir to diff --git a/cmd/spinifex/cmd/templates/spinifex.toml b/cmd/spinifex/cmd/templates/spinifex.toml index 6bde7133d..7805b427d 100644 --- a/cmd/spinifex/cmd/templates/spinifex.toml +++ b/cmd/spinifex/cmd/templates/spinifex.toml @@ -93,6 +93,9 @@ token = "{{.NatsToken}}" [nodes.{{.Node}}.viperblock] shardwal = false +{{- if .EncryptionKeyFile}} +encryption_key_file = "{{.EncryptionKeyFile}}" +{{- end}} [nodes.{{.Node}}.vpcd] ovn_nb_addr = "{{.OVNNBAddr}}" diff --git a/scripts/setup.sh b/scripts/setup.sh index cc4c1ca15..d41c65dbd 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -416,6 +416,14 @@ TMPEOF $SUDO chown "spinifex-gw:$SPINIFEX_GROUP" /etc/spinifex/awsgw $SUDO chmod 0750 /etc/spinifex/awsgw + # Viperblock's at-rest encryption key dir. 0750 group-traversable; the key + # itself is set to root:spinifex 0640 by SetServiceOwnership because both + # viperblockd (spinifex-viperblock) and the awsgw handlers (spinifex-gw) + # load it. + $SUDO mkdir -p /etc/spinifex/viperblock + $SUDO chown "spinifex-viperblock:$SPINIFEX_GROUP" /etc/spinifex/viperblock + $SUDO chmod 0750 /etc/spinifex/viperblock + # Per-service data directories $SUDO mkdir -p /var/lib/spinifex/nats $SUDO chown "spinifex-nats:$SPINIFEX_GROUP" /var/lib/spinifex/nats diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index 389055f11..0228d172d 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -104,6 +104,12 @@ type ConfigSettings struct { // IPSecEnabled toggles cluster-wide OVN native IPsec on intra-AZ Geneve. // Written under [network] in spinifex.toml; daemon reads it via cluster config. IPSecEnabled bool + + // EncryptionKeyFile is the path to the cluster-wide viperblock at-rest + // encryption key, rendered into [nodes.X.viperblock].encryption_key_file. + // Empty means no key was provisioned and volumes are written cleartext + // (legacy mode); the template omits the field entirely in that case. + EncryptionKeyFile string } // PredastoreNodeConfig describes a single Predastore node for multi-node config generation. @@ -308,6 +314,7 @@ func SetServiceOwnership() { "/var/lib/spinifex/nats": "spinifex-nats", "/etc/spinifex/predastore": "spinifex-storage", "/var/lib/spinifex/predastore": "spinifex-storage", + "/etc/spinifex/viperblock": "spinifex-viperblock", "/var/lib/spinifex/spinifex": "spinifex-daemon", "/var/lib/spinifex/viperblock": "spinifex-viperblock", "/var/lib/spinifex/vpcd": "spinifex-vpcd", @@ -342,11 +349,12 @@ func SetServiceOwnership() { // bootstrap.json lives in the awsgw data dir (not /etc/spinifex), // so /etc/spinifex stays at 0750 (no group-write needed). for path, mode := range map[string]os.FileMode{ - "/etc/spinifex/spinifex.toml": 0640, - "/etc/spinifex/master.key": 0640, - "/etc/spinifex/server.pem": 0644, - "/etc/spinifex/server.key": 0640, - "/etc/spinifex/ca.pem": 0644, + "/etc/spinifex/spinifex.toml": 0640, + "/etc/spinifex/master.key": 0640, + "/etc/spinifex/viperblock/encryption.key": 0640, + "/etc/spinifex/server.pem": 0644, + "/etc/spinifex/server.key": 0640, + "/etc/spinifex/ca.pem": 0644, } { if _, err := os.Stat(path); err != nil { continue diff --git a/spinifex/admin/images_remove.go b/spinifex/admin/images_remove.go index 45642d71a..bed1d940d 100644 --- a/spinifex/admin/images_remove.go +++ b/spinifex/admin/images_remove.go @@ -318,7 +318,7 @@ func readAMIConfig(store objectstore.ObjectStore, bucket, imageID string) (viper } var state viperblock.VBState - if err := json.Unmarshal(body, &state); err != nil { + if err := json.Unmarshal(viperblock.StateBody(body), &state); err != nil { return viperblock.AMIMetadata{}, fmt.Errorf("%w: %s: %v", handlers_ec2_image.ErrCorruptAMIConfig, key, err) } return state.VolumeConfig.AMIMetadata, nil @@ -350,7 +350,7 @@ func readVolumeConfig(store objectstore.ObjectStore, bucket, volumeID string) (* } var w volumeConfigWrapper - if err := json.Unmarshal(body, &w); err != nil { + if err := json.Unmarshal(viperblock.StateBody(body), &w); err != nil { return nil, fmt.Errorf("%w: %s: %v", errCorruptVolumeConfig, key, err) } return &w.VolumeConfig, nil diff --git a/spinifex/formation/formation.go b/spinifex/formation/formation.go index e0fdc8f3a..569a1c3ad 100644 --- a/spinifex/formation/formation.go +++ b/spinifex/formation/formation.go @@ -55,6 +55,7 @@ type StatusResponse struct { CACert string `json:"ca_cert,omitempty"` CAKey string `json:"ca_key,omitempty"` MasterKey string `json:"master_key,omitempty"` + ViperblockKey string `json:"viperblock_key,omitempty"` NetworkConfig *NetworkConfig `json:"network_config,omitempty"` } @@ -110,6 +111,7 @@ type FormationServer struct { caCert string caKey string masterKey string + viperblockKey string networkConfig *NetworkConfig joinToken string tokenExpiry time.Time @@ -218,6 +220,16 @@ func (fs *FormationServer) SetMasterKey(key string) { fs.masterKey = key } +// SetViperblockKey sets the base64-encoded cluster-wide viperblock at-rest +// encryption key for distribution to joining nodes. Unlike the per-node +// predastore key, this key is shared so a volume sealed on one node can be +// opened on any other. +func (fs *FormationServer) SetViperblockKey(key string) { + fs.mu.Lock() + defer fs.mu.Unlock() + fs.viperblockKey = key +} + // Start launches the HTTPS server on the given address (e.g. "10.0.0.1:4432") // using the cluster CA certificate and key for TLS. func (fs *FormationServer) Start(bindAddr string) error { @@ -360,6 +372,7 @@ func (fs *FormationServer) handleStatus(w http.ResponseWriter, r *http.Request) resp.CACert = fs.caCert resp.CAKey = fs.caKey resp.MasterKey = fs.masterKey + resp.ViperblockKey = fs.viperblockKey resp.NetworkConfig = fs.networkConfig } diff --git a/spinifex/formation/formation_test.go b/spinifex/formation/formation_test.go index 480d1b9f9..eab000277 100644 --- a/spinifex/formation/formation_test.go +++ b/spinifex/formation/formation_test.go @@ -346,6 +346,31 @@ func TestStatusEndpointMasterKey(t *testing.T) { assert.Equal(t, "dGVzdC1tYXN0ZXIta2V5LWJhc2U2NC1lbmNvZGVk", sr.MasterKey) } +func TestStatusEndpointViperblockKey(t *testing.T) { + t.Parallel() + fs := NewFormationServer(2, testCreds(), "ca-cert", "ca-key", nil, testToken, testTokenTTL) + fs.SetViperblockKey("dmlwZXJibG9jay1zaGFyZWQta2V5LWJhc2U2NA==") + ts := testServer(t, fs) + defer ts.Close() + + // Before completion: shared key must not leak + resp := authGet(t, ts.URL+"/formation/status", testToken) + var sr StatusResponse + require.NoError(t, json.NewDecoder(resp.Body).Decode(&sr)) + resp.Body.Close() + assert.Empty(t, sr.ViperblockKey) + + require.NoError(t, fs.RegisterNode(testNode("node1", "10.0.0.1"))) + require.NoError(t, fs.RegisterNode(testNode("node2", "10.0.0.2"))) + + // After completion: joiners receive the cluster-wide key + resp = authGet(t, ts.URL+"/formation/status", testToken) + defer resp.Body.Close() + require.NoError(t, json.NewDecoder(resp.Body).Decode(&sr)) + assert.True(t, sr.Complete) + assert.Equal(t, "dmlwZXJibG9jay1zaGFyZWQta2V5LWJhc2U2NA==", sr.ViperblockKey) +} + func TestHealthEndpoint_NoAuth(t *testing.T) { t.Parallel() fs := NewFormationServer(1, testCreds(), "", "", nil, testToken, testTokenTTL) diff --git a/spinifex/handlers/ec2/image/service_impl.go b/spinifex/handlers/ec2/image/service_impl.go index 4090db445..41b70cd2a 100644 --- a/spinifex/handlers/ec2/image/service_impl.go +++ b/spinifex/handlers/ec2/image/service_impl.go @@ -170,11 +170,13 @@ func (s *ImageServiceImpl) DescribeImages(input *ec2.DescribeImagesInput, accoun continue } - // Parse the viperblock config which contains VolumeConfig with AMIMetadata + // Parse the viperblock config which contains VolumeConfig with AMIMetadata. + // StateBody unwraps the at-rest encryption envelope (the metadata ships + // authenticated-but-plaintext); it is a no-op for unencrypted volumes. var vbConfig struct { VolumeConfig viperblock.VolumeConfig `json:"VolumeConfig"` } - if err := json.Unmarshal(body, &vbConfig); err != nil { + if err := json.Unmarshal(viperblock.StateBody(body), &vbConfig); err != nil { slog.Debug("Failed to unmarshal config", "key", configKey, "err", err) continue } @@ -580,8 +582,12 @@ func (s *ImageServiceImpl) getVolumeConfig(volumeID string) (*viperblock.VolumeC } defer result.Body.Close() + body, err := io.ReadAll(result.Body) + if err != nil { + return nil, err + } var vbState viperblock.VBState - if err := json.NewDecoder(result.Body).Decode(&vbState); err != nil { + if err := json.Unmarshal(viperblock.StateBody(body), &vbState); err != nil { return nil, err } return &vbState.VolumeConfig, nil @@ -616,10 +622,13 @@ func (s *ImageServiceImpl) amiNameExists(name string) (bool, error) { return false, fmt.Errorf("amiNameExists: failed to read %s: %w", configKey, err) } - var vbState viperblock.VBState - decodeErr := json.NewDecoder(result.Body).Decode(&vbState) + body, readErr := io.ReadAll(result.Body) _ = result.Body.Close() - if decodeErr != nil { + if readErr != nil { + return false, fmt.Errorf("amiNameExists: failed to read %s: %w", configKey, readErr) + } + var vbState viperblock.VBState + if decodeErr := json.Unmarshal(viperblock.StateBody(body), &vbState); decodeErr != nil { return false, fmt.Errorf("amiNameExists: failed to decode %s: %w", configKey, decodeErr) } @@ -649,8 +658,12 @@ func (s *ImageServiceImpl) GetAMIConfig(imageID string) (viperblock.AMIMetadata, } defer result.Body.Close() + body, err := io.ReadAll(result.Body) + if err != nil { + return viperblock.AMIMetadata{}, fmt.Errorf("%w: %s: %v", ErrCorruptAMIConfig, configKey, err) + } var vbState viperblock.VBState - if err := json.NewDecoder(result.Body).Decode(&vbState); err != nil { + if err := json.Unmarshal(viperblock.StateBody(body), &vbState); err != nil { return viperblock.AMIMetadata{}, fmt.Errorf("%w: %s: %v", ErrCorruptAMIConfig, configKey, err) } return vbState.VolumeConfig.AMIMetadata, nil diff --git a/spinifex/handlers/ec2/volume/service_impl.go b/spinifex/handlers/ec2/volume/service_impl.go index 40b76ea4b..a11a39403 100644 --- a/spinifex/handlers/ec2/volume/service_impl.go +++ b/spinifex/handlers/ec2/volume/service_impl.go @@ -1000,6 +1000,9 @@ func (s *VolumeServiceImpl) getVolumeConfigAndEncryption(volumeID string) (*vipe if err != nil { return nil, false, fmt.Errorf("failed to read config body: %w", err) } + // Unwrap the at-rest encryption envelope (authenticated-but-plaintext + // metadata); no-op for unencrypted volumes. + body = viperblock.StateBody(body) // Try full VBState first (matches mergeVolumeConfig's careful decode // pattern). A populated BlockSize is the marker that the blob is a full @@ -1062,10 +1065,11 @@ func (s *VolumeServiceImpl) mergeVolumeConfig(configKey string, cfg *viperblock. return nil, fmt.Errorf("failed to read existing config: %w", err) } - // Use Decoder (not Unmarshal): a trailing AES-GCM tag must not cause a parse error - // that would silently route to the wrapper-only path and brick the volume. + // StateBody unwraps the at-rest encryption envelope so the guard below sees + // the real VBState. Without it the wrapper decodes to a zero-valued state + // (BlockSize==0), routing to the wrapper-only path and bricking the volume. var state viperblock.VBState - if decodeErr := json.NewDecoder(bytes.NewReader(body)).Decode(&state); decodeErr != nil || state.BlockSize == 0 { + if decodeErr := json.Unmarshal(viperblock.StateBody(body), &state); decodeErr != nil || state.BlockSize == 0 { // Not a full VBState (new volume or wrapper-only) -- write wrapper return json.Marshal(volumeConfigWrapper{VolumeConfig: *cfg}) }