From 107a5f6ea4ef87c82af32ebaf57aae20e3b5e8e8 Mon Sep 17 00:00:00 2001 From: ChengyuZhu6 Date: Mon, 19 May 2025 17:59:26 +0800 Subject: [PATCH 01/25] internal:dmverity: Implement dmverity functionality Signed-off-by: ChengyuZhu6 --- internal/dmverity/dmverity.go | 143 ++++++++++++++++++++++++++++ internal/dmverity/dmverity_linux.go | 136 ++++++++++++++++++++++++++ internal/dmverity/dmverity_other.go | 39 ++++++++ internal/dmverity/dmverity_test.go | 123 ++++++++++++++++++++++++ 4 files changed, 441 insertions(+) create mode 100644 internal/dmverity/dmverity.go create mode 100644 internal/dmverity/dmverity_linux.go create mode 100644 internal/dmverity/dmverity_other.go create mode 100644 internal/dmverity/dmverity_test.go diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go new file mode 100644 index 0000000000000..15cf2101e15a9 --- /dev/null +++ b/internal/dmverity/dmverity.go @@ -0,0 +1,143 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +// Package dmverity provides functions for working with dm-verity for integrity verification +package dmverity + +import ( + "bufio" + "strconv" + "strings" +) + +// VeritySetupCommand represents the type of veritysetup command to execute +type VeritySetupCommand string + +const ( + // FormatCommand corresponds to "veritysetup format" + FormatCommand VeritySetupCommand = "format" + // OpenCommand corresponds to "veritysetup open" + OpenCommand VeritySetupCommand = "open" + // CloseCommand corresponds to "veritysetup close" + CloseCommand VeritySetupCommand = "close" +) + +// DmverityOptions contains configuration options for dm-verity operations +type DmverityOptions struct { + // Salt for hashing, represented as a hex string + Salt string + // Hash algorithm to use (default: sha256) + HashAlgorithm string + // Size of data blocks in bytes (default: 4096) + DataBlockSize uint32 + // Size of hash blocks in bytes (default: 4096) + HashBlockSize uint32 + // Number of data blocks + DataBlocks uint64 + // Offset of hash area in bytes + HashOffset uint64 + // Hash type (default: 1) + HashType uint32 + // Superblock usage flag (false meaning --no-superblock) + UseSuperblock bool + // Debug flag + Debug bool + // UUID for device to use + UUID string +} + +// DefaultDmverityOptions returns a DmverityOptions struct with default values +func DefaultDmverityOptions() DmverityOptions { + return DmverityOptions{ + HashAlgorithm: "sha256", + DataBlockSize: 4096, + HashBlockSize: 4096, + HashType: 1, + UseSuperblock: true, + Salt: "0000000000000000000000000000000000000000000000000000000000000000", + } +} + +// FormatOutputInfo represents the parsed information from veritysetup format command output +type FormatOutputInfo struct { + // Basic dm-verity options, reused from DmverityOptions + DmverityOptions + // Number of hash blocks in the hash area + HashBlocks int64 + // Root hash value for verification + RootHash string +} + +// ParseFormatOutput parses the output from veritysetup format command +// and returns a structured representation of the information +func ParseFormatOutput(output string) (*FormatOutputInfo, error) { + info := &FormatOutputInfo{} + + scanner := bufio.NewScanner(strings.NewReader(output)) + for scanner.Scan() { + line := scanner.Text() + // Skip the header line and command echo line + if strings.HasPrefix(line, "VERITY header") || strings.HasPrefix(line, "# veritysetup") { + continue + } + + parts := strings.Split(line, ":") + if len(parts) != 2 { + continue + } + + key := strings.TrimSpace(parts[0]) + value := strings.TrimSpace(parts[1]) + + switch key { + case "UUID": + info.UUID = value + case "Hash type": + hashType, err := strconv.Atoi(value) + if err == nil { + info.HashType = uint32(hashType) + } + case "Data blocks": + dataBlocks, err := strconv.ParseInt(value, 10, 64) + if err == nil { + info.DataBlocks = uint64(dataBlocks) + } + case "Data block size": + dataBlockSize, err := strconv.ParseInt(value, 10, 64) + if err == nil { + info.DataBlockSize = uint32(dataBlockSize) + } + case "Hash blocks": + hashBlocks, err := strconv.ParseInt(value, 10, 64) + if err == nil { + info.HashBlocks = hashBlocks + } + case "Hash block size": + hashBlockSize, err := strconv.ParseInt(value, 10, 64) + if err == nil { + info.HashBlockSize = uint32(hashBlockSize) + } + case "Hash algorithm": + info.HashAlgorithm = value + case "Salt": + info.Salt = value + case "Root hash": + info.RootHash = value + } + } + + return info, scanner.Err() +} diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go new file mode 100644 index 0000000000000..48f6e0d4e46df --- /dev/null +++ b/internal/dmverity/dmverity_linux.go @@ -0,0 +1,136 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dmverity + +import ( + "bytes" + "fmt" + "os" + "os/exec" +) + +func IsSupported() (bool, error) { + moduleData, err := os.ReadFile("/proc/modules") + if err != nil || !bytes.Contains(moduleData, []byte("dm_verity")) { + return false, fmt.Errorf("dm_verity module not loaded") + } + if _, err := exec.LookPath("veritysetup"); err == nil { + cmd := exec.Command("veritysetup", "--version") + if _, err := cmd.CombinedOutput(); err != nil { + return false, fmt.Errorf("veritysetup not found") + } + } + return true, nil +} + +// runVeritySetup executes a veritysetup command with the given arguments and options +func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (string, error) { + cmdArgs := []string{string(cmd)} + + if opts == nil { + defaultOpts := DefaultDmverityOptions() + opts = &defaultOpts + } + + if opts.UUID != "" { + cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) + } + + if !opts.UseSuperblock { + cmdArgs = append(cmdArgs, "--no-superblock") + } + + if opts.HashType == 0 { + cmdArgs = append(cmdArgs, fmt.Sprintf("--format=%d", opts.HashType)) + } + + if opts.Debug { + cmdArgs = append(cmdArgs, "--debug") + } + + if opts.HashAlgorithm != "" { + cmdArgs = append(cmdArgs, "--hash="+opts.HashAlgorithm) + } + + if opts.DataBlockSize > 0 { + cmdArgs = append(cmdArgs, fmt.Sprintf("--data-block-size=%d", opts.DataBlockSize)) + } + + if opts.HashBlockSize > 0 { + cmdArgs = append(cmdArgs, fmt.Sprintf("--hash-block-size=%d", opts.HashBlockSize)) + } + + if opts.DataBlocks > 0 { + cmdArgs = append(cmdArgs, "--data-blocks", fmt.Sprintf("%d", opts.DataBlocks)) + } + + if opts.HashOffset > 0 { + cmdArgs = append(cmdArgs, "--hash-offset", fmt.Sprintf("%d", opts.HashOffset)) + } + + if opts.Salt != "" { + cmdArgs = append(cmdArgs, "-s", opts.Salt) + } + + cmdArgs = append(cmdArgs, args...) + + execCmd := exec.Command("veritysetup", cmdArgs...) + output, err := execCmd.CombinedOutput() + if err != nil { + return "", fmt.Errorf("veritysetup %s failed: %w, output: %s", cmd, err, string(output)) + } + + return string(output), nil +} + +// Format creates a dm-verity hash for a data device +// If hashDevice is the same as dataDevice, the hash will be stored on the same device +func Format(dataDevice, hashDevice string, opts *DmverityOptions) (*FormatOutputInfo, error) { + args := []string{dataDevice, hashDevice} + output, err := actions(FormatCommand, args, opts) + if err != nil { + return nil, fmt.Errorf("failed to format dm-verity device: %w, output: %s", err, output) + } + + // Parse the output to extract structured information + info, err := ParseFormatOutput(output) + if err != nil { + return nil, fmt.Errorf("failed to parse format output: %w", err) + } + + return info, nil +} + +// Open creates a read-only device-mapper target for transparent integrity verification +func Open(dataDevice string, name string, hashDevice string, rootHash string, opts *DmverityOptions) (string, error) { + args := []string{dataDevice, name, hashDevice, rootHash} + output, err := actions(OpenCommand, args, opts) + if err != nil { + return "", fmt.Errorf("failed to open dm-verity device: %w, output: %s", err, output) + } + return output, nil +} + +// Close removes a dm-verity target and its underlying device from the device mapper table +func Close(name string) (string, error) { + args := []string{name} + output, err := actions(CloseCommand, args, nil) + if err != nil { + return "", fmt.Errorf("failed to close dm-verity device: %w, output: %s", err, output) + } + return output, nil +} diff --git a/internal/dmverity/dmverity_other.go b/internal/dmverity/dmverity_other.go new file mode 100644 index 0000000000000..92caa0a2e8bbd --- /dev/null +++ b/internal/dmverity/dmverity_other.go @@ -0,0 +1,39 @@ +//go:build !linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dmverity + +import "fmt" + +var errUnsupported = fmt.Errorf("dmverity is only supported on Linux systems") + +func IsSupported() (bool, error) { + return false, errUnsupported +} + +func Format(_ string, _ string, _ *DmverityOptions) (*FormatOutputInfo, error) { + return nil, errUnsupported +} + +func Open(_ string, _ string, _ string, _ string, _ *DmverityOptions) (string, error) { + return "", errUnsupported +} + +func Close(_ string) (string, error) { + return "", errUnsupported +} diff --git a/internal/dmverity/dmverity_test.go b/internal/dmverity/dmverity_test.go new file mode 100644 index 0000000000000..11594a0b1620b --- /dev/null +++ b/internal/dmverity/dmverity_test.go @@ -0,0 +1,123 @@ +//go:build linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dmverity + +import ( + "os" + "testing" + + "github.com/containerd/containerd/v2/core/mount" + "github.com/containerd/containerd/v2/pkg/testutil" + "github.com/docker/go-units" + "github.com/stretchr/testify/assert" +) + +const ( + testDeviceName = "test-verity-device" +) + +func TestDMVerity(t *testing.T) { + testutil.RequiresRoot(t) + + // Skip if dm-verity is not supported + supported, err := IsSupported() + if !supported || err != nil { + t.Skipf("dm-verity is not supported on this system: %v", err) + } + + // Create temp directory for test files + tempDir := t.TempDir() + + // Create a single loop device for both data and hash + loopImage, loopDevice := createLoopbackDevice(t, tempDir, "1Mb") + + defer func() { + err := mount.DetachLoopDevice(loopDevice) + assert.NoError(t, err, "failed to detach loop device for image: %s", loopImage) + }() + + // Create default options for the tests + opts := DmverityOptions{ + HashAlgorithm: "sha256", + Salt: "0000000000000000000000000000000000000000000000000000000000000000", + DataBlockSize: 4096, + HashBlockSize: 4096, + HashType: 1, + UseSuperblock: true, + Debug: false, + DataBlocks: 256, + HashOffset: 1048576, + } + + t.Run("IsSupported", func(t *testing.T) { + supported, err := IsSupported() + assert.True(t, supported, "dm-verity should be supported") + assert.NoError(t, err, "IsSupported should not return an error") + }) + + var formatInfo *FormatOutputInfo + + t.Run("Format", func(t *testing.T) { + var err error + // Use the same device for both data and hash + formatInfo, err = Format(loopDevice, loopDevice, &opts) + assert.NoError(t, err, "failed to format dm-verity device") + assert.NotEmpty(t, formatInfo.RootHash, "root hash should not be empty") + t.Logf("Root hash: %s", formatInfo.RootHash) + }) + + t.Run("Open", func(t *testing.T) { + output, err := Open(loopDevice, testDeviceName, loopDevice, formatInfo.RootHash, &opts) + assert.NoError(t, err, "failed to open dm-verity device") + t.Logf("Open output: %s", output) + + _, err = os.Stat("/dev/mapper/" + testDeviceName) + assert.NoError(t, err, "device should exist in /dev/mapper") + }) + + t.Run("Close", func(t *testing.T) { + output, err := Close(testDeviceName) + assert.NoError(t, err, "failed to close dm-verity device") + t.Logf("Close output: %s", output) + + _, err = os.Stat("/dev/mapper/" + testDeviceName) + assert.True(t, os.IsNotExist(err), "device should not exist after closing") + }) +} + +func createLoopbackDevice(t *testing.T, dir string, size string) (string, string) { + file, err := os.CreateTemp(dir, "dmverity-tests-") + assert.NoError(t, err) + + sizeInBytes, err := units.RAMInBytes(size) + assert.NoError(t, err) + + err = file.Truncate(sizeInBytes * 2) + assert.NoError(t, err) + + err = file.Close() + assert.NoError(t, err) + + imagePath := file.Name() + + loopDevice, err := mount.AttachLoopDevice(imagePath) + assert.NoError(t, err) + + return imagePath, loopDevice +} From dcefa2ef91fd94afdc77dccf252a17f058da8b8c Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Thu, 9 Oct 2025 23:44:55 +0000 Subject: [PATCH 02/25] Improve error handling --- internal/dmverity/dmverity.go | 41 +++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index 15cf2101e15a9..1cacf7e85e143 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -19,6 +19,7 @@ package dmverity import ( "bufio" + "fmt" "strconv" "strings" ) @@ -84,6 +85,10 @@ type FormatOutputInfo struct { // ParseFormatOutput parses the output from veritysetup format command // and returns a structured representation of the information func ParseFormatOutput(output string) (*FormatOutputInfo, error) { + if output == "" { + return nil, fmt.Errorf("output is empty") + } + info := &FormatOutputInfo{} scanner := bufio.NewScanner(strings.NewReader(output)) @@ -107,29 +112,34 @@ func ParseFormatOutput(output string) (*FormatOutputInfo, error) { info.UUID = value case "Hash type": hashType, err := strconv.Atoi(value) - if err == nil { - info.HashType = uint32(hashType) + if err != nil { + return nil, fmt.Errorf("failed to parse hash type %q: %w", value, err) } + info.HashType = uint32(hashType) case "Data blocks": dataBlocks, err := strconv.ParseInt(value, 10, 64) - if err == nil { - info.DataBlocks = uint64(dataBlocks) + if err != nil { + return nil, fmt.Errorf("failed to parse data blocks %q: %w", value, err) } + info.DataBlocks = uint64(dataBlocks) case "Data block size": dataBlockSize, err := strconv.ParseInt(value, 10, 64) - if err == nil { - info.DataBlockSize = uint32(dataBlockSize) + if err != nil { + return nil, fmt.Errorf("failed to parse data block size %q: %w", value, err) } + info.DataBlockSize = uint32(dataBlockSize) case "Hash blocks": hashBlocks, err := strconv.ParseInt(value, 10, 64) - if err == nil { - info.HashBlocks = hashBlocks + if err != nil { + return nil, fmt.Errorf("failed to parse hash blocks %q: %w", value, err) } + info.HashBlocks = hashBlocks case "Hash block size": hashBlockSize, err := strconv.ParseInt(value, 10, 64) - if err == nil { - info.HashBlockSize = uint32(hashBlockSize) + if err != nil { + return nil, fmt.Errorf("failed to parse hash block size %q: %w", value, err) } + info.HashBlockSize = uint32(hashBlockSize) case "Hash algorithm": info.HashAlgorithm = value case "Salt": @@ -139,5 +149,14 @@ func ParseFormatOutput(output string) (*FormatOutputInfo, error) { } } - return info, scanner.Err() + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error scanning output: %w", err) + } + + // Validate required fields + if info.RootHash == "" { + return nil, fmt.Errorf("root hash not found in output") + } + + return info, nil } From 091104c50dbdfc6fdd1abed086f2f8ad31913dae Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 10 Oct 2025 20:44:25 +0000 Subject: [PATCH 03/25] Improve IsSupported fn error handling --- internal/dmverity/dmverity_linux.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index 48f6e0d4e46df..f0b22bf79ad81 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -25,15 +25,23 @@ import ( func IsSupported() (bool, error) { moduleData, err := os.ReadFile("/proc/modules") - if err != nil || !bytes.Contains(moduleData, []byte("dm_verity")) { + if err != nil { + return false, fmt.Errorf("failed to read /proc/modules: %w", err) + } + if !bytes.Contains(moduleData, []byte("dm_verity")) { return false, fmt.Errorf("dm_verity module not loaded") } - if _, err := exec.LookPath("veritysetup"); err == nil { - cmd := exec.Command("veritysetup", "--version") - if _, err := cmd.CombinedOutput(); err != nil { - return false, fmt.Errorf("veritysetup not found") - } + + veritysetupPath, err := exec.LookPath("veritysetup") + if err != nil { + return false, fmt.Errorf("veritysetup not found in PATH: %w", err) } + + cmd := exec.Command(veritysetupPath, "--version") + if _, err := cmd.CombinedOutput(); err != nil { + return false, fmt.Errorf("veritysetup not functional: %w", err) + } + return true, nil } From 50f861c17cdfff1d5ed07cbbaa5d7efc41c629cc Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 10 Oct 2025 21:21:41 +0000 Subject: [PATCH 04/25] Validate dm verity options and root hash --- internal/dmverity/dmverity.go | 52 +++++++++++++++++++++++++++-- internal/dmverity/dmverity_linux.go | 5 +++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index 1cacf7e85e143..1d738e622ce7a 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -19,6 +19,7 @@ package dmverity import ( "bufio" + "encoding/hex" "fmt" "strconv" "strings" @@ -72,6 +73,51 @@ func DefaultDmverityOptions() DmverityOptions { } } +// ValidateOptions validates dm-verity options to ensure they are valid +// before being passed to veritysetup commands +func ValidateOptions(opts *DmverityOptions) error { + if opts == nil { + return fmt.Errorf("options cannot be nil") + } + + // Validate block sizes are power of 2 (kernel requirement) + if opts.DataBlockSize > 0 { + if opts.DataBlockSize&(opts.DataBlockSize-1) != 0 { + return fmt.Errorf("data block size %d must be a power of 2", opts.DataBlockSize) + } + } + + if opts.HashBlockSize > 0 { + if opts.HashBlockSize&(opts.HashBlockSize-1) != 0 { + return fmt.Errorf("hash block size %d must be a power of 2", opts.HashBlockSize) + } + } + + // Validate salt format (must be hex string) + if opts.Salt != "" { + // Use hex.DecodeString to validate - it checks format and even length + if _, err := hex.DecodeString(opts.Salt); err != nil { + return fmt.Errorf("salt must be a valid hex string: %w", err) + } + } + + return nil +} + +// ValidateRootHash validates that a root hash string is in valid hexadecimal format +func ValidateRootHash(rootHash string) error { + if rootHash == "" { + return fmt.Errorf("root hash cannot be empty") + } + + // Validate root hash (must be hex string) + if _, err := hex.DecodeString(rootHash); err != nil { + return fmt.Errorf("root hash must be a valid hex string: %w", err) + } + + return nil +} + // FormatOutputInfo represents the parsed information from veritysetup format command output type FormatOutputInfo struct { // Basic dm-verity options, reused from DmverityOptions @@ -153,9 +199,9 @@ func ParseFormatOutput(output string) (*FormatOutputInfo, error) { return nil, fmt.Errorf("error scanning output: %w", err) } - // Validate required fields - if info.RootHash == "" { - return nil, fmt.Errorf("root hash not found in output") + // Validate root hash format + if err := ValidateRootHash(info.RootHash); err != nil { + return nil, fmt.Errorf("parsed root hash is invalid: %w", err) } return info, nil diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index f0b22bf79ad81..b91e5f0364488 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -54,6 +54,11 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri opts = &defaultOpts } + // Validate options before building command + if err := ValidateOptions(opts); err != nil { + return "", fmt.Errorf("invalid dm-verity options: %w", err) + } + if opts.UUID != "" { cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) } From c8310fcc5249e47e0d9be7250f2effd83c09ff1c Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 10 Oct 2025 21:28:02 +0000 Subject: [PATCH 05/25] Ensure order of dm verity options is consistent --- internal/dmverity/dmverity.go | 2 +- internal/dmverity/dmverity_linux.go | 32 ++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index 1d738e622ce7a..b3ce9712d40f1 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -64,12 +64,12 @@ type DmverityOptions struct { // DefaultDmverityOptions returns a DmverityOptions struct with default values func DefaultDmverityOptions() DmverityOptions { return DmverityOptions{ + Salt: "0000000000000000000000000000000000000000000000000000000000000000", HashAlgorithm: "sha256", DataBlockSize: 4096, HashBlockSize: 4096, HashType: 1, UseSuperblock: true, - Salt: "0000000000000000000000000000000000000000000000000000000000000000", } } diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index b91e5f0364488..50e53e4bc597e 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -59,20 +59,8 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri return "", fmt.Errorf("invalid dm-verity options: %w", err) } - if opts.UUID != "" { - cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) - } - - if !opts.UseSuperblock { - cmdArgs = append(cmdArgs, "--no-superblock") - } - - if opts.HashType == 0 { - cmdArgs = append(cmdArgs, fmt.Sprintf("--format=%d", opts.HashType)) - } - - if opts.Debug { - cmdArgs = append(cmdArgs, "--debug") + if opts.Salt != "" { + cmdArgs = append(cmdArgs, "-s", opts.Salt) } if opts.HashAlgorithm != "" { @@ -95,8 +83,20 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri cmdArgs = append(cmdArgs, "--hash-offset", fmt.Sprintf("%d", opts.HashOffset)) } - if opts.Salt != "" { - cmdArgs = append(cmdArgs, "-s", opts.Salt) + if opts.HashType == 0 { + cmdArgs = append(cmdArgs, fmt.Sprintf("--format=%d", opts.HashType)) + } + + if !opts.UseSuperblock { + cmdArgs = append(cmdArgs, "--no-superblock") + } + + if opts.Debug { + cmdArgs = append(cmdArgs, "--debug") + } + + if opts.UUID != "" { + cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) } cmdArgs = append(cmdArgs, args...) From dba744daa1c7e1c5285da1aeef0bf7f4af74f806 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 10 Oct 2025 21:29:26 +0000 Subject: [PATCH 06/25] Remove extra comment --- internal/dmverity/dmverity.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index b3ce9712d40f1..5732c93b30ea7 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -95,7 +95,6 @@ func ValidateOptions(opts *DmverityOptions) error { // Validate salt format (must be hex string) if opts.Salt != "" { - // Use hex.DecodeString to validate - it checks format and even length if _, err := hex.DecodeString(opts.Salt); err != nil { return fmt.Errorf("salt must be a valid hex string: %w", err) } From 201187210858094ee0a46ed3ed52389ccef2ace0 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 10 Oct 2025 21:56:48 +0000 Subject: [PATCH 07/25] add root hash file option --- internal/dmverity/ROOT_HASH_FILE.md | 125 ++++++++++++++++++++++++++++ internal/dmverity/dmverity.go | 64 ++++++++------ internal/dmverity/dmverity_linux.go | 7 +- 3 files changed, 171 insertions(+), 25 deletions(-) create mode 100644 internal/dmverity/ROOT_HASH_FILE.md diff --git a/internal/dmverity/ROOT_HASH_FILE.md b/internal/dmverity/ROOT_HASH_FILE.md new file mode 100644 index 0000000000000..2ef7a8cc53095 --- /dev/null +++ b/internal/dmverity/ROOT_HASH_FILE.md @@ -0,0 +1,125 @@ +# Root Hash File Support + +## Overview + +The `--root-hash-file` option allows saving the generated root hash to a file during the `format` operation, instead of only printing it to stdout. + +## Usage + +### Basic Example + +```go +opts := dmverity.DefaultDmverityOptions() +opts.RootHashFile = "/tmp/root-hash.txt" + +info, err := dmverity.Format("/dev/loop0", "/dev/loop1", &opts) +if err != nil { + log.Fatal(err) +} + +// Root hash is available in info.RootHash +// AND has been saved to /tmp/root-hash.txt +fmt.Printf("Root hash: %s\n", info.RootHash) +``` + +## Benefits + +1. **Automation-Friendly**: Eliminates need to parse stdout +2. **Secure Storage**: Hash can be written directly to secure location +3. **Scripting**: Easier to use in shell scripts and CI/CD pipelines +4. **Persistence**: Hash is saved even if program crashes after formatting + +## Implementation Details + +### When RootHashFile is Specified + +1. `--root-hash-file ` is passed to veritysetup +2. veritysetup writes the root hash (hex-encoded) to the specified file +3. After formatting completes, we read the file to populate `FormatOutputInfo.RootHash` +4. The hash is validated using `ValidateRootHash()` + +### File Format + +The file contains the root hash as a hex-encoded string (no newlines or whitespace at the end after trimming): + +``` +4d8f71f8a5b9c3e2f7a1d6b4e9c8a7f5d2e1b3c4a5e7d9f8b6c3a2e1f7d8b9a4 +``` + +For SHA-256: 64 hex characters (32 bytes) +For SHA-512: 128 hex characters (64 bytes) + +### Fallback Behavior + +If `RootHashFile` is NOT specified: +- Root hash is parsed from stdout (existing behavior) +- `ParseFormatOutput()` extracts it from the "Root hash:" line + +If `RootHashFile` IS specified: +- Root hash may not appear in stdout +- We read it from the file after formatting +- Both sources are validated with `ValidateRootHash()` + +## Error Handling + +Possible errors when using RootHashFile: + +```go +// File cannot be created (permissions, path doesn't exist) +"failed to format dm-verity device: ..." + +// File cannot be read after formatting +"failed to read root hash from file \"/path\": ..." + +// File contains invalid hash +"root hash from file is invalid: must be a valid hex string" +``` + +## Security Considerations + +1. **File Permissions**: Ensure the target file has appropriate permissions +2. **Path Validation**: Validate the path before passing to Format() +3. **Atomic Writes**: veritysetup writes atomically, but cleanup on failure is caller's responsibility +4. **Cleanup**: Consider removing the file if Format() fails + +## Example: Secure Usage + +```go +// Create a secure temporary file +tmpFile, err := os.CreateTemp("", "root-hash-*.txt") +if err != nil { + return err +} +defer os.Remove(tmpFile.Name()) // Cleanup on any error +tmpFile.Close() + +// Use it for root hash +opts := dmverity.DefaultDmverityOptions() +opts.RootHashFile = tmpFile.Name() + +info, err := dmverity.Format(dataDevice, hashDevice, &opts) +if err != nil { + return fmt.Errorf("format failed: %w", err) +} + +// Move to permanent location if needed +if err := os.Rename(tmpFile.Name(), "/secure/location/root-hash"); err != nil { + return err +} +``` + +## Compatibility + +- Requires veritysetup version 1.0 or later +- Option is ignored for `Open()` and `Close()` commands (format-only) +- Works with or without `--no-superblock` flag +- Compatible with all hash algorithms (sha256, sha512, sha1) + +## Testing + +When testing, verify: +1. File is created with correct permissions +2. File contains valid hex-encoded hash +3. Hash in file matches hash in FormatOutputInfo +4. File is readable after formatting +5. Error handling when file path is invalid diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index 5732c93b30ea7..7759730b6358d 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -19,8 +19,10 @@ package dmverity import ( "bufio" + "bytes" "encoding/hex" "fmt" + "os" "strconv" "strings" ) @@ -59,6 +61,8 @@ type DmverityOptions struct { Debug bool // UUID for device to use UUID string + // RootHashFile specifies a file path where the root hash should be saved + RootHashFile string } // DefaultDmverityOptions returns a DmverityOptions struct with default values @@ -128,14 +132,15 @@ type FormatOutputInfo struct { } // ParseFormatOutput parses the output from veritysetup format command -// and returns a structured representation of the information -func ParseFormatOutput(output string) (*FormatOutputInfo, error) { +// and returns a structured representation of the information. +func ParseFormatOutput(output string, opts *DmverityOptions) (*FormatOutputInfo, error) { if output == "" { return nil, fmt.Errorf("output is empty") } info := &FormatOutputInfo{} + // Parse stdout output line by line scanner := bufio.NewScanner(strings.NewReader(output)) for scanner.Scan() { line := scanner.Text() @@ -153,42 +158,42 @@ func ParseFormatOutput(output string) (*FormatOutputInfo, error) { value := strings.TrimSpace(parts[1]) switch key { - case "UUID": - info.UUID = value - case "Hash type": - hashType, err := strconv.Atoi(value) + case "Salt": + info.Salt = value + case "Hash algorithm": + info.HashAlgorithm = value + case "Data block size": + dataBlockSize, err := strconv.ParseInt(value, 10, 64) if err != nil { - return nil, fmt.Errorf("failed to parse hash type %q: %w", value, err) + return nil, fmt.Errorf("failed to parse data block size %q: %w", value, err) } - info.HashType = uint32(hashType) + info.DataBlockSize = uint32(dataBlockSize) + case "Hash block size": + hashBlockSize, err := strconv.ParseInt(value, 10, 64) + if err != nil { + return nil, fmt.Errorf("failed to parse hash block size %q: %w", value, err) + } + info.HashBlockSize = uint32(hashBlockSize) case "Data blocks": dataBlocks, err := strconv.ParseInt(value, 10, 64) if err != nil { return nil, fmt.Errorf("failed to parse data blocks %q: %w", value, err) } info.DataBlocks = uint64(dataBlocks) - case "Data block size": - dataBlockSize, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, fmt.Errorf("failed to parse data block size %q: %w", value, err) - } - info.DataBlockSize = uint32(dataBlockSize) case "Hash blocks": hashBlocks, err := strconv.ParseInt(value, 10, 64) if err != nil { return nil, fmt.Errorf("failed to parse hash blocks %q: %w", value, err) } info.HashBlocks = hashBlocks - case "Hash block size": - hashBlockSize, err := strconv.ParseInt(value, 10, 64) + case "Hash type": + hashType, err := strconv.Atoi(value) if err != nil { - return nil, fmt.Errorf("failed to parse hash block size %q: %w", value, err) + return nil, fmt.Errorf("failed to parse hash type %q: %w", value, err) } - info.HashBlockSize = uint32(hashBlockSize) - case "Hash algorithm": - info.HashAlgorithm = value - case "Salt": - info.Salt = value + info.HashType = uint32(hashType) + case "UUID": + info.UUID = value case "Root hash": info.RootHash = value } @@ -198,9 +203,20 @@ func ParseFormatOutput(output string) (*FormatOutputInfo, error) { return nil, fmt.Errorf("error scanning output: %w", err) } - // Validate root hash format + // If a root hash file was specified and we haven't found a root hash in stdout, + // read it from the file (veritysetup writes it there instead of stdout when using --root-hash-file) + if opts != nil && opts.RootHashFile != "" && info.RootHash == "" { + hashBytes, err := os.ReadFile(opts.RootHashFile) + if err != nil { + return nil, fmt.Errorf("failed to read root hash from file %q: %w", opts.RootHashFile, err) + } + // Trim any whitespace/newlines + info.RootHash = string(bytes.TrimSpace(hashBytes)) + } + + // Validate root hash if err := ValidateRootHash(info.RootHash); err != nil { - return nil, fmt.Errorf("parsed root hash is invalid: %w", err) + return nil, fmt.Errorf("root hash is invalid: %w", err) } return info, nil diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index 50e53e4bc597e..cd6648c7220b4 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -99,6 +99,10 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) } + if opts.RootHashFile != "" { + cmdArgs = append(cmdArgs, "--root-hash-file", opts.RootHashFile) + } + cmdArgs = append(cmdArgs, args...) execCmd := exec.Command("veritysetup", cmdArgs...) @@ -120,7 +124,8 @@ func Format(dataDevice, hashDevice string, opts *DmverityOptions) (*FormatOutput } // Parse the output to extract structured information - info, err := ParseFormatOutput(output) + // Pass opts so ParseFormatOutput can read root hash from file if RootHashFile was specified + info, err := ParseFormatOutput(output, opts) if err != nil { return nil, fmt.Errorf("failed to parse format output: %w", err) } From 1ce79e018e42cd31a2929e23d76a4e9de6bb6a54 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 10 Oct 2025 22:45:09 +0000 Subject: [PATCH 08/25] Improve testing --- internal/dmverity/ROOT_HASH_FILE.md | 125 --------- internal/dmverity/dmverity.go | 8 - internal/dmverity/dmverity_test.go | 387 ++++++++++++++++++++++++++-- 3 files changed, 361 insertions(+), 159 deletions(-) delete mode 100644 internal/dmverity/ROOT_HASH_FILE.md diff --git a/internal/dmverity/ROOT_HASH_FILE.md b/internal/dmverity/ROOT_HASH_FILE.md deleted file mode 100644 index 2ef7a8cc53095..0000000000000 --- a/internal/dmverity/ROOT_HASH_FILE.md +++ /dev/null @@ -1,125 +0,0 @@ -# Root Hash File Support - -## Overview - -The `--root-hash-file` option allows saving the generated root hash to a file during the `format` operation, instead of only printing it to stdout. - -## Usage - -### Basic Example - -```go -opts := dmverity.DefaultDmverityOptions() -opts.RootHashFile = "/tmp/root-hash.txt" - -info, err := dmverity.Format("/dev/loop0", "/dev/loop1", &opts) -if err != nil { - log.Fatal(err) -} - -// Root hash is available in info.RootHash -// AND has been saved to /tmp/root-hash.txt -fmt.Printf("Root hash: %s\n", info.RootHash) -``` - -## Benefits - -1. **Automation-Friendly**: Eliminates need to parse stdout -2. **Secure Storage**: Hash can be written directly to secure location -3. **Scripting**: Easier to use in shell scripts and CI/CD pipelines -4. **Persistence**: Hash is saved even if program crashes after formatting - -## Implementation Details - -### When RootHashFile is Specified - -1. `--root-hash-file ` is passed to veritysetup -2. veritysetup writes the root hash (hex-encoded) to the specified file -3. After formatting completes, we read the file to populate `FormatOutputInfo.RootHash` -4. The hash is validated using `ValidateRootHash()` - -### File Format - -The file contains the root hash as a hex-encoded string (no newlines or whitespace at the end after trimming): - -``` -4d8f71f8a5b9c3e2f7a1d6b4e9c8a7f5d2e1b3c4a5e7d9f8b6c3a2e1f7d8b9a4 -``` - -For SHA-256: 64 hex characters (32 bytes) -For SHA-512: 128 hex characters (64 bytes) - -### Fallback Behavior - -If `RootHashFile` is NOT specified: -- Root hash is parsed from stdout (existing behavior) -- `ParseFormatOutput()` extracts it from the "Root hash:" line - -If `RootHashFile` IS specified: -- Root hash may not appear in stdout -- We read it from the file after formatting -- Both sources are validated with `ValidateRootHash()` - -## Error Handling - -Possible errors when using RootHashFile: - -```go -// File cannot be created (permissions, path doesn't exist) -"failed to format dm-verity device: ..." - -// File cannot be read after formatting -"failed to read root hash from file \"/path\": ..." - -// File contains invalid hash -"root hash from file is invalid: must be a valid hex string" -``` - -## Security Considerations - -1. **File Permissions**: Ensure the target file has appropriate permissions -2. **Path Validation**: Validate the path before passing to Format() -3. **Atomic Writes**: veritysetup writes atomically, but cleanup on failure is caller's responsibility -4. **Cleanup**: Consider removing the file if Format() fails - -## Example: Secure Usage - -```go -// Create a secure temporary file -tmpFile, err := os.CreateTemp("", "root-hash-*.txt") -if err != nil { - return err -} -defer os.Remove(tmpFile.Name()) // Cleanup on any error -tmpFile.Close() - -// Use it for root hash -opts := dmverity.DefaultDmverityOptions() -opts.RootHashFile = tmpFile.Name() - -info, err := dmverity.Format(dataDevice, hashDevice, &opts) -if err != nil { - return fmt.Errorf("format failed: %w", err) -} - -// Move to permanent location if needed -if err := os.Rename(tmpFile.Name(), "/secure/location/root-hash"); err != nil { - return err -} -``` - -## Compatibility - -- Requires veritysetup version 1.0 or later -- Option is ignored for `Open()` and `Close()` commands (format-only) -- Works with or without `--no-superblock` flag -- Compatible with all hash algorithms (sha256, sha512, sha1) - -## Testing - -When testing, verify: -1. File is created with correct permissions -2. File contains valid hex-encoded hash -3. Hash in file matches hash in FormatOutputInfo -4. File is readable after formatting -5. Error handling when file path is invalid diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index 7759730b6358d..a78a96ad9586c 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -125,8 +125,6 @@ func ValidateRootHash(rootHash string) error { type FormatOutputInfo struct { // Basic dm-verity options, reused from DmverityOptions DmverityOptions - // Number of hash blocks in the hash area - HashBlocks int64 // Root hash value for verification RootHash string } @@ -180,12 +178,6 @@ func ParseFormatOutput(output string, opts *DmverityOptions) (*FormatOutputInfo, return nil, fmt.Errorf("failed to parse data blocks %q: %w", value, err) } info.DataBlocks = uint64(dataBlocks) - case "Hash blocks": - hashBlocks, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, fmt.Errorf("failed to parse hash blocks %q: %w", value, err) - } - info.HashBlocks = hashBlocks case "Hash type": hashType, err := strconv.Atoi(value) if err != nil { diff --git a/internal/dmverity/dmverity_test.go b/internal/dmverity/dmverity_test.go index 11594a0b1620b..aee2815ba74ad 100644 --- a/internal/dmverity/dmverity_test.go +++ b/internal/dmverity/dmverity_test.go @@ -19,6 +19,7 @@ package dmverity import ( + "bytes" "os" "testing" @@ -35,69 +36,101 @@ const ( func TestDMVerity(t *testing.T) { testutil.RequiresRoot(t) - // Skip if dm-verity is not supported supported, err := IsSupported() if !supported || err != nil { t.Skipf("dm-verity is not supported on this system: %v", err) } - // Create temp directory for test files tempDir := t.TempDir() - - // Create a single loop device for both data and hash - loopImage, loopDevice := createLoopbackDevice(t, tempDir, "1Mb") - + _, loopDevice := createLoopbackDevice(t, tempDir, "1Mb") defer func() { - err := mount.DetachLoopDevice(loopDevice) - assert.NoError(t, err, "failed to detach loop device for image: %s", loopImage) + assert.NoError(t, mount.DetachLoopDevice(loopDevice)) }() - // Create default options for the tests opts := DmverityOptions{ - HashAlgorithm: "sha256", Salt: "0000000000000000000000000000000000000000000000000000000000000000", + HashAlgorithm: "sha256", DataBlockSize: 4096, HashBlockSize: 4096, + DataBlocks: 256, + HashOffset: 1048576, HashType: 1, UseSuperblock: true, Debug: false, - DataBlocks: 256, - HashOffset: 1048576, } t.Run("IsSupported", func(t *testing.T) { supported, err := IsSupported() - assert.True(t, supported, "dm-verity should be supported") - assert.NoError(t, err, "IsSupported should not return an error") + assert.True(t, supported) + assert.NoError(t, err) }) var formatInfo *FormatOutputInfo t.Run("Format", func(t *testing.T) { var err error - // Use the same device for both data and hash formatInfo, err = Format(loopDevice, loopDevice, &opts) - assert.NoError(t, err, "failed to format dm-verity device") - assert.NotEmpty(t, formatInfo.RootHash, "root hash should not be empty") - t.Logf("Root hash: %s", formatInfo.RootHash) + assert.NoError(t, err) + assert.NotEmpty(t, formatInfo.RootHash) + assert.NotEmpty(t, formatInfo.Salt) + assert.Equal(t, "sha256", formatInfo.HashAlgorithm) + assert.Equal(t, uint32(4096), formatInfo.DataBlockSize) + assert.Equal(t, uint32(4096), formatInfo.HashBlockSize) + }) + + t.Run("Format_WithRootHashFile", func(t *testing.T) { + rootHashFile, err := os.CreateTemp(tempDir, "root-hash-*.txt") + assert.NoError(t, err) + rootHashFilePath := rootHashFile.Name() + rootHashFile.Close() + defer os.Remove(rootHashFilePath) + + _, loopDevice2 := createLoopbackDevice(t, tempDir, "1Mb") + defer func() { + assert.NoError(t, mount.DetachLoopDevice(loopDevice2)) + }() + + optsWithFile := opts + optsWithFile.RootHashFile = rootHashFilePath + + formatInfo, err := Format(loopDevice2, loopDevice2, &optsWithFile) + assert.NoError(t, err) + assert.NotEmpty(t, formatInfo.RootHash) + + fileContent, err := os.ReadFile(rootHashFilePath) + assert.NoError(t, err) + fileHash := string(bytes.TrimSpace(fileContent)) + assert.Equal(t, formatInfo.RootHash, fileHash) + }) + + t.Run("Format_NoSuperblock", func(t *testing.T) { + _, loopDevice3 := createLoopbackDevice(t, tempDir, "1Mb") + defer func() { + assert.NoError(t, mount.DetachLoopDevice(loopDevice3)) + }() + + optsNoSuperblock := opts + optsNoSuperblock.UseSuperblock = false + + formatInfo, err := Format(loopDevice3, loopDevice3, &optsNoSuperblock) + assert.NoError(t, err) + assert.NotEmpty(t, formatInfo.RootHash) }) t.Run("Open", func(t *testing.T) { - output, err := Open(loopDevice, testDeviceName, loopDevice, formatInfo.RootHash, &opts) - assert.NoError(t, err, "failed to open dm-verity device") - t.Logf("Open output: %s", output) + _, err := Open(loopDevice, testDeviceName, loopDevice, formatInfo.RootHash, &opts) + assert.NoError(t, err) _, err = os.Stat("/dev/mapper/" + testDeviceName) - assert.NoError(t, err, "device should exist in /dev/mapper") + assert.NoError(t, err) }) t.Run("Close", func(t *testing.T) { - output, err := Close(testDeviceName) - assert.NoError(t, err, "failed to close dm-verity device") - t.Logf("Close output: %s", output) + _, err := Close(testDeviceName) + assert.NoError(t, err) _, err = os.Stat("/dev/mapper/" + testDeviceName) - assert.True(t, os.IsNotExist(err), "device should not exist after closing") + assert.True(t, os.IsNotExist(err)) }) } @@ -121,3 +154,305 @@ func createLoopbackDevice(t *testing.T, dir string, size string) (string, string return imagePath, loopDevice } + +func TestValidateOptions(t *testing.T) { + tests := []struct { + name string + opts *DmverityOptions + wantErr bool + errMsg string + }{ + { + name: "nil options", + opts: nil, + wantErr: true, + errMsg: "options cannot be nil", + }, + { + name: "valid options", + opts: func() *DmverityOptions { o := DefaultDmverityOptions(); return &o }(), + wantErr: false, + }, + { + name: "invalid data block size", + opts: func() *DmverityOptions { + o := DefaultDmverityOptions() + o.DataBlockSize = 4097 // Not a power of 2 + return &o + }(), + wantErr: true, + errMsg: "data block size", + }, + { + name: "invalid hash block size", + opts: func() *DmverityOptions { + o := DefaultDmverityOptions() + o.HashBlockSize = 4097 // Not a power of 2 + return &o + }(), + wantErr: true, + errMsg: "hash block size", + }, + { + name: "invalid salt hex", + opts: func() *DmverityOptions { + o := DefaultDmverityOptions() + o.Salt = "not-a-hex-string" + return &o + }(), + wantErr: true, + errMsg: "salt must be a valid hex string", + }, + { + name: "empty salt allowed", + opts: func() *DmverityOptions { + o := DefaultDmverityOptions() + o.Salt = "" + return &o + }(), + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateOptions(tt.opts) + if tt.wantErr { + assert.Error(t, err) + if tt.errMsg != "" { + assert.Contains(t, err.Error(), tt.errMsg) + } + } else { + assert.NoError(t, err) + } + }) + } + + // Test multiple valid power-of-2 sizes + t.Run("valid power of 2 sizes", func(t *testing.T) { + validSizes := []uint32{512, 1024, 2048, 4096, 8192} + for _, size := range validSizes { + opts := DefaultDmverityOptions() + opts.DataBlockSize = size + opts.HashBlockSize = size + err := ValidateOptions(&opts) + assert.NoError(t, err, "size %d should be valid", size) + } + }) +} + +func TestValidateRootHash(t *testing.T) { + tests := []struct { + name string + hash string + wantErr bool + errMsg string + }{ + { + name: "empty root hash", + hash: "", + wantErr: true, + errMsg: "root hash cannot be empty", + }, + { + name: "valid root hash", + hash: "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", + wantErr: false, + }, + { + name: "invalid hex characters", + hash: "not-a-valid-hex-hash", + wantErr: true, + errMsg: "root hash must be a valid hex string", + }, + { + name: "odd length hex", + hash: "abc", + wantErr: true, + errMsg: "root hash must be a valid hex string", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateRootHash(tt.hash) + if tt.wantErr { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestParseFormatOutput(t *testing.T) { + validOutput := `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: 256 +Data block size: 4096 +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807` + + tests := []struct { + name string + output string + opts *DmverityOptions + wantErr bool + errMsg string + check func(*testing.T, *FormatOutputInfo) + }{ + { + name: "empty output", + output: "", + wantErr: true, + errMsg: "output is empty", + }, + { + name: "valid output", + output: validOutput, + wantErr: false, + check: func(t *testing.T, info *FormatOutputInfo) { + assert.Equal(t, "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", info.RootHash) + assert.Equal(t, "sha256", info.HashAlgorithm) + assert.Equal(t, uint32(4096), info.DataBlockSize) + assert.Equal(t, uint32(4096), info.HashBlockSize) + assert.Equal(t, uint64(256), info.DataBlocks) + assert.Equal(t, uint32(1), info.HashType) + }, + }, + { + name: "missing root hash", + output: `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: 256 +Data block size: 4096 +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000`, + wantErr: true, + errMsg: "root hash", + }, + { + name: "invalid root hash", + output: `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: 256 +Data block size: 4096 +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: not-a-valid-hex-hash`, + wantErr: true, + errMsg: "root hash is invalid", + }, + { + name: "invalid data block size", + output: `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: 256 +Data block size: not-a-number +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, + wantErr: true, + errMsg: "failed to parse data block size", + }, + { + name: "invalid hash block size", + output: `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: 256 +Data block size: 4096 +Hash block size: not-a-number +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, + wantErr: true, + errMsg: "failed to parse hash block size", + }, + { + name: "invalid data blocks", + output: `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: not-a-number +Data block size: 4096 +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, + wantErr: true, + errMsg: "failed to parse data blocks", + }, + { + name: "invalid hash type", + output: `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: not-a-number +Data blocks: 256 +Data block size: 4096 +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, + wantErr: true, + errMsg: "failed to parse hash type", + }, + { + name: "root hash from nonexistent file", + output: `VERITY header information for /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: 256 +Data block size: 4096 +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000`, + opts: &DmverityOptions{RootHashFile: "/nonexistent/path/to/hash/file"}, + wantErr: true, + errMsg: "failed to read root hash from file", + }, + { + name: "skips header lines", + output: `VERITY header information for /dev/loop0 +# veritysetup format /dev/loop0 /dev/loop0 +UUID: eebbec4a-a914-4089-aca0-22266b21bd2b +Hash type: 1 +Data blocks: 256 +Data block size: 4096 +Hash block size: 4096 +Hash algorithm: sha256 +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, + wantErr: false, + check: func(t *testing.T, info *FormatOutputInfo) { + assert.Equal(t, "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", info.RootHash) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + info, err := ParseFormatOutput(tt.output, tt.opts) + if tt.wantErr { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + assert.NoError(t, err) + assert.NotNil(t, info) + if tt.check != nil { + tt.check(t, info) + } + } + }) + } +} From bf1a08311c02237f5ef2c3690e132c3635787686 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Sat, 11 Oct 2025 00:09:07 +0000 Subject: [PATCH 09/25] Each command has specific options --- internal/dmverity/dmverity_linux.go | 97 +++++++++++++++++------------ 1 file changed, 57 insertions(+), 40 deletions(-) diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index cd6648c7220b4..901e6df621a57 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -59,50 +59,67 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri return "", fmt.Errorf("invalid dm-verity options: %w", err) } - if opts.Salt != "" { - cmdArgs = append(cmdArgs, "-s", opts.Salt) - } - - if opts.HashAlgorithm != "" { - cmdArgs = append(cmdArgs, "--hash="+opts.HashAlgorithm) - } - - if opts.DataBlockSize > 0 { - cmdArgs = append(cmdArgs, fmt.Sprintf("--data-block-size=%d", opts.DataBlockSize)) - } - - if opts.HashBlockSize > 0 { - cmdArgs = append(cmdArgs, fmt.Sprintf("--hash-block-size=%d", opts.HashBlockSize)) - } - - if opts.DataBlocks > 0 { - cmdArgs = append(cmdArgs, "--data-blocks", fmt.Sprintf("%d", opts.DataBlocks)) - } - - if opts.HashOffset > 0 { - cmdArgs = append(cmdArgs, "--hash-offset", fmt.Sprintf("%d", opts.HashOffset)) - } - - if opts.HashType == 0 { - cmdArgs = append(cmdArgs, fmt.Sprintf("--format=%d", opts.HashType)) - } - - if !opts.UseSuperblock { - cmdArgs = append(cmdArgs, "--no-superblock") - } - + // Apply options based on command type according to veritysetup man page + switch cmd { + case FormatCommand: + // FORMAT options: --hash, --no-superblock, --format, --data-block-size, + // --hash-block-size, --data-blocks, --hash-offset, --salt, --uuid, --root-hash-file + if opts.Salt != "" { + cmdArgs = append(cmdArgs, "-s", opts.Salt) + } + if opts.HashAlgorithm != "" { + cmdArgs = append(cmdArgs, "--hash="+opts.HashAlgorithm) + } + if opts.DataBlockSize > 0 { + cmdArgs = append(cmdArgs, fmt.Sprintf("--data-block-size=%d", opts.DataBlockSize)) + } + if opts.HashBlockSize > 0 { + cmdArgs = append(cmdArgs, fmt.Sprintf("--hash-block-size=%d", opts.HashBlockSize)) + } + if opts.DataBlocks > 0 { + cmdArgs = append(cmdArgs, "--data-blocks", fmt.Sprintf("%d", opts.DataBlocks)) + } + if opts.HashOffset > 0 { + cmdArgs = append(cmdArgs, "--hash-offset", fmt.Sprintf("%d", opts.HashOffset)) + } + if opts.HashType == 0 { + cmdArgs = append(cmdArgs, fmt.Sprintf("--format=%d", opts.HashType)) + } + if !opts.UseSuperblock { + cmdArgs = append(cmdArgs, "--no-superblock") + } + if opts.UUID != "" { + cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) + } + if opts.RootHashFile != "" { + cmdArgs = append(cmdArgs, "--root-hash-file", opts.RootHashFile) + } + + case OpenCommand: + // OPEN options: --hash-offset, --no-superblock, --root-hash-file + // (ignoring advanced options we don't have: --ignore-corruption, --restart-on-corruption, + // --panic-on-corruption, --ignore-zero-blocks, --check-at-most-once, + // --root-hash-signature, --use-tasklets, --shared) + if opts.HashOffset > 0 { + cmdArgs = append(cmdArgs, "--hash-offset", fmt.Sprintf("%d", opts.HashOffset)) + } + if !opts.UseSuperblock { + cmdArgs = append(cmdArgs, "--no-superblock") + } + if opts.RootHashFile != "" { + cmdArgs = append(cmdArgs, "--root-hash-file", opts.RootHashFile) + } + + case CloseCommand: + // CLOSE has minimal options (--deferred, --cancel-deferred not implemented) + // No options from DmverityOptions apply to close + } + + // Debug is not command-specific, can be used with any command if opts.Debug { cmdArgs = append(cmdArgs, "--debug") } - if opts.UUID != "" { - cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) - } - - if opts.RootHashFile != "" { - cmdArgs = append(cmdArgs, "--root-hash-file", opts.RootHashFile) - } - cmdArgs = append(cmdArgs, args...) execCmd := exec.Command("veritysetup", cmdArgs...) From 99a8f9cceee07ed11192864c2af2dcc0e11f8d74 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Thu, 16 Oct 2025 01:07:42 +0000 Subject: [PATCH 10/25] Unify the options usage and prevent localization issues --- internal/dmverity/dmverity.go | 3 +++ internal/dmverity/dmverity_linux.go | 18 +++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index a78a96ad9586c..12e837086a5b7 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -131,6 +131,9 @@ type FormatOutputInfo struct { // ParseFormatOutput parses the output from veritysetup format command // and returns a structured representation of the information. +// +// Note: This function expects English output. The calling code ensures veritysetup +// runs with LC_ALL=C and LANG=C to prevent localization issues. func ParseFormatOutput(output string, opts *DmverityOptions) (*FormatOutputInfo, error) { if output == "" { return nil, fmt.Errorf("output is empty") diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index 901e6df621a57..fe7773278ffa2 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -65,10 +65,10 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri // FORMAT options: --hash, --no-superblock, --format, --data-block-size, // --hash-block-size, --data-blocks, --hash-offset, --salt, --uuid, --root-hash-file if opts.Salt != "" { - cmdArgs = append(cmdArgs, "-s", opts.Salt) + cmdArgs = append(cmdArgs, fmt.Sprintf("--salt=%s", opts.Salt)) } if opts.HashAlgorithm != "" { - cmdArgs = append(cmdArgs, "--hash="+opts.HashAlgorithm) + cmdArgs = append(cmdArgs, fmt.Sprintf("--hash=%s", opts.HashAlgorithm)) } if opts.DataBlockSize > 0 { cmdArgs = append(cmdArgs, fmt.Sprintf("--data-block-size=%d", opts.DataBlockSize)) @@ -77,10 +77,10 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri cmdArgs = append(cmdArgs, fmt.Sprintf("--hash-block-size=%d", opts.HashBlockSize)) } if opts.DataBlocks > 0 { - cmdArgs = append(cmdArgs, "--data-blocks", fmt.Sprintf("%d", opts.DataBlocks)) + cmdArgs = append(cmdArgs, fmt.Sprintf("--data-blocks=%d", opts.DataBlocks)) } if opts.HashOffset > 0 { - cmdArgs = append(cmdArgs, "--hash-offset", fmt.Sprintf("%d", opts.HashOffset)) + cmdArgs = append(cmdArgs, fmt.Sprintf("--hash-offset=%d", opts.HashOffset)) } if opts.HashType == 0 { cmdArgs = append(cmdArgs, fmt.Sprintf("--format=%d", opts.HashType)) @@ -92,7 +92,7 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri cmdArgs = append(cmdArgs, fmt.Sprintf("--uuid=%s", opts.UUID)) } if opts.RootHashFile != "" { - cmdArgs = append(cmdArgs, "--root-hash-file", opts.RootHashFile) + cmdArgs = append(cmdArgs, fmt.Sprintf("--root-hash-file=%s", opts.RootHashFile)) } case OpenCommand: @@ -101,13 +101,13 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri // --panic-on-corruption, --ignore-zero-blocks, --check-at-most-once, // --root-hash-signature, --use-tasklets, --shared) if opts.HashOffset > 0 { - cmdArgs = append(cmdArgs, "--hash-offset", fmt.Sprintf("%d", opts.HashOffset)) + cmdArgs = append(cmdArgs, fmt.Sprintf("--hash-offset=%d", opts.HashOffset)) } if !opts.UseSuperblock { cmdArgs = append(cmdArgs, "--no-superblock") } if opts.RootHashFile != "" { - cmdArgs = append(cmdArgs, "--root-hash-file", opts.RootHashFile) + cmdArgs = append(cmdArgs, fmt.Sprintf("--root-hash-file=%s", opts.RootHashFile)) } case CloseCommand: @@ -123,6 +123,10 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri cmdArgs = append(cmdArgs, args...) execCmd := exec.Command("veritysetup", cmdArgs...) + // Force C locale to ensure consistent, non-localized output that we can parse reliably + // This prevents localization issues where field names like "Root hash", "Salt", etc. + // might be translated to other languages, breaking our text parsing + execCmd.Env = append(os.Environ(), "LC_ALL=C", "LANG=C") output, err := execCmd.CombinedOutput() if err != nil { return "", fmt.Errorf("veritysetup %s failed: %w, output: %s", cmd, err, string(output)) From be93f84f8d3120b74cc57d3855d888b3f3db8a8e Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Thu, 16 Oct 2025 19:03:36 +0000 Subject: [PATCH 11/25] Execute command with context to prevent goroutine leak, better handle error and timeout --- internal/dmverity/dmverity_linux.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index fe7773278ffa2..7fe7de77ed416 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -18,9 +18,17 @@ package dmverity import ( "bytes" + "context" "fmt" "os" "os/exec" + "time" +) + +const ( + // veritysetupTimeout is the maximum time allowed for a veritysetup command to complete + // Format operations can take time for large devices, but should complete within 5 minutes + veritysetupTimeout = 5 * time.Minute ) func IsSupported() (bool, error) { @@ -122,7 +130,10 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri cmdArgs = append(cmdArgs, args...) - execCmd := exec.Command("veritysetup", cmdArgs...) + ctx, cancel := context.WithTimeout(context.Background(), veritysetupTimeout) + defer cancel() + + execCmd := exec.CommandContext(ctx, "veritysetup", cmdArgs...) // Force C locale to ensure consistent, non-localized output that we can parse reliably // This prevents localization issues where field names like "Root hash", "Salt", etc. // might be translated to other languages, breaking our text parsing From 6116d154eaacfcaf18a538c31610f0529b12c2a7 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Thu, 16 Oct 2025 21:48:39 +0000 Subject: [PATCH 12/25] Load dm_verity module --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9870d9483b78a..630eb4c49436e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -437,6 +437,10 @@ jobs: run: | sudo modprobe erofs + - name: Load dm-verity kernel module + run: | + sudo modprobe dm_verity + - name: Install containerd env: CGO_ENABLED: 1 From c004710979cb1dd4bcc6907e8c24dc6f45fb5245 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Thu, 16 Oct 2025 22:18:04 +0000 Subject: [PATCH 13/25] Verify veritysetup version --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 630eb4c49436e..5f136ccc15a83 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -441,6 +441,10 @@ jobs: run: | sudo modprobe dm_verity + - name: Verify veritysetup version + run: | + veritysetup --version + - name: Install containerd env: CGO_ENABLED: 1 From 26d9ceaa881f722ca50bde8aa6388958ee82aaec Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Mon, 20 Oct 2025 17:53:52 +0000 Subject: [PATCH 14/25] We only use the roothash from the veritysetup format command --- internal/dmverity/dmverity.go | 116 +++++++++----------------- internal/dmverity/dmverity_linux.go | 18 ++--- internal/dmverity/dmverity_other.go | 4 +- internal/dmverity/dmverity_test.go | 121 ++++++++++------------------ 4 files changed, 90 insertions(+), 169 deletions(-) diff --git a/internal/dmverity/dmverity.go b/internal/dmverity/dmverity.go index 12e837086a5b7..eec65afd7c027 100644 --- a/internal/dmverity/dmverity.go +++ b/internal/dmverity/dmverity.go @@ -23,8 +23,9 @@ import ( "encoding/hex" "fmt" "os" - "strconv" "strings" + + "github.com/containerd/log" ) // VeritySetupCommand represents the type of veritysetup command to execute @@ -121,98 +122,53 @@ func ValidateRootHash(rootHash string) error { return nil } -// FormatOutputInfo represents the parsed information from veritysetup format command output -type FormatOutputInfo struct { - // Basic dm-verity options, reused from DmverityOptions - DmverityOptions - // Root hash value for verification - RootHash string -} - -// ParseFormatOutput parses the output from veritysetup format command -// and returns a structured representation of the information. +// ExtractRootHash extracts the root hash from veritysetup format command output. +// It first attempts to read from the root hash file (if specified in opts.RootHashFile), +// then falls back to parsing the stdout output. // -// Note: This function expects English output. The calling code ensures veritysetup -// runs with LC_ALL=C and LANG=C to prevent localization issues. -func ParseFormatOutput(output string, opts *DmverityOptions) (*FormatOutputInfo, error) { - if output == "" { - return nil, fmt.Errorf("output is empty") - } +// Note: This function expects English output when parsing stdout. The calling code +// ensures veritysetup runs with LC_ALL=C and LANG=C to prevent localization issues. +func ExtractRootHash(output string, opts *DmverityOptions) (string, error) { + log.L.Debugf("veritysetup format output:\n%s", output) - info := &FormatOutputInfo{} + var rootHash string - // Parse stdout output line by line - scanner := bufio.NewScanner(strings.NewReader(output)) - for scanner.Scan() { - line := scanner.Text() - // Skip the header line and command echo line - if strings.HasPrefix(line, "VERITY header") || strings.HasPrefix(line, "# veritysetup") { - continue + // Try to read from root hash file first (if specified) + if opts != nil && opts.RootHashFile != "" { + hashBytes, err := os.ReadFile(opts.RootHashFile) + if err != nil { + return "", fmt.Errorf("failed to read root hash from file %q: %w", opts.RootHashFile, err) } - - parts := strings.Split(line, ":") - if len(parts) != 2 { - continue + // Trim any whitespace/newlines + rootHash = string(bytes.TrimSpace(hashBytes)) + } else { + // Parse stdout output to find the root hash + if output == "" { + return "", fmt.Errorf("output is empty") } - key := strings.TrimSpace(parts[0]) - value := strings.TrimSpace(parts[1]) - - switch key { - case "Salt": - info.Salt = value - case "Hash algorithm": - info.HashAlgorithm = value - case "Data block size": - dataBlockSize, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, fmt.Errorf("failed to parse data block size %q: %w", value, err) - } - info.DataBlockSize = uint32(dataBlockSize) - case "Hash block size": - hashBlockSize, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, fmt.Errorf("failed to parse hash block size %q: %w", value, err) + scanner := bufio.NewScanner(strings.NewReader(output)) + for scanner.Scan() { + line := scanner.Text() + // Look for the "Root hash:" line + if strings.HasPrefix(line, "Root hash:") { + parts := strings.Split(line, ":") + if len(parts) == 2 { + rootHash = strings.TrimSpace(parts[1]) + break + } } - info.HashBlockSize = uint32(hashBlockSize) - case "Data blocks": - dataBlocks, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, fmt.Errorf("failed to parse data blocks %q: %w", value, err) - } - info.DataBlocks = uint64(dataBlocks) - case "Hash type": - hashType, err := strconv.Atoi(value) - if err != nil { - return nil, fmt.Errorf("failed to parse hash type %q: %w", value, err) - } - info.HashType = uint32(hashType) - case "UUID": - info.UUID = value - case "Root hash": - info.RootHash = value } - } - - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("error scanning output: %w", err) - } - // If a root hash file was specified and we haven't found a root hash in stdout, - // read it from the file (veritysetup writes it there instead of stdout when using --root-hash-file) - if opts != nil && opts.RootHashFile != "" && info.RootHash == "" { - hashBytes, err := os.ReadFile(opts.RootHashFile) - if err != nil { - return nil, fmt.Errorf("failed to read root hash from file %q: %w", opts.RootHashFile, err) + if err := scanner.Err(); err != nil { + return "", fmt.Errorf("error scanning output: %w", err) } - // Trim any whitespace/newlines - info.RootHash = string(bytes.TrimSpace(hashBytes)) } // Validate root hash - if err := ValidateRootHash(info.RootHash); err != nil { - return nil, fmt.Errorf("root hash is invalid: %w", err) + if err := ValidateRootHash(rootHash); err != nil { + return "", fmt.Errorf("root hash is invalid: %w", err) } - return info, nil + return rootHash, nil } diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index 7fe7de77ed416..505c8270d4d16 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -146,23 +146,23 @@ func actions(cmd VeritySetupCommand, args []string, opts *DmverityOptions) (stri return string(output), nil } -// Format creates a dm-verity hash for a data device -// If hashDevice is the same as dataDevice, the hash will be stored on the same device -func Format(dataDevice, hashDevice string, opts *DmverityOptions) (*FormatOutputInfo, error) { +// Format creates a dm-verity hash for a data device and returns the root hash. +// If hashDevice is the same as dataDevice, the hash will be stored on the same device. +func Format(dataDevice, hashDevice string, opts *DmverityOptions) (string, error) { args := []string{dataDevice, hashDevice} output, err := actions(FormatCommand, args, opts) if err != nil { - return nil, fmt.Errorf("failed to format dm-verity device: %w, output: %s", err, output) + return "", fmt.Errorf("failed to format dm-verity device: %w, output: %s", err, output) } - // Parse the output to extract structured information - // Pass opts so ParseFormatOutput can read root hash from file if RootHashFile was specified - info, err := ParseFormatOutput(output, opts) + // Extract the root hash from the format output + // Pass opts so ExtractRootHash can read root hash from file if RootHashFile was specified + rootHash, err := ExtractRootHash(output, opts) if err != nil { - return nil, fmt.Errorf("failed to parse format output: %w", err) + return "", fmt.Errorf("failed to extract root hash: %w", err) } - return info, nil + return rootHash, nil } // Open creates a read-only device-mapper target for transparent integrity verification diff --git a/internal/dmverity/dmverity_other.go b/internal/dmverity/dmverity_other.go index 92caa0a2e8bbd..c62e55335e1b9 100644 --- a/internal/dmverity/dmverity_other.go +++ b/internal/dmverity/dmverity_other.go @@ -26,8 +26,8 @@ func IsSupported() (bool, error) { return false, errUnsupported } -func Format(_ string, _ string, _ *DmverityOptions) (*FormatOutputInfo, error) { - return nil, errUnsupported +func Format(_ string, _ string, _ *DmverityOptions) (string, error) { + return "", errUnsupported } func Open(_ string, _ string, _ string, _ string, _ *DmverityOptions) (string, error) { diff --git a/internal/dmverity/dmverity_test.go b/internal/dmverity/dmverity_test.go index aee2815ba74ad..79977846571d4 100644 --- a/internal/dmverity/dmverity_test.go +++ b/internal/dmverity/dmverity_test.go @@ -65,17 +65,13 @@ func TestDMVerity(t *testing.T) { assert.NoError(t, err) }) - var formatInfo *FormatOutputInfo + var rootHash string t.Run("Format", func(t *testing.T) { var err error - formatInfo, err = Format(loopDevice, loopDevice, &opts) + rootHash, err = Format(loopDevice, loopDevice, &opts) assert.NoError(t, err) - assert.NotEmpty(t, formatInfo.RootHash) - assert.NotEmpty(t, formatInfo.Salt) - assert.Equal(t, "sha256", formatInfo.HashAlgorithm) - assert.Equal(t, uint32(4096), formatInfo.DataBlockSize) - assert.Equal(t, uint32(4096), formatInfo.HashBlockSize) + assert.NotEmpty(t, rootHash) }) t.Run("Format_WithRootHashFile", func(t *testing.T) { @@ -93,14 +89,14 @@ func TestDMVerity(t *testing.T) { optsWithFile := opts optsWithFile.RootHashFile = rootHashFilePath - formatInfo, err := Format(loopDevice2, loopDevice2, &optsWithFile) + rootHashFromFormat, err := Format(loopDevice2, loopDevice2, &optsWithFile) assert.NoError(t, err) - assert.NotEmpty(t, formatInfo.RootHash) + assert.NotEmpty(t, rootHashFromFormat) fileContent, err := os.ReadFile(rootHashFilePath) assert.NoError(t, err) fileHash := string(bytes.TrimSpace(fileContent)) - assert.Equal(t, formatInfo.RootHash, fileHash) + assert.Equal(t, rootHashFromFormat, fileHash) }) t.Run("Format_NoSuperblock", func(t *testing.T) { @@ -112,13 +108,13 @@ func TestDMVerity(t *testing.T) { optsNoSuperblock := opts optsNoSuperblock.UseSuperblock = false - formatInfo, err := Format(loopDevice3, loopDevice3, &optsNoSuperblock) + rootHashNoSuperblock, err := Format(loopDevice3, loopDevice3, &optsNoSuperblock) assert.NoError(t, err) - assert.NotEmpty(t, formatInfo.RootHash) + assert.NotEmpty(t, rootHashNoSuperblock) }) t.Run("Open", func(t *testing.T) { - _, err := Open(loopDevice, testDeviceName, loopDevice, formatInfo.RootHash, &opts) + _, err := Open(loopDevice, testDeviceName, loopDevice, rootHash, &opts) assert.NoError(t, err) _, err = os.Stat("/dev/mapper/" + testDeviceName) @@ -286,7 +282,7 @@ func TestValidateRootHash(t *testing.T) { } } -func TestParseFormatOutput(t *testing.T) { +func TestExtractRootHash(t *testing.T) { validOutput := `VERITY header information for /dev/loop0 UUID: eebbec4a-a914-4089-aca0-22266b21bd2b Hash type: 1 @@ -303,7 +299,7 @@ Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae792 opts *DmverityOptions wantErr bool errMsg string - check func(*testing.T, *FormatOutputInfo) + check func(*testing.T, string) }{ { name: "empty output", @@ -315,13 +311,8 @@ Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae792 name: "valid output", output: validOutput, wantErr: false, - check: func(t *testing.T, info *FormatOutputInfo) { - assert.Equal(t, "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", info.RootHash) - assert.Equal(t, "sha256", info.HashAlgorithm) - assert.Equal(t, uint32(4096), info.DataBlockSize) - assert.Equal(t, uint32(4096), info.HashBlockSize) - assert.Equal(t, uint64(256), info.DataBlocks) - assert.Equal(t, uint32(1), info.HashType) + check: func(t *testing.T, rootHash string) { + assert.Equal(t, "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", rootHash) }, }, { @@ -352,63 +343,21 @@ Root hash: not-a-valid-hex-hash`, errMsg: "root hash is invalid", }, { - name: "invalid data block size", - output: `VERITY header information for /dev/loop0 -UUID: eebbec4a-a914-4089-aca0-22266b21bd2b -Hash type: 1 -Data blocks: 256 -Data block size: not-a-number -Hash block size: 4096 -Hash algorithm: sha256 -Salt: 0000000000000000000000000000000000000000000000000000000000000000 -Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, - wantErr: true, - errMsg: "failed to parse data block size", - }, - { - name: "invalid hash block size", - output: `VERITY header information for /dev/loop0 -UUID: eebbec4a-a914-4089-aca0-22266b21bd2b -Hash type: 1 -Data blocks: 256 -Data block size: 4096 -Hash block size: not-a-number -Hash algorithm: sha256 -Salt: 0000000000000000000000000000000000000000000000000000000000000000 -Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, - wantErr: true, - errMsg: "failed to parse hash block size", - }, - { - name: "invalid data blocks", + name: "root hash from nonexistent file", output: `VERITY header information for /dev/loop0 UUID: eebbec4a-a914-4089-aca0-22266b21bd2b Hash type: 1 -Data blocks: not-a-number -Data block size: 4096 -Hash block size: 4096 -Hash algorithm: sha256 -Salt: 0000000000000000000000000000000000000000000000000000000000000000 -Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, - wantErr: true, - errMsg: "failed to parse data blocks", - }, - { - name: "invalid hash type", - output: `VERITY header information for /dev/loop0 -UUID: eebbec4a-a914-4089-aca0-22266b21bd2b -Hash type: not-a-number Data blocks: 256 Data block size: 4096 Hash block size: 4096 Hash algorithm: sha256 -Salt: 0000000000000000000000000000000000000000000000000000000000000000 -Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, +Salt: 0000000000000000000000000000000000000000000000000000000000000000`, + opts: &DmverityOptions{RootHashFile: "/nonexistent/path/to/hash/file"}, wantErr: true, - errMsg: "failed to parse hash type", + errMsg: "failed to read root hash from file", }, { - name: "root hash from nonexistent file", + name: "root hash from file takes priority", output: `VERITY header information for /dev/loop0 UUID: eebbec4a-a914-4089-aca0-22266b21bd2b Hash type: 1 @@ -416,10 +365,26 @@ Data blocks: 256 Data block size: 4096 Hash block size: 4096 Hash algorithm: sha256 -Salt: 0000000000000000000000000000000000000000000000000000000000000000`, - opts: &DmverityOptions{RootHashFile: "/nonexistent/path/to/hash/file"}, - wantErr: true, - errMsg: "failed to read root hash from file", +Salt: 0000000000000000000000000000000000000000000000000000000000000000 +Root hash: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`, + opts: func() *DmverityOptions { + // Create a temporary file with a different hash + tmpFile, err := os.CreateTemp("", "root-hash-test-*.txt") + assert.NoError(t, err) + tmpFilePath := tmpFile.Name() + fileHash := "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807" + _, err = tmpFile.WriteString(fileHash) + assert.NoError(t, err) + tmpFile.Close() + // Cleanup will happen after the test runs + t.Cleanup(func() { os.Remove(tmpFilePath) }) + return &DmverityOptions{RootHashFile: tmpFilePath} + }(), + wantErr: false, + check: func(t *testing.T, rootHash string) { + // Should get the hash from file, not from output + assert.Equal(t, "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", rootHash) + }, }, { name: "skips header lines", @@ -434,23 +399,23 @@ Hash algorithm: sha256 Salt: 0000000000000000000000000000000000000000000000000000000000000000 Root hash: bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807`, wantErr: false, - check: func(t *testing.T, info *FormatOutputInfo) { - assert.Equal(t, "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", info.RootHash) + check: func(t *testing.T, rootHash string) { + assert.Equal(t, "bef46122f85025cf37061b16c04e2a19960a5bbcdbb656b5e91ae7927c0ad807", rootHash) }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - info, err := ParseFormatOutput(tt.output, tt.opts) + rootHash, err := ExtractRootHash(tt.output, tt.opts) if tt.wantErr { assert.Error(t, err) assert.Contains(t, err.Error(), tt.errMsg) } else { assert.NoError(t, err) - assert.NotNil(t, info) + assert.NotEmpty(t, rootHash) if tt.check != nil { - tt.check(t, info) + tt.check(t, rootHash) } } }) From 0f0041dc1d61d8852010682e79298f4b4633bc5f Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Mon, 20 Oct 2025 18:49:25 +0000 Subject: [PATCH 15/25] Open can also use root hash file --- internal/dmverity/dmverity_linux.go | 8 +++++- internal/dmverity/dmverity_test.go | 39 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/internal/dmverity/dmverity_linux.go b/internal/dmverity/dmverity_linux.go index 505c8270d4d16..ac31b8cbbd2b1 100644 --- a/internal/dmverity/dmverity_linux.go +++ b/internal/dmverity/dmverity_linux.go @@ -167,7 +167,13 @@ func Format(dataDevice, hashDevice string, opts *DmverityOptions) (string, error // Open creates a read-only device-mapper target for transparent integrity verification func Open(dataDevice string, name string, hashDevice string, rootHash string, opts *DmverityOptions) (string, error) { - args := []string{dataDevice, name, hashDevice, rootHash} + var args []string + // If RootHashFile is provided, use the alternate open syntax without root hash as command arg + if opts != nil && opts.RootHashFile != "" { + args = []string{dataDevice, name, hashDevice} + } else { + args = []string{dataDevice, name, hashDevice, rootHash} + } output, err := actions(OpenCommand, args, opts) if err != nil { return "", fmt.Errorf("failed to open dm-verity device: %w, output: %s", err, output) diff --git a/internal/dmverity/dmverity_test.go b/internal/dmverity/dmverity_test.go index 79977846571d4..0d7fb373dd689 100644 --- a/internal/dmverity/dmverity_test.go +++ b/internal/dmverity/dmverity_test.go @@ -121,6 +121,45 @@ func TestDMVerity(t *testing.T) { assert.NoError(t, err) }) + t.Run("Open_WithRootHashFile", func(t *testing.T) { + // Create a root hash file + rootHashFile, err := os.CreateTemp(tempDir, "root-hash-open-*.txt") + assert.NoError(t, err) + rootHashFilePath := rootHashFile.Name() + + // Write the root hash to the file + _, err = rootHashFile.WriteString(rootHash) + assert.NoError(t, err) + rootHashFile.Close() + defer os.Remove(rootHashFilePath) + + // Create a new loopback device for this test + _, loopDevice4 := createLoopbackDevice(t, tempDir, "1Mb") + defer func() { + assert.NoError(t, mount.DetachLoopDevice(loopDevice4)) + }() + + // Format the device first + optsFormat := opts + _, err = Format(loopDevice4, loopDevice4, &optsFormat) + assert.NoError(t, err) + + // Open with root hash file instead of command-line arg + optsOpen := opts + optsOpen.RootHashFile = rootHashFilePath + deviceName := "test-verity-roothashfile" + _, err = Open(loopDevice4, deviceName, loopDevice4, "", &optsOpen) + assert.NoError(t, err) + + // Verify device was created + _, err = os.Stat("/dev/mapper/" + deviceName) + assert.NoError(t, err) + + // Clean up + _, err = Close(deviceName) + assert.NoError(t, err) + }) + t.Run("Close", func(t *testing.T) { _, err := Close(testDeviceName) assert.NoError(t, err) From 8d0f6f82d16d36580005a34403a4d721ac20b86c Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Wed, 15 Oct 2025 22:14:19 +0000 Subject: [PATCH 16/25] Initial commit to integrate dm-verity support into the erofs snapshotter --- plugins/snapshots/erofs/erofs_linux.go | 255 ++++++++++++++++-- .../snapshots/erofs/plugin/plugin_linux.go | 8 +- 2 files changed, 234 insertions(+), 29 deletions(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index 5e254df7a3a3b..a6e5075c74536 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -23,19 +23,21 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "syscall" - - "github.com/containerd/continuity/fs" - "github.com/containerd/log" - "github.com/containerd/plugin" - "golang.org/x/sys/unix" + "time" "github.com/containerd/containerd/v2/core/mount" "github.com/containerd/containerd/v2/core/snapshots" "github.com/containerd/containerd/v2/core/snapshots/storage" + "github.com/containerd/containerd/v2/internal/dmverity" "github.com/containerd/containerd/v2/internal/erofsutils" "github.com/containerd/containerd/v2/internal/fsverity" + "github.com/containerd/continuity/fs" + "github.com/containerd/log" + "github.com/containerd/plugin" + "golang.org/x/sys/unix" ) // SnapshotterConfig is used to configure the erofs snapshotter instance @@ -44,8 +46,8 @@ type SnapshotterConfig struct { ovlOptions []string // enableFsverity enables fsverity for EROFS layers enableFsverity bool - // setImmutable enables IMMUTABLE_FL file attribute for EROFS layers - setImmutable bool + // enableDmverity enables dmverity for EROFS layers + enableDmverity bool } // Opt is an option to configure the erofs snapshotter @@ -65,10 +67,10 @@ func WithFsverity() Opt { } } -// WithImmutable enables IMMUTABLE_FL file attribute for EROFS layers -func WithImmutable() Opt { +// WithDmverity enables dmverity for EROFS layers +func WithDmverity() Opt { return func(config *SnapshotterConfig) { - config.setImmutable = true + config.enableDmverity = true } } @@ -83,7 +85,7 @@ type snapshotter struct { ms *storage.MetaStore ovlOptions []string enableFsverity bool - setImmutable bool + enableDmverity bool } // check if EROFS kernel filesystem is registered or not @@ -142,6 +144,16 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { } } + if config.enableDmverity { + supported, err := dmverity.IsSupported() + if err != nil { + return nil, fmt.Errorf("failed to check dmverity support on %q: %w", root, err) + } + if !supported { + return nil, fmt.Errorf("dmverity is not supported on the filesystem of %q", root) + } + } + ms, err := storage.NewMetaStore(filepath.Join(root, "metadata.db")) if err != nil { return nil, err @@ -156,12 +168,30 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { ms: ms, ovlOptions: config.ovlOptions, enableFsverity: config.enableFsverity, - setImmutable: config.setImmutable, + enableDmverity: config.enableDmverity, }, nil } // Close closes the snapshotter func (s *snapshotter) Close() error { + // If dmverity is enabled, try to close all devices + if s.enableDmverity { + // Get a list of all snapshots + err := s.ms.WithTransaction(context.Background(), false, func(ctx context.Context) error { + return storage.WalkInfo(ctx, func(ctx context.Context, info snapshots.Info) error { + if info.Kind == snapshots.KindCommitted { + // Close the device if it exists + if err := s.closeDmverityDevice(info.Name); err != nil { + log.L.WithError(err).Warnf("failed to close dmverity device for %v", info.Name) + } + } + return nil + }) + }) + if err != nil { + log.L.WithError(err).Warn("error closing dmverity devices") + } + } return s.ms.Close() } @@ -177,6 +207,94 @@ func (s *snapshotter) workPath(id string) string { func (s *snapshotter) layerBlobPath(id string) string { return filepath.Join(s.root, "snapshots", id, "layer.erofs") } +func (s *snapshotter) formatLayerBlob(id string) error { + layerBlob := s.layerBlobPath(id) + if _, err := os.Stat(layerBlob); err != nil { + return fmt.Errorf("failed to find valid erofs layer blob: %w", err) + } + if !s.isLayerWithDmverity(id) { + opts := dmverity.DefaultDmverityOptions() + fileinfo, err := os.Stat(layerBlob) + if err != nil { + return fmt.Errorf("failed to stat layer blob: %w", err) + } + + // Open file for truncating + f, err := os.OpenFile(layerBlob, os.O_RDWR, 0644) + if err != nil { + return fmt.Errorf("failed to open layer blob for truncating: %w", err) + } + defer f.Close() + file_size := fileinfo.Size() + // Truncate the file to double its size + if err := f.Truncate(file_size * 2); err != nil { + return fmt.Errorf("failed to truncate layer blob: %w", err) + } + opts.HashOffset = uint64(file_size) + info, err := dmverity.Format(layerBlob, layerBlob, &opts) + if err != nil { + return fmt.Errorf("failed to format dmverity: %w", err) + } + dmverityData := fmt.Sprintf("%s|%d", info.RootHash, fileinfo.Size()) + if err := os.WriteFile(filepath.Join(s.root, "snapshots", id, ".dmverity"), []byte(dmverityData), 0644); err != nil { + return fmt.Errorf("failed to write dmverity root hash: %w", err) + } + } + return nil +} +func (s *snapshotter) runDmverity(id string) (string, error) { + layerBlob := s.layerBlobPath(id) + if _, err := os.Stat(layerBlob); err != nil { + return "", fmt.Errorf("failed to find valid erofs layer blob: %w", err) + } + dmName := fmt.Sprintf("containerd-erofs-%s", id) + devicePath := fmt.Sprintf("/dev/mapper/%s", dmName) + if _, err := os.Stat(devicePath); err == nil { + status, err := dmverity.Status(dmName) + fmt.Println("dmverity device status: ", status) + if err != nil { + return "", fmt.Errorf("failed to get dmverity device status: %w", err) + } + if !status.IsVerified() { + return "", fmt.Errorf("dmverity device %s is not verified, status: %s", dmName, status.Status) + } + + return devicePath, nil + } + dmverityContent, err := os.ReadFile(filepath.Join(s.root, "snapshots", id, ".dmverity")) + if err != nil { + return "", fmt.Errorf("failed to read dmverity root hash: %w", err) + } + + parts := strings.Split(string(dmverityContent), "|") + rootHash := parts[0] + var originalSize uint64 + if len(parts) > 1 { + var err error + originalSize, err = strconv.ParseUint(parts[1], 10, 64) + if err != nil { + return "", fmt.Errorf("failed to parse original size: %w", err) + } + } + + if _, err := os.Stat(devicePath); err != nil { + fmt.Println("openning dmverity") + opts := dmverity.DefaultDmverityOptions() + opts.HashOffset = originalSize + _, err = dmverity.Open(layerBlob, dmName, layerBlob, string(rootHash), &opts) + if err != nil { + return "", fmt.Errorf("failed to open dmverity device: %w", err) + } + + for i := 0; i < 50; i++ { + if _, err := os.Stat(devicePath); err == nil { + break + } + time.Sleep(10 * time.Millisecond) + } + } + return devicePath, nil +} func (s *snapshotter) lowerPath(id string) (mount.Mount, string, error) { layerBlob := s.layerBlobPath(id) @@ -217,8 +335,12 @@ func (s *snapshotter) prepareDirectory(ctx context.Context, snapshotDir string, func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]mount.Mount, error) { var options []string + fmt.Println("mounts called, info :", info) + fmt.Println("snap: ", snap) if len(snap.ParentIDs) == 0 { - m, _, err := s.lowerPath(snap.ID) + fmt.Println("no parent ids") + m, mntpoint, err := s.lowerPath(snap.ID) + fmt.Printf("lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) if err == nil { if snap.Kind != snapshots.KindView { return nil, fmt.Errorf("only works for snapshots.KindView on a committed snapshot: %w", err) @@ -228,8 +350,17 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun return nil, err } } + fmt.Println("formatting layer blob m: ", m) + if s.enableDmverity { + if err := s.formatLayerBlob(snap.ID); err != nil { + return nil, err + } + } // We have to force a loop device here since mount[] is static. - m.Options = append(m.Options, "loop") + // However, if we're using dmverity, it's already a block device + if !strings.HasPrefix(m.Source, "/dev/mapper/") { + m.Options = append(m.Options, "loop") + } return []mount.Mount{m}, nil } // if we only have one layer/no parents then just return a bind mount as overlay @@ -250,34 +381,48 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun }, nil } + fmt.Println("snap.Kind", snap.Kind) if snap.Kind == snapshots.KindActive { options = append(options, fmt.Sprintf("workdir=%s", s.workPath(snap.ID)), fmt.Sprintf("upperdir=%s", s.upperPath(snap.ID)), ) } else if len(snap.ParentIDs) == 1 { - m, _, err := s.lowerPath(snap.ParentIDs[0]) + fmt.Println("len(snap.ParentIDs) == 1") + m, mntpoint, err := s.lowerPath(snap.ParentIDs[0]) if err != nil { return nil, err } + fmt.Printf("lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) + if s.enableDmverity { + if err := s.formatLayerBlob(snap.ParentIDs[0]); err != nil { + return nil, err + } + } + fmt.Printf("lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) // We have to force a loop device here too since mount[] is static. - m.Options = append(m.Options, "loop") + // However, if we're using dmverity, it's already a block device + if !strings.HasPrefix(m.Source, "/dev/mapper/") { + m.Options = append(m.Options, "loop") + } return []mount.Mount{m}, nil } + fmt.Println("snap.ParentIDs", snap.ParentIDs) var lowerdirs []string for i := range snap.ParentIDs { m, mntpoint, err := s.lowerPath(snap.ParentIDs[i]) if err != nil { return nil, err } - + fmt.Printf("active lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) // If the lowerdir is actually an EROFS committed layer but // doesn't have an EROFS mount. Let's recover now. - if mntpoint != m.Source && !isErofs(mntpoint) { + if !s.enableDmverity && mntpoint != m.Source && !isErofs(mntpoint) { err := m.Mount(mntpoint) // Use loop if the current kernel (6.12+) doesn't support file-backed mount - if errors.Is(err, unix.ENOTBLK) { + // Skip 'loop' if using dmverity device + if errors.Is(err, unix.ENOTBLK) && (!s.enableDmverity || !strings.HasPrefix(m.Source, "/dev/mapper/")) { m.Options = append(m.Options, "loop") err = m.Mount(mntpoint) } @@ -285,11 +430,33 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun return nil, err } } + if s.enableDmverity { + devicePath, err := s.runDmverity(snap.ParentIDs[i]) + if err != nil { + return nil, err + } + dmName := fmt.Sprintf("containerd-erofs-%s", snap.ParentIDs[i]) + if _, err := os.Stat(devicePath); err == nil { + status, err := dmverity.Status(dmName) + if err != nil { + return nil, fmt.Errorf("failed to get dmverity device status: %w", err) + } + m.Source = devicePath + if !status.IsInUse() { + err = m.Mount(mntpoint) + if err != nil { + return nil, err + } + } + } + + } lowerdirs = append(lowerdirs, mntpoint) } + fmt.Println("lowerdirs: ", lowerdirs) options = append(options, fmt.Sprintf("lowerdir=%s", strings.Join(lowerdirs, ":"))) options = append(options, s.ovlOptions...) - + fmt.Printf("options = %v\n", options) return []mount.Mount{{ Type: "overlay", Source: "overlay", @@ -370,8 +537,15 @@ func (s *snapshotter) View(ctx context.Context, key, parent string, opts ...snap return s.createSnapshot(ctx, snapshots.KindView, key, parent, opts) } +func (s *snapshotter) isLayerWithDmverity(id string) bool { + if _, err := os.Stat(filepath.Join(s.root, "snapshots", id, ".dmverity")); err != nil { + return false + } + return true +} + func setImmutable(path string, enable bool) error { - //nolint:revive,staticcheck // silence "don't use ALL_CAPS in Go names; use CamelCase" + //nolint:revive // silence "don't use ALL_CAPS in Go names; use CamelCase" const ( FS_IMMUTABLE_FL = 0x10 ) @@ -444,12 +618,18 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap } } - // Set IMMUTABLE_FL on the EROFS layer to avoid artificial data loss - if s.setImmutable { - if err := setImmutable(layerBlob, true); err != nil { - log.G(ctx).WithError(err).Warnf("failed to set IMMUTABLE_FL for %s", layerBlob) + if s.enableDmverity { + err := s.formatLayerBlob(id) + // _, err := s.runDmverity(id) + if err != nil { + return fmt.Errorf("failed to run dmverity: %w", err) } } + + // Set IMMUTABLE_FL on the EROFS layer to avoid artificial data loss + // if err := setImmutable(layerBlob, true); err != nil { + // log.G(ctx).WithError(err).Warnf("failed to set IMMUTABLE_FL for %s", layerBlob) + // } return nil }) @@ -527,6 +707,7 @@ func (s *snapshotter) getCleanupDirectories(ctx context.Context) ([]string, erro func (s *snapshotter) Remove(ctx context.Context, key string) (err error) { var removals []string var id string + fmt.Println("Remove called, key: ", key) // Remove directories after the transaction is closed, failures must not // return error since the transaction is committed with the removal // key no longer available. @@ -536,6 +717,10 @@ func (s *snapshotter) Remove(ctx context.Context, key string) (err error) { log.G(ctx).Warnf("failed to unmount EROFS mount for %v", id) } + if err := s.closeDmverityDevice(id); err != nil { + log.G(ctx).WithError(err).Warnf("failed to close dmverity device for %v", id) + } + for _, dir := range removals { if err := os.RemoveAll(dir); err != nil { log.G(ctx).WithError(err).WithField("path", dir).Warn("failed to remove directory") @@ -557,6 +742,11 @@ func (s *snapshotter) Remove(ctx context.Context, key string) (err error) { } // The layer blob is only persisted for committed snapshots. if k == snapshots.KindCommitted { + fmt.Println("closing dmverity device for ", id) + if err := s.closeDmverityDevice(id); err != nil { + log.G(ctx).WithError(err).Warnf("failed to close dmverity device for %v", id) + } + // Clear IMMUTABLE_FL before removal, since this flag avoids it. err = setImmutable(s.layerBlobPath(id), false) if err != nil { @@ -642,3 +832,18 @@ func (s *snapshotter) verifyFsverity(path string) error { } return nil } + +// closeDmverityDevice closes the dmverity device for a specific snapshot ID +func (s *snapshotter) closeDmverityDevice(id string) error { + if !s.enableDmverity || !s.isLayerWithDmverity(id) { + return nil + } + + dmName := fmt.Sprintf("containerd-erofs-%s", id) + devicePath := fmt.Sprintf("/dev/mapper/%s", dmName) + if _, err := os.Stat(devicePath); err == nil { + _, err = dmverity.Close(dmName) + return err + } + return nil +} diff --git a/plugins/snapshots/erofs/plugin/plugin_linux.go b/plugins/snapshots/erofs/plugin/plugin_linux.go index ff64f2d0ea32c..e96a091819e84 100644 --- a/plugins/snapshots/erofs/plugin/plugin_linux.go +++ b/plugins/snapshots/erofs/plugin/plugin_linux.go @@ -37,8 +37,8 @@ type Config struct { // EnableFsverity enables fsverity for EROFS layers EnableFsverity bool `toml:"enable_fsverity"` - // If `SetImmutable` is enabled, IMMUTABLE_FL will be set on layer blobs. - SetImmutable bool `toml:"set_immutable"` + // EnableDmverity enables dmverity for EROFS layers + EnableDmverity bool `toml:"enable_dmverity"` } func init() { @@ -68,8 +68,8 @@ func init() { opts = append(opts, erofs.WithFsverity()) } - if config.SetImmutable { - opts = append(opts, erofs.WithImmutable()) + if config.EnableDmverity { + opts = append(opts, erofs.WithDmverity()) } ic.Meta.Exports[plugins.SnapshotterRootDir] = root From 88c365f5bd7675c868f8f7c0a720b85684deeee7 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Wed, 15 Oct 2025 22:38:19 +0000 Subject: [PATCH 17/25] Initial commit to address comments --- plugins/snapshots/erofs/erofs_linux.go | 113 ++++++++++++------------- 1 file changed, 52 insertions(+), 61 deletions(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index a6e5075c74536..b1bd39197df01 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -207,6 +207,12 @@ func (s *snapshotter) workPath(id string) string { func (s *snapshotter) layerBlobPath(id string) string { return filepath.Join(s.root, "snapshots", id, "layer.erofs") } + +// dmverityDeviceName returns the dm-verity device name for a snapshot ID +func (s *snapshotter) dmverityDeviceName(id string) string { + return fmt.Sprintf("containerd-erofs-%s", id) +} + func (s *snapshotter) formatLayerBlob(id string) error { layerBlob := s.layerBlobPath(id) if _, err := os.Stat(layerBlob); err != nil { @@ -235,7 +241,7 @@ func (s *snapshotter) formatLayerBlob(id string) error { if err != nil { return fmt.Errorf("failed to format dmverity: %w", err) } - dmverityData := fmt.Sprintf("%s|%d", info.RootHash, fileinfo.Size()) + dmverityData := fmt.Sprintf("roothash=%s\noriginalsize=%d\n", info.RootHash, fileinfo.Size()) if err := os.WriteFile(filepath.Join(s.root, "snapshots", id, ".dmverity"), []byte(dmverityData), 0644); err != nil { return fmt.Errorf("failed to write dmverity root hash: %w", err) } @@ -247,18 +253,9 @@ func (s *snapshotter) runDmverity(id string) (string, error) { if _, err := os.Stat(layerBlob); err != nil { return "", fmt.Errorf("failed to find valid erofs layer blob: %w", err) } - dmName := fmt.Sprintf("containerd-erofs-%s", id) + dmName := s.dmverityDeviceName(id) devicePath := fmt.Sprintf("/dev/mapper/%s", dmName) if _, err := os.Stat(devicePath); err == nil { - status, err := dmverity.Status(dmName) - fmt.Println("dmverity device status: ", status) - if err != nil { - return "", fmt.Errorf("failed to get dmverity device status: %w", err) - } - if !status.IsVerified() { - return "", fmt.Errorf("dmverity device %s is not verified, status: %s", dmName, status.Status) - } - return devicePath, nil } dmverityContent, err := os.ReadFile(filepath.Join(s.root, "snapshots", id, ".dmverity")) @@ -266,19 +263,42 @@ func (s *snapshotter) runDmverity(id string) (string, error) { return "", fmt.Errorf("failed to read dmverity root hash: %w", err) } - parts := strings.Split(string(dmverityContent), "|") - rootHash := parts[0] + var rootHash string var originalSize uint64 - if len(parts) > 1 { - var err error - originalSize, err = strconv.ParseUint(parts[1], 10, 64) - if err != nil { - return "", fmt.Errorf("failed to parse original size: %w", err) + + content := string(dmverityContent) + // Try new format first (key=value pairs) + if strings.Contains(content, "roothash=") { + lines := strings.Split(strings.TrimSpace(content), "\n") + for _, line := range lines { + parts := strings.SplitN(line, "=", 2) + if len(parts) != 2 { + continue + } + key, value := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) + switch key { + case "roothash": + rootHash = value + case "originalsize": + originalSize, err = strconv.ParseUint(value, 10, 64) + if err != nil { + return "", fmt.Errorf("failed to parse original size: %w", err) + } + } + } + } else { + // Fall back to old format (roothash|originalsize) + parts := strings.Split(content, "|") + rootHash = parts[0] + if len(parts) > 1 { + originalSize, err = strconv.ParseUint(parts[1], 10, 64) + if err != nil { + return "", fmt.Errorf("failed to parse original size: %w", err) + } } } if _, err := os.Stat(devicePath); err != nil { - fmt.Println("openning dmverity") opts := dmverity.DefaultDmverityOptions() opts.HashOffset = originalSize _, err = dmverity.Open(layerBlob, dmName, layerBlob, string(rootHash), &opts) @@ -335,12 +355,8 @@ func (s *snapshotter) prepareDirectory(ctx context.Context, snapshotDir string, func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]mount.Mount, error) { var options []string - fmt.Println("mounts called, info :", info) - fmt.Println("snap: ", snap) if len(snap.ParentIDs) == 0 { - fmt.Println("no parent ids") - m, mntpoint, err := s.lowerPath(snap.ID) - fmt.Printf("lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) + m, _, err := s.lowerPath(snap.ID) if err == nil { if snap.Kind != snapshots.KindView { return nil, fmt.Errorf("only works for snapshots.KindView on a committed snapshot: %w", err) @@ -350,15 +366,14 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun return nil, err } } - fmt.Println("formatting layer blob m: ", m) if s.enableDmverity { if err := s.formatLayerBlob(snap.ID); err != nil { return nil, err } } // We have to force a loop device here since mount[] is static. - // However, if we're using dmverity, it's already a block device - if !strings.HasPrefix(m.Source, "/dev/mapper/") { + // However, if we're using dmverity, we don't add loop here + if !s.enableDmverity { m.Options = append(m.Options, "loop") } return []mount.Mount{m}, nil @@ -381,82 +396,64 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun }, nil } - fmt.Println("snap.Kind", snap.Kind) if snap.Kind == snapshots.KindActive { options = append(options, fmt.Sprintf("workdir=%s", s.workPath(snap.ID)), fmt.Sprintf("upperdir=%s", s.upperPath(snap.ID)), ) } else if len(snap.ParentIDs) == 1 { - fmt.Println("len(snap.ParentIDs) == 1") - m, mntpoint, err := s.lowerPath(snap.ParentIDs[0]) + m, _, err := s.lowerPath(snap.ParentIDs[0]) if err != nil { return nil, err } - fmt.Printf("lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) if s.enableDmverity { if err := s.formatLayerBlob(snap.ParentIDs[0]); err != nil { return nil, err } } - fmt.Printf("lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) // We have to force a loop device here too since mount[] is static. // However, if we're using dmverity, it's already a block device - if !strings.HasPrefix(m.Source, "/dev/mapper/") { + if !s.enableDmverity { m.Options = append(m.Options, "loop") } return []mount.Mount{m}, nil } - fmt.Println("snap.ParentIDs", snap.ParentIDs) var lowerdirs []string for i := range snap.ParentIDs { m, mntpoint, err := s.lowerPath(snap.ParentIDs[i]) if err != nil { return nil, err } - fmt.Printf("active lowerPath: m = %v, mntpoint = %v\n", m, mntpoint) // If the lowerdir is actually an EROFS committed layer but - // doesn't have an EROFS mount. Let's recover now. + // doesn't have an EROFS mount. Let's recover now. if !s.enableDmverity && mntpoint != m.Source && !isErofs(mntpoint) { err := m.Mount(mntpoint) // Use loop if the current kernel (6.12+) doesn't support file-backed mount - // Skip 'loop' if using dmverity device - if errors.Is(err, unix.ENOTBLK) && (!s.enableDmverity || !strings.HasPrefix(m.Source, "/dev/mapper/")) { + if errors.Is(err, unix.ENOTBLK) { m.Options = append(m.Options, "loop") err = m.Mount(mntpoint) } if err != nil { return nil, err } - } - if s.enableDmverity { + } else if s.enableDmverity { devicePath, err := s.runDmverity(snap.ParentIDs[i]) if err != nil { return nil, err } - dmName := fmt.Sprintf("containerd-erofs-%s", snap.ParentIDs[i]) - if _, err := os.Stat(devicePath); err == nil { - status, err := dmverity.Status(dmName) + m.Source = devicePath + if mntpoint != m.Source && !isErofs(mntpoint) { + err = m.Mount(mntpoint) if err != nil { - return nil, fmt.Errorf("failed to get dmverity device status: %w", err) - } - m.Source = devicePath - if !status.IsInUse() { - err = m.Mount(mntpoint) - if err != nil { - return nil, err - } + return nil, err } } - } lowerdirs = append(lowerdirs, mntpoint) } - fmt.Println("lowerdirs: ", lowerdirs) options = append(options, fmt.Sprintf("lowerdir=%s", strings.Join(lowerdirs, ":"))) options = append(options, s.ovlOptions...) - fmt.Printf("options = %v\n", options) return []mount.Mount{{ Type: "overlay", Source: "overlay", @@ -626,10 +623,6 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap } } - // Set IMMUTABLE_FL on the EROFS layer to avoid artificial data loss - // if err := setImmutable(layerBlob, true); err != nil { - // log.G(ctx).WithError(err).Warnf("failed to set IMMUTABLE_FL for %s", layerBlob) - // } return nil }) @@ -707,7 +700,6 @@ func (s *snapshotter) getCleanupDirectories(ctx context.Context) ([]string, erro func (s *snapshotter) Remove(ctx context.Context, key string) (err error) { var removals []string var id string - fmt.Println("Remove called, key: ", key) // Remove directories after the transaction is closed, failures must not // return error since the transaction is committed with the removal // key no longer available. @@ -742,7 +734,6 @@ func (s *snapshotter) Remove(ctx context.Context, key string) (err error) { } // The layer blob is only persisted for committed snapshots. if k == snapshots.KindCommitted { - fmt.Println("closing dmverity device for ", id) if err := s.closeDmverityDevice(id); err != nil { log.G(ctx).WithError(err).Warnf("failed to close dmverity device for %v", id) } @@ -839,7 +830,7 @@ func (s *snapshotter) closeDmverityDevice(id string) error { return nil } - dmName := fmt.Sprintf("containerd-erofs-%s", id) + dmName := s.dmverityDeviceName(id) devicePath := fmt.Sprintf("/dev/mapper/%s", dmName) if _, err := os.Stat(devicePath); err == nil { _, err = dmverity.Close(dmName) From 7a6f889b5717d9858a4ad8383959c95bb9fbb64d Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Thu, 16 Oct 2025 23:11:56 +0000 Subject: [PATCH 18/25] Do not allow fsverity and dmverity to be enabled at the same time --- plugins/snapshots/erofs/erofs_linux.go | 5 +++++ plugins/snapshots/erofs/erofs_linux_test.go | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index b1bd39197df01..629c86db9e97e 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -133,6 +133,11 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { return nil, fmt.Errorf("EROFS unsupported, please `modprobe erofs`: %w", plugin.ErrSkipPlugin) } + // Ensure fsverity and dmverity are not both enabled + if config.enableFsverity && config.enableDmverity { + return nil, fmt.Errorf("fsverity and dmverity cannot be enabled simultaneously") + } + // Check fsverity support if enabled if config.enableFsverity { supported, err := fsverity.IsSupported(root) diff --git a/plugins/snapshots/erofs/erofs_linux_test.go b/plugins/snapshots/erofs/erofs_linux_test.go index 195cc41c9302f..95639c7c4a8f2 100644 --- a/plugins/snapshots/erofs/erofs_linux_test.go +++ b/plugins/snapshots/erofs/erofs_linux_test.go @@ -131,3 +131,22 @@ func TestErofsFsverity(t *testing.T) { t.Fatal("Expected direct write to fsverity-enabled layer to fail") } } + +func TestFsverityAndDmverityMutualExclusion(t *testing.T) { + testutil.RequiresRoot(t) + + root := t.TempDir() + + if !findErofs() { + t.Skip("check for erofs kernel support failed, skipping test") + } + + // Try to create snapshotter with both fsverity and dmverity enabled + _, err := NewSnapshotter(root, WithFsverity(), WithDmverity()) + if err == nil { + t.Fatal("Expected error when enabling both fsverity and dmverity") + } + if err.Error() != "fsverity and dmverity cannot be enabled simultaneously" { + t.Fatalf("Unexpected error message: %v", err) + } +} From b69f2b35b8f54caa83fe7927b0f23170901bc2f2 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Thu, 16 Oct 2025 23:33:00 +0000 Subject: [PATCH 19/25] Improve function name --- plugins/snapshots/erofs/erofs_linux.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index 629c86db9e97e..21b42346ed660 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -253,7 +253,10 @@ func (s *snapshotter) formatLayerBlob(id string) error { } return nil } -func (s *snapshotter) runDmverity(id string) (string, error) { + +// getDmverityDevicePath opens a dm-verity device for the given snapshot ID if not already open, +// and returns the device path. If the device is already open, returns the existing device path. +func (s *snapshotter) getDmverityDevicePath(id string) (string, error) { layerBlob := s.layerBlobPath(id) if _, err := os.Stat(layerBlob); err != nil { return "", fmt.Errorf("failed to find valid erofs layer blob: %w", err) @@ -443,7 +446,7 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun return nil, err } } else if s.enableDmverity { - devicePath, err := s.runDmverity(snap.ParentIDs[i]) + devicePath, err := s.getDmverityDevicePath(snap.ParentIDs[i]) if err != nil { return nil, err } @@ -622,9 +625,9 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap if s.enableDmverity { err := s.formatLayerBlob(id) - // _, err := s.runDmverity(id) + // Note: Device opening is deferred until mount time via getDmverityDevicePath if err != nil { - return fmt.Errorf("failed to run dmverity: %w", err) + return fmt.Errorf("failed to format dmverity: %w", err) } } From 4525e0962597614b03dabe9a7c5c177f49bab275 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 17 Oct 2025 22:23:44 +0000 Subject: [PATCH 20/25] Keep setImmutable and maintain order --- plugins/snapshots/erofs/erofs_linux.go | 19 ++++++++++++++++++- .../snapshots/erofs/plugin/plugin_linux.go | 7 +++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index 21b42346ed660..ae820ebebcf75 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -46,6 +46,8 @@ type SnapshotterConfig struct { ovlOptions []string // enableFsverity enables fsverity for EROFS layers enableFsverity bool + // setImmutable enables IMMUTABLE_FL file attribute for EROFS layers + setImmutable bool // enableDmverity enables dmverity for EROFS layers enableDmverity bool } @@ -67,6 +69,13 @@ func WithFsverity() Opt { } } +// WithImmutable enables IMMUTABLE_FL file attribute for EROFS layers +func WithImmutable() Opt { + return func(config *SnapshotterConfig) { + config.setImmutable = true + } +} + // WithDmverity enables dmverity for EROFS layers func WithDmverity() Opt { return func(config *SnapshotterConfig) { @@ -85,6 +94,7 @@ type snapshotter struct { ms *storage.MetaStore ovlOptions []string enableFsverity bool + setImmutable bool enableDmverity bool } @@ -173,6 +183,7 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { ms: ms, ovlOptions: config.ovlOptions, enableFsverity: config.enableFsverity, + setImmutable: config.setImmutable, enableDmverity: config.enableDmverity, }, nil } @@ -550,7 +561,7 @@ func (s *snapshotter) isLayerWithDmverity(id string) bool { } func setImmutable(path string, enable bool) error { - //nolint:revive // silence "don't use ALL_CAPS in Go names; use CamelCase" + //nolint:revive,staticcheck // silence "don't use ALL_CAPS in Go names; use CamelCase" const ( FS_IMMUTABLE_FL = 0x10 ) @@ -631,6 +642,12 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap } } + // Set IMMUTABLE_FL on the EROFS layer to avoid artificial data loss + if s.setImmutable { + if err := setImmutable(layerBlob, true); err != nil { + log.G(ctx).WithError(err).Warnf("failed to set IMMUTABLE_FL for %s", layerBlob) + } + } return nil }) diff --git a/plugins/snapshots/erofs/plugin/plugin_linux.go b/plugins/snapshots/erofs/plugin/plugin_linux.go index e96a091819e84..dd525496c4cb0 100644 --- a/plugins/snapshots/erofs/plugin/plugin_linux.go +++ b/plugins/snapshots/erofs/plugin/plugin_linux.go @@ -37,6 +37,9 @@ type Config struct { // EnableFsverity enables fsverity for EROFS layers EnableFsverity bool `toml:"enable_fsverity"` + // If `SetImmutable` is enabled, IMMUTABLE_FL will be set on layer blobs. + SetImmutable bool `toml:"set_immutable"` + // EnableDmverity enables dmverity for EROFS layers EnableDmverity bool `toml:"enable_dmverity"` } @@ -68,6 +71,10 @@ func init() { opts = append(opts, erofs.WithFsverity()) } + if config.SetImmutable { + opts = append(opts, erofs.WithImmutable()) + } + if config.EnableDmverity { opts = append(opts, erofs.WithDmverity()) } From f1c80b4bf0cb1a97f8c7a232db2467ba72f43446 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 17 Oct 2025 22:48:15 +0000 Subject: [PATCH 21/25] Do not allow immutable and dmverity to be enabled at the same time --- plugins/snapshots/erofs/erofs_linux.go | 6 ++++++ plugins/snapshots/erofs/erofs_linux_test.go | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index ae820ebebcf75..cb5a86dfefa09 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -148,6 +148,12 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { return nil, fmt.Errorf("fsverity and dmverity cannot be enabled simultaneously") } + // Ensure setImmutable and dmverity are not both enabled + // dmverity needs to modify the file to add hash trees, which conflicts with IMMUTABLE_FL + if config.setImmutable && config.enableDmverity { + return nil, fmt.Errorf("setImmutable and dmverity cannot be enabled simultaneously") + } + // Check fsverity support if enabled if config.enableFsverity { supported, err := fsverity.IsSupported(root) diff --git a/plugins/snapshots/erofs/erofs_linux_test.go b/plugins/snapshots/erofs/erofs_linux_test.go index 95639c7c4a8f2..8da5f468478c1 100644 --- a/plugins/snapshots/erofs/erofs_linux_test.go +++ b/plugins/snapshots/erofs/erofs_linux_test.go @@ -150,3 +150,22 @@ func TestFsverityAndDmverityMutualExclusion(t *testing.T) { t.Fatalf("Unexpected error message: %v", err) } } + +func TestSetImmutableAndDmverityMutualExclusion(t *testing.T) { + testutil.RequiresRoot(t) + + root := t.TempDir() + + if !findErofs() { + t.Skip("check for erofs kernel support failed, skipping test") + } + + // Try to create snapshotter with both setImmutable and dmverity enabled + _, err := NewSnapshotter(root, WithImmutable(), WithDmverity()) + if err == nil { + t.Fatal("Expected error when enabling both setImmutable and dmverity") + } + if err.Error() != "setImmutable and dmverity cannot be enabled simultaneously" { + t.Fatalf("Unexpected error message: %v", err) + } +} From a68615e2843a20899fdb21e7cd4845f03dd6c4a2 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Fri, 17 Oct 2025 23:36:27 +0000 Subject: [PATCH 22/25] Update calls to getDmverityDevicePath --- plugins/snapshots/erofs/erofs_linux.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index cb5a86dfefa09..7c61f6756a073 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -392,9 +392,11 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun } } if s.enableDmverity { - if err := s.formatLayerBlob(snap.ID); err != nil { + devicePath, err := s.getDmverityDevicePath(snap.ID) + if err != nil { return nil, err } + m.Source = devicePath } // We have to force a loop device here since mount[] is static. // However, if we're using dmverity, we don't add loop here @@ -432,9 +434,11 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun return nil, err } if s.enableDmverity { - if err := s.formatLayerBlob(snap.ParentIDs[0]); err != nil { + devicePath, err := s.getDmverityDevicePath(snap.ParentIDs[0]) + if err != nil { return nil, err } + m.Source = devicePath } // We have to force a loop device here too since mount[] is static. // However, if we're using dmverity, it's already a block device @@ -640,6 +644,13 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap } } + // Set IMMUTABLE_FL on the EROFS layer to avoid artificial data loss + if s.setImmutable { + if err := setImmutable(layerBlob, true); err != nil { + log.G(ctx).WithError(err).Warnf("failed to set IMMUTABLE_FL for %s", layerBlob) + } + } + if s.enableDmverity { err := s.formatLayerBlob(id) // Note: Device opening is deferred until mount time via getDmverityDevicePath @@ -647,13 +658,6 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap return fmt.Errorf("failed to format dmverity: %w", err) } } - - // Set IMMUTABLE_FL on the EROFS layer to avoid artificial data loss - if s.setImmutable { - if err := setImmutable(layerBlob, true); err != nil { - log.G(ctx).WithError(err).Warnf("failed to set IMMUTABLE_FL for %s", layerBlob) - } - } return nil }) From d3ac12d62b6f5b4f08f6db1ef635430afcfe95a2 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Mon, 20 Oct 2025 18:43:25 +0000 Subject: [PATCH 23/25] Use root hash file --- plugins/snapshots/erofs/erofs_linux.go | 28 +++++++++++++++----------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index 7c61f6756a073..c093df4ecba94 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -259,13 +259,20 @@ func (s *snapshotter) formatLayerBlob(id string) error { return fmt.Errorf("failed to truncate layer blob: %w", err) } opts.HashOffset = uint64(file_size) - info, err := dmverity.Format(layerBlob, layerBlob, &opts) + + // Create a persistent file for the root hash + rootHashFile := filepath.Join(s.root, "snapshots", id, ".roothash") + opts.RootHashFile = rootHashFile + + _, err = dmverity.Format(layerBlob, layerBlob, &opts) if err != nil { return fmt.Errorf("failed to format dmverity: %w", err) } - dmverityData := fmt.Sprintf("roothash=%s\noriginalsize=%d\n", info.RootHash, fileinfo.Size()) + + // Write the dmverity metadata with original size + dmverityData := fmt.Sprintf("originalsize=%d\n", fileinfo.Size()) if err := os.WriteFile(filepath.Join(s.root, "snapshots", id, ".dmverity"), []byte(dmverityData), 0644); err != nil { - return fmt.Errorf("failed to write dmverity root hash: %w", err) + return fmt.Errorf("failed to write dmverity metadata: %w", err) } } return nil @@ -285,15 +292,14 @@ func (s *snapshotter) getDmverityDevicePath(id string) (string, error) { } dmverityContent, err := os.ReadFile(filepath.Join(s.root, "snapshots", id, ".dmverity")) if err != nil { - return "", fmt.Errorf("failed to read dmverity root hash: %w", err) + return "", fmt.Errorf("failed to read dmverity metadata: %w", err) } - var rootHash string var originalSize uint64 content := string(dmverityContent) // Try new format first (key=value pairs) - if strings.Contains(content, "roothash=") { + if strings.Contains(content, "originalsize=") { lines := strings.Split(strings.TrimSpace(content), "\n") for _, line := range lines { parts := strings.SplitN(line, "=", 2) @@ -301,10 +307,7 @@ func (s *snapshotter) getDmverityDevicePath(id string) (string, error) { continue } key, value := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) - switch key { - case "roothash": - rootHash = value - case "originalsize": + if key == "originalsize" { originalSize, err = strconv.ParseUint(value, 10, 64) if err != nil { return "", fmt.Errorf("failed to parse original size: %w", err) @@ -314,7 +317,6 @@ func (s *snapshotter) getDmverityDevicePath(id string) (string, error) { } else { // Fall back to old format (roothash|originalsize) parts := strings.Split(content, "|") - rootHash = parts[0] if len(parts) > 1 { originalSize, err = strconv.ParseUint(parts[1], 10, 64) if err != nil { @@ -326,7 +328,9 @@ func (s *snapshotter) getDmverityDevicePath(id string) (string, error) { if _, err := os.Stat(devicePath); err != nil { opts := dmverity.DefaultDmverityOptions() opts.HashOffset = originalSize - _, err = dmverity.Open(layerBlob, dmName, layerBlob, string(rootHash), &opts) + // Use the root hash file instead of passing root hash as command-line arg + opts.RootHashFile = filepath.Join(s.root, "snapshots", id, ".roothash") + _, err = dmverity.Open(layerBlob, dmName, layerBlob, "", &opts) if err != nil { return "", fmt.Errorf("failed to open dmverity device: %w", err) } From 965f7e7631530155ef0e1f19f63c1a0ca047bc38 Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Mon, 20 Oct 2025 19:03:39 +0000 Subject: [PATCH 24/25] Do not need .dmverity as well --- plugins/snapshots/erofs/erofs_linux.go | 47 ++++---------------------- 1 file changed, 7 insertions(+), 40 deletions(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index c093df4ecba94..16221abc9ca67 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -23,7 +23,6 @@ import ( "fmt" "os" "path/filepath" - "strconv" "strings" "syscall" "time" @@ -268,12 +267,6 @@ func (s *snapshotter) formatLayerBlob(id string) error { if err != nil { return fmt.Errorf("failed to format dmverity: %w", err) } - - // Write the dmverity metadata with original size - dmverityData := fmt.Sprintf("originalsize=%d\n", fileinfo.Size()) - if err := os.WriteFile(filepath.Join(s.root, "snapshots", id, ".dmverity"), []byte(dmverityData), 0644); err != nil { - return fmt.Errorf("failed to write dmverity metadata: %w", err) - } } return nil } @@ -290,40 +283,14 @@ func (s *snapshotter) getDmverityDevicePath(id string) (string, error) { if _, err := os.Stat(devicePath); err == nil { return devicePath, nil } - dmverityContent, err := os.ReadFile(filepath.Join(s.root, "snapshots", id, ".dmverity")) - if err != nil { - return "", fmt.Errorf("failed to read dmverity metadata: %w", err) - } - var originalSize uint64 - - content := string(dmverityContent) - // Try new format first (key=value pairs) - if strings.Contains(content, "originalsize=") { - lines := strings.Split(strings.TrimSpace(content), "\n") - for _, line := range lines { - parts := strings.SplitN(line, "=", 2) - if len(parts) != 2 { - continue - } - key, value := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) - if key == "originalsize" { - originalSize, err = strconv.ParseUint(value, 10, 64) - if err != nil { - return "", fmt.Errorf("failed to parse original size: %w", err) - } - } - } - } else { - // Fall back to old format (roothash|originalsize) - parts := strings.Split(content, "|") - if len(parts) > 1 { - originalSize, err = strconv.ParseUint(parts[1], 10, 64) - if err != nil { - return "", fmt.Errorf("failed to parse original size: %w", err) - } - } + // Calculate the hash offset dynamically from the file size + // The file was truncated to double its original size in formatLayerBlob + fileInfo, err := os.Stat(layerBlob) + if err != nil { + return "", fmt.Errorf("failed to stat layer blob: %w", err) } + originalSize := uint64(fileInfo.Size() / 2) if _, err := os.Stat(devicePath); err != nil { opts := dmverity.DefaultDmverityOptions() @@ -568,7 +535,7 @@ func (s *snapshotter) View(ctx context.Context, key, parent string, opts ...snap } func (s *snapshotter) isLayerWithDmverity(id string) bool { - if _, err := os.Stat(filepath.Join(s.root, "snapshots", id, ".dmverity")); err != nil { + if _, err := os.Stat(filepath.Join(s.root, "snapshots", id, ".roothash")); err != nil { return false } return true From e0c627d815f73d1f697259fbf5143e9b51235d3a Mon Sep 17 00:00:00 2001 From: Aadhar Agarwal Date: Mon, 20 Oct 2025 19:34:17 +0000 Subject: [PATCH 25/25] Add comment for file truncating and simplify Close function --- plugins/snapshots/erofs/erofs_linux.go | 27 ++++++++------------------ 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/plugins/snapshots/erofs/erofs_linux.go b/plugins/snapshots/erofs/erofs_linux.go index 16221abc9ca67..61ff6e86f601a 100644 --- a/plugins/snapshots/erofs/erofs_linux.go +++ b/plugins/snapshots/erofs/erofs_linux.go @@ -195,24 +195,11 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { // Close closes the snapshotter func (s *snapshotter) Close() error { - // If dmverity is enabled, try to close all devices - if s.enableDmverity { - // Get a list of all snapshots - err := s.ms.WithTransaction(context.Background(), false, func(ctx context.Context) error { - return storage.WalkInfo(ctx, func(ctx context.Context, info snapshots.Info) error { - if info.Kind == snapshots.KindCommitted { - // Close the device if it exists - if err := s.closeDmverityDevice(info.Name); err != nil { - log.L.WithError(err).Warnf("failed to close dmverity device for %v", info.Name) - } - } - return nil - }) - }) - if err != nil { - log.L.WithError(err).Warn("error closing dmverity devices") - } - } + // Close the metadata store. We intentionally do not close dm-verity devices here + // to avoid disrupting running containers. Devices will be cleaned up when: + // - Snapshots are removed via Remove() + // - The system reboots + // - Manual cleanup is performed return s.ms.Close() } @@ -253,7 +240,9 @@ func (s *snapshotter) formatLayerBlob(id string) error { } defer f.Close() file_size := fileinfo.Size() - // Truncate the file to double its size + // Truncate the file to double its size to provide space for the dm-verity hash tree. + // The hash tree will never exceed the original data size. + // Most filesystems use sparse allocation, so unused space doesn't consume disk. if err := f.Truncate(file_size * 2); err != nil { return fmt.Errorf("failed to truncate layer blob: %w", err) }