Skip to content

Commit 3c00f20

Browse files
committed
Fix c/image fails to pull OCI image with non-http(s):// urls
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
1 parent f5b23d3 commit 3c00f20

File tree

2 files changed

+41
-11
lines changed

2 files changed

+41
-11
lines changed

docker/docker_image_src.go

+18-4
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,9 @@ func (s *dockerImageSource) ensureManifestIsLoaded(ctx context.Context) error {
236236
return nil
237237
}
238238

239+
// getExternalBlob returns the reader of the first available blob URL from urls, which must not be empty.
240+
// This function can return nil reader when no url is supported by this function. In this case, the caller
241+
// should fallback to fetch the non-external blob (i.e. pull from the registry).
239242
func (s *dockerImageSource) getExternalBlob(ctx context.Context, urls []string) (io.ReadCloser, int64, error) {
240243
var (
241244
resp *http.Response
@@ -244,21 +247,27 @@ func (s *dockerImageSource) getExternalBlob(ctx context.Context, urls []string)
244247
if len(urls) == 0 {
245248
return nil, 0, errors.New("internal error: getExternalBlob called with no URLs")
246249
}
247-
for _, url := range urls {
250+
for _, u := range urls {
251+
if u, err := url.Parse(u); err != nil || (u.Scheme != "http" && u.Scheme != "https") {
252+
continue // unsupported url. skip this url.
253+
}
248254
// NOTE: we must not authenticate on additional URLs as those
249255
// can be abused to leak credentials or tokens. Please
250256
// refer to CVE-2020-15157 for more information.
251-
resp, err = s.c.makeRequestToResolvedURL(ctx, http.MethodGet, url, nil, nil, -1, noAuth, nil)
257+
resp, err = s.c.makeRequestToResolvedURL(ctx, http.MethodGet, u, nil, nil, -1, noAuth, nil)
252258
if err == nil {
253259
if resp.StatusCode != http.StatusOK {
254-
err = errors.Errorf("error fetching external blob from %q: %d (%s)", url, resp.StatusCode, http.StatusText(resp.StatusCode))
260+
err = errors.Errorf("error fetching external blob from %q: %d (%s)", u, resp.StatusCode, http.StatusText(resp.StatusCode))
255261
logrus.Debug(err)
256262
resp.Body.Close()
257263
continue
258264
}
259265
break
260266
}
261267
}
268+
if resp == nil && err == nil {
269+
return nil, 0, nil // fallback to non-external blob
270+
}
262271
if err != nil {
263272
return nil, 0, err
264273
}
@@ -408,7 +417,12 @@ func (s *dockerImageSource) GetBlobAt(ctx context.Context, info types.BlobInfo,
408417
// May update BlobInfoCache, preferably after it knows for certain that a blob truly exists at a specific location.
409418
func (s *dockerImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache) (io.ReadCloser, int64, error) {
410419
if len(info.URLs) != 0 {
411-
return s.getExternalBlob(ctx, info.URLs)
420+
r, s, err := s.getExternalBlob(ctx, info.URLs)
421+
if err != nil {
422+
return nil, 0, err
423+
} else if r != nil {
424+
return r, s, nil
425+
}
412426
}
413427

414428
path := fmt.Sprintf(blobsPath, reference.Path(s.physicalRef.ref), info.Digest.String())

oci/layout/oci_src.go

+23-7
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"io"
66
"io/ioutil"
77
"net/http"
8+
"net/url"
89
"os"
910
"strconv"
1011

@@ -113,7 +114,12 @@ func (s *ociImageSource) HasThreadSafeGetBlob() bool {
113114
// May update BlobInfoCache, preferably after it knows for certain that a blob truly exists at a specific location.
114115
func (s *ociImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache) (io.ReadCloser, int64, error) {
115116
if len(info.URLs) != 0 {
116-
return s.getExternalBlob(ctx, info.URLs)
117+
r, s, err := s.getExternalBlob(ctx, info.URLs)
118+
if err != nil {
119+
return nil, 0, err
120+
} else if r != nil {
121+
return r, s, nil
122+
}
117123
}
118124

119125
path, err := s.ref.blobPath(info.Digest, s.sharedBlobDir)
@@ -140,34 +146,44 @@ func (s *ociImageSource) GetSignatures(ctx context.Context, instanceDigest *dige
140146
return [][]byte{}, nil
141147
}
142148

149+
// getExternalBlob returns the reader of the first available blob URL from urls, which must not be empty.
150+
// This function can return nil reader when no url is supported by this function. In this case, the caller
151+
// should fallback to fetch the non-external blob (i.e. pull from the registry).
143152
func (s *ociImageSource) getExternalBlob(ctx context.Context, urls []string) (io.ReadCloser, int64, error) {
144153
if len(urls) == 0 {
145154
return nil, 0, errors.New("internal error: getExternalBlob called with no URLs")
146155
}
147156

148157
errWrap := errors.New("failed fetching external blob from all urls")
149-
for _, url := range urls {
150-
151-
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
158+
hasSupportedURL := false
159+
for _, u := range urls {
160+
if u, err := url.Parse(u); err != nil || (u.Scheme != "http" && u.Scheme != "https") {
161+
continue // unsupported url. skip this url.
162+
}
163+
hasSupportedURL = true
164+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
152165
if err != nil {
153-
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", url, err.Error())
166+
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", u, err.Error())
154167
continue
155168
}
156169

157170
resp, err := s.client.Do(req)
158171
if err != nil {
159-
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", url, err.Error())
172+
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", u, err.Error())
160173
continue
161174
}
162175

163176
if resp.StatusCode != http.StatusOK {
164177
resp.Body.Close()
165-
errWrap = errors.Wrapf(errWrap, "fetching %s failed, response code not 200", url)
178+
errWrap = errors.Wrapf(errWrap, "fetching %s failed, response code not 200", u)
166179
continue
167180
}
168181

169182
return resp.Body, getBlobSize(resp), nil
170183
}
184+
if !hasSupportedURL {
185+
return nil, 0, nil // fallback to non-external blob
186+
}
171187

172188
return nil, 0, errWrap
173189
}

0 commit comments

Comments
 (0)