Skip to content

Commit 0243e6f

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 0243e6f

File tree

2 files changed

+55
-19
lines changed

2 files changed

+55
-19
lines changed

docker/docker_image_src.go

+26-8
Original file line numberDiff line numberDiff line change
@@ -236,33 +236,46 @@ func (s *dockerImageSource) ensureManifestIsLoaded(ctx context.Context) error {
236236
return nil
237237
}
238238

239-
func (s *dockerImageSource) getExternalBlob(ctx context.Context, urls []string) (io.ReadCloser, int64, error) {
239+
// getExternalBlob returns the reader of the first available blob URL.
240+
// This returns a bool value (named "fallback") which indicates whether the caller can fallback to the pull of
241+
// the non-external blob (i.e. pull from the registry).
242+
// This parameter becomes true only if no url is supported by this function.
243+
// Do not pass zero-length "urls". This is considered a fatal error and returns an error without allowing fallback.
244+
func (s *dockerImageSource) getExternalBlob(ctx context.Context, urls []string) (_ io.ReadCloser, _ int64, fallback bool, _ error) {
240245
var (
241246
resp *http.Response
242247
err error
243248
)
244249
if len(urls) == 0 {
245-
return nil, 0, errors.New("internal error: getExternalBlob called with no URLs")
250+
return nil, 0, false, errors.New("internal error: getExternalBlob called with no URLs") // fatal error
246251
}
247-
for _, url := range urls {
252+
noSupportedURL := true
253+
for _, u := range urls {
254+
if u, err := url.Parse(u); err != nil || (u.Scheme != "http" && u.Scheme != "https") {
255+
continue // unsupported url. skip this url.
256+
}
257+
noSupportedURL = false
248258
// NOTE: we must not authenticate on additional URLs as those
249259
// can be abused to leak credentials or tokens. Please
250260
// refer to CVE-2020-15157 for more information.
251-
resp, err = s.c.makeRequestToResolvedURL(ctx, http.MethodGet, url, nil, nil, -1, noAuth, nil)
261+
resp, err = s.c.makeRequestToResolvedURL(ctx, http.MethodGet, u, nil, nil, -1, noAuth, nil)
252262
if err == nil {
253263
if resp.StatusCode != http.StatusOK {
254-
err = errors.Errorf("error fetching external blob from %q: %d (%s)", url, resp.StatusCode, http.StatusText(resp.StatusCode))
264+
err = errors.Errorf("error fetching external blob from %q: %d (%s)", u, resp.StatusCode, http.StatusText(resp.StatusCode))
255265
logrus.Debug(err)
256266
resp.Body.Close()
257267
continue
258268
}
259269
break
260270
}
261271
}
272+
if noSupportedURL {
273+
return nil, 0, true, errors.New("no supported url is specified") // fallback to non-external blob
274+
}
262275
if err != nil {
263-
return nil, 0, err
276+
return nil, 0, false, err
264277
}
265-
return resp.Body, getBlobSize(resp), nil
278+
return resp.Body, getBlobSize(resp), false, nil
266279
}
267280

268281
func getBlobSize(resp *http.Response) int64 {
@@ -408,7 +421,12 @@ func (s *dockerImageSource) GetBlobAt(ctx context.Context, info types.BlobInfo,
408421
// May update BlobInfoCache, preferably after it knows for certain that a blob truly exists at a specific location.
409422
func (s *dockerImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache) (io.ReadCloser, int64, error) {
410423
if len(info.URLs) != 0 {
411-
return s.getExternalBlob(ctx, info.URLs)
424+
r, s, fallback, err := s.getExternalBlob(ctx, info.URLs)
425+
if err == nil {
426+
return r, s, nil
427+
} else if !fallback {
428+
return nil, 0, err
429+
}
412430
}
413431

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

oci/layout/oci_src.go

+29-11
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, fallback, err := s.getExternalBlob(ctx, info.URLs)
118+
if err == nil {
119+
return r, s, nil
120+
} else if !fallback {
121+
return nil, 0, err
122+
}
117123
}
118124

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

143-
func (s *ociImageSource) getExternalBlob(ctx context.Context, urls []string) (io.ReadCloser, int64, error) {
149+
// getExternalBlob returns the reader of the first available blob URL.
150+
// This returns a bool value (named "fallback") which indicates whether the caller can fallback to the pull of
151+
// the non-external blob (i.e. pull from the registry).
152+
// This parameter becomes true only if no url is supported by this function.
153+
// Do not pass zero-length "urls". This is considered a fatal error and returns an error without allowing fallback.
154+
func (s *ociImageSource) getExternalBlob(ctx context.Context, urls []string) (_ io.ReadCloser, _ int64, fallback bool, _ error) {
144155
if len(urls) == 0 {
145-
return nil, 0, errors.New("internal error: getExternalBlob called with no URLs")
156+
return nil, 0, false, errors.New("internal error: getExternalBlob called with no URLs") // fatal error
146157
}
147158

148159
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)
160+
noSupportedURL := true
161+
for _, u := range urls {
162+
if u, err := url.Parse(u); err != nil || (u.Scheme != "http" && u.Scheme != "https") {
163+
continue // unsupported url. skip this url.
164+
}
165+
noSupportedURL = false
166+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
152167
if err != nil {
153-
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", url, err.Error())
168+
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", u, err.Error())
154169
continue
155170
}
156171

157172
resp, err := s.client.Do(req)
158173
if err != nil {
159-
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", url, err.Error())
174+
errWrap = errors.Wrapf(errWrap, "fetching %s failed %s", u, err.Error())
160175
continue
161176
}
162177

163178
if resp.StatusCode != http.StatusOK {
164179
resp.Body.Close()
165-
errWrap = errors.Wrapf(errWrap, "fetching %s failed, response code not 200", url)
180+
errWrap = errors.Wrapf(errWrap, "fetching %s failed, response code not 200", u)
166181
continue
167182
}
168183

169-
return resp.Body, getBlobSize(resp), nil
184+
return resp.Body, getBlobSize(resp), false, nil
185+
}
186+
if noSupportedURL {
187+
return nil, 0, true, errors.New("no supported url is specified") // fallback to non-external blob
170188
}
171189

172-
return nil, 0, errWrap
190+
return nil, 0, false, errWrap
173191
}
174192

175193
// LayerInfosForCopy returns either nil (meaning the values in the manifest are fine), or updated values for the layer

0 commit comments

Comments
 (0)