From 2a36074db65941c87eb448acc033779f520587cd Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 5 Aug 2021 11:15:20 +0200 Subject: [PATCH] libimage: {un}tag: reject digests Make sure that tag and untag reject digested input. Also add unit tests for both to make sure we're not regressing in the future. Fixes: containers/common#710 Signed-off-by: Valentin Rothberg --- common/libimage/image.go | 17 +++++- common/libimage/image_test.go | 104 ++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/common/libimage/image.go b/common/libimage/image.go index c47e633394..b02bf9783a 100644 --- a/common/libimage/image.go +++ b/common/libimage/image.go @@ -448,14 +448,24 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma return parent.removeRecursive(ctx, rmMap, processedIDs, "", options) } +var errTagDigest = errors.New("tag by digest not supported") + // Tag the image with the specified name and store it in the local containers // storage. The name is normalized according to the rules of NormalizeName. func (i *Image) Tag(name string) error { + if strings.HasPrefix(name, "sha256:") { // ambiguous input + return errors.Wrap(errTagDigest, name) + } + ref, err := NormalizeName(name) if err != nil { return errors.Wrapf(err, "error normalizing name %q", name) } + if _, isDigested := ref.(reference.Digested); isDigested { + return errors.Wrap(errTagDigest, name) + } + logrus.Debugf("Tagging image %s with %q", i.ID(), ref.String()) if i.runtime.eventChannel != nil { defer i.runtime.writeEvent(&Event{ID: i.ID(), Name: name, Time: time.Now(), Type: EventTypeImageTag}) @@ -480,7 +490,7 @@ var errUntagDigest = errors.New("untag by digest not supported") // the local containers storage. The name is normalized according to the rules // of NormalizeName. func (i *Image) Untag(name string) error { - if strings.HasPrefix(name, "sha256:") { + if strings.HasPrefix(name, "sha256:") { // ambiguous input return errors.Wrap(errUntagDigest, name) } @@ -488,6 +498,11 @@ func (i *Image) Untag(name string) error { if err != nil { return errors.Wrapf(err, "error normalizing name %q", name) } + + if _, isDigested := ref.(reference.Digested); isDigested { + return errors.Wrap(errUntagDigest, name) + } + name = ref.String() logrus.Debugf("Untagging %q from image %s", ref.String(), i.ID()) diff --git a/common/libimage/image_test.go b/common/libimage/image_test.go index 3e27fa98b8..29836fafff 100644 --- a/common/libimage/image_test.go +++ b/common/libimage/image_test.go @@ -164,3 +164,107 @@ func TestImageFunctions(t *testing.T) { require.Equal(t, labels, imageData.Labels, "inspect data should match") require.Equal(t, image.NamesHistory(), imageData.NamesHistory, "inspect data should match") } + +func TestTag(t *testing.T) { + // Note: this will resolve pull from the GCR registry (see + // testdata/registries.conf). + busyboxLatest := "docker.io/library/busybox:latest" + + runtime, cleanup := testNewRuntime(t) + defer cleanup() + ctx := context.Background() + + pullOptions := &PullOptions{} + pullOptions.Writer = os.Stdout + pulledImages, err := runtime.Pull(ctx, busyboxLatest, config.PullPolicyMissing, pullOptions) + require.NoError(t, err) + require.Len(t, pulledImages, 1) + + image := pulledImages[0] + + digest := "sha256:adab3844f497ab9171f070d4cae4114b5aec565ac772e2f2579405b78be67c96" + + // Tag + for _, test := range []struct { + tag string + resolvesTo string + expectError bool + }{ + {"foo", "localhost/foo:latest", false}, + {"docker.io/foo", "docker.io/library/foo:latest", false}, + {"quay.io/bar/foo:tag", "quay.io/bar/foo:tag", false}, + {"registry.com/$invalid", "", true}, + {digest, "", true}, + {"foo@" + digest, "", true}, + {"quay.io/foo@" + digest, "", true}, + {"", "", true}, + } { + err := image.Tag(test.tag) + if test.expectError { + require.Error(t, err, "tag should have failed: %v", test) + continue + } + require.NoError(t, err, "tag should have succeeded: %v", test) + + _, resolvedName, err := runtime.LookupImage(test.tag, nil) + require.NoError(t, err, "image should have resolved locally: %v", test) + require.Equal(t, test.resolvesTo, resolvedName, "image should have resolved correctly: %v", test) + } + + // Check for specific error. + err = image.Tag("foo@" + digest) + require.True(t, errors.Cause(err) == errTagDigest, "check for specific digest error") +} + +func TestUntag(t *testing.T) { + // Note: this will resolve pull from the GCR registry (see + // testdata/registries.conf). + busyboxLatest := "docker.io/library/busybox:latest" + + runtime, cleanup := testNewRuntime(t) + defer cleanup() + ctx := context.Background() + + pullOptions := &PullOptions{} + pullOptions.Writer = os.Stdout + pulledImages, err := runtime.Pull(ctx, busyboxLatest, config.PullPolicyMissing, pullOptions) + require.NoError(t, err) + require.Len(t, pulledImages, 1) + + image := pulledImages[0] + + digest := "sha256:adab3844f497ab9171f070d4cae4114b5aec565ac772e2f2579405b78be67c96" + + // Untag + for _, test := range []struct { + tag string + untag string + expectError bool + }{ + {"foo", "foo", false}, + {"foo", "foo:latest", false}, + {"foo", "localhost/foo", false}, + {"foo", "localhost/foo:latest", false}, + {"quay.io/image/foo", "quay.io/image/foo", false}, + {"foo", "doNotExist", true}, + {"foo", digest, true}, + {"foo", "foo@" + digest, true}, + {"foo", "localhost/foo@" + digest, true}, + } { + err := image.Tag(test.tag) + require.NoError(t, err, "tag should have succeeded: %v", test) + + err = image.Untag(test.untag) + if test.expectError { + require.Error(t, err, "untag should have failed: %v", test) + continue + } + require.NoError(t, err, "untag should have succeedded: %v", test) + _, resolvedName, err := runtime.LookupImage(test.tag, nil) + require.Error(t, err, "image should not resolve after untag anymore (%s): %v", resolvedName, test) + } + + // Check for specific error. + err = image.Untag("foo@" + digest) + require.True(t, errors.Cause(err) == errUntagDigest, "check for specific digest error") +}