From ecbfe731932f49f4565f3824fd2c8249c2f1fb73 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 3 Apr 2022 21:38:24 +0200 Subject: [PATCH 1/2] opts: ParseTCPAddr(): fix validation of hosts to not ignore path elements There was a discrepancy between what `ParseTCPAddr()` accepted, and what the daemon was able to use, resulting in the daemon to start, but fail to create listeners for the specified host. Before this patch: dockerd -H tcp://127.0.0.1:2375/ INFO[2022-04-03T10:18:06.417502600Z] Starting up ... failed to load listeners: listen tcp: address tcp/2375/: unknown port dockerd -H 127.0.0.1:2375/path INFO[2022-04-03T10:18:06.417502600Z] Starting up ... failed to load listeners: listen tcp: address tcp/5555/path: unknown port After this patch: dockerd -H tcp://127.0.0.1:2375/ Status: invalid argument "tcp://127.0.0.1:2375/" for "-H, --host" flag: invalid bind address (127.0.0.1:2375/): should not contain a path element See 'dockerd --help'., Code: 125 dockerd -H 127.0.0.1:2375/path Status: invalid argument "127.0.0.1:2375/path" for "-H, --host" flag: invalid bind address (127.0.0.1:2375/path): should not contain a path element See 'dockerd --help'., Code: 125 Signed-off-by: Sebastiaan van Stijn --- opts/hosts.go | 7 +++- opts/hosts_test.go | 91 +++++++++++++++++++++++++--------------------- 2 files changed, 55 insertions(+), 43 deletions(-) diff --git a/opts/hosts.go b/opts/hosts.go index a3123adefe..aaa41ec97f 100644 --- a/opts/hosts.go +++ b/opts/hosts.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/docker/docker/pkg/homedir" + "github.com/pkg/errors" ) const ( @@ -162,7 +163,11 @@ func ParseTCPAddr(tryAddr string, defaultAddr string) (string, error) { return "", fmt.Errorf("Invalid bind address format: %s", tryAddr) } - return fmt.Sprintf("tcp://%s%s", net.JoinHostPort(host, port), u.Path), nil + if u.Path != "" { + return "", errors.Errorf("invalid bind address (%s): should not contain a path element", tryAddr) + } + + return "tcp://" + net.JoinHostPort(host, port), nil } // ValidateExtraHost validates that the specified string is a valid extrahost and returns it. diff --git a/opts/hosts_test.go b/opts/hosts_test.go index 7a0a943adf..42e0018285 100644 --- a/opts/hosts_test.go +++ b/opts/hosts_test.go @@ -13,6 +13,14 @@ func TestParseHost(t *testing.T) { "unknown://", "tcp://:port", "tcp://invalid:port", + "tcp://:5555/", + "tcp://:5555/p", + "tcp://0.0.0.0:5555/", + "tcp://0.0.0.0:5555/p", + "tcp://[::1]:/", + "tcp://[::1]:5555/", + "tcp://[::1]:5555/p", + " tcp://:5555/path ", } valid := map[string]string{ @@ -29,7 +37,6 @@ func TestParseHost(t *testing.T) { "tcp://192.168.0.0:12000": "tcp://192.168.0.0:12000", "tcp://192.168:8080": "tcp://192.168:8080", "tcp://0.0.0.0:1234567890": "tcp://0.0.0.0:1234567890", // yeah it's valid :P - " tcp://:7777/path ": fmt.Sprintf("tcp://%s:7777/path", DefaultHTTPHost), "tcp://docker.com:2375": "tcp://docker.com:2375", "unix://": "unix://" + DefaultUnixSocket, "unix://path/to/socket": "unix://path/to/socket", @@ -60,27 +67,27 @@ func TestParseDockerDaemonHost(t *testing.T) { "tcp://unix:///run/docker.sock": "Invalid proto, expected tcp: unix:///run/docker.sock", " tcp://:7777/path ": "Invalid bind address format: tcp://:7777/path ", "": "Invalid bind address format: ", + ":5555/path": "invalid bind address (:5555/path): should not contain a path element", + "0.0.0.1:5555/path": "invalid bind address (0.0.0.1:5555/path): should not contain a path element", + "[::1]:5555/path": "invalid bind address ([::1]:5555/path): should not contain a path element", + "[0:0:0:0:0:0:0:1]:5555/path": "invalid bind address ([0:0:0:0:0:0:0:1]:5555/path): should not contain a path element", + "tcp://:5555/path": "invalid bind address (:5555/path): should not contain a path element", + "localhost:5555/path": "invalid bind address (localhost:5555/path): should not contain a path element", } valids := map[string]string{ - "0.0.0.1:": "tcp://0.0.0.1:2375", - "0.0.0.1:5555": "tcp://0.0.0.1:5555", - "0.0.0.1:5555/path": "tcp://0.0.0.1:5555/path", - "[::1]:": "tcp://[::1]:2375", - "[::1]:5555/path": "tcp://[::1]:5555/path", - "[0:0:0:0:0:0:0:1]:": "tcp://[0:0:0:0:0:0:0:1]:2375", - "[0:0:0:0:0:0:0:1]:5555/path": "tcp://[0:0:0:0:0:0:0:1]:5555/path", - ":6666": fmt.Sprintf("tcp://%s:6666", DefaultHTTPHost), - ":6666/path": fmt.Sprintf("tcp://%s:6666/path", DefaultHTTPHost), - "tcp://": DefaultTCPHost, - "tcp://:7777": fmt.Sprintf("tcp://%s:7777", DefaultHTTPHost), - "tcp://:7777/path": fmt.Sprintf("tcp://%s:7777/path", DefaultHTTPHost), - "unix:///run/docker.sock": "unix:///run/docker.sock", - "unix://": "unix://" + DefaultUnixSocket, - "fd://": "fd://", - "fd://something": "fd://something", - "localhost:": "tcp://localhost:2375", - "localhost:5555": "tcp://localhost:5555", - "localhost:5555/path": "tcp://localhost:5555/path", + "0.0.0.1:": "tcp://0.0.0.1:2375", + "0.0.0.1:5555": "tcp://0.0.0.1:5555", + "[::1]:": "tcp://[::1]:2375", + "[0:0:0:0:0:0:0:1]:": "tcp://[0:0:0:0:0:0:0:1]:2375", + ":6666": fmt.Sprintf("tcp://%s:6666", DefaultHTTPHost), + "tcp://": DefaultTCPHost, + "tcp://:7777": fmt.Sprintf("tcp://%s:7777", DefaultHTTPHost), + "unix:///run/docker.sock": "unix:///run/docker.sock", + "unix://": "unix://" + DefaultUnixSocket, + "fd://": "fd://", + "fd://something": "fd://something", + "localhost:": "tcp://localhost:2375", + "localhost:5555": "tcp://localhost:5555", } for invalidAddr, expectedError := range invalids { if addr, err := parseDaemonHost(invalidAddr); err == nil || expectedError != "" && err.Error() != expectedError { @@ -99,30 +106,30 @@ func TestParseTCP(t *testing.T) { defaultHTTPHost = "tcp://127.0.0.1:2376" ) invalids := map[string]string{ - "tcp:a.b.c.d": "", - "tcp:a.b.c.d/path": "", - "udp://127.0.0.1": "Invalid proto, expected tcp: udp://127.0.0.1", - "udp://127.0.0.1:2375": "Invalid proto, expected tcp: udp://127.0.0.1:2375", + "tcp:a.b.c.d": "", + "tcp:a.b.c.d/path": "", + "udp://127.0.0.1": "Invalid proto, expected tcp: udp://127.0.0.1", + "udp://127.0.0.1:2375": "Invalid proto, expected tcp: udp://127.0.0.1:2375", + ":5555/path": "invalid bind address (:5555/path): should not contain a path element", + "0.0.0.1:5555/path": "invalid bind address (0.0.0.1:5555/path): should not contain a path element", + "[::1]:5555/path": "invalid bind address ([::1]:5555/path): should not contain a path element", + "[0:0:0:0:0:0:0:1]:5555/path": "invalid bind address ([0:0:0:0:0:0:0:1]:5555/path): should not contain a path element", + "tcp://:5555/path": "invalid bind address (tcp://:5555/path): should not contain a path element", + "localhost:5555/path": "invalid bind address (localhost:5555/path): should not contain a path element", } valids := map[string]string{ - "": defaultHTTPHost, - "tcp://": defaultHTTPHost, - "0.0.0.1:": "tcp://0.0.0.1:2376", - "0.0.0.1:5555": "tcp://0.0.0.1:5555", - "0.0.0.1:5555/path": "tcp://0.0.0.1:5555/path", - ":6666": "tcp://127.0.0.1:6666", - ":6666/path": "tcp://127.0.0.1:6666/path", - "tcp://:7777": "tcp://127.0.0.1:7777", - "tcp://:7777/path": "tcp://127.0.0.1:7777/path", - "[::1]:": "tcp://[::1]:2376", - "[::1]:5555": "tcp://[::1]:5555", - "[::1]:5555/path": "tcp://[::1]:5555/path", - "[0:0:0:0:0:0:0:1]:": "tcp://[0:0:0:0:0:0:0:1]:2376", - "[0:0:0:0:0:0:0:1]:5555": "tcp://[0:0:0:0:0:0:0:1]:5555", - "[0:0:0:0:0:0:0:1]:5555/path": "tcp://[0:0:0:0:0:0:0:1]:5555/path", - "localhost:": "tcp://localhost:2376", - "localhost:5555": "tcp://localhost:5555", - "localhost:5555/path": "tcp://localhost:5555/path", + "": defaultHTTPHost, + "tcp://": defaultHTTPHost, + "0.0.0.1:": "tcp://0.0.0.1:2376", + "0.0.0.1:5555": "tcp://0.0.0.1:5555", + ":6666": "tcp://127.0.0.1:6666", + "tcp://:7777": "tcp://127.0.0.1:7777", + "[::1]:": "tcp://[::1]:2376", + "[::1]:5555": "tcp://[::1]:5555", + "[0:0:0:0:0:0:0:1]:": "tcp://[0:0:0:0:0:0:0:1]:2376", + "[0:0:0:0:0:0:0:1]:5555": "tcp://[0:0:0:0:0:0:0:1]:5555", + "localhost:": "tcp://localhost:2376", + "localhost:5555": "tcp://localhost:5555", } for invalidAddr, expectedError := range invalids { if addr, err := ParseTCPAddr(invalidAddr, defaultHTTPHost); err == nil || expectedError != "" && err.Error() != expectedError { From a35b4ac54a5c8b7e822293ba62f198047a52a0aa Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 4 Apr 2022 15:18:01 +0200 Subject: [PATCH 2/2] daemon/config: Validate(): validate hosts The config.Validate() function did not validate hosts that were configured in the daemon.json configuration file, resulting in `--validate` to pass, but the daemon failing to start. before this patch: echo '{"hosts":["127.0.0.1:2375/path"]}' > /etc/docker/daemon.json dockerd --validate configuration OK dockerd INFO[2022-04-03T11:42:22.162366200Z] Starting up failed to load listeners: error parsing -H 127.0.0.1:2375/path: invalid bind address (127.0.0.1:2375/path): should not contain a path element with this patch: echo '{"hosts":["127.0.0.1:2375/path"]}' > /etc/docker/daemon.json dockerd --validate unable to configure the Docker daemon with file /etc/docker/daemon.json: configuration validation from file failed: invalid bind address (127.0.0.1:2375/path): should not contain a path element Signed-off-by: Sebastiaan van Stijn --- daemon/config/config.go | 6 ++++++ daemon/config/config_test.go | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/daemon/config/config.go b/daemon/config/config.go index ab8380e15c..173830f340 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -601,6 +601,12 @@ func Validate(config *Config) error { } } + for _, h := range config.Hosts { + if _, err := opts.ValidateHost(h); err != nil { + return err + } + } + // validate platform-specific settings return config.ValidatePlatformConfig() } diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index fee8f6f004..4ba24a4b38 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -336,6 +336,15 @@ func TestValidateConfigurationErrors(t *testing.T) { }, expectedErr: "could not parse GenericResource: mixed discrete and named resources in expression 'foo=[bar 1]'", }, + { + name: "with invalid hosts", + config: &Config{ + CommonConfig: CommonConfig{ + Hosts: []string{"127.0.0.1:2375/path"}, + }, + }, + expectedErr: "invalid bind address (127.0.0.1:2375/path): should not contain a path element", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -420,6 +429,14 @@ func TestValidateConfiguration(t *testing.T) { }, }, }, + { + name: "with hosts", + config: &Config{ + CommonConfig: CommonConfig{ + Hosts: []string{"tcp://127.0.0.1:2375"}, + }, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) {