overlay, composefs: use mkcomposefs to generate composefs #1727

giuseppe · 2023-10-12T20:50:05Z

use mkcomposefs to generate composefs, instead of using the custom composefs-from-json.

As part of the integration, retrieve the verity fs measurement after the files are written so that they can be embedded in the composefs image and their payload is validated each time the files are accessed.

giuseppe · 2023-10-12T20:50:51Z

@alexlarsson PTAL

giuseppe · 2023-10-13T06:24:05Z

marked as a draft for now, I'll try to move the fsverity logic into the chunked package so we don't need to carry the digests around

giuseppe · 2023-10-13T07:21:45Z

marked as a draft for now, I'll try to move the fsverity logic into the chunked package so we don't need to carry the digests around

looked into it, we will still need to propagate the digests anyway since composefs lives in the overlay driver.

Moving the fsverity measurement in the chunked package can be done later.

pkg/chunked/dump/dump.go

+	ESCAPE_STANDARD
+)
+
+func escaped(val string, length int, escape int) string {


pkg/chunked/dump/dump.go

+		length = len(val)
+	}
+
+	if escapeLoneDash && length == 1 && val[0] == '-' {


pkg/chunked/dump/dump.go

+	NOESCAPE_SPACE = 1 << iota
+	ESCAPE_EQUAL
+	ESCAPE_LONE_DASH
+	ESCAPE_STANDARD


pkg/chunked/dump/dump.go

+		path = "/" + path
+	}
+
+	if _, err := fmt.Fprint(out, escaped(path, len(path), ESCAPE_STANDARD)); err != nil {


drivers/overlay/composefs_supported.go

@@ -71,24 +87,42 @@ func enableVerityRecursive(path string) error {
 		if err := enableVerity(path, int(f.Fd())); err != nil {
 			return err
 		}
+
+		verity, err := measureVerity(path, int(f.Fd()))


drivers/overlay/composefs_supported.go

@@ -109,10 +143,10 @@ func generateComposeFsBlob(toc []byte, composefsDir string) error {
 		// a scope to close outFd before setting fsverity on the read-only fd.
 		defer outFd.Close()

-		cmd := exec.Command(writerJson, "--format=erofs", "--out=/proc/self/fd/3", "/proc/self/fd/0")
+		cmd := exec.Command(writerJson, "--from-file", "/proc/self/fd/0", "/proc/self/fd/3")


alexlarsson · 2023-10-13T12:57:44Z

I just released composefs 1.0.1 with the required dependencies for this. @smooge is packaging it up for fedora.

giuseppe · 2023-10-13T20:19:21Z

thanks for the review. I've addressed the comments and pushed a new version

pkg/chunked/dump/dump.go

if the file doesn't have a digest but its size is 0, we can hard code the known sha256 digest. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

so the differ can export arbitrary data that is not written as a big file. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

so that the users of the function can get access to the already unmarshalled TOC instead of having to unmarshal it again. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2023-10-16T11:13:19Z

@alexlarsson I've encountered a small issue (still investigating):

something like the following doesn't seem to preserve the st_nlink:

/bin/busybox-hardlink 847 100755 1 0 0 0 1690477944.0 89/f85a8b92bdb5ada51bd8ee0d9d687da1a759a0d80f8d012e7f8cbb3933a3a0 - d4ced2c28e68aa87530507099801f1ab575ee18054847d301bad5a27a4984855
/bin/busybox-hardlink2 0 @100755 1 0 0 0 1690477944.0 /bin/busybox-hardlink - -
/bin/busybox-hardlink3 0 @100755 1 0 0 0 1690477944.0 /bin/busybox-hardlink - -
/bin/busybox-hardlink4 0 @100755 1 0 0 0 1690477944.0 /bin/busybox-hardlink - -

# bin/podman run --rm quay.io/giuseppe/zstd-chunked:hard-link sh -c 'stat -c "%n -> %h" /bin/busybox-hardlink*' 
/bin/busybox-hardlink -> 1
/bin/busybox-hardlink2 -> 1
/bin/busybox-hardlink3 -> 1
/bin/busybox-hardlink4 -> 1

anything obviously wrong?

EDIT: overseen the nlink is part of the dump itself, going to fix it.

giuseppe · 2023-10-16T12:00:19Z

addressed the new comments and pushed a new version

provide a way to format the TOC in the same format that is generated by composefs-info, this is a preparation patch for the next commit. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

use mkcomposefs to generate composefs, instead of using the custom composefs-from-json. As part of the integration, retrieve the verity fs measurement after the files are written so that they can be embedded in the composefs image and their payload is validated each time the files are accessed. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

alexlarsson

lgtm

openshift-ci · 2023-10-16T14:52:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexlarsson, giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhatdan · 2023-10-16T15:00:45Z

@nalind @mtrmac PTAL

giuseppe · 2023-10-17T12:17:09Z

@flouthoc PTAL

rhatdan · 2023-10-17T13:43:00Z

LGTM

nalind · 2023-10-18T15:21:19Z

Please let the bot handle merges.

cgwalters · 2023-12-05T19:14:02Z

I know this question isn't related to this PR, just reusing it as a handy discussion forum...it looks like right now enabling this logic requires a build tag. And once the build tag is set, we unconditionally use composefs?

Is there any reason not to switch to having this be an option in storage.conf instead, and have the code always be enabled?

giuseppe · 2023-12-05T20:05:26Z

I agree, it is better to drop the build tag. I've used it when it was just a PoC and that required some custom binary to be present, but now there is no need to hide the feature

giuseppe force-pushed the mkcomposefs-dump branch from 1bfe620 to 9beb89f Compare October 12, 2023 20:53

openshift-ci bot added the approved label Oct 12, 2023

giuseppe force-pushed the mkcomposefs-dump branch 2 times, most recently from d01ce8b to aa7a442 Compare October 12, 2023 21:13

giuseppe marked this pull request as draft October 13, 2023 06:23

openshift-ci bot added the do-not-merge/work-in-progress label Oct 13, 2023

giuseppe marked this pull request as ready for review October 13, 2023 07:21

openshift-ci bot removed the do-not-merge/work-in-progress label Oct 13, 2023