From 44d97c1fd016990ef0287625457fa3b16a1ed7df Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 11 Nov 2014 17:37:44 -0500 Subject: [PATCH 1/4] 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 6dd4e1f60..2eac41ce3 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 000000000..0ec1220d9 --- /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 c9a9fc81b..7e63ee92a 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 7051d9343..53e8278b0 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 2/4] 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 379dc78f4..fc2b46b9b 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 7e63ee92a..aaba3f0a3 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 3/4] 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 7c0709a47..dad58c163 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 2eac41ce3..eb0e9a1fa 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 0ec1220d9..54105ec17 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 aaba3f0a3..dbedefe05 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 4/4] 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 eb0e9a1fa..d65fd7e8a 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 fc2b46b9b..50724f0f9 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 dbedefe05..1ffb44f31 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) } } }