1. docker
  2. Commit 8dadeaf8ea4d3a3f06ab3283e353887039675ff2
Browse code

Return error when insecure registry contains scheme

While investigating 29936 I noticed one potential issue
in `LoadInsecureRegistries`.

The implementation of the func assumes that the format
of insecure registry should be `host:port` if not CIDR.
However, it is very common that user may incorrectly
provide a registry with a scheme (e.g, `http://myregistry.com:5000`)
Such a registry format with a scheme will cause docker pull to
always try https endpoint.

The reason is that the func of `isSecureIndex()` actually will
check for the map of the index server for `myregistry.com:5000`
while the insecure registry only has a record of `http://myregistry.com:5000`.
As a consequence, docker assumes that `myregistry.com:5000` is not
a insecure registry and will go ahead with https endpoint.

This fix addresses the issue by error out insecure registries with scheme.

A unit test has been added.

This fix is related to 29936.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Yong Tang authored on 2017/01/09 12:30:58
Showing 2 changed files
registry/config.go
History View file @ 8dadeaf
... ... 
@@ -7,6 +7,7 @@ import (
7 7 
 	"net/url"
8 8 
 	"strings"
9 9
10 
+	"github.com/Sirupsen/logrus"
10 11 
 	registrytypes "github.com/docker/docker/api/types/registry"
11 12 
 	"github.com/docker/docker/opts"
12 13 
 	"github.com/docker/docker/reference"
... ... 
@@ -150,6 +151,19 @@ skip:
150 150 
 			config.ServiceConfig.IndexConfigs = originalIndexInfos
151 151 
 			return err
152 152 
 		}
153 
+		if strings.HasPrefix(strings.ToLower(r), "http://") {
154 
+			logrus.Warnf("insecure registry %s should not contain 'http://' and 'http://' has been removed from the insecure registry config", r)
155 
+			r = r[7:]
156 
+		} else if strings.HasPrefix(strings.ToLower(r), "https://") {
157 
+			logrus.Warnf("insecure registry %s should not contain 'https://' and 'https://' has been removed from the insecure registry config", r)
158 
+			r = r[8:]
159 
+		} else if validateNoScheme(r) != nil {
160 
+			// Insecure registry should not contain '://'
161 
+			// before returning err, roll back to original data
162 
+			config.ServiceConfig.InsecureRegistryCIDRs = originalCIDRs
163 
+			config.ServiceConfig.IndexConfigs = originalIndexInfos
164 
+			return fmt.Errorf("insecure registry %s should not contain '://'", r)
165 
+		}
153 166 
 		// Check if CIDR was passed to --insecure-registry
154 167 
 		_, ipnet, err := net.ParseCIDR(r)
155 168 
 		if err == nil {
registry/config_test.go
History View file @ 8dadeaf
... ... 
@@ -1,6 +1,7 @@
1 1 
 package registry
2 2
3 3 
 import (
4 
+	"strings"
4 5 
 	"testing"
5 6 
 )
6 7
... ... 
@@ -48,3 +49,57 @@ func TestValidateMirror(t *testing.T) {
48 48 
 		}
49 49 
 	}
50 50 
 }
51 
+
52 
+func TestLoadInsecureRegistries(t *testing.T) {
53 
+	testCases := []struct {
54 
+		registries []string
55 
+		index      string
56 
+		err        string
57 
+	}{
58 
+		{
59 
+			registries: []string{"http://mytest.com"},
60 
+			index:      "mytest.com",
61 
+		},
62 
+		{
63 
+			registries: []string{"https://mytest.com"},
64 
+			index:      "mytest.com",
65 
+		},
66 
+		{
67 
+			registries: []string{"HTTP://mytest.com"},
68 
+			index:      "mytest.com",
69 
+		},
70 
+		{
71 
+			registries: []string{"svn://mytest.com"},
72 
+			err:        "insecure registry svn://mytest.com should not contain '://'",
73 
+		},
74 
+		{
75 
+			registries: []string{"-invalid-registry"},
76 
+			err:        "Cannot begin or end with a hyphen",
77 
+		},
78 
+	}
79 
+	for _, testCase := range testCases {
80 
+		config := newServiceConfig(ServiceOptions{})
81 
+		err := config.LoadInsecureRegistries(testCase.registries)
82 
+		if testCase.err == "" {
83 
+			if err != nil {
84 
+				t.Fatalf("expect no error, got '%s'", err)
85 
+			}
86 
+			match := false
87 
+			for index := range config.IndexConfigs {
88 
+				if index == testCase.index {
89 
+					match = true
90 
+				}
91 
+			}
92 
+			if !match {
93 
+				t.Fatalf("expect index configs to contain '%s', got %+v", testCase.index, config.IndexConfigs)
94 
+			}
95 
+		} else {
96 
+			if err == nil {
97 
+				t.Fatalf("expect error '%s', got no error", testCase.err)
98 
+			}
99 
+			if !strings.Contains(err.Error(), testCase.err) {
100 
+				t.Fatalf("expect error '%s', got '%s'", testCase.err, err)
101 
+			}
102 
+		}
103 
+	}
104 
+}