snap-bootstrap: load partition information from a manifest file for cloudimg-rootfs mode #14713

sespiros · 2024-11-12T09:52:24Z

~~Rebased on top of #14178 which is approved but not merged. New commits start at commit 2035d4d.~~

~~Rebased on top of #14789 which is approved but not merged. New commits start at commit c55f509.~~

This refactors cloudimg-rootfs mode and adds support for externally providing a dm-verity root hash for the rootfs that is mounted. This root hash is part of a new manifest file that resides on the ESP.

Subsequent PRs will add support for measuring the manifest to the TPM for remote attestation purposes.

Jira: https://warthogs.atlassian.net/browse/FR-9480

alfonsosanchezbeato · 2024-11-20T18:55:31Z

@sespiros probably it would be good to rebase this now that the prerequisite has been merged

pedronis

@sespiros I think we discussed this at the sprint, my strong preference would be to first move the existing CVM support to a _cvm.go and _cmv_test.go file, and then do this change

codecov · 2024-12-03T13:53:35Z

Codecov Report

Attention: Patch coverage is 64.10256% with 56 lines in your changes missing coverage. Please review.

Project coverage is 78.24%. Comparing base (24a0034) to head (c4ab893).
Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go	64.10%	44 Missing and 12 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #14713      +/-   ##
==========================================
+ Coverage   78.20%   78.24%   +0.03%     
==========================================
  Files        1151     1146       -5     
  Lines      151396   151656     +260     
==========================================
+ Hits       118402   118656     +254     
+ Misses      25662    25656       -6     
- Partials     7332     7344      +12

Flag	Coverage Δ
unittests	`78.24% <64.10%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This is a preparatory commit which modifies the existing CVM mode to support mounting multiple overlaid partitions for the rootfs.

In order to provide integrity protection for the root fs, the root fs will be mounted read-only and will be accompanied by dm-verity data from a new auto-discovered partition. To support writable state for the ephemeral CVM scenario, an overlayfs mount is created with an upper layer residing on tmpfs. A writable layer residing on disk can be also supported but it is currently disabled. The filesystem layout in the initramfs will look like: . /run/mnt ├── ... ├── data (where the final merged overlay will be mounted) ├── cloudimg-rootfs (read-only mount of the root fs, folder name is the fs label) ├── .. (future WIP, other partition mounts as separate mounts) └── cloudimg-rootfs-writable (tmpfs mount for the writable upper layer) ├── work └── upper An example final overlay mount command will look like: mount -t overlay overlay -olowerdir=/run/mnt/cloudimg-rootfs,\ upperdir=/run/mnt/cloudimg-rootfs-writable/upper,\ workdir=/run/mnt/cloudimg-rootfs-writable/work \ /run/mnt/data

…verity partition instead

v1 CVM images use the filesystem label to auto-discover an encrypted rootfs partition based on whether it has the '-enc' suffix. The new images use the GPT label to auto-discover the verity partition. In order to keep backwards compatibility, the customization tool will use set both labels to the same one. This commit clarifies that the field in the new structs actually contain GPT label information.

creating the upper and work folders used for setting up the overlayfs mount, was initially handled by snap-bootstrap whether it set up a tmpfs or detected a writable partition on disk but in order to support the writable partition case, it was later discovered that these folders might need to be created during the customization step and might already exist. snap-bootstrap will now create those folders only if it has detected no writable partition in the manifest and has auto-created a tmpfs-based writable partition.

trying to boot an image on a system without a TPM present was failing due to not-finding a TPM device. The source of the error was not the provision step as secbotProvisionForCVM already ignores errors (returns nil) if a TPM device is not found or a template file for provision the TPM is not found.

CVM mode used to mount the rootfs partition with the Ephemeral flag as cloud images contain a fixed fstab entry.

This is a legacy check that is not actually needed now as secbootUnlockVolumeUsingSealedKeyIfEncrypted will only find partitions residing on the disk that the ESP is from.

alfonsosanchezbeato

Thanks, I have some comments

alfonsosanchezbeato · 2024-12-05T16:42:11Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+	Opts     *systemdMountOptions
+}
+
+type ImageManifestPartition struct {


It looks like this and other new types like ImageManifest or ManifestError do not need to be exported.

alfonsosanchezbeato · 2024-12-05T16:45:37Z

cmd/snap-bootstrap/export_test.go

@@ -94,6 +94,14 @@ func MockOsutilIsMounted(f func(string) (bool, error)) (restore func()) {
 	}
 }

+func MockOsReadFile(f func(string) ([]byte, error)) (restore func()) {


In tests we tend to create the files instead of mocking the read, note that dirs.GlobalRootDir is set to a temporary folder so this is in principle easy to do.

alfonsosanchezbeato · 2024-12-05T16:47:20Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+	GptLabel string `json:"label"`
+	RootHash string `json:"root_hash"`
+	Overlay  string `json:"overlay"`
+}


A description on what is each of these fields would be helpful, or maybe a link to a description of the format if available.

alfonsosanchezbeato · 2024-12-05T16:47:55Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+}
+
+type ImageManifest struct {
+	Partitions []ImageManifestPartition `json:"partitions"`


same: re describing the field

alfonsosanchezbeato · 2024-12-05T16:59:19Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+	return im, nil
+}
+
+// generateMountsFromManifest is used to parse a manifest file which contains information about which


strictly speaking, the manifest passed around parsed already, right?

alfonsosanchezbeato · 2024-12-05T17:03:39Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+		if p.Overlay == "lowerdir" {
+			// XXX: currently only one lower layer is permitted. The rest, if found, are ignored.
+			if foundReadOnlyPartition != "" {
+				continue


Should not this as a minimum be logged? Or should it be considered an error in the manifest?

alfonsosanchezbeato · 2024-12-05T17:06:28Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+		} else {
+			// Only one writable partition is permitted.
+			if foundWritablePartition != "" {
+				continue


Same regarding log/error out

alfonsosanchezbeato · 2024-12-05T17:12:04Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+		GptLabel: "cloudimg-rootfs",
+		Opts: &systemdMountOptions{
+			Overlayfs: true,
+			LowerDirs: []string{filepath.Join(boot.InitramfsRunMntDir, partitionMounts[0].GptLabel)},


is using partitionMounts[0] assuming an order in the definitions of the partitions in the file? should this always be the read-only partition?

alfonsosanchezbeato · 2024-12-05T17:14:53Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm.go

+	pm.Opts.UpperDir = filepath.Join(boot.InitramfsRunMntDir, foundWritablePartition, "upper")
+	pm.Opts.WorkDir = filepath.Join(boot.InitramfsRunMntDir, foundWritablePartition, "work")


This could be done when pm is defined, splitting the initialization of Opts does not seem necessary.

alfonsosanchezbeato · 2024-12-05T17:17:10Z

cmd/snap-bootstrap/cmd_initramfs_mounts_cvm_test.go

+	restore = main.MockOsReadFile(func(path string) ([]byte, error) {
+		// check attempt to read manifest from specific path
+		c.Assert(path, Equals, filepath.Join(boot.InitramfsUbuntuSeedDir, "EFI/ubuntu", "manifest.json"))
+		return []byte(fmt.Sprintf(`{"partitions":[{"label":"cloudimg-rootfs","root_hash":%q,"overlay":"lowerdir"}]}`, expectedRootHash)), nil


Do we need a test case for when more than one partition is defined?

pedronis requested review from pedronis, valentindavid and alfonsosanchezbeato November 12, 2024 09:58

alfonsosanchezbeato added the Run nested The PR also runs tests inluded in nested suite label Nov 20, 2024

sespiros force-pushed the snap-bootstrap-manifest branch from 5c312d1 to 45d1ecd Compare November 21, 2024 17:28

pedronis requested changes Dec 2, 2024

View reviewed changes

sespiros mentioned this pull request Dec 3, 2024

c/snap-bootstrap: split CVM related functionality in separate files #14789

Merged

sespiros force-pushed the snap-bootstrap-manifest branch from 45d1ecd to b4d37ea Compare December 3, 2024 13:36

sespiros requested a review from pedronis December 3, 2024 13:36

sespiros force-pushed the snap-bootstrap-manifest branch from b4d37ea to 6b717d2 Compare December 4, 2024 11:53

sespiros added 12 commits December 5, 2024 12:48

c/snap-bootstrap: refactor CVM mode

6e0520c

This is a preparatory commit which modifies the existing CVM mode to support mounting multiple overlaid partitions for the rootfs.

c/snap-bootstrap: use partition label to auto-discover the rootfs dm-…

979f766

…verity partition instead

c/snap-bootstrap: refactor manifest parsing code for CVM mode

7c04d9e

c/snap-bootstrap: change path name of writable tmpfs partition

375d3ee

c/snap-bootstrap: make comment more generic

0d15271

c/snap-bootstrap: fix test for CVM mode

54a9460

CVM mode used to mount the rootfs partition with the Ephemeral flag as cloud images contain a fixed fstab entry.

c/snap-bootstrap: remove redundant disk check

7543b7e

This is a legacy check that is not actually needed now as secbootUnlockVolumeUsingSealedKeyIfEncrypted will only find partitions residing on the disk that the ESP is from.

c/snap-bootstrap: add test for CVM ephemeral mode with manifest

c4ab893

sespiros force-pushed the snap-bootstrap-manifest branch from 6b717d2 to c4ab893 Compare December 5, 2024 10:48

alfonsosanchezbeato reviewed Dec 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

snap-bootstrap: load partition information from a manifest file for cloudimg-rootfs mode #14713

snap-bootstrap: load partition information from a manifest file for cloudimg-rootfs mode #14713

sespiros commented Nov 12, 2024 •

edited

Loading

alfonsosanchezbeato commented Nov 20, 2024

pedronis left a comment

codecov bot commented Dec 3, 2024 •

edited

Loading

alfonsosanchezbeato left a comment

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

alfonsosanchezbeato Dec 5, 2024

		pm.Opts.UpperDir = filepath.Join(boot.InitramfsRunMntDir, foundWritablePartition, "upper")
		pm.Opts.WorkDir = filepath.Join(boot.InitramfsRunMntDir, foundWritablePartition, "work")

snap-bootstrap: load partition information from a manifest file for cloudimg-rootfs mode #14713

Are you sure you want to change the base?

snap-bootstrap: load partition information from a manifest file for cloudimg-rootfs mode #14713

Conversation

sespiros commented Nov 12, 2024 • edited Loading

alfonsosanchezbeato commented Nov 20, 2024

pedronis left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 3, 2024 • edited Loading

Codecov Report

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sespiros commented Nov 12, 2024 •

edited

Loading

codecov bot commented Dec 3, 2024 •

edited

Loading