Skip to content
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.

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

Allow checking credentials for image imports against repo url, not just passed url #14851

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 26 additions & 7 deletions pkg/image/importer/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,24 +86,26 @@ func (c Context) WithCredentials(credentials auth.CredentialStore) RepositoryRet
}),
auth.NewBasicHandler(credentials),
}
})
}, credentials)
}

type AuthHandlersFunc func(transport http.RoundTripper, registry *url.URL, repoName string) []auth.AuthenticationHandler

func (c Context) WithAuthHandlers(fn AuthHandlersFunc) RepositoryRetriever {
func (c Context) WithAuthHandlers(fn AuthHandlersFunc, credentials auth.CredentialStore) RepositoryRetriever {
return &repositoryRetriever{
context: c,
credentials: fn,
context: c,
authHandlerFunc: fn,
credentials: credentials,

pings: make(map[url.URL]error),
redirect: make(map[url.URL]*url.URL),
}
}

type repositoryRetriever struct {
context Context
credentials AuthHandlersFunc
context Context
authHandlerFunc AuthHandlersFunc
credentials auth.CredentialStore

pings map[url.URL]error
redirect map[url.URL]*url.URL
Expand Down Expand Up @@ -146,7 +148,7 @@ func (r *repositoryRetriever) Repository(ctx gocontext.Context, registry *url.UR
// TODO: make multiple attempts if the first credential fails
auth.NewAuthorizer(
r.context.Challenges,
r.credentials(t, registry, repoName)...,
r.authHandlerFunc(t, registry, repoName)...,
),
)

Expand Down Expand Up @@ -197,6 +199,23 @@ func (r *repositoryRetriever) ping(registry url.URL, insecure bool, transport ht
}

r.context.Challenges.AddResponse(resp)
if challenges, err := r.context.Challenges.GetChallenges(*resp.Request.URL); err == nil {
for _, ch := range challenges {
if ch.Scheme != "bearer" && ch.Scheme != "basic" {
// we only support bearer and basic scheme
continue
}
// TODO: support multiple realm headers
if realm, ok := ch.Parameters["realm"]; ok {
if url, err := url.Parse(realm); err == nil {
if scs, ok := r.credentials.(*SecretCredentialStore); ok {
glog.V(4).Infof("Adding realm entry: %v - %v", registry, url)
scs.AddRealm(&registry, url)
}
}
}
}
}

return nil, nil
}
Expand Down
62 changes: 62 additions & 0 deletions pkg/image/importer/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/docker/distribution/registry/api/errcode"

kapi "k8s.io/kubernetes/pkg/api"
kapiv1 "k8s.io/kubernetes/pkg/api/v1"

imageapi "github.com/openshift/origin/pkg/image/apis/image"
dockerregistry "github.com/openshift/origin/pkg/image/importer/dockerv1client"
Expand Down Expand Up @@ -299,6 +300,67 @@ func TestPing(t *testing.T) {
}
}

func TestPingRecordRealm(t *testing.T) {
testCases := []struct {
header string
realm string
}{
{
header: `Bearer realm="https://auth.example.com/token"`,
realm: "https://auth.example.com/token",
},
{
header: `Basic realm="https://auth.example.com/basic"`,
realm: "https://auth.example.com/basic",
},
{
header: `Digest realm="https://auth.example.com/digest"`,
realm: "",
},
}

for _, tc := range testCases {
t.Run("", func(t *testing.T) {
retriever := NewContext(http.DefaultTransport, http.DefaultTransport).WithCredentials(NewCredentialsForSecrets([]kapiv1.Secret{})).(*repositoryRetriever)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Docker-Distribution-API-Version", "registry/2.0")
w.Header().Set("WWW-Authenticate", tc.header)
w.WriteHeader(http.StatusUnauthorized)
}))
uri, _ := url.Parse(server.URL)

_, err := retriever.ping(*uri, true, retriever.context.InsecureTransport)
if err != nil {
t.Errorf("Unexpected error, got %v", err)
}
scs, ok := retriever.credentials.(*SecretCredentialStore)
if !ok {
t.Fatalf("Unexpected credential store type %T", retriever.credentials)
}
if len(tc.realm) > 0 {
if len(scs.realmStore.List()) != 1 {
t.Errorf("Unexpected realm # entries, expected 1, got %d", len(scs.realmStore.List()))
}
obj := scs.realmStore.List()[0]
ru, ok := obj.(*realmURL)
if !ok {
t.Errorf("Unexpected object type, expected realmURL, got %T", obj)
}
if ru.realm.String() != tc.realm {
t.Errorf("Unexpected realm url, expected %v got %v", tc.realm, ru.realm)
}
if ru.registry.String() != server.URL {
t.Errorf("Unexpected registry url, expected %s, got %v", server.URL, ru.registry)
}
} else {
if len(scs.realmStore.List()) > 0 {
t.Errorf("Unexpected realm # entries, expected 0, got %d", len(scs.realmStore.List()))
}
}
})
}
}

func TestShouldRetry(t *testing.T) {
r := NewRetryRepository(nil, 1, 0).(*retryRepository)

Expand Down
93 changes: 79 additions & 14 deletions pkg/image/importer/credentials.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
package importer

import (
"fmt"
"net/url"
"strings"
"sync"
"time"

"github.com/golang/glog"

"github.com/docker/distribution/registry/client/auth"

"k8s.io/client-go/tools/cache"
kapiv1 "k8s.io/kubernetes/pkg/api/v1"
"k8s.io/kubernetes/pkg/credentialprovider"
)

const (
defaultRealmCacheTTL = time.Minute
)

var (
NoCredentials auth.CredentialStore = &noopCredentialStore{}

Expand Down Expand Up @@ -47,17 +54,14 @@ func (s *refreshTokenStore) SetRefreshToken(url *url.URL, service string, token
type noopCredentialStore struct{}

func (s *noopCredentialStore) Basic(url *url.URL) (string, string) {
glog.Infof("asked to provide Basic credentials for %s", url)
return "", ""
}

func (s *noopCredentialStore) RefreshToken(url *url.URL, service string) string {
glog.Infof("asked to provide RefreshToken for %s", url)
return ""
}

func (s *noopCredentialStore) SetRefreshToken(url *url.URL, service string, token string) {
glog.Infof("asked to provide SetRefreshToken for %s", url)
}

func NewBasicCredentials() *BasicCredentials {
Expand Down Expand Up @@ -104,35 +108,47 @@ type keyringCredentialStore struct {
}

func (s *keyringCredentialStore) Basic(url *url.URL) (string, string) {
return basicCredentialsFromKeyring(s.DockerKeyring, url)
return basicCredentialsFromKeyring(s.DockerKeyring, url, nil)
}

func NewCredentialsForSecrets(secrets []kapiv1.Secret) *SecretCredentialStore {
return &SecretCredentialStore{
secrets: secrets,
refreshTokenStore: &refreshTokenStore{},
realmStore: cache.NewTTLStore(realmKeyFunc, defaultRealmCacheTTL),
}
}

func NewLazyCredentialsForSecrets(secretsFn func() ([]kapiv1.Secret, error)) *SecretCredentialStore {
return &SecretCredentialStore{
secretsFn: secretsFn,
refreshTokenStore: &refreshTokenStore{},
realmStore: cache.NewTTLStore(realmKeyFunc, defaultRealmCacheTTL),
}
}

type SecretCredentialStore struct {
lock sync.Mutex
secrets []kapiv1.Secret
secretsFn func() ([]kapiv1.Secret, error)
err error
keyring credentialprovider.DockerKeyring
lock sync.Mutex
realmStore cache.Store
secrets []kapiv1.Secret
secretsFn func() ([]kapiv1.Secret, error)
err error
keyring credentialprovider.DockerKeyring

*refreshTokenStore
}

func (s *SecretCredentialStore) Basic(url *url.URL) (string, string) {
return basicCredentialsFromKeyring(s.init(), url)
// the store holds realm entries, if the target URL matches one it means
// we should auth against registry URL rather than realm one
entry, exists, err := s.realmStore.GetByKey(url.String())
if exists && err == nil {
if ru, ok := entry.(*realmURL); ok {
return basicCredentialsFromKeyring(s.init(), url, &ru.registries)
}
}

return basicCredentialsFromKeyring(s.init(), url, nil)
}

func (s *SecretCredentialStore) Err() error {
Expand All @@ -141,6 +157,33 @@ func (s *SecretCredentialStore) Err() error {
return s.err
}

func (s *SecretCredentialStore) AddRealm(registry, realm *url.URL) {
entry, exists, err := s.realmStore.GetByKey(realm.String())
if !exists {
ru := &realmURL{
realm: *realm,
registry: {*registry},
}
s.realmStore.Add(ru)
}
if exists && err == nil {
if ru, ok := entry.(*realmURL); ok {
found := false
for _, reg := range ru.registries {
if reg.String() == registry.String() {
found = true
}
}
if !found {
s.lock.Lock()
defer s.lock.Unlock()
ru.registries = append(ru.registries, *registry)
s.realmStore.Update(ru)
}
}
}
}

func (s *SecretCredentialStore) init() credentialprovider.DockerKeyring {
s.lock.Lock()
defer s.lock.Unlock()
Expand All @@ -166,7 +209,7 @@ func (s *SecretCredentialStore) init() credentialprovider.DockerKeyring {
return keyring
}

func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, target *url.URL) (string, string) {
func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, target *url.URL, nonRealmURL *url.URL) (string, string) {
// TODO: compare this logic to Docker authConfig in v2 configuration
var value string
if len(target.Scheme) == 0 || target.Scheme == "https" {
Expand All @@ -190,12 +233,16 @@ func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, targe
// do a special case check for docker.io to match historical lookups when we respond to a challenge
if value == "auth.docker.io/token" {
glog.V(5).Infof("Being asked for %s, trying %s for legacy behavior", target, "index.docker.io/v1")
return basicCredentialsFromKeyring(keyring, &url.URL{Host: "index.docker.io", Path: "/v1"})
return basicCredentialsFromKeyring(keyring, &url.URL{Host: "index.docker.io", Path: "/v1"}, nil)
}
// docker 1.9 saves 'docker.io' in config in f23, see https://bugzilla.redhat.com/show_bug.cgi?id=1309739
if value == "index.docker.io" {
glog.V(5).Infof("Being asked for %s, trying %s for legacy behavior", target, "docker.io")
return basicCredentialsFromKeyring(keyring, &url.URL{Host: "docker.io"})
return basicCredentialsFromKeyring(keyring, &url.URL{Host: "docker.io"}, nil)
}
if nonRealmURL != nil {
glog.V(5).Infof("Trying non realm url %s for target %s", nonRealmURL, target)
return basicCredentialsFromKeyring(keyring, nonRealmURL, nil)
}

// try removing the canonical ports for the given requests
Expand All @@ -204,7 +251,7 @@ func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, targe
host := strings.SplitN(target.Host, ":", 2)[0]
glog.V(5).Infof("Being asked for %s, trying %s without port", target, host)

return basicCredentialsFromKeyring(keyring, &url.URL{Scheme: target.Scheme, Host: host, Path: target.Path})
return basicCredentialsFromKeyring(keyring, &url.URL{Scheme: target.Scheme, Host: host, Path: target.Path}, nil)
}

glog.V(5).Infof("Unable to find a secret to match %s (%s)", target, value)
Expand All @@ -213,3 +260,21 @@ func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, targe
glog.V(5).Infof("Found secret to match %s (%s): %s", target, value, configs[0].ServerAddress)
return configs[0].Username, configs[0].Password
}

// realmURL is a container associating a realm URL with an actual registry URL
type realmURL struct {
realm url.URL
registries []url.URL
}

// realmKeyFunc returns an actual registry URL for given realm URL
func realmKeyFunc(obj interface{}) (string, error) {
if key, ok := obj.(cache.ExplicitKey); ok {
return string(key), nil
}
ru, ok := obj.(*realmURL)
if !ok {
return "", fmt.Errorf("object %T is not a realmURL object", obj)
}
return ru.realm.String(), nil
}
4 changes: 2 additions & 2 deletions pkg/image/importer/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (k *mockKeyring) Lookup(image string) ([]credentialprovider.LazyAuthConfigu

func TestHubFallback(t *testing.T) {
k := &mockKeyring{}
basicCredentialsFromKeyring(k, &url.URL{Host: "auth.docker.io", Path: "/token"})
basicCredentialsFromKeyring(k, &url.URL{Host: "auth.docker.io", Path: "/token"}, nil)
if !reflect.DeepEqual([]string{"auth.docker.io/token", "index.docker.io", "docker.io"}, k.calls) {
t.Errorf("unexpected calls: %v", k.calls)
}
Expand Down Expand Up @@ -113,7 +113,7 @@ func Test_basicCredentialsFromKeyring(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
user, password := basicCredentialsFromKeyring(tt.args.keyring, tt.args.target)
user, password := basicCredentialsFromKeyring(tt.args.keyring, tt.args.target, nil)
if user != tt.user {
t.Errorf("basicCredentialsFromKeyring() user = %v, actual = %v", user, tt.user)
}
Expand Down