From a4558eaccf6f4bd40b575b0b7ef5c663138423f8 Mon Sep 17 00:00:00 2001 From: Nima Talebi Date: Mon, 19 Sep 2022 22:17:09 -0700 Subject: [PATCH] fix: Scrutinize references more closely Signed-off-by: Nima Talebi --- registry/reference.go | 36 +++++++++-------- registry/remote/repository.go | 10 ++++- registry/remote/repository_test.go | 62 +++++++++++++++++++++++++++--- 3 files changed, 86 insertions(+), 22 deletions(-) diff --git a/registry/reference.go b/registry/reference.go index 71aee49c3..4ddb29902 100644 --- a/registry/reference.go +++ b/registry/reference.go @@ -81,12 +81,12 @@ type Reference struct { // That is, of all the possible forms that `path` can take, there are exactly 4 // that are considered valid. Expanding the BNF notation for `path`, the 4 are: // -// <---------------------- path ----------------------> | - Decode `path` -// <=== REPOSITORY ===><---------- reference ---------> | - Decode `reference` -// <=== REPOSITORY ===><=================== digest ===> | - Valid Form A -// <=== REPOSITORY ===> @ <=== digest ===> | - Valid Form B -// <=== REPOSITORY ===><=== TAG ======================> | - Valid Form C -// <=== REPOSITORY ===================================> | - Valid Form D +// <--- path --------------------------------------------> | - Decode `path` +// <=== REPOSITORY ===> <--- reference ------------------> | - Decode `reference` +// <=== REPOSITORY ===> @ <=================== digest ===> | - Valid Form A +// <=== REPOSITORY ===> : @ <=== digest ===> | - Valid Form B (tag is dropped) +// <=== REPOSITORY ===> : <=== TAG ======================> | - Valid Form C +// <=== REPOSITORY ======================================> | - Valid Form D // // Note: In the case of Valid Form B, TAG is dropped without any validation or // further consideration. @@ -98,10 +98,12 @@ func ParseReference(artifact string) (Reference, error) { } registry, path := parts[0], parts[1] + var isTag bool var repository string var reference string if index := strings.Index(path, "@"); index != -1 { // `digest` found; Valid Form A (if not B) + isTag = false repository = path[:index] reference = path[index+1:] @@ -112,6 +114,7 @@ func ParseReference(artifact string) (Reference, error) { } } else if index := strings.Index(path, ":"); index != -1 { // `tag` found; Valid Form C + isTag = true repository = path[:index] reference = path[index+1:] } else { @@ -123,14 +126,14 @@ func ParseReference(artifact string) (Reference, error) { Repository: repository, Reference: reference, } - if err := res.Validate(); err != nil { + if err := res.Validate(isTag); err != nil { return Reference{}, err } return res, nil } // Validate validates the entire reference. -func (r Reference) Validate() error { +func (r Reference) Validate(isTag bool) error { err := r.ValidateRegistry() if err != nil { return err @@ -139,7 +142,7 @@ func (r Reference) Validate() error { if err != nil { return err } - return r.ValidateReference() + return r.ValidateReference(isTag) } // ValidateRegistry validates the registry. @@ -160,16 +163,19 @@ func (r Reference) ValidateRepository() error { } // ValidateReference validates the reference. -func (r Reference) ValidateReference() error { - if r.Reference == "" { - return nil - } - if _, err := r.Digest(); err == nil { +func (r Reference) ValidateReference(isTag bool) error { + if r.Reference == "" || isTag && tagRegexp.MatchString(r.Reference) { return nil } - if !tagRegexp.MatchString(r.Reference) { + + if isTag { return fmt.Errorf("%w: invalid tag", errdef.ErrInvalidReference) } + + if _, err := r.Digest(); err != nil { + return fmt.Errorf("%w: invalid digest; %v", errdef.ErrInvalidReference, err) + } + return nil } diff --git a/registry/remote/repository.go b/registry/remote/repository.go index 1032a396c..73e17244e 100644 --- a/registry/remote/repository.go +++ b/registry/remote/repository.go @@ -196,17 +196,25 @@ func (r *Repository) FetchReference(ctx context.Context, reference string) (ocis func (r *Repository) ParseReference(reference string) (registry.Reference, error) { ref, err := registry.ParseReference(reference) if err != nil { + var isTag = true + // reference is not a FQDN if index := strings.IndexByte(reference, '@'); index != -1 { // drop tag since the digest is present reference = reference[index+1:] + isTag = false + } else if index = strings.IndexByte(reference, ':'); index != -1 { + // no @ but :, so it can only be a digest + isTag = false } + ref = registry.Reference{ Registry: r.Reference.Registry, Repository: r.Reference.Repository, Reference: reference, } - if err = ref.ValidateReference(); err != nil { + + if err = ref.ValidateReference(isTag); err != nil { return registry.Reference{}, err } } else if ref.Registry != r.Reference.Registry || ref.Repository != r.Reference.Repository { diff --git a/registry/remote/repository_test.go b/registry/remote/repository_test.go index 022fce211..e72f4b815 100644 --- a/registry/remote/repository_test.go +++ b/registry/remote/repository_test.go @@ -3592,37 +3592,87 @@ func TestRepository_ParseReference(t *testing.T) { wantErr: errdef.ErrInvalidReference, }, { - name: "missing reference after @", + name: "registry mismatch", repoRef: registry.Reference{ Registry: "registry.example.com", Repository: "hello-world", }, args: args{ - reference: "registry.example.com/hello-world@", + reference: "myregistry.example.com/hello-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9", }, want: registry.Reference{}, wantErr: errdef.ErrInvalidReference, }, { - name: "registry mismatch", + name: "repository mismatch", repoRef: registry.Reference{ Registry: "registry.example.com", Repository: "hello-world", }, args: args{ - reference: "myregistry.example.com/hello-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9", + reference: "registry.example.com/goodbye-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9", }, want: registry.Reference{}, wantErr: errdef.ErrInvalidReference, }, { - name: "repository mismatch", + name: "digest posing as a tag", repoRef: registry.Reference{ Registry: "registry.example.com", Repository: "hello-world", }, args: args{ - reference: "registry.example.com/goodbye-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9", + reference: "registry.example.com:5000/hello-world:sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }, + want: registry.Reference{}, + wantErr: errdef.ErrInvalidReference, + }, + { + name: "missing reference after the at sign", + repoRef: registry.Reference{ + Registry: "registry.example.com", + Repository: "hello-world", + }, + args: args{ + reference: "registry.example.com/hello-world@", + }, + want: registry.Reference{}, + wantErr: errdef.ErrInvalidReference, + }, + { + name: "missing reference after the colon", + repoRef: registry.Reference{ + Registry: "localhost", + }, + args: args{ + reference: "localhost:5000/hello:", + }, + want: registry.Reference{}, + wantErr: errdef.ErrInvalidReference, + }, + { + name: "zero-size tag, zero-size digest", + repoRef: registry.Reference{}, + args: args{ + reference: "localhost:5000/hello:@", + }, + want: registry.Reference{}, + wantErr: errdef.ErrInvalidReference, + }, + { + name: "zero-size tag with valid digest", + repoRef: registry.Reference{}, + args: args{ + reference: "localhost:5000/hello:@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }, + want: registry.Reference{}, + wantErr: errdef.ErrInvalidReference, + }, + { + name: "valid tag with zero-size digest", + repoRef: registry.Reference{}, + args: args{ + reference: "localhost:5000/hello:foobar@", }, want: registry.Reference{}, wantErr: errdef.ErrInvalidReference,