diff --git a/pkg/image/importer/client.go b/pkg/image/importer/client.go index 8e30d0c4c0a8..2977cb864abe 100644 --- a/pkg/image/importer/client.go +++ b/pkg/image/importer/client.go @@ -47,6 +47,7 @@ func NewContext(transport, insecureTransport http.RoundTripper) Context { Transport: transport, InsecureTransport: insecureTransport, Challenges: challenge.NewSimpleManager(), + Actions: []string{"pull"}, } } @@ -54,12 +55,46 @@ type Context struct { Transport http.RoundTripper InsecureTransport http.RoundTripper Challenges challenge.Manager + Scopes []auth.Scope + Actions []string +} + +func (c Context) WithScopes(scopes ...auth.Scope) Context { + c.Scopes = scopes + return c +} + +func (c Context) WithActions(actions ...string) Context { + c.Actions = actions + return c } func (c Context) WithCredentials(credentials auth.CredentialStore) RepositoryRetriever { + return c.WithAuthHandlers(func(rt http.RoundTripper, _ *url.URL, repoName string) []auth.AuthenticationHandler { + scopes := make([]auth.Scope, 0, 1+len(c.Scopes)) + scopes = append(scopes, c.Scopes...) + if len(c.Actions) == 0 { + scopes = append(scopes, auth.RepositoryScope{Repository: repoName, Actions: []string{"pull"}}) + } else { + scopes = append(scopes, auth.RepositoryScope{Repository: repoName, Actions: c.Actions}) + } + return []auth.AuthenticationHandler{ + auth.NewTokenHandlerWithOptions(auth.TokenHandlerOptions{ + Transport: rt, + Credentials: credentials, + Scopes: scopes, + }), + auth.NewBasicHandler(credentials), + } + }) +} + +type AuthHandlersFunc func(transport http.RoundTripper, registry *url.URL, repoName string) []auth.AuthenticationHandler + +func (c Context) WithAuthHandlers(fn AuthHandlersFunc) RepositoryRetriever { return &repositoryRetriever{ context: c, - credentials: credentials, + credentials: fn, pings: make(map[url.URL]error), redirect: make(map[url.URL]*url.URL), @@ -68,7 +103,7 @@ func (c Context) WithCredentials(credentials auth.CredentialStore) RepositoryRet type repositoryRetriever struct { context Context - credentials auth.CredentialStore + credentials AuthHandlersFunc pings map[url.URL]error redirect map[url.URL]*url.URL @@ -111,8 +146,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, - auth.NewTokenHandler(t, r.credentials, repoName, "pull"), - auth.NewBasicHandler(r.credentials), + r.credentials(t, registry, repoName)..., ), ) diff --git a/pkg/image/importer/credentials.go b/pkg/image/importer/credentials.go index 57fb528affae..e6b0014d29a2 100644 --- a/pkg/image/importer/credentials.go +++ b/pkg/image/importer/credentials.go @@ -19,6 +19,31 @@ var ( emptyKeyring = &credentialprovider.BasicDockerKeyring{} ) +type refreshTokenKey struct { + url string + service string +} + +type refreshTokenStore struct { + lock sync.Mutex + store map[refreshTokenKey]string +} + +func (s *refreshTokenStore) RefreshToken(url *url.URL, service string) string { + s.lock.Lock() + defer s.lock.Unlock() + return s.store[refreshTokenKey{url: url.String(), service: service}] +} + +func (s *refreshTokenStore) SetRefreshToken(url *url.URL, service string, token string) { + s.lock.Lock() + defer s.lock.Unlock() + if s.store == nil { + s.store = make(map[refreshTokenKey]string) + } + s.store[refreshTokenKey{url: url.String(), service: service}] = token +} + type noopCredentialStore struct{} func (s *noopCredentialStore) Basic(url *url.URL) (string, string) { @@ -36,7 +61,7 @@ func (s *noopCredentialStore) SetRefreshToken(url *url.URL, service string, toke } func NewBasicCredentials() *BasicCredentials { - return &BasicCredentials{} + return &BasicCredentials{refreshTokenStore: &refreshTokenStore{}} } type basicForURL struct { @@ -46,6 +71,7 @@ type basicForURL struct { type BasicCredentials struct { creds []basicForURL + *refreshTokenStore } func (c *BasicCredentials) Add(url *url.URL, username, password string) { @@ -65,38 +91,34 @@ func (c *BasicCredentials) Basic(url *url.URL) (string, string) { return "", "" } -func (c *BasicCredentials) RefreshToken(url *url.URL, service string) string { - return "" -} - -func (c *BasicCredentials) SetRefreshToken(url *url.URL, service string, token string) { -} - func NewLocalCredentials() auth.CredentialStore { - return &keyringCredentialStore{credentialprovider.NewDockerKeyring()} + return &keyringCredentialStore{ + DockerKeyring: credentialprovider.NewDockerKeyring(), + refreshTokenStore: &refreshTokenStore{}, + } } type keyringCredentialStore struct { credentialprovider.DockerKeyring + *refreshTokenStore } func (s *keyringCredentialStore) Basic(url *url.URL) (string, string) { return basicCredentialsFromKeyring(s.DockerKeyring, url) } -func (s *keyringCredentialStore) RefreshToken(url *url.URL, service string) string { - return "" -} - -func (s *keyringCredentialStore) SetRefreshToken(url *url.URL, service string, token string) { -} - func NewCredentialsForSecrets(secrets []kapiv1.Secret) *SecretCredentialStore { - return &SecretCredentialStore{secrets: secrets} + return &SecretCredentialStore{ + secrets: secrets, + refreshTokenStore: &refreshTokenStore{}, + } } func NewLazyCredentialsForSecrets(secretsFn func() ([]kapiv1.Secret, error)) *SecretCredentialStore { - return &SecretCredentialStore{secretsFn: secretsFn} + return &SecretCredentialStore{ + secretsFn: secretsFn, + refreshTokenStore: &refreshTokenStore{}, + } } type SecretCredentialStore struct { @@ -105,19 +127,14 @@ type SecretCredentialStore struct { 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) } -func (s *SecretCredentialStore) RefreshToken(url *url.URL, service string) string { - return "" -} - -func (s *SecretCredentialStore) SetRefreshToken(url *url.URL, service string, token string) { -} - func (s *SecretCredentialStore) Err() error { s.lock.Lock() defer s.lock.Unlock() @@ -151,7 +168,13 @@ func (s *SecretCredentialStore) init() credentialprovider.DockerKeyring { func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, target *url.URL) (string, string) { // TODO: compare this logic to Docker authConfig in v2 configuration - value := target.Host + target.Path + var value string + if len(target.Scheme) == 0 || target.Scheme == "https" { + value = target.Host + target.Path + } else { + // only lookup credential for http that say they are for http + value = target.String() + } // Lookup(...) expects an image (not a URL path). // The keyring strips /v1/ and /v2/ version prefixes, @@ -174,6 +197,16 @@ func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, targe glog.V(5).Infof("Being asked for %s, trying %s for legacy behavior", target, "docker.io") return basicCredentialsFromKeyring(keyring, &url.URL{Host: "docker.io"}) } + + // try removing the canonical ports for the given requests + if (strings.HasSuffix(target.Host, ":443") && target.Scheme == "https") || + (strings.HasSuffix(target.Host, ":80") && target.Scheme == "http") { + 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}) + } + glog.V(5).Infof("Unable to find a secret to match %s (%s)", target, value) return "", "" } diff --git a/pkg/image/importer/credentials_test.go b/pkg/image/importer/credentials_test.go index fa71e1d1e664..a7ec0526d3af 100644 --- a/pkg/image/importer/credentials_test.go +++ b/pkg/image/importer/credentials_test.go @@ -11,7 +11,6 @@ import ( kapiv1 "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/credentialprovider" - "github.com/golang/glog" _ "github.com/openshift/origin/pkg/api/install" ) @@ -29,7 +28,7 @@ func TestCredentialsForSecrets(t *testing.T) { for i, secret := range secrets.Items { err := kapiv1.Convert_api_Secret_To_v1_Secret(&secret, &secretsv1[i], nil) if err != nil { - glog.V(2).Infof("Unable to make the Docker keyring for %s/%s secret: %v", secret.Name, secret.Namespace, err) + t.Logf("Unable to make the Docker keyring for %s/%s secret: %v", secret.Name, secret.Namespace, err) continue } } @@ -70,3 +69,57 @@ func TestBasicCredentials(t *testing.T) { t.Fatalf("unexpected response: %s %s", u, p) } } + +func Test_basicCredentialsFromKeyring(t *testing.T) { + fn := func(host string, entry credentialprovider.DockerConfigEntry) credentialprovider.DockerKeyring { + k := &credentialprovider.BasicDockerKeyring{} + k.Add(map[string]credentialprovider.DockerConfigEntry{host: entry}) + return k + } + def := credentialprovider.DockerConfigEntry{ + Username: "local_user", + Password: "local_pass", + } + type args struct { + keyring credentialprovider.DockerKeyring + target *url.URL + } + tests := []struct { + name string + args args + user string + password string + }{ + {name: "exact", args: args{keyring: fn("localhost", def), target: &url.URL{Host: "localhost"}}, user: def.Username, password: def.Password}, + {name: "https scheme", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: def.Username, password: def.Password}, + {name: "canonical https", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "https", Host: "localhost:443"}}, user: def.Username, password: def.Password}, + {name: "only https", args: args{keyring: fn("https://localhost", def), target: &url.URL{Host: "localhost"}}, user: def.Username, password: def.Password}, + {name: "only https scheme", args: args{keyring: fn("https://localhost", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: def.Username, password: def.Password}, + {name: "mismatched scheme - http", args: args{keyring: fn("http://localhost", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: def.Username, password: def.Password}, + + // this is not allowed by the credential keyring, but should be + {name: "exact http", args: args{keyring: fn("http://localhost", def), target: &url.URL{Scheme: "http", Host: "localhost:80"}}, user: "", password: ""}, + {name: "keyring canonical https", args: args{keyring: fn("localhost:443", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: "", password: ""}, + + // these should not be allowed + {name: "host is for port 80 only", args: args{keyring: fn("localhost:80", def), target: &url.URL{Host: "localhost"}}, user: "", password: ""}, + {name: "host is for port 443 only", args: args{keyring: fn("localhost:443", def), target: &url.URL{Host: "localhost"}}, user: "", password: ""}, + {name: "don't assume port 80 in keyring is https", args: args{keyring: fn("localhost:80", def), target: &url.URL{Scheme: "http", Host: "localhost"}}, user: "", password: ""}, + {name: "canonical http", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "http", Host: "localhost:80"}}, user: "", password: ""}, + {name: "http scheme", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "http", Host: "localhost"}}, user: "", password: ""}, + {name: "https not canonical", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "https", Host: "localhost:80"}}, user: "", password: ""}, + {name: "http not canonical", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "http", Host: "localhost:443"}}, user: "", password: ""}, + {name: "mismatched scheme", args: args{keyring: fn("https://localhost", def), target: &url.URL{Scheme: "http", Host: "localhost"}}, user: "", password: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + user, password := basicCredentialsFromKeyring(tt.args.keyring, tt.args.target) + if user != tt.user { + t.Errorf("basicCredentialsFromKeyring() user = %v, actual = %v", user, tt.user) + } + if password != tt.password { + t.Errorf("basicCredentialsFromKeyring() password = %v, actual = %v", password, tt.password) + } + }) + } +}