Merge pull request #16859 from diogomonica/changing-trust-server-checks

Changing trustServer allowed URL validation

api/client/trust.go

@@ -15,7 +15,6 @@ import (
 	"regexp"
 	"sort"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/Sirupsen/logrus"
@@ -79,17 +78,19 @@ func (cli *DockerCli) certificateDirectory(server string) (string, error) {
 	return filepath.Join(cliconfig.ConfigDir(), "tls", u.Host), nil
 }
 
-func trustServer(index *registry.IndexInfo) string {
+func trustServer(index *registry.IndexInfo) (string, error) {
 	if s := os.Getenv("DOCKER_CONTENT_TRUST_SERVER"); s != "" {
-		if !strings.HasPrefix(s, "https://") {
-			return "https://" + s
+		urlObj, err := url.Parse(s)
+		if err != nil || urlObj.Scheme != "https" {
+			return "", fmt.Errorf("valid https URL required for trust server, got %s", s)
 		}
-		return s
+
+		return s, nil
 	}
 	if index.Official {
-		return registry.NotaryServer
+		return registry.NotaryServer, nil
 	}
-	return "https://" + index.Name
+	return "https://" + index.Name, nil
 }
 
 type simpleCredentialStore struct {
@@ -101,9 +102,9 @@ func (scs simpleCredentialStore) Basic(u *url.URL) (string, string) {
 }
 
 func (cli *DockerCli) getNotaryRepository(repoInfo *registry.RepositoryInfo, authConfig cliconfig.AuthConfig) (*client.NotaryRepository, error) {
-	server := trustServer(repoInfo.Index)
-	if !strings.HasPrefix(server, "https://") {
-		return nil, errors.New("unsupported scheme: https required for trust server")
+	server, err := trustServer(repoInfo.Index)
+	if err != nil {
+		return nil, err
 	}
 
 	var cfg = tlsconfig.ClientDefault

api/client/trust_test.go

@@ -0,0 +1,55 @@
+package client
+
+import (
+	"os"
+	"testing"
+
+	"github.com/docker/docker/registry"
+)
+
+func unsetENV() {
+	os.Unsetenv("DOCKER_CONTENT_TRUST")
+	os.Unsetenv("DOCKER_CONTENT_TRUST_SERVER")
+}
+
+func TestENVTrustServer(t *testing.T) {
+	defer unsetENV()
+	indexInfo := &registry.IndexInfo{Name: "testserver"}
+	if err := os.Setenv("DOCKER_CONTENT_TRUST_SERVER", "https://notary-test.com:5000"); err != nil {
+		t.Fatal("Failed to set ENV variable")
+	}
+	output, err := trustServer(indexInfo)
+	expectedStr := "https://notary-test.com:5000"
+	if err != nil || output != expectedStr {
+		t.Fatalf("Expected server to be %s, got %s", expectedStr, output)
+	}
+}
+
+func TestHTTPENVTrustServer(t *testing.T) {
+	defer unsetENV()
+	indexInfo := &registry.IndexInfo{Name: "testserver"}
+	if err := os.Setenv("DOCKER_CONTENT_TRUST_SERVER", "http://notary-test.com:5000"); err != nil {
+		t.Fatal("Failed to set ENV variable")
+	}
+	_, err := trustServer(indexInfo)
+	if err == nil {
+		t.Fatal("Expected error with invalid scheme")
+	}
+}
+
+func TestOfficialTrustServer(t *testing.T) {
+	indexInfo := &registry.IndexInfo{Name: "testserver", Official: true}
+	output, err := trustServer(indexInfo)
+	if err != nil || output != registry.NotaryServer {
+		t.Fatalf("Expected server to be %s, got %s", registry.NotaryServer, output)
+	}
+}
+
+func TestNonOfficialTrustServer(t *testing.T) {
+	indexInfo := &registry.IndexInfo{Name: "testserver", Official: false}
+	output, err := trustServer(indexInfo)
+	expectedStr := "https://" + indexInfo.Name
+	if err != nil || output != expectedStr {
+		t.Fatalf("Expected server to be %s, got %s", expectedStr, output)
+	}
+}

integration-cli/docker_cli_push_test.go

@@ -148,7 +148,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithFaillingServer(c *check.C) {
 	dockerCmd(c, "tag", "busybox", repoName)
 
 	pushCmd := exec.Command(dockerBinary, "push", repoName)
-	s.trustedCmdWithServer(pushCmd, "example/")
+	s.trustedCmdWithServer(pushCmd, "https://example.com:81/")
 	out, _, err := runCommandWithOutput(pushCmd)
 	if err == nil {
 		c.Fatalf("Missing error while running trusted push w/ no server")
@@ -165,7 +165,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithoutServerAndUntrusted(c *check.C)
 	dockerCmd(c, "tag", "busybox", repoName)
 
 	pushCmd := exec.Command(dockerBinary, "push", "--disable-content-trust", repoName)
-	s.trustedCmdWithServer(pushCmd, "example/")
+	s.trustedCmdWithServer(pushCmd, "https://example.com/")
 	out, _, err := runCommandWithOutput(pushCmd)
 	if err != nil {
 		c.Fatalf("trusted push with no server and --disable-content-trust failed: %s\n%s", err, out)

integration-cli/trust_server.go

@@ -22,6 +22,9 @@ type testNotary struct {
 	dir string
 }
 
+const notaryHost = "localhost:4443"
+const notaryURL = "https://" + notaryHost
+
 func newTestNotary(c *check.C) (*testNotary, error) {
 	template := `{
 	"server": {
@@ -48,7 +51,7 @@ func newTestNotary(c *check.C) (*testNotary, error) {
 	if err != nil {
 		return nil, err
 	}
-	if _, err := fmt.Fprintf(config, template, "localhost:4443"); err != nil {
+	if _, err := fmt.Fprintf(config, template, notaryHost); err != nil {
 		os.RemoveAll(tmp)
 		return nil, err
 	}
@@ -82,10 +85,6 @@ func newTestNotary(c *check.C) (*testNotary, error) {
 	return testNotary, nil
 }
 
-func (t *testNotary) address() string {
-	return "localhost:4443"
-}
-
 func (t *testNotary) Ping() error {
 	tlsConfig := tlsconfig.ClientDefault
 	tlsConfig.InsecureSkipVerify = true
@@ -100,7 +99,7 @@ func (t *testNotary) Ping() error {
 			TLSClientConfig:     &tlsConfig,
 		},
 	}
-	resp, err := client.Get(fmt.Sprintf("https://%s/v2/", t.address()))
+	resp, err := client.Get(fmt.Sprintf("%s/v2/", notaryURL))
 	if err != nil {
 		return err
 	}
@@ -117,7 +116,7 @@ func (t *testNotary) Close() {
 
 func (s *DockerTrustSuite) trustedCmd(cmd *exec.Cmd) {
 	pwd := "12345678"
-	trustCmdEnv(cmd, s.not.address(), pwd, pwd)
+	trustCmdEnv(cmd, notaryURL, pwd, pwd)
 }
 
 func (s *DockerTrustSuite) trustedCmdWithServer(cmd *exec.Cmd, server string) {
@@ -126,7 +125,7 @@ func (s *DockerTrustSuite) trustedCmdWithServer(cmd *exec.Cmd, server string) {
 }
 
 func (s *DockerTrustSuite) trustedCmdWithPassphrases(cmd *exec.Cmd, offlinePwd, taggingPwd string) {
-	trustCmdEnv(cmd, s.not.address(), offlinePwd, taggingPwd)
+	trustCmdEnv(cmd, notaryURL, offlinePwd, taggingPwd)
 }
 
 func trustCmdEnv(cmd *exec.Cmd, server, offlinePwd, taggingPwd string) {