From 8b1c40732aeca2c993fdb53febf5596701c869f2 Mon Sep 17 00:00:00 2001 From: unclejack Date: Sat, 16 Aug 2014 13:27:04 +0300 Subject: [PATCH 01/12] make http usage for registry explicit Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) Conflicts: daemon/config.go daemon/daemon.go graph/pull.go graph/push.go graph/tags.go registry/registry.go registry/service.go --- docs/registry.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ docs/service.go | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/docs/registry.go b/docs/registry.go index fd74b751..8f4ae6fa 100644 --- a/docs/registry.go +++ b/docs/registry.go @@ -202,6 +202,55 @@ func ResolveRepositoryName(reposName string) (string, string, error) { return hostname, reposName, nil } +// this method expands the registry name as used in the prefix of a repo +// to a full url. if it already is a url, there will be no change. +func ExpandAndVerifyRegistryUrl(hostname string, secure bool) (endpoint string, err error) { + if strings.HasPrefix(hostname, "http:") || strings.HasPrefix(hostname, "https:") { + // if there is no slash after https:// (8 characters) then we have no path in the url + if strings.LastIndex(hostname, "/") < 9 { + // there is no path given. Expand with default path + hostname = hostname + "/v1/" + } + if _, err := pingRegistryEndpoint(hostname); err != nil { + return "", errors.New("Invalid Registry endpoint: " + err.Error()) + } + return hostname, nil + } + + // use HTTPS if secure, otherwise use HTTP + if secure { + endpoint = fmt.Sprintf("https://%s/v1/", hostname) + } else { + endpoint = fmt.Sprintf("http://%s/v1/", hostname) + } + _, err = pingRegistryEndpoint(endpoint) + if err != nil { + //TODO: triggering highland build can be done there without "failing" + err = fmt.Errorf("Invalid registry endpoint '%s': %s ", endpoint, err) + if secure { + err = fmt.Errorf("%s. If this private registry supports only HTTP, please add `--insecure-registry %s` to the daemon's arguments.", err, hostname) + } + return "", err + } + return endpoint, nil +} + +// this method verifies if the provided hostname is part of the list of +// insecure registries and returns false if HTTP should be used +func IsSecure(hostname string, insecureRegistries []string) (secure bool) { + secure = true + for _, h := range insecureRegistries { + if hostname == h { + secure = false + break + } + } + if hostname == IndexServerAddress() { + secure = true + } + return +} + func trustedLocation(req *http.Request) bool { var ( trusteds = []string{"docker.com", "docker.io"} diff --git a/docs/service.go b/docs/service.go index f7b35300..334e7c2e 100644 --- a/docs/service.go +++ b/docs/service.go @@ -40,7 +40,7 @@ func (s *Service) Auth(job *engine.Job) engine.Status { job.GetenvJson("authConfig", authConfig) // TODO: this is only done here because auth and registry need to be merged into one pkg if addr := authConfig.ServerAddress; addr != "" && addr != IndexServerAddress() { - endpoint, err := NewEndpoint(addr) + endpoint, err := NewEndpoint(addr, true) if err != nil { return job.Error(err) } From 2b9798fa190ac8aef2a6f7630fb07f116bad6289 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Tue, 19 Aug 2014 11:54:42 -0700 Subject: [PATCH 02/12] Refactor IsSecure change Fix issue with restoring the tag store and setting static configuration from the daemon. i.e. the field on the TagStore struct must be made internal or the json.Unmarshal in restore will overwrite the insecure registries to be an empty struct. Signed-off-by: Michael Crosby Conflicts: graph/pull.go graph/push.go graph/tags.go --- docs/registry.go | 44 +++++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/docs/registry.go b/docs/registry.go index 8f4ae6fa..bcbce401 100644 --- a/docs/registry.go +++ b/docs/registry.go @@ -204,51 +204,45 @@ func ResolveRepositoryName(reposName string) (string, string, error) { // this method expands the registry name as used in the prefix of a repo // to a full url. if it already is a url, there will be no change. -func ExpandAndVerifyRegistryUrl(hostname string, secure bool) (endpoint string, err error) { - if strings.HasPrefix(hostname, "http:") || strings.HasPrefix(hostname, "https:") { - // if there is no slash after https:// (8 characters) then we have no path in the url - if strings.LastIndex(hostname, "/") < 9 { - // there is no path given. Expand with default path - hostname = hostname + "/v1/" - } - if _, err := pingRegistryEndpoint(hostname); err != nil { - return "", errors.New("Invalid Registry endpoint: " + err.Error()) - } +func ExpandAndVerifyRegistryUrl(hostname string, secure bool) (string, error) { + if hostname == IndexServerAddress() { return hostname, nil } - // use HTTPS if secure, otherwise use HTTP + endpoint := fmt.Sprintf("http://%s/v1/", hostname) + if secure { endpoint = fmt.Sprintf("https://%s/v1/", hostname) - } else { - endpoint = fmt.Sprintf("http://%s/v1/", hostname) } - _, err = pingRegistryEndpoint(endpoint) - if err != nil { + + if _, oerr := pingRegistryEndpoint(endpoint); oerr != nil { //TODO: triggering highland build can be done there without "failing" - err = fmt.Errorf("Invalid registry endpoint '%s': %s ", endpoint, err) + err := fmt.Errorf("Invalid registry endpoint '%s': %s ", endpoint, oerr) + if secure { - err = fmt.Errorf("%s. If this private registry supports only HTTP, please add `--insecure-registry %s` to the daemon's arguments.", err, hostname) + err = fmt.Errorf("%s. If this private registry supports only HTTP, please add `--insecure-registry %s` to the daemon's arguments.", oerr, hostname) } + return "", err } + return endpoint, nil } // this method verifies if the provided hostname is part of the list of // insecure registries and returns false if HTTP should be used -func IsSecure(hostname string, insecureRegistries []string) (secure bool) { - secure = true +func IsSecure(hostname string, insecureRegistries []string) bool { + if hostname == IndexServerAddress() { + return true + } + for _, h := range insecureRegistries { if hostname == h { - secure = false - break + return false } } - if hostname == IndexServerAddress() { - secure = true - } - return + + return true } func trustedLocation(req *http.Request) bool { From 27ddc260e215e97c3d4f8b3f787a40dfe60e1241 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 20 Aug 2014 08:31:24 -0700 Subject: [PATCH 03/12] Don't hard code true for auth job Signed-off-by: Michael Crosby Conflicts: registry/service.go --- docs/service.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/service.go b/docs/service.go index 334e7c2e..890837ca 100644 --- a/docs/service.go +++ b/docs/service.go @@ -13,12 +13,15 @@ import ( // 'pull': Download images from any registry (TODO) // 'push': Upload images to any registry (TODO) type Service struct { + insecureRegistries []string } // NewService returns a new instance of Service ready to be // installed no an engine. -func NewService() *Service { - return &Service{} +func NewService(insecureRegistries []string) *Service { + return &Service{ + insecureRegistries: insecureRegistries, + } } // Install installs registry capabilities to eng. @@ -32,15 +35,12 @@ func (s *Service) Install(eng *engine.Engine) error { // and returns OK if authentication was sucessful. // It can be used to verify the validity of a client's credentials. func (s *Service) Auth(job *engine.Job) engine.Status { - var ( - err error - authConfig = &AuthConfig{} - ) + var authConfig = new(AuthConfig) job.GetenvJson("authConfig", authConfig) - // TODO: this is only done here because auth and registry need to be merged into one pkg + if addr := authConfig.ServerAddress; addr != "" && addr != IndexServerAddress() { - endpoint, err := NewEndpoint(addr, true) + endpoint, err := NewEndpoint(addr, IsSecure(addr, s.insecureRegistries)) if err != nil { return job.Error(err) } @@ -49,11 +49,11 @@ func (s *Service) Auth(job *engine.Job) engine.Status { } authConfig.ServerAddress = endpoint.String() } - status, err := Login(authConfig, HTTPRequestFactory(nil)) - if err != nil { + + if _, err := Login(authConfig, HTTPRequestFactory(nil)); err != nil { return job.Error(err) } - job.Printf("%s\n", status) + return engine.StatusOK } From 798fd3c7646559ec410b7147583c8bc959355716 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Fri, 10 Oct 2014 23:22:12 -0400 Subject: [PATCH 04/12] Do not verify certificate when using --insecure-registry on an HTTPS registry Signed-off-by: Tibor Vass Conflicts: registry/registry.go registry/registry_test.go registry/service.go registry/session.go --- docs/endpoint.go | 47 +++++++++++--- docs/registry.go | 143 +++++++++++++++++------------------------- docs/registry_test.go | 4 +- docs/service.go | 5 +- docs/session.go | 2 +- 5 files changed, 101 insertions(+), 100 deletions(-) diff --git a/docs/endpoint.go b/docs/endpoint.go index 5313a807..6dd4e1f6 100644 --- a/docs/endpoint.go +++ b/docs/endpoint.go @@ -2,7 +2,6 @@ package registry import ( "encoding/json" - "errors" "fmt" "io/ioutil" "net/http" @@ -34,9 +33,9 @@ func scanForApiVersion(hostname string) (string, APIVersion) { return hostname, DefaultAPIVersion } -func NewEndpoint(hostname string) (*Endpoint, error) { +func NewEndpoint(hostname string, secure bool) (*Endpoint, error) { var ( - endpoint Endpoint + endpoint = Endpoint{secure: secure} trimmedHostname string err error ) @@ -49,14 +48,27 @@ func NewEndpoint(hostname string) (*Endpoint, error) { return nil, err } + // Try HTTPS ping to registry endpoint.URL.Scheme = "https" if _, err := endpoint.Ping(); err != nil { - log.Debugf("Registry %s does not work (%s), falling back to http", endpoint, err) - // TODO: Check if http fallback is enabled - endpoint.URL.Scheme = "http" - if _, err = endpoint.Ping(); err != nil { - return nil, errors.New("Invalid Registry endpoint: " + err.Error()) + + //TODO: triggering highland build can be done there without "failing" + + if secure { + // If registry is secure and HTTPS failed, show user the error and tell them about `--insecure-registry` + // in case that's what they need. DO NOT accept unknown CA certificates, and DO NOT fallback to HTTP. + return nil, fmt.Errorf("Invalid registry endpoint %s: %v. If this private registry supports only HTTP or HTTPS with an unknown CA certificate, please add `--insecure-registry %s` to the daemon's arguments. In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; simply place the CA certificate at /etc/docker/certs.d/%s/ca.crt", endpoint, err, endpoint.URL.Host, endpoint.URL.Host) } + + // If registry is insecure and HTTPS failed, fallback to HTTP. + log.Debugf("Error from registry %q marked as insecure: %v. Insecurely falling back to HTTP", endpoint, err) + endpoint.URL.Scheme = "http" + _, err2 := endpoint.Ping() + if err2 == nil { + return &endpoint, nil + } + + return nil, fmt.Errorf("Invalid registry endpoint %q. HTTPS attempt: %v. HTTP attempt: %v", endpoint, err, err2) } return &endpoint, nil @@ -65,6 +77,7 @@ func NewEndpoint(hostname string) (*Endpoint, error) { type Endpoint struct { URL *url.URL Version APIVersion + secure bool } // Get the formated URL for the root of this registry Endpoint @@ -88,7 +101,7 @@ func (e Endpoint) Ping() (RegistryInfo, error) { return RegistryInfo{Standalone: false}, err } - resp, _, err := doRequest(req, nil, ConnectTimeout) + resp, _, err := doRequest(req, nil, ConnectTimeout, e.secure) if err != nil { return RegistryInfo{Standalone: false}, err } @@ -127,3 +140,19 @@ func (e Endpoint) Ping() (RegistryInfo, error) { log.Debugf("RegistryInfo.Standalone: %t", info.Standalone) return info, nil } + +// IsSecure returns false if the provided hostname is part of the list of insecure registries. +// Insecure registries accept HTTP and/or accept HTTPS with certificates from unknown CAs. +func IsSecure(hostname string, insecureRegistries []string) bool { + if hostname == IndexServerAddress() { + return true + } + + for _, h := range insecureRegistries { + if hostname == h { + return false + } + } + + return true +} diff --git a/docs/registry.go b/docs/registry.go index bcbce401..15fed1b8 100644 --- a/docs/registry.go +++ b/docs/registry.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/docker/docker/pkg/log" "github.com/docker/docker/utils" ) @@ -35,13 +36,17 @@ const ( ConnectTimeout ) -func newClient(jar http.CookieJar, roots *x509.CertPool, cert *tls.Certificate, timeout TimeoutType) *http.Client { +func newClient(jar http.CookieJar, roots *x509.CertPool, cert *tls.Certificate, timeout TimeoutType, secure bool) *http.Client { tlsConfig := tls.Config{RootCAs: roots} if cert != nil { tlsConfig.Certificates = append(tlsConfig.Certificates, *cert) } + if !secure { + tlsConfig.InsecureSkipVerify = true + } + httpTransport := &http.Transport{ DisableKeepAlives: true, Proxy: http.ProxyFromEnvironment, @@ -78,69 +83,76 @@ func newClient(jar http.CookieJar, roots *x509.CertPool, cert *tls.Certificate, } } -func doRequest(req *http.Request, jar http.CookieJar, timeout TimeoutType) (*http.Response, *http.Client, error) { - hasFile := func(files []os.FileInfo, name string) bool { - for _, f := range files { - if f.Name() == name { - return true - } - } - return false - } - - hostDir := path.Join("/etc/docker/certs.d", req.URL.Host) - fs, err := ioutil.ReadDir(hostDir) - if err != nil && !os.IsNotExist(err) { - return nil, nil, err - } - +func doRequest(req *http.Request, jar http.CookieJar, timeout TimeoutType, secure bool) (*http.Response, *http.Client, error) { var ( pool *x509.CertPool certs []*tls.Certificate ) - for _, f := range fs { - if strings.HasSuffix(f.Name(), ".crt") { - if pool == nil { - pool = x509.NewCertPool() + if secure && req.URL.Scheme == "https" { + hasFile := func(files []os.FileInfo, name string) bool { + for _, f := range files { + if f.Name() == name { + return true + } } - data, err := ioutil.ReadFile(path.Join(hostDir, f.Name())) - if err != nil { - return nil, nil, err - } - pool.AppendCertsFromPEM(data) + return false } - if strings.HasSuffix(f.Name(), ".cert") { - certName := f.Name() - keyName := certName[:len(certName)-5] + ".key" - if !hasFile(fs, keyName) { - return nil, nil, fmt.Errorf("Missing key %s for certificate %s", keyName, certName) - } - cert, err := tls.LoadX509KeyPair(path.Join(hostDir, certName), path.Join(hostDir, keyName)) - if err != nil { - return nil, nil, err - } - certs = append(certs, &cert) + + hostDir := path.Join("/etc/docker/certs.d", req.URL.Host) + log.Debugf("hostDir: %s", hostDir) + fs, err := ioutil.ReadDir(hostDir) + if err != nil && !os.IsNotExist(err) { + return nil, nil, err } - if strings.HasSuffix(f.Name(), ".key") { - keyName := f.Name() - certName := keyName[:len(keyName)-4] + ".cert" - if !hasFile(fs, certName) { - return nil, nil, fmt.Errorf("Missing certificate %s for key %s", certName, keyName) + + for _, f := range fs { + if strings.HasSuffix(f.Name(), ".crt") { + if pool == nil { + pool = x509.NewCertPool() + } + log.Debugf("crt: %s", hostDir+"/"+f.Name()) + data, err := ioutil.ReadFile(path.Join(hostDir, f.Name())) + if err != nil { + return nil, nil, err + } + pool.AppendCertsFromPEM(data) + } + if strings.HasSuffix(f.Name(), ".cert") { + certName := f.Name() + keyName := certName[:len(certName)-5] + ".key" + log.Debugf("cert: %s", hostDir+"/"+f.Name()) + if !hasFile(fs, keyName) { + return nil, nil, fmt.Errorf("Missing key %s for certificate %s", keyName, certName) + } + cert, err := tls.LoadX509KeyPair(path.Join(hostDir, certName), path.Join(hostDir, keyName)) + if err != nil { + return nil, nil, err + } + certs = append(certs, &cert) + } + if strings.HasSuffix(f.Name(), ".key") { + keyName := f.Name() + certName := keyName[:len(keyName)-4] + ".cert" + log.Debugf("key: %s", hostDir+"/"+f.Name()) + if !hasFile(fs, certName) { + return nil, nil, fmt.Errorf("Missing certificate %s for key %s", certName, keyName) + } } } } if len(certs) == 0 { - client := newClient(jar, pool, nil, timeout) + client := newClient(jar, pool, nil, timeout, secure) res, err := client.Do(req) if err != nil { return nil, nil, err } return res, client, nil } + for i, cert := range certs { - client := newClient(jar, pool, cert, timeout) + client := newClient(jar, pool, cert, timeout, secure) res, err := client.Do(req) // If this is the last cert, otherwise, continue to next cert if 403 or 5xx if i == len(certs)-1 || err == nil && res.StatusCode != 403 && res.StatusCode < 500 { @@ -202,49 +214,6 @@ func ResolveRepositoryName(reposName string) (string, string, error) { return hostname, reposName, nil } -// this method expands the registry name as used in the prefix of a repo -// to a full url. if it already is a url, there will be no change. -func ExpandAndVerifyRegistryUrl(hostname string, secure bool) (string, error) { - if hostname == IndexServerAddress() { - return hostname, nil - } - - endpoint := fmt.Sprintf("http://%s/v1/", hostname) - - if secure { - endpoint = fmt.Sprintf("https://%s/v1/", hostname) - } - - if _, oerr := pingRegistryEndpoint(endpoint); oerr != nil { - //TODO: triggering highland build can be done there without "failing" - err := fmt.Errorf("Invalid registry endpoint '%s': %s ", endpoint, oerr) - - if secure { - err = fmt.Errorf("%s. If this private registry supports only HTTP, please add `--insecure-registry %s` to the daemon's arguments.", oerr, hostname) - } - - return "", err - } - - return endpoint, nil -} - -// this method verifies if the provided hostname is part of the list of -// insecure registries and returns false if HTTP should be used -func IsSecure(hostname string, insecureRegistries []string) bool { - if hostname == IndexServerAddress() { - return true - } - - for _, h := range insecureRegistries { - if hostname == h { - return false - } - } - - return true -} - func trustedLocation(req *http.Request) bool { var ( trusteds = []string{"docker.com", "docker.io"} diff --git a/docs/registry_test.go b/docs/registry_test.go index ab417812..c9a9fc81 100644 --- a/docs/registry_test.go +++ b/docs/registry_test.go @@ -18,7 +18,7 @@ var ( func spawnTestRegistrySession(t *testing.T) *Session { authConfig := &AuthConfig{} - endpoint, err := NewEndpoint(makeURL("/v1/")) + endpoint, err := NewEndpoint(makeURL("/v1/"), false) if err != nil { t.Fatal(err) } @@ -30,7 +30,7 @@ func spawnTestRegistrySession(t *testing.T) *Session { } func TestPingRegistryEndpoint(t *testing.T) { - ep, err := NewEndpoint(makeURL("/v1/")) + ep, err := NewEndpoint(makeURL("/v1/"), false) if err != nil { t.Fatal(err) } diff --git a/docs/service.go b/docs/service.go index 890837ca..32274f40 100644 --- a/docs/service.go +++ b/docs/service.go @@ -89,7 +89,10 @@ func (s *Service) Search(job *engine.Job) engine.Status { if err != nil { return job.Error(err) } - endpoint, err := NewEndpoint(hostname) + + secure := IsSecure(hostname, s.insecureRegistries) + + endpoint, err := NewEndpoint(hostname, secure) if err != nil { return job.Error(err) } diff --git a/docs/session.go b/docs/session.go index 5067b8d5..28959967 100644 --- a/docs/session.go +++ b/docs/session.go @@ -64,7 +64,7 @@ func NewSession(authConfig *AuthConfig, factory *utils.HTTPRequestFactory, endpo } func (r *Session) doRequest(req *http.Request) (*http.Response, *http.Client, error) { - return doRequest(req, r.jar, r.timeout) + return doRequest(req, r.jar, r.timeout, r.indexEndpoint.secure) } // Retrieve the history of a given image from the Registry. From dff06789099b1c515219e5df63a760150515b1d1 Mon Sep 17 00:00:00 2001 From: "Daniel, Dao Quang Minh" Date: Wed, 15 Oct 2014 22:39:51 -0400 Subject: [PATCH 05/12] Avoid fallback to SSL protocols < TLS1.0 Signed-off-by: Tibor Vass Docker-DCO-1.1-Signed-off-by: Daniel, Dao Quang Minh (github: dqminh) Conflicts: registry/registry.go --- docs/registry.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/registry.go b/docs/registry.go index 15fed1b8..a03790af 100644 --- a/docs/registry.go +++ b/docs/registry.go @@ -37,7 +37,11 @@ const ( ) func newClient(jar http.CookieJar, roots *x509.CertPool, cert *tls.Certificate, timeout TimeoutType, secure bool) *http.Client { - tlsConfig := tls.Config{RootCAs: roots} + tlsConfig := tls.Config{ + RootCAs: roots, + // Avoid fallback to SSL protocols < TLS1.0 + MinVersion: tls.VersionTLS10, + } if cert != nil { tlsConfig.Certificates = append(tlsConfig.Certificates, *cert) From 0481c669c7ee82ddcb6b7ce78f14d5aa562505e5 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 28 Oct 2014 21:20:30 -0400 Subject: [PATCH 06/12] Fix login command Signed-off-by: Tibor Vass --- docs/service.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/service.go b/docs/service.go index 32274f40..7051d934 100644 --- a/docs/service.go +++ b/docs/service.go @@ -50,9 +50,11 @@ func (s *Service) Auth(job *engine.Job) engine.Status { authConfig.ServerAddress = endpoint.String() } - if _, err := Login(authConfig, HTTPRequestFactory(nil)); err != nil { + status, err := Login(authConfig, HTTPRequestFactory(nil)) + if err != nil { return job.Error(err) } + job.Printf("%s\n", status) return engine.StatusOK } From 44d97c1fd016990ef0287625457fa3b16a1ed7df Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 11 Nov 2014 17:37:44 -0500 Subject: [PATCH 07/12] registry: refactor registry.IsSecure calls into registry.NewEndpoint Signed-off-by: Tibor Vass Conflicts: registry/endpoint.go registry/endpoint_test.go registry/registry_test.go --- docs/endpoint.go | 38 ++++++++++++++++++++++++-------------- docs/endpoint_test.go | 27 +++++++++++++++++++++++++++ docs/registry_test.go | 29 +++++++++++++++++++++++++++++ docs/service.go | 6 ++---- 4 files changed, 82 insertions(+), 18 deletions(-) create mode 100644 docs/endpoint_test.go diff --git a/docs/endpoint.go b/docs/endpoint.go index 6dd4e1f6..2eac41ce 100644 --- a/docs/endpoint.go +++ b/docs/endpoint.go @@ -33,21 +33,15 @@ func scanForApiVersion(hostname string) (string, APIVersion) { return hostname, DefaultAPIVersion } -func NewEndpoint(hostname string, secure bool) (*Endpoint, error) { - var ( - endpoint = Endpoint{secure: secure} - trimmedHostname string - err error - ) - if !strings.HasPrefix(hostname, "http") { - hostname = "https://" + hostname - } - trimmedHostname, endpoint.Version = scanForApiVersion(hostname) - endpoint.URL, err = url.Parse(trimmedHostname) +func NewEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { + endpoint, err := newEndpoint(hostname) if err != nil { return nil, err } + secure := isSecure(endpoint.URL.Host, insecureRegistries) + endpoint.secure = secure + // Try HTTPS ping to registry endpoint.URL.Scheme = "https" if _, err := endpoint.Ping(); err != nil { @@ -65,12 +59,28 @@ func NewEndpoint(hostname string, secure bool) (*Endpoint, error) { endpoint.URL.Scheme = "http" _, err2 := endpoint.Ping() if err2 == nil { - return &endpoint, nil + return endpoint, nil } return nil, fmt.Errorf("Invalid registry endpoint %q. HTTPS attempt: %v. HTTP attempt: %v", endpoint, err, err2) } + return endpoint, nil +} +func newEndpoint(hostname string) (*Endpoint, error) { + var ( + endpoint = Endpoint{secure: true} + trimmedHostname string + err error + ) + if !strings.HasPrefix(hostname, "http") { + hostname = "https://" + hostname + } + trimmedHostname, endpoint.Version = scanForApiVersion(hostname) + endpoint.URL, err = url.Parse(trimmedHostname) + if err != nil { + return nil, err + } return &endpoint, nil } @@ -141,9 +151,9 @@ func (e Endpoint) Ping() (RegistryInfo, error) { return info, nil } -// IsSecure returns false if the provided hostname is part of the list of insecure registries. +// isSecure returns false if the provided hostname is part of the list of insecure registries. // Insecure registries accept HTTP and/or accept HTTPS with certificates from unknown CAs. -func IsSecure(hostname string, insecureRegistries []string) bool { +func isSecure(hostname string, insecureRegistries []string) bool { if hostname == IndexServerAddress() { return true } diff --git a/docs/endpoint_test.go b/docs/endpoint_test.go new file mode 100644 index 00000000..0ec1220d --- /dev/null +++ b/docs/endpoint_test.go @@ -0,0 +1,27 @@ +package registry + +import "testing" + +func TestEndpointParse(t *testing.T) { + testData := []struct { + str string + expected string + }{ + {IndexServerAddress(), IndexServerAddress()}, + {"http://0.0.0.0:5000", "http://0.0.0.0:5000/v1/"}, + {"0.0.0.0:5000", "https://0.0.0.0:5000/v1/"}, + } + for _, td := range testData { + e, err := newEndpoint(td.str) + if err != nil { + t.Errorf("%q: %s", td.str, err) + } + if e == nil { + t.Logf("something's fishy, endpoint for %q is nil", td.str) + continue + } + if e.String() != td.expected { + t.Errorf("expected %q, got %q", td.expected, e.String()) + } + } +} diff --git a/docs/registry_test.go b/docs/registry_test.go index c9a9fc81..7e63ee92 100644 --- a/docs/registry_test.go +++ b/docs/registry_test.go @@ -316,3 +316,32 @@ func TestAddRequiredHeadersToRedirectedRequests(t *testing.T) { } } } + +func TestIsSecure(t *testing.T) { + tests := []struct { + addr string + insecureRegistries []string + expected bool + }{ + {"example.com", []string{}, true}, + {"example.com", []string{"example.com"}, false}, + {"localhost", []string{"localhost:5000"}, false}, + {"localhost:5000", []string{"localhost:5000"}, false}, + {"localhost", []string{"example.com"}, false}, + {"127.0.0.1:5000", []string{"127.0.0.1:5000"}, false}, + {"localhost", []string{}, false}, + {"localhost:5000", []string{}, false}, + {"127.0.0.1", []string{}, false}, + {"localhost", []string{"example.com"}, false}, + {"127.0.0.1", []string{"example.com"}, false}, + {"example.com", []string{}, true}, + {"example.com", []string{"example.com"}, false}, + {"127.0.0.1", []string{"example.com"}, false}, + {"127.0.0.1:5000", []string{"example.com"}, false}, + } + for _, tt := range tests { + if sec := isSecure(tt.addr, tt.insecureRegistries); sec != tt.expected { + t.Errorf("isSecure failed for %q %v, expected %v got %v", tt.addr, tt.insecureRegistries, tt.expected, sec) + } + } +} diff --git a/docs/service.go b/docs/service.go index 7051d934..53e8278b 100644 --- a/docs/service.go +++ b/docs/service.go @@ -40,7 +40,7 @@ func (s *Service) Auth(job *engine.Job) engine.Status { job.GetenvJson("authConfig", authConfig) if addr := authConfig.ServerAddress; addr != "" && addr != IndexServerAddress() { - endpoint, err := NewEndpoint(addr, IsSecure(addr, s.insecureRegistries)) + endpoint, err := NewEndpoint(addr, s.insecureRegistries) if err != nil { return job.Error(err) } @@ -92,9 +92,7 @@ func (s *Service) Search(job *engine.Job) engine.Status { return job.Error(err) } - secure := IsSecure(hostname, s.insecureRegistries) - - endpoint, err := NewEndpoint(hostname, secure) + endpoint, err := NewEndpoint(hostname, s.insecureRegistries) if err != nil { return job.Error(err) } From 8b0e8b66212a3e5cbd0f15973f53fa0154058145 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 11 Nov 2014 20:08:59 -0500 Subject: [PATCH 08/12] Put mock registry address in insecureRegistries for unit tests Signed-off-by: Tibor Vass Conflicts: registry/registry_mock_test.go --- docs/registry_mock_test.go | 16 +++++++++++----- docs/registry_test.go | 4 ++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/registry_mock_test.go b/docs/registry_mock_test.go index 379dc78f..fc2b46b9 100644 --- a/docs/registry_mock_test.go +++ b/docs/registry_mock_test.go @@ -19,8 +19,9 @@ import ( ) var ( - testHttpServer *httptest.Server - testLayers = map[string]map[string]string{ + testHTTPServer *httptest.Server + insecureRegistries []string + testLayers = map[string]map[string]string{ "77dbf71da1d00e3fbddc480176eac8994025630c6590d11cfc8fe1209c2a1d20": { "json": `{"id":"77dbf71da1d00e3fbddc480176eac8994025630c6590d11cfc8fe1209c2a1d20", "comment":"test base image","created":"2013-03-23T12:53:11.10432-07:00", @@ -99,7 +100,12 @@ func init() { // /v2/ r.HandleFunc("/v2/version", handlerGetPing).Methods("GET") - testHttpServer = httptest.NewServer(handlerAccessLog(r)) + testHTTPServer = httptest.NewServer(handlerAccessLog(r)) + URL, err := url.Parse(testHTTPServer.URL) + if err != nil { + panic(err) + } + insecureRegistries = []string{URL.Host} } func handlerAccessLog(handler http.Handler) http.Handler { @@ -111,7 +117,7 @@ func handlerAccessLog(handler http.Handler) http.Handler { } func makeURL(req string) string { - return testHttpServer.URL + req + return testHTTPServer.URL + req } func writeHeaders(w http.ResponseWriter) { @@ -301,7 +307,7 @@ func handlerUsers(w http.ResponseWriter, r *http.Request) { } func handlerImages(w http.ResponseWriter, r *http.Request) { - u, _ := url.Parse(testHttpServer.URL) + u, _ := url.Parse(testHTTPServer.URL) w.Header().Add("X-Docker-Endpoints", fmt.Sprintf("%s , %s ", u.Host, "test.example.com")) w.Header().Add("X-Docker-Token", fmt.Sprintf("FAKE-SESSION-%d", time.Now().UnixNano())) if r.Method == "PUT" { diff --git a/docs/registry_test.go b/docs/registry_test.go index 7e63ee92..aaba3f0a 100644 --- a/docs/registry_test.go +++ b/docs/registry_test.go @@ -18,7 +18,7 @@ var ( func spawnTestRegistrySession(t *testing.T) *Session { authConfig := &AuthConfig{} - endpoint, err := NewEndpoint(makeURL("/v1/"), false) + endpoint, err := NewEndpoint(makeURL("/v1/"), insecureRegistries) if err != nil { t.Fatal(err) } @@ -30,7 +30,7 @@ func spawnTestRegistrySession(t *testing.T) *Session { } func TestPingRegistryEndpoint(t *testing.T) { - ep, err := NewEndpoint(makeURL("/v1/"), false) + ep, err := NewEndpoint(makeURL("/v1/"), insecureRegistries) if err != nil { t.Fatal(err) } From 8065dad50b9bacdf4a2f5cbc54a10745c81ab341 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 13 Nov 2014 06:56:36 -0800 Subject: [PATCH 09/12] registry: parse INDEXSERVERADDRESS into a URL for easier check in isSecure Signed-off-by: Tibor Vass --- docs/auth.go | 10 ++++++++++ docs/endpoint.go | 14 ++++++-------- docs/endpoint_test.go | 2 +- docs/registry_test.go | 1 + 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/docs/auth.go b/docs/auth.go index 7c0709a4..dad58c16 100644 --- a/docs/auth.go +++ b/docs/auth.go @@ -7,6 +7,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/url" "os" "path" "strings" @@ -27,8 +28,17 @@ const ( var ( ErrConfigFileMissing = errors.New("The Auth config file is missing") + IndexServerURL *url.URL ) +func init() { + url, err := url.Parse(INDEXSERVER) + if err != nil { + panic(err) + } + IndexServerURL = url +} + type AuthConfig struct { Username string `json:"username,omitempty"` Password string `json:"password,omitempty"` diff --git a/docs/endpoint.go b/docs/endpoint.go index 2eac41ce..eb0e9a1f 100644 --- a/docs/endpoint.go +++ b/docs/endpoint.go @@ -34,21 +34,18 @@ func scanForApiVersion(hostname string) (string, APIVersion) { } func NewEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { - endpoint, err := newEndpoint(hostname) + endpoint, err := newEndpoint(hostname, insecureRegistries) if err != nil { return nil, err } - secure := isSecure(endpoint.URL.Host, insecureRegistries) - endpoint.secure = secure - // Try HTTPS ping to registry endpoint.URL.Scheme = "https" if _, err := endpoint.Ping(); err != nil { //TODO: triggering highland build can be done there without "failing" - if secure { + if endpoint.secure { // If registry is secure and HTTPS failed, show user the error and tell them about `--insecure-registry` // in case that's what they need. DO NOT accept unknown CA certificates, and DO NOT fallback to HTTP. return nil, fmt.Errorf("Invalid registry endpoint %s: %v. If this private registry supports only HTTP or HTTPS with an unknown CA certificate, please add `--insecure-registry %s` to the daemon's arguments. In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; simply place the CA certificate at /etc/docker/certs.d/%s/ca.crt", endpoint, err, endpoint.URL.Host, endpoint.URL.Host) @@ -67,9 +64,9 @@ func NewEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error return endpoint, nil } -func newEndpoint(hostname string) (*Endpoint, error) { +func newEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { var ( - endpoint = Endpoint{secure: true} + endpoint = Endpoint{} trimmedHostname string err error ) @@ -81,6 +78,7 @@ func newEndpoint(hostname string) (*Endpoint, error) { if err != nil { return nil, err } + endpoint.secure = isSecure(endpoint.URL.Host, insecureRegistries) return &endpoint, nil } @@ -154,7 +152,7 @@ func (e Endpoint) Ping() (RegistryInfo, error) { // isSecure returns false if the provided hostname is part of the list of insecure registries. // Insecure registries accept HTTP and/or accept HTTPS with certificates from unknown CAs. func isSecure(hostname string, insecureRegistries []string) bool { - if hostname == IndexServerAddress() { + if hostname == IndexServerURL.Host { return true } diff --git a/docs/endpoint_test.go b/docs/endpoint_test.go index 0ec1220d..54105ec1 100644 --- a/docs/endpoint_test.go +++ b/docs/endpoint_test.go @@ -12,7 +12,7 @@ func TestEndpointParse(t *testing.T) { {"0.0.0.0:5000", "https://0.0.0.0:5000/v1/"}, } for _, td := range testData { - e, err := newEndpoint(td.str) + e, err := newEndpoint(td.str, insecureRegistries) if err != nil { t.Errorf("%q: %s", td.str, err) } diff --git a/docs/registry_test.go b/docs/registry_test.go index aaba3f0a..dbedefe0 100644 --- a/docs/registry_test.go +++ b/docs/registry_test.go @@ -323,6 +323,7 @@ func TestIsSecure(t *testing.T) { insecureRegistries []string expected bool }{ + {IndexServerURL.Host, nil, true}, {"example.com", []string{}, true}, {"example.com", []string{"example.com"}, false}, {"localhost", []string{"localhost:5000"}, false}, From 6638cd7bc73015ca11a44abd14a6d06e4b6f49e9 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 11 Nov 2014 16:31:15 -0500 Subject: [PATCH 10/12] Add the possibility of specifying a subnet for --insecure-registry Signed-off-by: Tibor Vass Conflicts: registry/endpoint.go --- docs/endpoint.go | 61 +++++++++++++++++++++++++++++++++----- docs/registry_mock_test.go | 26 ++++++++++++++++ docs/registry_test.go | 19 ++++++++---- 3 files changed, 93 insertions(+), 13 deletions(-) diff --git a/docs/endpoint.go b/docs/endpoint.go index eb0e9a1f..d65fd7e8 100644 --- a/docs/endpoint.go +++ b/docs/endpoint.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net" "net/http" "net/url" "strings" @@ -11,6 +12,9 @@ import ( "github.com/docker/docker/pkg/log" ) +// for mocking in unit tests +var lookupIP = net.LookupIP + // scans string for api version in the URL path. returns the trimmed hostname, if version found, string and API version. func scanForApiVersion(hostname string) (string, APIVersion) { var ( @@ -78,7 +82,10 @@ func newEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error if err != nil { return nil, err } - endpoint.secure = isSecure(endpoint.URL.Host, insecureRegistries) + endpoint.secure, err = isSecure(endpoint.URL.Host, insecureRegistries) + if err != nil { + return nil, err + } return &endpoint, nil } @@ -151,16 +158,56 @@ func (e Endpoint) Ping() (RegistryInfo, error) { // isSecure returns false if the provided hostname is part of the list of insecure registries. // Insecure registries accept HTTP and/or accept HTTPS with certificates from unknown CAs. -func isSecure(hostname string, insecureRegistries []string) bool { +// +// The list of insecure registries can contain an element with CIDR notation to specify a whole subnet. +// If the subnet contains one of the IPs of the registry specified by hostname, the latter is considered +// insecure. +// +// hostname should be a URL.Host (`host:port` or `host`) +func isSecure(hostname string, insecureRegistries []string) (bool, error) { if hostname == IndexServerURL.Host { - return true + return true, nil } - for _, h := range insecureRegistries { - if hostname == h { - return false + host, _, err := net.SplitHostPort(hostname) + if err != nil { + // assume hostname is of the form `host` without the port and go on. + host = hostname + } + addrs, err := lookupIP(host) + if err != nil { + ip := net.ParseIP(host) + if ip == nil { + // if resolving `host` fails, error out, since host is to be net.Dial-ed anyway + return true, fmt.Errorf("issecure: could not resolve %q: %v", host, err) + } + addrs = []net.IP{ip} + } + if len(addrs) == 0 { + return true, fmt.Errorf("issecure: could not resolve %q", host) + } + + for _, addr := range addrs { + for _, r := range insecureRegistries { + // hostname matches insecure registry + if hostname == r { + return false, nil + } + + // now assume a CIDR was passed to --insecure-registry + _, ipnet, err := net.ParseCIDR(r) + if err != nil { + // if could not parse it as a CIDR, even after removing + // assume it's not a CIDR and go on with the next candidate + continue + } + + // check if the addr falls in the subnet + if ipnet.Contains(addr) { + return false, nil + } } } - return true + return true, nil } diff --git a/docs/registry_mock_test.go b/docs/registry_mock_test.go index fc2b46b9..50724f0f 100644 --- a/docs/registry_mock_test.go +++ b/docs/registry_mock_test.go @@ -2,9 +2,11 @@ package registry import ( "encoding/json" + "errors" "fmt" "io" "io/ioutil" + "net" "net/http" "net/http/httptest" "net/url" @@ -80,6 +82,11 @@ var ( "latest": "42d718c941f5c532ac049bf0b0ab53f0062f09a03afd4aa4a02c098e46032b9d", }, } + mockHosts = map[string][]net.IP{ + "": {net.ParseIP("0.0.0.0")}, + "localhost": {net.ParseIP("127.0.0.1"), net.ParseIP("::1")}, + "example.com": {net.ParseIP("42.42.42.42")}, + } ) func init() { @@ -106,6 +113,25 @@ func init() { panic(err) } insecureRegistries = []string{URL.Host} + + // override net.LookupIP + lookupIP = func(host string) ([]net.IP, error) { + if host == "127.0.0.1" { + // I believe in future Go versions this will fail, so let's fix it later + return net.LookupIP(host) + } + for h, addrs := range mockHosts { + if host == h { + return addrs, nil + } + for _, addr := range addrs { + if addr.String() == host { + return []net.IP{addr}, nil + } + } + } + return nil, errors.New("lookup: no such host") + } } func handlerAccessLog(handler http.Handler) http.Handler { diff --git a/docs/registry_test.go b/docs/registry_test.go index dbedefe0..1ffb44f3 100644 --- a/docs/registry_test.go +++ b/docs/registry_test.go @@ -330,19 +330,26 @@ func TestIsSecure(t *testing.T) { {"localhost:5000", []string{"localhost:5000"}, false}, {"localhost", []string{"example.com"}, false}, {"127.0.0.1:5000", []string{"127.0.0.1:5000"}, false}, - {"localhost", []string{}, false}, - {"localhost:5000", []string{}, false}, - {"127.0.0.1", []string{}, false}, + {"localhost", nil, false}, + {"localhost:5000", nil, false}, + {"127.0.0.1", nil, false}, {"localhost", []string{"example.com"}, false}, {"127.0.0.1", []string{"example.com"}, false}, - {"example.com", []string{}, true}, + {"example.com", nil, true}, {"example.com", []string{"example.com"}, false}, {"127.0.0.1", []string{"example.com"}, false}, {"127.0.0.1:5000", []string{"example.com"}, false}, + {"example.com:5000", []string{"42.42.0.0/16"}, false}, + {"example.com", []string{"42.42.0.0/16"}, false}, + {"example.com:5000", []string{"42.42.42.42/8"}, false}, + {"127.0.0.1:5000", []string{"127.0.0.0/8"}, false}, + {"42.42.42.42:5000", []string{"42.1.1.1/8"}, false}, } for _, tt := range tests { - if sec := isSecure(tt.addr, tt.insecureRegistries); sec != tt.expected { - t.Errorf("isSecure failed for %q %v, expected %v got %v", tt.addr, tt.insecureRegistries, tt.expected, sec) + // TODO: remove this once we remove localhost insecure by default + insecureRegistries := append(tt.insecureRegistries, "127.0.0.0/8") + if sec, err := isSecure(tt.addr, insecureRegistries); err != nil || sec != tt.expected { + t.Fatalf("isSecure failed for %q %v, expected %v got %v. Error: %v", tt.addr, insecureRegistries, tt.expected, sec, err) } } } From b62a9ac9895553bd14259e7769026e9f4d2a60d1 Mon Sep 17 00:00:00 2001 From: unclejack Date: Thu, 27 Nov 2014 23:55:03 +0200 Subject: [PATCH 11/12] validate image ID properly & before load Signed-off-by: Cristian Staretu --- docs/registry.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/registry.go b/docs/registry.go index a03790af..e0285a23 100644 --- a/docs/registry.go +++ b/docs/registry.go @@ -23,7 +23,6 @@ var ( ErrInvalidRepositoryName = errors.New("Invalid repository name (ex: \"registry.domain.tld/myrepos\")") ErrDoesNotExist = errors.New("Image does not exist") errLoginRequired = errors.New("Authentication is required.") - validHex = regexp.MustCompile(`^([a-f0-9]{64})$`) validNamespace = regexp.MustCompile(`^([a-z0-9_]{4,30})$`) validRepo = regexp.MustCompile(`^([a-z0-9-_.]+)$`) ) @@ -177,7 +176,8 @@ func validateRepositoryName(repositoryName string) error { namespace = "library" name = nameParts[0] - if validHex.MatchString(name) { + // the repository name must not be a valid image ID + if err := utils.ValidateID(name); err == nil { return fmt.Errorf("Invalid repository name (%s), cannot specify 64-byte hexadecimal strings", name) } } else { From cb4f91608e945a8cb370c1324a193cd36b49aad1 Mon Sep 17 00:00:00 2001 From: Jessica Frazelle Date: Thu, 11 Dec 2014 17:14:53 -0800 Subject: [PATCH 12/12] Fix conflicts. Docker-DCO-1.1-Signed-off-by: Jessica Frazelle (github: jfrazelle) --- docs/registry.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/registry.go b/docs/registry.go index f3a4a340..d503a63d 100644 --- a/docs/registry.go +++ b/docs/registry.go @@ -47,10 +47,6 @@ func newClient(jar http.CookieJar, roots *x509.CertPool, certs []tls.Certificate tlsConfig.InsecureSkipVerify = true } - if !secure { - tlsConfig.InsecureSkipVerify = true - } - httpTransport := &http.Transport{ DisableKeepAlives: true, Proxy: http.ProxyFromEnvironment,