From 27456cae7ca90f46db985baea456c4d6064b98f8 Mon Sep 17 00:00:00 2001 From: Avi Deitcher Date: Tue, 16 Jan 2024 18:52:22 +0200 Subject: [PATCH 1/5] pillar: Encode media type in verifier filenames. Implemented a feature to save the media type hashed in the verifier filename. This enhancement is used in both config and status, ensuring that the media type is persistently available and survives reboots. Modified several functions in verifier.go to include media type in the filename generation. Also updated related structures in verifiertypes.go to include the media type. Issue: #3704 Signed-off-by: Avi Deitcher (cherry picked from commit c1a8e94c584a140a2834923e6c0e1108b996d53b) --- pkg/pillar/cmd/verifier/verifier.go | 36 +++++++++++++++++----- pkg/pillar/cmd/volumemgr/handleverifier.go | 1 + pkg/pillar/types/verifiertypes.go | 2 ++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/pkg/pillar/cmd/verifier/verifier.go b/pkg/pillar/cmd/verifier/verifier.go index 5a7dcdd137..814a44d300 100644 --- a/pkg/pillar/cmd/verifier/verifier.go +++ b/pkg/pillar/cmd/verifier/verifier.go @@ -14,6 +14,7 @@ package verifier import ( "flag" "fmt" + "net/url" "os" "path" "strings" @@ -308,6 +309,7 @@ func handleCreate(ctx *verifierContext, status := types.VerifyImageStatus{ Name: config.Name, ImageSha256: config.ImageSha256, + MediaType: config.MediaType, PendingAdd: true, State: types.VERIFYING, RefCount: config.RefCount, @@ -500,12 +502,20 @@ func handleGlobalConfigDelete(ctxArg interface{}, key string, } // ImageVerifierFilenames - Returns pendingFilename, verifierFilename, verifiedFilename -// for the image -func ImageVerifierFilenames(infile, sha256, tmpID string) (string, string, string) { +// for the image. The verifierFilename and verifiedFilename always will have an extension +// of the media-type, e.g. abcdeff112.application-vnd.oci.image.manifest.v1+json +// This is because we need the media-type to process the blob. Normally, we carry +// it around in the status (DownloadStatus -> BlobStatus), but those are ephemeral and +// lost during a reboot. We need that information to be persistent and survive reboot, +// so we can reconstruct it. Hence, we preserve it in the filename. It is PathEscape'd +// so it is filename-safe. +func ImageVerifierFilenames(infile, sha256, tmpID, mediaType string) (string, string, string) { verifierDirname, verifiedDirname := getVerifierDir(), getVerifiedDir() // Handle names which are paths - verified := tmpID + "." + sha256 - return infile, path.Join(verifierDirname, verified), path.Join(verifiedDirname, sha256) + mediaTypeSafe := url.PathEscape(mediaType) + verifierFilename := strings.Join([]string{tmpID, sha256, mediaTypeSafe}, ".") + verifiedFilename := strings.Join([]string{sha256, mediaTypeSafe}, ".") + return infile, path.Join(verifierDirname, verifierFilename), path.Join(verifiedDirname, verifiedFilename) } // Returns ok, size of object @@ -514,7 +524,7 @@ func markObjectAsVerifying(ctx *verifierContext, status *types.VerifyImageStatus, tmpID uuid.UUID) (bool, int64) { verifierDirname := getVerifierDir() - pendingFilename, verifierFilename, _ := ImageVerifierFilenames(config.FileLocation, config.ImageSha256, tmpID.String()) + pendingFilename, verifierFilename, _ := ImageVerifierFilenames(config.FileLocation, config.ImageSha256, tmpID.String(), config.MediaType) // Move to verifier directory which is RO // XXX should have dom0 do this and/or have RO mounts @@ -561,7 +571,7 @@ func markObjectAsVerifying(ctx *verifierContext, func markObjectAsVerified(config *types.VerifyImageConfig, status *types.VerifyImageStatus, tmpID uuid.UUID) { verifiedDirname := getVerifiedDir() - _, verifierFilename, verifiedFilename := ImageVerifierFilenames(config.FileLocation, config.ImageSha256, tmpID.String()) + _, verifierFilename, verifiedFilename := ImageVerifierFilenames(config.FileLocation, config.ImageSha256, tmpID.String(), config.MediaType) // Move directory from DownloadDirname/verifier to // DownloadDirname/verified // XXX should have dom0 do this and/or have RO mounts @@ -609,10 +619,22 @@ func handleInitVerifiedObjects(ctx *verifierContext) { func verifyImageStatusFromImageFile(imageFileName string, size int64, pathname string) *types.VerifyImageStatus { + // filename might have two parts, separated by '.': digest and PathEscape(mediaType) + var ( + mediaType string + digest string + ) + parts := strings.SplitN(imageFileName, ".", 2) + digest = parts[0] + if len(parts) == 2 { + // just ignore the error and treat mediaType as empty + mediaType, _ = url.PathUnescape(parts[1]) + } status := types.VerifyImageStatus{ Name: imageFileName, FileLocation: pathname, - ImageSha256: imageFileName, + ImageSha256: digest, + MediaType: mediaType, Size: size, State: types.VERIFIED, RefCount: 0, diff --git a/pkg/pillar/cmd/volumemgr/handleverifier.go b/pkg/pillar/cmd/volumemgr/handleverifier.go index 13eacffe9f..f76cade519 100644 --- a/pkg/pillar/cmd/volumemgr/handleverifier.go +++ b/pkg/pillar/cmd/volumemgr/handleverifier.go @@ -71,6 +71,7 @@ func MaybeAddVerifyImageConfigBlob(ctx *volumemgrContext, blob types.BlobStatus) ImageSha256: blob.Sha256, // the sha to verify Name: blob.Sha256, // we are just going to use the sha for the verifier display RefCount: refcount, + MediaType: blob.MediaType, } log.Tracef("MaybeAddVerifyImageConfigBlob - config: %+v", vic) } diff --git a/pkg/pillar/types/verifiertypes.go b/pkg/pillar/types/verifiertypes.go index 4b7dfef503..be20c29e87 100644 --- a/pkg/pillar/types/verifiertypes.go +++ b/pkg/pillar/types/verifiertypes.go @@ -21,6 +21,7 @@ import ( type VerifyImageConfig struct { ImageSha256 string // sha256 of immutable image Name string + MediaType string // MIME type FileLocation string // Current location; should be info about file Size int64 //FileLocation size RefCount uint @@ -91,6 +92,7 @@ type VerifyImageStatus struct { Name string FileLocation string // Current location Size int64 + MediaType string // MIME type PendingAdd bool PendingModify bool PendingDelete bool From d523212637c9f360d112d2d51014e98906c0a904 Mon Sep 17 00:00:00 2001 From: Nikolay Martyanov Date: Wed, 17 Jan 2024 17:26:32 +0100 Subject: [PATCH 2/5] pillar: Add unit tests for media type encoding/decoding in Verifier. This commit introduces new unit tests in verifier_test.go for the verifier module. The tests focus on ensuring that media types, specifically 'application/vnd.docker.distribution.manifest.v2+json', are correctly encoded into file names and then accurately decoded back into the VerifyImageStatus structure. Issue: #3704 Signed-off-by: Nikolay Martyanov Signed-off-by: Avi Deitcher (cherry picked from commit 41a9f96e4d62224977aff7de139fd26e09ddab2c) --- pkg/pillar/cmd/verifier/verifier.go | 7 ++++-- pkg/pillar/cmd/verifier/verifier_test.go | 30 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 pkg/pillar/cmd/verifier/verifier_test.go diff --git a/pkg/pillar/cmd/verifier/verifier.go b/pkg/pillar/cmd/verifier/verifier.go index 814a44d300..d54209a450 100644 --- a/pkg/pillar/cmd/verifier/verifier.go +++ b/pkg/pillar/cmd/verifier/verifier.go @@ -616,7 +616,10 @@ func handleInitVerifiedObjects(ctx *verifierContext) { } } -func verifyImageStatusFromImageFile(imageFileName string, +// verifyImageStatusFromVerifiedImageFile given a verified image file, +// return a VerifyImageStatus. Note that this is for a verified file, not a +// verifying file. +func verifyImageStatusFromVerifiedImageFile(imageFileName string, size int64, pathname string) *types.VerifyImageStatus { // filename might have two parts, separated by '.': digest and PathEscape(mediaType) @@ -680,7 +683,7 @@ func populateInitialStatusFromVerified(ctx *verifierContext, } log.Tracef("populateInitialStatusFromVerified: Processing %s: %d Mbytes", pathname, size/(1024*1024)) - status := verifyImageStatusFromImageFile( + status := verifyImageStatusFromVerifiedImageFile( location.Name(), size, pathname) if status != nil { imageHash, err := fileutils.ComputeShaFile(pathname) diff --git a/pkg/pillar/cmd/verifier/verifier_test.go b/pkg/pillar/cmd/verifier/verifier_test.go new file mode 100644 index 0000000000..2e2eab87cd --- /dev/null +++ b/pkg/pillar/cmd/verifier/verifier_test.go @@ -0,0 +1,30 @@ +// Copyright (c) 2024 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package verifier + +import ( + "github.com/lf-edge/eve/pkg/pillar/base" + "github.com/sirupsen/logrus" + "path" + "testing" +) + +func TestMediaTypeInStatusFromVerifiedFilename(t *testing.T) { + log = base.NewSourceLogObject(logrus.StandardLogger(), "verifier_test", 0) + mediaType := "application/vnd.docker.distribution.manifest.v2+json" + tmpID := "tempID" + sha256 := "dummySha256" + infile := "dummyFile" + + _, _, verifiedFilename := ImageVerifierFilenames(infile, sha256, tmpID, mediaType) + status := verifyImageStatusFromVerifiedImageFile(verifiedFilename, 12345, "dummyPath") + + if status == nil { + t.Fatalf("Status is nil") + } + + if status.MediaType != mediaType { + t.Errorf("MediaType in status %v does not match original %v", status.MediaType, mediaType) + } +} From 325558df873d3e60f393e025c4a10ba1b9d3a054 Mon Sep 17 00:00:00 2001 From: Nikolay Martyanov Date: Thu, 18 Jan 2024 14:41:40 +0100 Subject: [PATCH 3/5] pillar: Forcing redownload for verified files lacking MediaType. This commit updates the verifier module, specifically in `verifier.go`, to enhance the handling of verified files that lack a MediaType. Changes made: 1. In `verifyImageStatusFromVerifiedImageFile`, a check is added for the absence of MediaType in the file name. If MediaType is missing, a warning is logged, and the function returns `nil` to initiate a redownload process for the image. 2. In `populateInitialStatusFromVerified`, we handle the scenario where `status` is `nil` (indicating a missing MediaType and potentially corrupted file). The verifier now logs this situation, checks for the file's existence, and removes the file if found. This step prevents the population of blob status for these files and ensures that only complete and verified files are Issue: #3704 Signed-off-by: Nikolay Martyanov (cherry picked from commit e03b830be61a7ba800239174fa150442c74a7891) --- pkg/pillar/cmd/verifier/verifier.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/pillar/cmd/verifier/verifier.go b/pkg/pillar/cmd/verifier/verifier.go index d54209a450..bd1c5e6729 100644 --- a/pkg/pillar/cmd/verifier/verifier.go +++ b/pkg/pillar/cmd/verifier/verifier.go @@ -632,6 +632,10 @@ func verifyImageStatusFromVerifiedImageFile(imageFileName string, if len(parts) == 2 { // just ignore the error and treat mediaType as empty mediaType, _ = url.PathUnescape(parts[1]) + } else { + // if there is no mediaType, we force the redownload process by returning nil + log.Warnf("verifyImageStatusFromVerifiedImageFile: no mediaType in %s", imageFileName) + return nil } status := types.VerifyImageStatus{ Name: imageFileName, @@ -685,7 +689,20 @@ func populateInitialStatusFromVerified(ctx *verifierContext, pathname, size/(1024*1024)) status := verifyImageStatusFromVerifiedImageFile( location.Name(), size, pathname) - if status != nil { + if status == nil { + log.Warnf("populateInitialStatusFromVerified: cannot create status for %s", location.Name()) + // If the file exists, but we cannot create a status from it, consider it as corrupted and remove it + _, err := os.Stat(pathname) + if err != nil { + // file does not exist, nothing to do + continue + } + log.Functionf("populateInitialStatusFromVerified: removing corrupted file %s", pathname) + err = os.Remove(pathname) + if err != nil { + log.Errorf("populateInitialStatusFromVerified: cannot remove broken file: %v", err) + } + } else { imageHash, err := fileutils.ComputeShaFile(pathname) if err != nil { log.Errorf("populateInitialStatusFromVerified: cannot compute sha: %v", err) From 9d4d5956630cf938c7bfa360bcf6b5104c165962 Mon Sep 17 00:00:00 2001 From: Nikolay Martyanov Date: Mon, 22 Jan 2024 19:26:31 +0100 Subject: [PATCH 4/5] pillar: Enhance logging for mediaType recovery and file removal. Add trace logging for mediaType recovery in verifyImageStatusFromVerifiedImageFile. Change logging level for removing corrupted files in populateInitialStatusFromVerified to Tracef for consistent traceability. Issue: #3704 Signed-off-by: Nikolay Martyanov (cherry picked from commit 633010fb530f3806df8c81f13a12dee0dd41299e) --- pkg/pillar/cmd/verifier/verifier.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/pillar/cmd/verifier/verifier.go b/pkg/pillar/cmd/verifier/verifier.go index bd1c5e6729..6b5ee519ed 100644 --- a/pkg/pillar/cmd/verifier/verifier.go +++ b/pkg/pillar/cmd/verifier/verifier.go @@ -632,6 +632,7 @@ func verifyImageStatusFromVerifiedImageFile(imageFileName string, if len(parts) == 2 { // just ignore the error and treat mediaType as empty mediaType, _ = url.PathUnescape(parts[1]) + log.Tracef("verifyImageStatusFromVerifiedImageFile: mediaType %s recovered from %s", mediaType, imageFileName) } else { // if there is no mediaType, we force the redownload process by returning nil log.Warnf("verifyImageStatusFromVerifiedImageFile: no mediaType in %s", imageFileName) @@ -697,7 +698,7 @@ func populateInitialStatusFromVerified(ctx *verifierContext, // file does not exist, nothing to do continue } - log.Functionf("populateInitialStatusFromVerified: removing corrupted file %s", pathname) + log.Tracef("populateInitialStatusFromVerified: removing corrupted file %s", pathname) err = os.Remove(pathname) if err != nil { log.Errorf("populateInitialStatusFromVerified: cannot remove broken file: %v", err) From 7eb4648c241851100010d81f28f24db4fd159e30 Mon Sep 17 00:00:00 2001 From: Nikolay Martyanov Date: Mon, 22 Jan 2024 20:23:11 +0100 Subject: [PATCH 5/5] pillar: Add verifier uint test for filename without MediaType. Introduces a new test in verifier_test.go to handle filenames without a media type, as might be generated by older EVE versions. Ensures correct processing of such filenames by the `verifyImageStatusFromVerifiedImageFile` function. Issue: #3704 Signed-off-by: Nikolay Martyanov (cherry picked from commit 25db0d86a50b8665cb2a2127ee48a106cdfc0c2f) --- pkg/pillar/cmd/verifier/verifier_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/pillar/cmd/verifier/verifier_test.go b/pkg/pillar/cmd/verifier/verifier_test.go index 2e2eab87cd..e8427194a6 100644 --- a/pkg/pillar/cmd/verifier/verifier_test.go +++ b/pkg/pillar/cmd/verifier/verifier_test.go @@ -28,3 +28,17 @@ func TestMediaTypeInStatusFromVerifiedFilename(t *testing.T) { t.Errorf("MediaType in status %v does not match original %v", status.MediaType, mediaType) } } + +func TestMediaTypeInStatusFromVerifiedFilenameWithNoMediaType(t *testing.T) { + log = base.NewSourceLogObject(logrus.StandardLogger(), "verifier_test", 0) + dummyPath := "dummyPath" + dummyFilename := "someSha256" + verifiedFilename := path.Join(dummyPath, dummyFilename) + + status := verifyImageStatusFromVerifiedImageFile(verifiedFilename, 12345, "dummyPath") + + if status != nil { + t.Errorf("Status is not nil") + } + +}