From e49589505a9c543006f9a54a85917c3349b6ec5a Mon Sep 17 00:00:00 2001
From: Yong Tang <yong.tang.github@outlook.com>
Date: Sat, 4 Feb 2017 23:58:12 -0800
Subject: [PATCH] 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>
---
 registry/config.go      | 36 ++++++++++++++++++++++++++++++++++++
 registry/config_test.go | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/registry/config.go b/registry/config.go
index ff72381dc5..7b6703f570 100644
--- a/registry/config.go
+++ b/registry/config.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"net"
 	"net/url"
+	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/Sirupsen/logrus"
@@ -62,6 +64,10 @@ var (
 	emptyServiceConfig = newServiceConfig(ServiceOptions{})
 )
 
+var (
+	validHostPortRegex = regexp.MustCompile(`^` + reference.DomainRegexp.String() + `$`)
+)
+
 // for mocking in unit tests
 var lookupIP = net.LookupIP
 
@@ -178,6 +184,12 @@ skip:
 			config.InsecureRegistryCIDRs = append(config.InsecureRegistryCIDRs, data)
 
 		} else {
+			if err := validateHostPort(r); err != nil {
+				config.ServiceConfig.InsecureRegistryCIDRs = originalCIDRs
+				config.ServiceConfig.IndexConfigs = originalIndexInfos
+				return fmt.Errorf("insecure registry %s is not valid: %v", r, err)
+
+			}
 			// Assume `host:port` if not CIDR.
 			config.IndexConfigs[r] = &registrytypes.IndexInfo{
 				Name:     r,
@@ -288,6 +300,30 @@ func validateNoScheme(reposName string) error {
 	return nil
 }
 
+func validateHostPort(s string) error {
+	// Split host and port, and in case s can not be splitted, assume host only
+	host, port, err := net.SplitHostPort(s)
+	if err != nil {
+		host = s
+		port = ""
+	}
+	// If match against the `host:port` pattern fails,
+	// it might be `IPv6:port`, which will be captured by net.ParseIP(host)
+	if !validHostPortRegex.MatchString(s) && net.ParseIP(host) == nil {
+		return fmt.Errorf("invalid host %q", host)
+	}
+	if port != "" {
+		v, err := strconv.Atoi(port)
+		if err != nil {
+			return err
+		}
+		if v < 0 || v > 65535 {
+			return fmt.Errorf("invalid port %q", port)
+		}
+	}
+	return nil
+}
+
 // newIndexInfo returns IndexInfo configuration from indexName
 func newIndexInfo(config *serviceConfig, indexName string) (*registrytypes.IndexInfo, error) {
 	var err error
diff --git a/registry/config_test.go b/registry/config_test.go
index 0555af2329..b57e515b94 100644
--- a/registry/config_test.go
+++ b/registry/config_test.go
@@ -56,6 +56,22 @@ func TestLoadInsecureRegistries(t *testing.T) {
 		index      string
 		err        string
 	}{
+		{
+			registries: []string{"127.0.0.1"},
+			index:      "127.0.0.1",
+		},
+		{
+			registries: []string{"127.0.0.1:8080"},
+			index:      "127.0.0.1:8080",
+		},
+		{
+			registries: []string{"2001:db8::1"},
+			index:      "2001:db8::1",
+		},
+		{
+			registries: []string{"[2001:db8::1]:80"},
+			index:      "[2001:db8::1]:80",
+		},
 		{
 			registries: []string{"http://mytest.com"},
 			index:      "mytest.com",
@@ -76,6 +92,26 @@ func TestLoadInsecureRegistries(t *testing.T) {
 			registries: []string{"-invalid-registry"},
 			err:        "Cannot begin or end with a hyphen",
 		},
+		{
+			registries: []string{`mytest-.com`},
+			err:        `insecure registry mytest-.com is not valid: invalid host "mytest-.com"`,
+		},
+		{
+			registries: []string{`1200:0000:AB00:1234:0000:2552:7777:1313:8080`},
+			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"`,
+		},
+		{
+			registries: []string{`mytest.com:500000`},
+			err:        `insecure registry mytest.com:500000 is not valid: invalid port "500000"`,
+		},
+		{
+			registries: []string{`"mytest.com"`},
+			err:        `insecure registry "mytest.com" is not valid: invalid host "\"mytest.com\""`,
+		},
+		{
+			registries: []string{`"mytest.com:5000"`},
+			err:        `insecure registry "mytest.com:5000" is not valid: invalid host "\"mytest.com"`,
+		},
 	}
 	for _, testCase := range testCases {
 		config := newServiceConfig(ServiceOptions{})