From 5445c5f44dc177fb6e3dc4beb448705c5b9d348d Mon Sep 17 00:00:00 2001 From: justinsb Date: Mon, 1 Apr 2024 08:25:33 -0400 Subject: [PATCH] Inline dependency on (Apache licensed) auth challenge This code was made internal in https://github.com/distribution/distribution/pull/4126, because it was not intended to be consumed externally. As it is Apache-licensed, start by inlining the dependency; we can then enhance test coverage and address any shortcomings. --- go.mod | 2 +- pkg/v1/remote/transport/bearer.go | 3 +- pkg/v1/remote/transport/ping.go | 5 +- .../v1/remote/transport/response_challenge.go | 120 +++++------------- .../transport/response_challenge_test.go | 54 ++++++++ .../registry/client/auth/challenge/addr.go | 27 ---- vendor/modules.txt | 1 - 7 files changed, 91 insertions(+), 121 deletions(-) rename vendor/github.com/docker/distribution/registry/client/auth/challenge/authchallenge.go => pkg/v1/remote/transport/response_challenge.go (56%) create mode 100644 pkg/v1/remote/transport/response_challenge_test.go delete mode 100644 vendor/github.com/docker/distribution/registry/client/auth/challenge/addr.go diff --git a/go.mod b/go.mod index 180a50012..bce1ae86b 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.18 require ( github.com/containerd/stargz-snapshotter/estargz v0.14.3 github.com/docker/cli v24.0.0+incompatible - github.com/docker/distribution v2.8.2+incompatible github.com/docker/docker v24.0.0+incompatible github.com/google/go-cmp v0.5.9 github.com/klauspost/compress v1.16.5 @@ -23,6 +22,7 @@ require ( cloud.google.com/go/compute/metadata v0.2.3 // indirect github.com/Microsoft/go-winio v0.6.1 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect + github.com/docker/distribution v2.8.2+incompatible // indirect github.com/docker/docker-credential-helpers v0.7.0 // indirect github.com/docker/go-connections v0.4.0 // indirect github.com/docker/go-units v0.5.0 // indirect diff --git a/pkg/v1/remote/transport/bearer.go b/pkg/v1/remote/transport/bearer.go index cb1567496..4f300a30d 100644 --- a/pkg/v1/remote/transport/bearer.go +++ b/pkg/v1/remote/transport/bearer.go @@ -25,7 +25,6 @@ import ( "net/url" "strings" - authchallenge "github.com/docker/distribution/registry/client/auth/challenge" "github.com/google/go-containerregistry/internal/redact" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/logs" @@ -151,7 +150,7 @@ func (bt *bearerTransport) RoundTrip(in *http.Request) (*http.Response, error) { } // If we hit a WWW-Authenticate challenge, it might be due to expired tokens or insufficient scope. - if challenges := authchallenge.ResponseChallenges(res); len(challenges) != 0 { + if challenges := authResponseChallenges(res); len(challenges) != 0 { // close out old response, since we will not return it. res.Body.Close() diff --git a/pkg/v1/remote/transport/ping.go b/pkg/v1/remote/transport/ping.go index 799c7ea08..9d39da728 100644 --- a/pkg/v1/remote/transport/ping.go +++ b/pkg/v1/remote/transport/ping.go @@ -23,7 +23,6 @@ import ( "strings" "time" - authchallenge "github.com/docker/distribution/registry/client/auth/challenge" "github.com/google/go-containerregistry/pkg/logs" "github.com/google/go-containerregistry/pkg/name" ) @@ -84,7 +83,7 @@ func pingSingle(ctx context.Context, reg name.Registry, t http.RoundTripper, sch Insecure: insecure, }, nil case http.StatusUnauthorized: - if challenges := authchallenge.ResponseChallenges(resp); len(challenges) != 0 { + if challenges := authResponseChallenges(resp); len(challenges) != 0 { // If we hit more than one, let's try to find one that we know how to handle. wac := pickFromMultipleChallenges(challenges) return &Challenge{ @@ -165,7 +164,7 @@ func pingParallel(ctx context.Context, reg name.Registry, t http.RoundTripper, s } } -func pickFromMultipleChallenges(challenges []authchallenge.Challenge) authchallenge.Challenge { +func pickFromMultipleChallenges(challenges []authChallenge) authChallenge { // It might happen there are multiple www-authenticate headers, e.g. `Negotiate` and `Basic`. // Picking simply the first one could result eventually in `unrecognized challenge` error, // that's why we're looping through the challenges in search for one that can be handled. diff --git a/vendor/github.com/docker/distribution/registry/client/auth/challenge/authchallenge.go b/pkg/v1/remote/transport/response_challenge.go similarity index 56% rename from vendor/github.com/docker/distribution/registry/client/auth/challenge/authchallenge.go rename to pkg/v1/remote/transport/response_challenge.go index fe238210c..306ee8e22 100644 --- a/vendor/github.com/docker/distribution/registry/client/auth/challenge/authchallenge.go +++ b/pkg/v1/remote/transport/response_challenge.go @@ -1,91 +1,28 @@ -package challenge +// Copyright 2024 Google LLC All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package transport + +// This code is copy-paste imported from the Apache-licensed https://github.com/distribution/distribution, +// as the dependency has been made internal upstream. +// There is an alternative implementation in https://fuchsia.googlesource.com/tools/+/efc566f8f0dcc061dac3d57989b24f496b109ecb/net/digest/digest.go import ( - "fmt" "net/http" - "net/url" "strings" - "sync" ) -// Challenge carries information from a WWW-Authenticate response header. -// See RFC 2617. -type Challenge struct { - // Scheme is the auth-scheme according to RFC 2617 - Scheme string - - // Parameters are the auth-params according to RFC 2617 - Parameters map[string]string -} - -// Manager manages the challenges for endpoints. -// The challenges are pulled out of HTTP responses. Only -// responses which expect challenges should be added to -// the manager, since a non-unauthorized request will be -// viewed as not requiring challenges. -type Manager interface { - // GetChallenges returns the challenges for the given - // endpoint URL. - GetChallenges(endpoint url.URL) ([]Challenge, error) - - // AddResponse adds the response to the challenge - // manager. The challenges will be parsed out of - // the WWW-Authenicate headers and added to the - // URL which was produced the response. If the - // response was authorized, any challenges for the - // endpoint will be cleared. - AddResponse(resp *http.Response) error -} - -// NewSimpleManager returns an instance of -// Manger which only maps endpoints to challenges -// based on the responses which have been added the -// manager. The simple manager will make no attempt to -// perform requests on the endpoints or cache the responses -// to a backend. -func NewSimpleManager() Manager { - return &simpleManager{ - Challenges: make(map[string][]Challenge), - } -} - -type simpleManager struct { - sync.RWMutex - Challenges map[string][]Challenge -} - -func normalizeURL(endpoint *url.URL) { - endpoint.Host = strings.ToLower(endpoint.Host) - endpoint.Host = canonicalAddr(endpoint) -} - -func (m *simpleManager) GetChallenges(endpoint url.URL) ([]Challenge, error) { - normalizeURL(&endpoint) - - m.RLock() - defer m.RUnlock() - challenges := m.Challenges[endpoint.String()] - return challenges, nil -} - -func (m *simpleManager) AddResponse(resp *http.Response) error { - challenges := ResponseChallenges(resp) - if resp.Request == nil { - return fmt.Errorf("missing request reference") - } - urlCopy := url.URL{ - Path: resp.Request.URL.Path, - Host: resp.Request.URL.Host, - Scheme: resp.Request.URL.Scheme, - } - normalizeURL(&urlCopy) - - m.Lock() - defer m.Unlock() - m.Challenges[urlCopy.String()] = challenges - return nil -} - // Octet types from RFC 2616. type octetType byte @@ -128,10 +65,10 @@ func init() { } } -// ResponseChallenges returns a list of authorization challenges +// authResponseChallenges returns a list of authorization challenges // for the given http Response. Challenges are only checked if // the response status code was a 401. -func ResponseChallenges(resp *http.Response) []Challenge { +func authResponseChallenges(resp *http.Response) []authChallenge { if resp.StatusCode == http.StatusUnauthorized { // Parse the WWW-Authenticate Header and store the challenges // on this endpoint object. @@ -141,17 +78,26 @@ func ResponseChallenges(resp *http.Response) []Challenge { return nil } -func parseAuthHeader(header http.Header) []Challenge { - challenges := []Challenge{} +func parseAuthHeader(header http.Header) []authChallenge { + challenges := []authChallenge{} for _, h := range header[http.CanonicalHeaderKey("WWW-Authenticate")] { v, p := parseValueAndParams(h) if v != "" { - challenges = append(challenges, Challenge{Scheme: v, Parameters: p}) + challenges = append(challenges, authChallenge{Scheme: v, Parameters: p}) } } return challenges } +// Note: we may be able to combine with Challenge here +type authChallenge struct { + Scheme string + + // Following the challenge there are often key/value pairs + // e.g. Bearer service="gcr.io",realm="https://auth.gcr.io/v36/tokenz" + Parameters map[string]string +} + func parseValueAndParams(header string) (value string, params map[string]string) { params = make(map[string]string) value, s := expectToken(header) diff --git a/pkg/v1/remote/transport/response_challenge_test.go b/pkg/v1/remote/transport/response_challenge_test.go new file mode 100644 index 000000000..6e0451f2b --- /dev/null +++ b/pkg/v1/remote/transport/response_challenge_test.go @@ -0,0 +1,54 @@ +// Copyright 2024 Google LLC All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package transport + +// This code is copy-paste imported from the Apache-licensed https://github.com/distribution/distribution, +// as the dependency has been made internal upstream. + +import ( + "net/http" + "testing" +) + +func TestAuthChallengeParse(t *testing.T) { + header := http.Header{} + header.Add("WWW-Authenticate", `Bearer realm="https://auth.example.com/token",service="registry.example.com",other=fun,slashed="he\"\l\lo"`) + + challenges := parseAuthHeader(header) + if len(challenges) != 1 { + t.Fatalf("Unexpected number of auth challenges: %d, expected 1", len(challenges)) + } + challenge := challenges[0] + + if expected := "bearer"; challenge.Scheme != expected { + t.Fatalf("Unexpected scheme: %s, expected: %s", challenge.Scheme, expected) + } + + if expected := "https://auth.example.com/token"; challenge.Parameters["realm"] != expected { + t.Fatalf("Unexpected param: %s, expected: %s", challenge.Parameters["realm"], expected) + } + + if expected := "registry.example.com"; challenge.Parameters["service"] != expected { + t.Fatalf("Unexpected param: %s, expected: %s", challenge.Parameters["service"], expected) + } + + if expected := "fun"; challenge.Parameters["other"] != expected { + t.Fatalf("Unexpected param: %s, expected: %s", challenge.Parameters["other"], expected) + } + + if expected := "he\"llo"; challenge.Parameters["slashed"] != expected { + t.Fatalf("Unexpected param: %s, expected: %s", challenge.Parameters["slashed"], expected) + } +} diff --git a/vendor/github.com/docker/distribution/registry/client/auth/challenge/addr.go b/vendor/github.com/docker/distribution/registry/client/auth/challenge/addr.go deleted file mode 100644 index 2c3ebe165..000000000 --- a/vendor/github.com/docker/distribution/registry/client/auth/challenge/addr.go +++ /dev/null @@ -1,27 +0,0 @@ -package challenge - -import ( - "net/url" - "strings" -) - -// FROM: https://golang.org/src/net/http/http.go -// Given a string of the form "host", "host:port", or "[ipv6::address]:port", -// return true if the string includes a port. -func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") } - -// FROM: http://golang.org/src/net/http/transport.go -var portMap = map[string]string{ - "http": "80", - "https": "443", -} - -// canonicalAddr returns url.Host but always with a ":port" suffix -// FROM: http://golang.org/src/net/http/transport.go -func canonicalAddr(url *url.URL) string { - addr := url.Host - if !hasPort(addr) { - return addr + ":" + portMap[url.Scheme] - } - return addr -} diff --git a/vendor/modules.txt b/vendor/modules.txt index fd2f3de30..5e1ceaebc 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -28,7 +28,6 @@ github.com/docker/cli/cli/config/types ## explicit github.com/docker/distribution/digestset github.com/docker/distribution/reference -github.com/docker/distribution/registry/client/auth/challenge # github.com/docker/docker v24.0.0+incompatible ## explicit github.com/docker/docker/api