Browse code

Validate insecure registry (`--insecure-registry`) values

This fix is based on:
https://github.com/docker/docker/issues/29936#issuecomment-277494885

Currently the insecure registry is only checked to see if it contains
scheme (`http(s)://`) or not. No fully validation is done and this
caused many confusions like in #29936.

This fix tries to address the issue.

This fix adds additional validation so that an insecure registry
is validated to make sure it is in `host:port` format where host
could be IPv4/IPv6 or a host name, and port could be an integer
between 0-65535.

Additional unit tests have been added.

This fix is related to #29936.

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

Yong Tang authored on 2017/02/05 16:58:12
Showing 2 changed files
... ...
@@ -4,6 +4,8 @@ import (
4 4
 	"fmt"
5 5
 	"net"
6 6
 	"net/url"
7
+	"regexp"
8
+	"strconv"
7 9
 	"strings"
8 10
 
9 11
 	"github.com/Sirupsen/logrus"
... ...
@@ -62,6 +64,10 @@ var (
62 62
 	emptyServiceConfig = newServiceConfig(ServiceOptions{})
63 63
 )
64 64
 
65
+var (
66
+	validHostPortRegex = regexp.MustCompile(`^` + reference.DomainRegexp.String() + `$`)
67
+)
68
+
65 69
 // for mocking in unit tests
66 70
 var lookupIP = net.LookupIP
67 71
 
... ...
@@ -178,6 +184,12 @@ skip:
178 178
 			config.InsecureRegistryCIDRs = append(config.InsecureRegistryCIDRs, data)
179 179
 
180 180
 		} else {
181
+			if err := validateHostPort(r); err != nil {
182
+				config.ServiceConfig.InsecureRegistryCIDRs = originalCIDRs
183
+				config.ServiceConfig.IndexConfigs = originalIndexInfos
184
+				return fmt.Errorf("insecure registry %s is not valid: %v", r, err)
185
+
186
+			}
181 187
 			// Assume `host:port` if not CIDR.
182 188
 			config.IndexConfigs[r] = &registrytypes.IndexInfo{
183 189
 				Name:     r,
... ...
@@ -288,6 +300,30 @@ func validateNoScheme(reposName string) error {
288 288
 	return nil
289 289
 }
290 290
 
291
+func validateHostPort(s string) error {
292
+	// Split host and port, and in case s can not be splitted, assume host only
293
+	host, port, err := net.SplitHostPort(s)
294
+	if err != nil {
295
+		host = s
296
+		port = ""
297
+	}
298
+	// If match against the `host:port` pattern fails,
299
+	// it might be `IPv6:port`, which will be captured by net.ParseIP(host)
300
+	if !validHostPortRegex.MatchString(s) && net.ParseIP(host) == nil {
301
+		return fmt.Errorf("invalid host %q", host)
302
+	}
303
+	if port != "" {
304
+		v, err := strconv.Atoi(port)
305
+		if err != nil {
306
+			return err
307
+		}
308
+		if v < 0 || v > 65535 {
309
+			return fmt.Errorf("invalid port %q", port)
310
+		}
311
+	}
312
+	return nil
313
+}
314
+
291 315
 // newIndexInfo returns IndexInfo configuration from indexName
292 316
 func newIndexInfo(config *serviceConfig, indexName string) (*registrytypes.IndexInfo, error) {
293 317
 	var err error
... ...
@@ -57,6 +57,22 @@ func TestLoadInsecureRegistries(t *testing.T) {
57 57
 		err        string
58 58
 	}{
59 59
 		{
60
+			registries: []string{"127.0.0.1"},
61
+			index:      "127.0.0.1",
62
+		},
63
+		{
64
+			registries: []string{"127.0.0.1:8080"},
65
+			index:      "127.0.0.1:8080",
66
+		},
67
+		{
68
+			registries: []string{"2001:db8::1"},
69
+			index:      "2001:db8::1",
70
+		},
71
+		{
72
+			registries: []string{"[2001:db8::1]:80"},
73
+			index:      "[2001:db8::1]:80",
74
+		},
75
+		{
60 76
 			registries: []string{"http://mytest.com"},
61 77
 			index:      "mytest.com",
62 78
 		},
... ...
@@ -76,6 +92,26 @@ func TestLoadInsecureRegistries(t *testing.T) {
76 76
 			registries: []string{"-invalid-registry"},
77 77
 			err:        "Cannot begin or end with a hyphen",
78 78
 		},
79
+		{
80
+			registries: []string{`mytest-.com`},
81
+			err:        `insecure registry mytest-.com is not valid: invalid host "mytest-.com"`,
82
+		},
83
+		{
84
+			registries: []string{`1200:0000:AB00:1234:0000:2552:7777:1313:8080`},
85
+			err:        `insecure registry 1200:0000:AB00:1234:0000:2552:7777:1313:8080 is not valid: invalid host "1200:0000:AB00:1234:0000:2552:7777:1313:8080"`,
86
+		},
87
+		{
88
+			registries: []string{`mytest.com:500000`},
89
+			err:        `insecure registry mytest.com:500000 is not valid: invalid port "500000"`,
90
+		},
91
+		{
92
+			registries: []string{`"mytest.com"`},
93
+			err:        `insecure registry "mytest.com" is not valid: invalid host "\"mytest.com\""`,
94
+		},
95
+		{
96
+			registries: []string{`"mytest.com:5000"`},
97
+			err:        `insecure registry "mytest.com:5000" is not valid: invalid host "\"mytest.com"`,
98
+		},
79 99
 	}
80 100
 	for _, testCase := range testCases {
81 101
 		config := newServiceConfig(ServiceOptions{})