[PATCH] treat manifest provided URLs differently
authorSergey Kanzhelev <S.Kanzhelev@live.com>
Thu, 24 Sep 2020 18:35:46 +0000 (18:35 +0000)
committerFelix Geyer <fgeyer@debian.org>
Sun, 21 Feb 2021 17:18:35 +0000 (17:18 +0000)
Gbp-Pq: Name cve-2020-15157.patch

containerd/remotes/docker/fetcher.go
containerd/remotes/docker/fetcher_test.go

index 4a2ce3c393b4c73f81ae9ba03f3fe3db8715eeb1..1708b68fea1d298407df75f4e417ccdb4ecb8d9e 100644 (file)
@@ -56,6 +56,23 @@ func (r dockerFetcher) Fetch(ctx context.Context, desc ocispec.Descriptor) (io.R
        }
 
        return newHTTPReadSeeker(desc.Size, func(offset int64) (io.ReadCloser, error) {
+               if len(desc.URLs) > 0 {
+                       db := *r.dockerBase
+                       db.auth = nil // do not authenticate
+                       nr := dockerFetcher{
+                               dockerBase: &db,
+                       }
+                       for _, u := range desc.URLs {
+                               log.G(ctx).WithField("url", u).Debug("trying alternative url")
+                               rc, err := nr.open(ctx, u, desc.MediaType, offset)
+                               if err != nil {
+                                       log.G(ctx).WithField("error", err).Debug("error trying url")
+                                       continue // try one of the other urls.
+                               }
+
+                               return rc, nil
+                       }
+               }
                for _, u := range urls {
                        rc, err := r.open(ctx, u, desc.MediaType, offset)
                        if err != nil {
@@ -142,14 +159,6 @@ func (r dockerFetcher) open(ctx context.Context, u, mediatype string, offset int
 func (r *dockerFetcher) getV2URLPaths(ctx context.Context, desc ocispec.Descriptor) ([]string, error) {
        var urls []string
 
-       if len(desc.URLs) > 0 {
-               // handle fetch via external urls.
-               for _, u := range desc.URLs {
-                       log.G(ctx).WithField("url", u).Debug("adding alternative url")
-                       urls = append(urls, u)
-               }
-       }
-
        switch desc.MediaType {
        case images.MediaTypeDockerSchema2Manifest, images.MediaTypeDockerSchema2ManifestList,
                images.MediaTypeDockerSchema1Manifest,
index 8b7beb758dd6236ea320c19926cfe26915f12f92..b72de6249be0394cf37dfa83158b183ec703867e 100644 (file)
@@ -23,7 +23,12 @@ import (
        "math/rand"
        "net/http"
        "net/http/httptest"
+       "net/url"
        "testing"
+
+       "github.com/containerd/containerd/images"
+       digest "github.com/opencontainers/go-digest"
+       ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 func TestFetcherOpen(t *testing.T) {
@@ -92,3 +97,135 @@ func TestFetcherOpen(t *testing.T) {
                t.Fatal("expected error opening with invalid server response")
        }
 }
+
+func TestFetcherFetch(t *testing.T) {
+       content := make([]byte, 128)
+       rand.New(rand.NewSource(1)).Read(content)
+       start := 0
+
+       s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+               t.Helper()
+
+               if r.RequestURI == "/404" {
+                       // no authorization must be provided with the initial GET
+                       if r.Header["Authorization"] != nil {
+                               t.Errorf("no authorization can be used with manifest-specified URLs")
+                               return
+                       }
+
+                       rw.WriteHeader(http.StatusNotFound)
+                       return
+               }
+
+               if r.RequestURI == "/401" {
+                       if r.Header["Authorization"] == nil {
+                               rw.Header().Set("Docker-Distribution-Api-Version", "registry/2.0")
+                               rw.Header().Set("WWW-Authenticate", "Basic realm=\"https://url\"")
+                               rw.WriteHeader(http.StatusUnauthorized)
+                               return
+                       }
+
+                       // no authorization must be provided for manifest-defined URLs
+                       t.Errorf("no authorization can be used with manifest-specified URLs")
+                       return
+               }
+
+               if r.Header["Authorization"] == nil {
+                       rw.Header().Set("Docker-Distribution-Api-Version", "registry/2.0")
+                       rw.Header().Set("WWW-Authenticate", "Basic realm=\"https://url\"")
+                       rw.WriteHeader(http.StatusUnauthorized)
+                       return
+               }
+
+               // authorizer must set Authorize header for the manifest URL
+               if start > 0 {
+                       rw.Header().Set("content-range", fmt.Sprintf("bytes %d-127/128", start))
+               }
+               rw.Header().Set("content-length", fmt.Sprintf("%d", len(content[start:])))
+               rw.Write(content[start:])
+       }))
+       defer s.Close()
+
+       baseURL, _ := url.Parse(s.URL)
+       db := &dockerBase{
+               client: s.Client(),
+               base:   *baseURL,
+       }
+       db.auth = NewAuthorizer(db.client, func(a string) (string, string, error) {
+               return "Authorize", "Basic blah", nil
+       })
+
+       f := dockerFetcher{dockerBase: db}
+
+       ctx := context.Background()
+
+       desc := ocispec.Descriptor{
+               MediaType:   images.MediaTypeDockerSchema2Manifest,
+               Digest:      digest.FromBytes([]byte("digest")),
+               Size:        10,
+               URLs:        []string{fmt.Sprintf("%s/404", s.URL), fmt.Sprintf("%s/401", s.URL)},
+               Annotations: map[string]string{},
+       }
+
+       rc, err := f.Fetch(ctx, desc)
+       if err != nil {
+               t.Fatalf("failed to open: %+v", err)
+       }
+       b, err := ioutil.ReadAll(rc)
+       if err != nil {
+               t.Fatal(err)
+       }
+       expected := content[0:]
+       if len(b) != len(expected) {
+               t.Errorf("unexpected length %d, expected %d", len(b), len(expected))
+               return
+       }
+       for i, c := range expected {
+               if b[i] != c {
+                       t.Errorf("unexpected byte %x at %d, expected %x", b[i], i, c)
+                       return
+               }
+       }
+}
+
+func TestFetcherGetV2URLPaths(t *testing.T) {
+       content := make([]byte, 128)
+       rand.New(rand.NewSource(1)).Read(content)
+       start := 0
+
+       s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+               if start > 0 {
+                       rw.Header().Set("content-range", fmt.Sprintf("bytes %d-127/128", start))
+               }
+               rw.Header().Set("content-length", fmt.Sprintf("%d", len(content[start:])))
+               rw.Write(content[start:])
+       }))
+       defer s.Close()
+
+       f := dockerFetcher{&dockerBase{
+               client: s.Client(),
+       }}
+       ctx := context.Background()
+
+       desc := ocispec.Descriptor{
+               MediaType:   images.MediaTypeDockerSchema2Manifest,
+               Digest:      "digest",
+               Size:        10,
+               URLs:        []string{"first", "second"},
+               Annotations: map[string]string{},
+       }
+
+       urls, err := f.getV2URLPaths(ctx, desc)
+
+       if err != nil {
+               t.Errorf("unexpected error %v", err)
+               return
+       }
+
+       // blobs and manifest/digest
+       // URLs from the descriptor should not be added to the list of alternative sources
+       if len(urls) != 2 {
+               t.Errorf("unexpected number of urls: %d, expected %d", len(urls), 2)
+               return
+       }
+}