From cf613d9ed8ce5b5d379479e040cbf4e7a058d86b Mon Sep 17 00:00:00 2001 From: Piotr Halama Date: Thu, 3 Oct 2024 15:39:54 +0200 Subject: [PATCH] Add support for image digest validation (#283) --- internal/validate/image.go | 49 ++++++++++++++++++++++----------- internal/validate/image_test.go | 23 +++++++++++++++- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/internal/validate/image.go b/internal/validate/image.go index 2263b89d..d958a614 100644 --- a/internal/validate/image.go +++ b/internal/validate/image.go @@ -20,12 +20,13 @@ import ( "context" "crypto/subtle" "encoding/hex" + "strings" + "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/kyma-project/warden/internal/helpers" "github.com/kyma-project/warden/pkg" "github.com/pkg/errors" - "strings" ) const ( @@ -79,16 +80,21 @@ func (s *notaryService) Validate(ctx context.Context, image string) error { return err } - shaBytes, err := s.loggedGetImageDigestHash(ctx, image) + shaImageBytes, shaManifestBytes, err := s.loggedGetImageDigestHash(ctx, image) if err != nil { return err } - if subtle.ConstantTimeCompare(shaBytes, expectedShaBytes) == 0 { - return pkg.NewValidationFailedErr(errors.New("unexpected image hash value")) + if subtle.ConstantTimeCompare(shaImageBytes, expectedShaBytes) == 1 { + return nil + } + + if subtle.ConstantTimeCompare(shaManifestBytes, expectedShaBytes) == 1 { + logger.Warn("deprecated: manifest hash was used for verification") + return nil } - return nil + return pkg.NewValidationFailedErr(errors.New("unexpected image hash value")) } func (s *notaryService) isImageAllowed(imgRepo string) bool { @@ -101,39 +107,50 @@ func (s *notaryService) isImageAllowed(imgRepo string) bool { return false } -func (s *notaryService) loggedGetImageDigestHash(ctx context.Context, image string) ([]byte, error) { +func (s *notaryService) loggedGetImageDigestHash(ctx context.Context, image string) ([]byte, []byte, error) { const message = "request to image registry" closeLog := helpers.LogStartTime(ctx, message) defer closeLog() - result, err := s.getImageDigestHash(image) - return result, err + return s.getImageDigestHash(image) } -func (s *notaryService) getImageDigestHash(image string) ([]byte, error) { +func (s *notaryService) getImageDigestHash(image string) ([]byte, []byte, error) { if len(image) == 0 { - return []byte{}, pkg.NewValidationFailedErr(errors.New("empty image provided")) + return []byte{}, []byte{}, pkg.NewValidationFailedErr(errors.New("empty image provided")) } ref, err := name.ParseReference(image) if err != nil { - return []byte{}, pkg.NewValidationFailedErr(errors.Wrap(err, "ref parse")) + return []byte{}, []byte{}, pkg.NewValidationFailedErr(errors.Wrap(err, "ref parse")) } i, err := remote.Image(ref) if err != nil { - return []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "get image")) + return []byte{}, []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "get image")) } + + // Deprecated: Remove manifest hash verification after all images has been signed using the new method m, err := i.Manifest() if err != nil { - return []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "image manifest")) + return []byte{}, []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "image manifest")) + } + + manifestBytes, err := hex.DecodeString(m.Config.Digest.Hex) + if err != nil { + return []byte{}, []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "manifest checksum error: %w")) + } + + digest, err := i.Digest() + if err != nil { + return []byte{}, []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "image digest")) } - bytes, err := hex.DecodeString(m.Config.Digest.Hex) + digestBytes, err := hex.DecodeString(digest.Hex) if err != nil { - return []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "checksum error: %w")) + return []byte{}, []byte{}, pkg.NewUnknownResultErr(errors.Wrap(err, "checksum error: %w")) } - return bytes, nil + return digestBytes, manifestBytes, nil } func (s *notaryService) loggedGetNotaryImageDigestHash(ctx context.Context, imgRepo, imgTag string) ([]byte, error) { diff --git a/internal/validate/image_test.go b/internal/validate/image_test.go index b8d1d741..64df5818 100644 --- a/internal/validate/image_test.go +++ b/internal/validate/image_test.go @@ -32,7 +32,14 @@ var ( trustedImage = image{ name: "europe-docker.pkg.dev/kyma-project/prod/function-controller", tag: "v20230428-1ea34f8e", - hash: []byte{244, 18, 77, 237, 156, 176, 43, 214, 188, 171, 25, 208, 79, 227, 163, 71, 22, 44, 31, 78, 117, 94, 30, 156, 54, 216, 160, 253, 117, 117, 78, 190}, + // image hash + hash: []byte{223, 6, 148, 15, 106, 95, 90, 178, 129, 233, 166, 72, 164, 160, 88, 104, 72, 130, 62, 48, 240, 49, 177, 42, 108, 15, 138, 138, 255, 113, 176, 239}, + } + trustedImageLegacy = image{ + name: "europe-docker.pkg.dev/kyma-project/prod/function-controller", + tag: "v20240731-b8af3f9c", + // manifest hash + hash: []byte{157, 125, 211, 253, 79, 175, 129, 184, 184, 72, 163, 165, 92, 251, 19, 70, 92, 162, 125, 90, 135, 102, 39, 28, 194, 201, 221, 188, 72, 73, 136, 239}, } differentHashImage = image{ name: "nginx", @@ -58,6 +65,15 @@ func Test_Validate_ProperImage_ShouldPass(t *testing.T) { require.NoError(t, err) } +func Test_Validate_ProperImageLegacy_ShouldPass(t *testing.T) { + cfg := validate.ServiceConfig{NotaryConfig: validate.NotaryConfig{}} + f := setupMockFactory() + + s := validate.NewImageValidator(&cfg, f) + err := s.Validate(context.TODO(), trustedImageLegacy.image()) + require.NoError(t, err) +} + func Test_Validate_InvalidImageName_ShouldReturnError(t *testing.T) { cfg := validate.ServiceConfig{NotaryConfig: validate.NotaryConfig{}} f := setupMockFactory() @@ -297,6 +313,10 @@ func setupMockFactory() validate.RepoFactory { Hashes: map[string][]byte{"ignored": trustedImage.hash}, Length: 1}} + trustedLegacy := client.TargetWithRole{Target: client.Target{Name: "ignored", + Hashes: map[string][]byte{"ignored": trustedImageLegacy.hash}, + Length: 1}} + unknown := client.TargetWithRole{Target: client.Target{Name: "ignored", Hashes: map[string][]byte{"ignored": unknownImage.hash}, Length: 1}} @@ -305,6 +325,7 @@ func setupMockFactory() validate.RepoFactory { Hashes: map[string][]byte{"ignored": differentHashImage.hash}}} notaryClient.On("GetTargetByName", trustedImage.tag).Return(&trusted, nil) + notaryClient.On("GetTargetByName", trustedImageLegacy.tag).Return(&trustedLegacy, nil) notaryClient.On("GetTargetByName", differentHashImage.tag).Return(&different, nil) notaryClient.On("GetTargetByName", unknownImage.tag).Return(&unknown, nil) notaryClient.On("GetTargetByName", untrustedImage.tag).