Skip to content

docker: mimic docker upstream registry authentication #211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 30, 2017
Merged

docker: mimic docker upstream registry authentication #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 48 additions & 98 deletions docker/docker_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,20 @@ var ErrV1NotSupported = errors.New("can't talk to a V1 docker registry")

// dockerClient is configuration for dealing with a single Docker registry.
type dockerClient struct {
ctx *types.SystemContext
registry string
username string
password string
wwwAuthenticate string // Cache of a value set by ping() if scheme is not empty
scheme string // Cache of a value returned by a successful ping() if not empty
client *http.Client
signatureBase signatureStorageBase
ctx *types.SystemContext
registry string
username string
password string
scheme string // Cache of a value returned by a successful ping() if not empty
client *http.Client
signatureBase signatureStorageBase
challenges []challenge
scope authScope
}

type authScope struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider inlining the fields into dockerClient directly, but this is perfectly fine as well.

remoteName string
actions string
}

// this is cloned from docker/go-connections because upstream docker has changed
Expand Down Expand Up @@ -147,7 +153,7 @@ func hasFile(files []os.FileInfo, name string) bool {

// newDockerClient returns a new dockerClient instance for refHostname (a host a specified in the Docker image reference, not canonicalized to dockerRegistry)
// “write” specifies whether the client will be used for "write" access (in particular passed to lookaside.go:toplevelFromSection)
func newDockerClient(ctx *types.SystemContext, ref dockerReference, write bool) (*dockerClient, error) {
func newDockerClient(ctx *types.SystemContext, ref dockerReference, write bool, actions string) (*dockerClient, error) {
registry := ref.ref.Hostname()
if registry == dockerHostname {
registry = dockerRegistry
Expand Down Expand Up @@ -184,19 +190,20 @@ func newDockerClient(ctx *types.SystemContext, ref dockerReference, write bool)
password: password,
client: client,
signatureBase: sigBase,
scope: authScope{
actions: actions,
remoteName: ref.ref.RemoteName(),
},
}, nil
}

// makeRequest creates and executes a http.Request with the specified parameters, adding authentication and TLS options for the Docker client.
// url is NOT an absolute URL, but a path relative to the /v2/ top-level API path. The host name and schema is taken from the client or autodetected.
func (c *dockerClient) makeRequest(method, url string, headers map[string][]string, stream io.Reader) (*http.Response, error) {
if c.scheme == "" {
pr, err := c.ping()
if err != nil {
if err := c.ping(); err != nil {
return nil, err
}
c.wwwAuthenticate = pr.WWWAuthenticate
c.scheme = pr.scheme
}

url = fmt.Sprintf(baseURL, c.scheme, c.registry) + url
Expand Down Expand Up @@ -224,7 +231,7 @@ func (c *dockerClient) makeRequestToResolvedURL(method, url string, headers map[
if c.ctx != nil && c.ctx.DockerRegistryUserAgent != "" {
req.Header.Add("User-Agent", c.ctx.DockerRegistryUserAgent)
}
if c.wwwAuthenticate != "" && sendAuth {
if sendAuth {
if err := c.setupRequestAuth(req); err != nil {
return nil, err
}
Expand All @@ -237,87 +244,38 @@ func (c *dockerClient) makeRequestToResolvedURL(method, url string, headers map[
return res, nil
}

// we're using the challenges from the /v2/ ping response and not the one from the destination
// URL in this request because:
//
// 1) docker does that as well
// 2) gcr.io is sending 401 without a WWW-Authenticate header in the real request
//
// debugging: https://github.com/containers/image/pull/211#issuecomment-273426236 and follows up
func (c *dockerClient) setupRequestAuth(req *http.Request) error {
tokens := strings.SplitN(strings.TrimSpace(c.wwwAuthenticate), " ", 2)
if len(tokens) != 2 {
return errors.Errorf("expected 2 tokens in WWW-Authenticate: %d, %s", len(tokens), c.wwwAuthenticate)
if len(c.challenges) == 0 {
return nil
}
switch tokens[0] {
case "Basic":
// assume just one...
challenge := c.challenges[0]
switch challenge.Scheme {
case "basic":
req.SetBasicAuth(c.username, c.password)
return nil
case "Bearer":
// FIXME? This gets a new token for every API request;
// we may be easily able to reuse a previous token, e.g.
// for OpenShift the token only identifies the user and does not vary
// across operations. Should we just try the request first, and
// only get a new token on failure?
// OTOH what to do with the single-use body stream in that case?

// Try performing the request, expecting it to fail.
testReq := *req
// Do not use the body stream, or we couldn't reuse it for the "real" call later.
testReq.Body = nil
testReq.ContentLength = 0
res, err := c.client.Do(&testReq)
if err != nil {
return err
}
chs := parseAuthHeader(res.Header)
// We could end up in this "if" statement if the /v2/ call (during ping)
// returned 401 with a valid WWW-Authenticate=Bearer header.
// That doesn't **always** mean, however, that the specific API request
// (different from /v2/) actually needs to be authorized.
// One example of this _weird_ scenario happens with GCR.io docker
// registries.
if res.StatusCode != http.StatusUnauthorized || chs == nil || len(chs) == 0 {
// With gcr.io, the /v2/ call returns a 401 with a valid WWW-Authenticate=Bearer
// header but the repository could be _public_ (no authorization is needed).
// Hence, the registry response contains no challenges and the status
// code is not 401.
// We just skip this case as it's not standard on docker/distribution
// registries (https://github.com/docker/distribution/blob/master/docs/spec/api.md#api-version-check)
if res.StatusCode != http.StatusUnauthorized {
return nil
}
// gcr.io private repositories pull instead requires us to send user:pass pair in
// order to retrieve a token and setup the correct Bearer token.
// try again one last time with Basic Auth
testReq2 := *req
// Do not use the body stream, or we couldn't reuse it for the "real" call later.
testReq2.Body = nil
testReq2.ContentLength = 0
testReq2.SetBasicAuth(c.username, c.password)
res, err := c.client.Do(&testReq2)
if err != nil {
return err
}
chs = parseAuthHeader(res.Header)
if res.StatusCode != http.StatusUnauthorized || chs == nil || len(chs) == 0 {
// no need for bearer? wtf?
return nil
}
}
// Arbitrarily use the first challenge, there is no reason to expect more than one.
challenge := chs[0]
if challenge.Scheme != "bearer" { // Another artifact of trying to handle WWW-Authenticate before it actually happens.
return errors.Errorf("Unimplemented: WWW-Authenticate Bearer replaced by %#v", challenge.Scheme)
}
case "bearer":
realm, ok := challenge.Parameters["realm"]
if !ok {
return errors.Errorf("missing realm in bearer auth challenge")
}
service, _ := challenge.Parameters["service"] // Will be "" if not present
scope, _ := challenge.Parameters["scope"] // Will be "" if not present
scope := fmt.Sprintf("repository:%s:%s", c.scope.remoteName, c.scope.actions)
token, err := c.getBearerToken(realm, service, scope)
if err != nil {
return err
}
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
return nil
}
return errors.Errorf("no handler for %s authentication", tokens[0])
// support docker bearer with authconfig's Auth string? see docker2aci
return errors.Errorf("no handler for %s authentication", challenge.Scheme)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some similarly explicit comments should remain in this method, explaining at least that we intentionally use the WWW-Authenticate values from /v2/ and not from the destination URL, which we completely ignore, because 1) that’s what docker does and 2) because gcr.io is sending 401-without-WWW-Authenticate in the real requests, and pointing to dumps in this PR.).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

func (c *dockerClient) getBearerToken(realm, service, scope string) (string, error) {
Expand Down Expand Up @@ -427,39 +385,31 @@ func getAuth(ctx *types.SystemContext, registry string) (string, string, error)
return "", "", nil
}

type pingResponse struct {
WWWAuthenticate string
APIVersion string
scheme string
}

func (c *dockerClient) ping() (*pingResponse, error) {
ping := func(scheme string) (*pingResponse, error) {
func (c *dockerClient) ping() error {
ping := func(scheme string) error {
url := fmt.Sprintf(baseURL, scheme, c.registry)
resp, err := c.makeRequestToResolvedURL("GET", url, nil, nil, -1, true)
logrus.Debugf("Ping %s err %#v", url, err)
if err != nil {
return nil, err
return err
}
defer resp.Body.Close()
logrus.Debugf("Ping %s status %d", scheme+"://"+c.registry+"/v2/", resp.StatusCode)
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusUnauthorized {
return nil, errors.Errorf("error pinging repository, response code %d", resp.StatusCode)
return errors.Errorf("error pinging repository, response code %d", resp.StatusCode)
}
pr := &pingResponse{}
pr.WWWAuthenticate = resp.Header.Get("WWW-Authenticate")
pr.APIVersion = resp.Header.Get("Docker-Distribution-Api-Version")
pr.scheme = scheme
return pr, nil
c.challenges = parseAuthHeader(resp.Header)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels unclean; other values are only set via the pr structure, and c.challenges differs for no obvious reason.

(We could probably get rid of pingResponse altogether, have the nested function return a (*http.Response, scheme string, error), and move the resp.Header.Get() calls and the data transfers which now happen at the top of makeRequest to the successful termination path of ping. Not sure whether that would be the cleanest way, it’s just one of many options. Just adding challenges to pingResponse would also work. Or dropping pingReponse and making all the c. assignments in here.)

c.scheme = scheme
return nil
}
pr, err := ping("https")
err := ping("https")
if err != nil && c.ctx != nil && c.ctx.DockerInsecureSkipTLSVerify {
pr, err = ping("http")
err = ping("http")
}
if err != nil {
err = errors.Wrap(err, "pinging docker registry returned")
if c.ctx != nil && c.ctx.DockerDisableV1Ping {
return nil, err
return err
}
// best effort to understand if we're talking to a V1 registry
pingV1 := func(scheme string) bool {
Expand All @@ -484,7 +434,7 @@ func (c *dockerClient) ping() (*pingResponse, error) {
err = ErrV1NotSupported
}
}
return pr, err
return err
}

func getDefaultConfigDir(confPath string) string {
Expand Down
2 changes: 1 addition & 1 deletion docker/docker_image_dest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type dockerImageDestination struct {

// newImageDestination creates a new ImageDestination for the specified image reference.
func newImageDestination(ctx *types.SystemContext, ref dockerReference) (types.ImageDestination, error) {
c, err := newDockerClient(ctx, ref, true)
c, err := newDockerClient(ctx, ref, true, "push")
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions docker/docker_image_src.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type dockerImageSource struct {
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
// The caller must call .Close() on the returned ImageSource.
func newImageSource(ctx *types.SystemContext, ref dockerReference, requestedManifestMIMETypes []string) (*dockerImageSource, error) {
c, err := newDockerClient(ctx, ref, false)
c, err := newDockerClient(ctx, ref, false, "pull")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -261,7 +261,7 @@ func (s *dockerImageSource) getOneSignature(url *url.URL) (signature []byte, mis

// deleteImage deletes the named image from the registry, if supported.
func deleteImage(ctx *types.SystemContext, ref dockerReference) error {
c, err := newDockerClient(ctx, ref, true)
c, err := newDockerClient(ctx, ref, true, "push")
if err != nil {
return err
}
Expand Down