Skip to content

Commit

Permalink
docker_client: Handle "invalid_scope" errors
Browse files Browse the repository at this point in the history 
By default docker_client just uses the auth challenges from the /v2/
ping request to request a Bearer Token. For some requests (e.g. for
/v2/_catalog on some registries) this might not be sufficient and return a
a HTTP Unauthorized Error with the "www-authenticate" header including
an "insufficient_scope" error. In that case the client will now retry
the request and fetch a new token with updated challenges to have the
"scope" matching for what the endpoint needs.

This fixes containers#1478

Signed-off-by: Ralf Haferkamp <[email protected]>
Signed-off-by: Ralf Haferkamp <[email protected]>
Signed-off-by: Dan Čermák <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>

Co-authored-by: Miloslav Trmač <[email protected]>
Co-authored-by: Ralf Haferkamp <[email protected]>
  • Loading branch information
@mtrmac @rhafer @dcermak
3 people authored and dcermak committed Aug 30, 2022
1 parent 2a56e49 commit 3ce7f05
Show file tree
Hide file tree
Showing 4 changed files with 228 additions and 10 deletions.
79 changes: 71 additions & 8 deletions docker/docker_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ type dockerClient struct {
}

type authScope struct {
remoteName string
actions string
resourceType string
remoteName string
actions string
}

// sendAuth determines whether we need authentication for v2 or v1 endpoint.
Expand Down Expand Up @@ -236,6 +237,7 @@ func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, regis
}
client.signatureBase = sigBase
client.useSigstoreAttachments = registryConfig.useSigstoreAttachments(ref)
client.scope.resourceType = "repository"
client.scope.actions = actions
client.scope.remoteName = reference.Path(ref.ref)
return client, nil
Expand Down Expand Up @@ -474,6 +476,33 @@ func (c *dockerClient) makeRequest(ctx context.Context, method, path string, hea
return c.makeRequestToResolvedURL(ctx, method, url, headers, stream, -1, auth, extraScope)
}

// Checks if the auth headers in the response contain an indication of a failed
// authorizdation because of an "insufficient_scope" error. If that's the case,
// returns the required scope to be used for fetching a new token.
func needsRetryWithUpdatedScope(err error, res *http.Response) (bool, *authScope) {
if err == nil && res.StatusCode == http.StatusUnauthorized {
challenges := parseAuthHeader(res.Header)
for _, challenge := range challenges {
if challenge.Scheme == "bearer" {
if errmsg, ok := challenge.Parameters["error"]; ok && errmsg == "insufficient_scope" {
if scope, ok := challenge.Parameters["scope"]; ok && scope != "" {
if newScope, err := parseAuthScope(scope); err == nil {
return true, newScope
} else {
logrus.WithFields(logrus.Fields{
"error": err,
"scope": scope,
"challenge": challenge,
}).Error("Failed to parse the authentication scope from the given challenge")
}
}
}
}
}
}
return false, nil
}

// parseRetryAfter determines the delay required by the "Retry-After" header in res and returns it,
// silently falling back to fallbackDelay if the header is missing or invalid.
func parseRetryAfter(res *http.Response, fallbackDelay time.Duration) time.Duration {
Expand Down Expand Up @@ -513,6 +542,29 @@ func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method stri
for {
res, err := c.makeRequestToResolvedURLOnce(ctx, method, url, headers, stream, streamLen, auth, extraScope)
attempts++

// By default we use pre-defined scopes per operation. In
// certain cases, this can fail when our authentication is
// insufficient, then we might be getting an error back with a
// Www-Authenticate Header indicating an insufficient scope.
//
// Check for that and update the client challenges to retry after
// requesting a new token
//
// We only try this on the first attempt, to not overload an
// already struggling server.
// We also cannot retry with a body (stream != nil) as stream
// was already read
if attempts == 1 && stream == nil && auth != noAuth {
if retry, newScope := needsRetryWithUpdatedScope(err, res); retry {
logrus.Debug("Detected insufficient_scope error, will retry request with updated scope")
// Note: This retry ignores extraScope. That’s, strictly speaking, incorrect, but we don’t currently
// expect the insufficient_scope errors to happen for those callers. If that changes, we can add support
// for more than one extra scope.
res, err = c.makeRequestToResolvedURLOnce(ctx, method, url, headers, stream, streamLen, auth, newScope)
extraScope = newScope
}
}
if res == nil || res.StatusCode != http.StatusTooManyRequests || // Only retry on StatusTooManyRequests, success or other failure is returned to caller immediately
stream != nil || // We can't retry with a body (which is not restartable in the general case)
attempts == backoffNumIterations {
Expand Down Expand Up @@ -592,8 +644,18 @@ func (c *dockerClient) setupRequestAuth(req *http.Request, extraScope *authScope
cacheKey := ""
scopes := []authScope{c.scope}
if extraScope != nil {
// Using ':' as a separator here is unambiguous because getBearerToken below uses the same separator when formatting a remote request (and because repository names can't contain colons).
cacheKey = fmt.Sprintf("%s:%s", extraScope.remoteName, extraScope.actions)
// Using ':' as a separator here is unambiguous because getBearerToken below
// uses the same separator when formatting a remote request (and because
// repository names that we create can't contain colons, and extraScope values
// coming from a server come from `parseAuthScope`, which also splits on colons).
cacheKey = fmt.Sprintf("%s:%s:%s", extraScope.resourceType, extraScope.remoteName, extraScope.actions)
if colonCount := strings.Count(cacheKey, ":"); colonCount != 2 {
return fmt.Errorf(
"Internal error: there must be exactly 2 colons in the cacheKey ('%s') but got %d",
cacheKey,
colonCount,
)
}
scopes = append(scopes, *extraScope)
}
var token bearerToken
Expand Down Expand Up @@ -648,9 +710,10 @@ func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, challenge chall
if service, ok := challenge.Parameters["service"]; ok && service != "" {
params.Add("service", service)
}

for _, scope := range scopes {
if scope.remoteName != "" && scope.actions != "" {
params.Add("scope", fmt.Sprintf("repository:%s:%s", scope.remoteName, scope.actions))
if scope.resourceType != "" && scope.remoteName != "" && scope.actions != "" {
params.Add("scope", fmt.Sprintf("%s:%s:%s", scope.resourceType, scope.remoteName, scope.actions))
}
}
params.Add("grant_type", "refresh_token")
Expand Down Expand Up @@ -700,8 +763,8 @@ func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge,
}

for _, scope := range scopes {
if scope.remoteName != "" && scope.actions != "" {
params.Add("scope", fmt.Sprintf("repository:%s:%s", scope.remoteName, scope.actions))
if scope.resourceType != "" && scope.remoteName != "" && scope.actions != "" {
params.Add("scope", fmt.Sprintf("%s:%s:%s", scope.resourceType, scope.remoteName, scope.actions))
}
}

Expand Down
141 changes: 141 additions & 0 deletions docker/docker_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package docker

import (
"context"
"errors"
"fmt"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -187,3 +188,143 @@ func TestUserAgent(t *testing.T) {
}
}
}

func TestNeedsRetryOnError(t *testing.T) {
needsRetry, _ := needsRetryWithUpdatedScope(errors.New("generic"), nil)
if needsRetry {
t.Fatal("Got needRetry for a connection that included an error")
}
}

var registrySuseComResp = http.Response{
Status: "401 Unauthorized",
StatusCode: http.StatusUnauthorized,
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
Header: map[string][]string{
"Content-Length": {"145"},
"Content-Type": {"application/json"},
"Date": {"Fri, 26 Aug 2022 08:03:13 GMT"},
"Docker-Distribution-Api-Version": {"registry/2.0"},
// "Www-Authenticate": {`Bearer realm="https://registry.suse.com/auth",service="SUSE Linux Docker Registry",scope="registry:catalog:*",error="insufficient_scope"`},
"X-Content-Type-Options": {"nosniff"},
},
Request: nil,
}

func TestNeedsRetryOnInsuficientScope(t *testing.T) {
resp := registrySuseComResp
resp.Header["Www-Authenticate"] = []string{
`Bearer realm="https://registry.suse.com/auth",service="SUSE Linux Docker Registry",scope="registry:catalog:*",error="insufficient_scope"`,
}
expectedScope := authScope{
resourceType: "registry",
remoteName: "catalog",
actions: "*",
}

needsRetry, scope := needsRetryWithUpdatedScope(nil, &resp)

if !needsRetry {
t.Fatal("Expected needing to retry")
}

if expectedScope != *scope {
t.Fatalf("Got an invalid scope, expected '%q' but got '%q'", expectedScope, *scope)
}
}

func TestNeedsRetryNoRetryWhenNoAuthHeader(t *testing.T) {
resp := registrySuseComResp
delete(resp.Header, "Www-Authenticate")

needsRetry, _ := needsRetryWithUpdatedScope(nil, &resp)

if needsRetry {
t.Fatal("Expected no need to retry, as no Authentication headers are present")
}
}

func TestNeedsRetryNoRetryWhenNoBearerAuthHeader(t *testing.T) {
resp := registrySuseComResp
resp.Header["Www-Authenticate"] = []string{
`OAuth2 realm="https://registry.suse.com/auth",service="SUSE Linux Docker Registry",scope="registry:catalog:*"`,
}

needsRetry, _ := needsRetryWithUpdatedScope(nil, &resp)

if needsRetry {
t.Fatal("Expected no need to retry, as no bearer authentication header is present")
}
}

func TestNeedsRetryNoRetryWhenNoErrorInBearer(t *testing.T) {
resp := registrySuseComResp
resp.Header["Www-Authenticate"] = []string{
`Bearer realm="https://registry.suse.com/auth",service="SUSE Linux Docker Registry",scope="registry:catalog:*"`,
}

needsRetry, _ := needsRetryWithUpdatedScope(nil, &resp)

if needsRetry {
t.Fatal("Expected no need to retry, as no insufficient error is present in the authentication header")
}
}

func TestNeedsRetryNoRetryWhenInvalidErrorInBearer(t *testing.T) {
resp := registrySuseComResp
resp.Header["Www-Authenticate"] = []string{
`Bearer realm="https://registry.suse.com/auth",service="SUSE Linux Docker Registry",scope="registry:catalog:*,error="random_error"`,
}

needsRetry, _ := needsRetryWithUpdatedScope(nil, &resp)

if needsRetry {
t.Fatal("Expected no need to retry, as no insufficient_error is present in the authentication header")
}
}

func TestNeedsRetryNoRetryWhenInvalidScope(t *testing.T) {
resp := registrySuseComResp
resp.Header["Www-Authenticate"] = []string{
`Bearer realm="https://registry.suse.com/auth",service="SUSE Linux Docker Registry",scope="foo:bar",error="insufficient_scope"`,
}

needsRetry, _ := needsRetryWithUpdatedScope(nil, &resp)

if needsRetry {
t.Fatal("Expected no need to retry, as no insufficient_error is present in the authentication header")
}
}

func TestNeedsNoRetry(t *testing.T) {
resp := http.Response{
Status: "200 OK",
StatusCode: http.StatusOK,
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
Header: map[string][]string{"Apptime": {"D=49722"},
"Content-Length": {"1683"},
"Content-Type": {"application/json; charset=utf-8"},
"Date": {"Fri, 26 Aug 2022 09:00:21 GMT"},
"Docker-Distribution-Api-Version": {"registry/2.0"},
"Link": {`</v2/_catalog?last=f35%2Fs2i-base&n=100>; rel="next"`},
"Referrer-Policy": {"same-origin"},
"Server": {"Apache"},
"Strict-Transport-Security": {"max-age=31536000; includeSubDomains; preload"},
"Vary": {"Accept"},
"X-Content-Type-Options": {"nosniff"},
"X-Fedora-Proxyserver": {"proxy10.iad2.fedoraproject.org"},
"X-Fedora-Requestid": {"YwiLpHEhLsbSTugJblBF8QAAAEI"},
"X-Frame-Options": {"SAMEORIGIN"},
"X-Xss-Protection": {"1; mode=block"},
},
}

needsRetry, _ := needsRetryWithUpdatedScope(nil, &resp)
if needsRetry {
t.Fatal("Got the need to retry, but none should be required")
}
}
5 changes: 3 additions & 2 deletions docker/docker_image_dest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,9 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context,
// Checking candidateRepo, and mounting from it, requires an
// expanded token scope.
extraScope := &authScope{
remoteName: reference.Path(candidateRepo),
actions: "pull",
resourceType: "repository",
remoteName: reference.Path(candidateRepo),
actions: "pull",
}
// This existence check is not, strictly speaking, necessary: We only _really_ need it to get the blob size, and we could record that in the cache instead.
// But a "failed" d.mountBlob currently leaves around an unterminated server-side upload, which we would try to cancel.
Expand Down
13 changes: 13 additions & 0 deletions docker/wwwauthenticate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package docker
// Based on github.com/docker/distribution/registry/client/auth/authchallenge.go, primarily stripping unnecessary dependencies.

import (
"fmt"
"net/http"
"strings"
)
Expand Down Expand Up @@ -70,6 +71,18 @@ func parseAuthHeader(header http.Header) []challenge {
return challenges
}

/// parses an authentication scope string of the form `$resource:$remote:$actions`
func parseAuthScope(scopeStr string) (*authScope, error) {
if parts := strings.Split(scopeStr, ":"); len(parts) == 3 {
return &authScope{
resourceType: parts[0],
remoteName: parts[1],
actions: parts[2],
}, nil
}
return nil, fmt.Errorf("error parsing auth scope: '%s'", scopeStr)
}

// NOTE: This is not a fully compliant parser per RFC 7235:
// Most notably it does not support more than one challenge within a single header
// Some of the whitespace parsing also seems noncompliant.
Expand Down

0 comments on commit 3ce7f05

Please sign in to comment.