Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

client: define a "dummy" hostname to use for local connections #45942

Merged
merged 4 commits into from Jul 14, 2023
Merged

client: define a "dummy" hostname to use for local connections #45942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a59188
client: TestSetHostHeader: don't use un-keyed literals
thaJeztah Jul 14, 2023
92975f0
client: define a "dummy" hostname to use for local connections
thaJeztah Jul 12, 2023
6b7705d
pkg/plugins: use a dummy hostname for local connections
thaJeztah Jul 12, 2023
e1db9e9
testutil: use dummyhost for non-tcp connections
thaJeztah Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions client/client.go
View file
Open in desktop
Expand Up @@ -56,6 +56,36 @@ import (
"github.com/pkg/errors"
)

// DummyHost is a hostname used for local communication.
//
// It acts as a valid formatted hostname for local connections (such as "unix://"
// or "npipe://") which do not require a hostname. It should never be resolved,
// but uses the special-purpose ".localhost" TLD (as defined in [RFC 2606, Section 2]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense (just in case) to use invalid. TLD that explicitly prohibits resolution while localhost typically resolves to loopback IP?

From https://www.rfc-editor.org/rfc/rfc2606.html#section-2

".invalid" is intended for use in online construction of domain
names that are sure to be invalid and which it is obvious at a
glance are invalid.

The ".localhost" TLD has traditionally been statically defined in
host DNS implementations as having an A record pointing to the
loop back IP address and is reserved for such use.  Any other use
would conflict with widely deployed code which assumes this use.

From https://www.rfc-editor.org/rfc/rfc6761#section-6.4

The domain "invalid." and any names falling within ".invalid." are
special in the ways listed below.  In the text below, the term
"invalid" is used in quotes to signify such names, as opposed to
names that may be invalid for other reasons (e.g., being too long).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. this.placeholder.host.is.intentionally.invalid

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was playing with the idea to use .invalid, but considered that it may be potentially confusing to use an "invalid" host as a "valid" hostname. I'm not 100% attached to that though, and now that we have a const for this, it would also be easy to change it later if we feel like doing so.

Perhaps one of Cory's ideas (. or -) could then also be considered.

// and [RFC 6761, Section 6.3]).
//
// [RFC 7230, Section 5.4] defines that an empty header must be used for such
// cases:
//
// If the authority component is missing or undefined for the target URI,
// then a client MUST send a Host header field with an empty field-value.
//
// However, [Go stdlib] enforces the semantics of HTTP(S) over TCP, does not
// allow an empty header to be used, and requires req.URL.Scheme to be either
// "http" or "https".
//
// For further details, refer to:
//
// - https://github.com/docker/engine-api/issues/189
// - https://github.com/golang/go/issues/13624
// - https://github.com/golang/go/issues/61076
// - https://github.com/moby/moby/issues/45935
//
// [RFC 2606, Section 2]: https://www.rfc-editor.org/rfc/rfc2606.html#section-2
// [RFC 6761, Section 6.3]: https://www.rfc-editor.org/rfc/rfc6761#section-6.3
// [RFC 7230, Section 5.4]: https://datatracker.ietf.org/doc/html/rfc7230#section-5.4
// [Go stdlib]: https://github.com/golang/go/blob/6244b1946bc2101b01955468f1be502dbadd6807/src/net/http/transport.go#L558-L569
const DummyHost = "api.moby.localhost"

// ErrRedirect is the error returned by checkRedirect when the request is non-GET.
var ErrRedirect = errors.New("unexpected redirect in response")

Expand Down
6 changes: 5 additions & 1 deletion client/hijack.go
View file
Open in desktop
Expand Up @@ -64,7 +64,11 @@ func fallbackDial(proto, addr string, tlsConfig *tls.Config) (net.Conn, error) {
}

func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto string) (net.Conn, string, error) {
req.Host = cli.addr
req.URL.Host = cli.addr
if cli.proto == "unix" || cli.proto == "npipe" {
// Override host header for non-tcp connections.
req.Host = DummyHost
}
req.Header.Set("Connection", "Upgrade")
req.Header.Set("Upgrade", proto)

Expand Down
13 changes: 4 additions & 9 deletions client/request.go
View file
Open in desktop
Expand Up @@ -102,19 +102,14 @@ func (cli *Client) buildRequest(method, path string, body io.Reader, headers htt
return nil, err
}
req = cli.addHeaders(req, headers)
req.URL.Scheme = cli.scheme
req.URL.Host = cli.addr

if cli.proto == "unix" || cli.proto == "npipe" {
// For local communications, it doesn't matter what the host is. We just
// need a valid and meaningful host name. For details, see:
//
// - https://github.com/docker/engine-api/issues/189
// - https://github.com/golang/go/issues/13624
req.Host = "docker"
// Override host header for non-tcp connections.
req.Host = DummyHost
thaJeztah marked this conversation as resolved.
Show resolved Hide resolved
}

req.URL.Host = cli.addr
req.URL.Scheme = cli.scheme

if body != nil && req.Header.Get("Content-Type") == "" {
req.Header.Set("Content-Type", "text/plain")
}
Expand Down
24 changes: 12 additions & 12 deletions client/request_test.go
View file
Open in desktop
Expand Up @@ -28,24 +28,24 @@ func TestSetHostHeader(t *testing.T) {
expectedURLHost string
}{
{
"unix:///var/run/docker.sock",
"docker",
"/var/run/docker.sock",
host: "unix:///var/run/docker.sock",
expectedHost: DummyHost,
expectedURLHost: "/var/run/docker.sock",
},
{
"npipe:////./pipe/docker_engine",
"docker",
"//./pipe/docker_engine",
host: "npipe:////./pipe/docker_engine",
expectedHost: DummyHost,
expectedURLHost: "//./pipe/docker_engine",
},
{
"tcp://0.0.0.0:4243",
"",
"0.0.0.0:4243",
host: "tcp://0.0.0.0:4243",
expectedHost: "",
expectedURLHost: "0.0.0.0:4243",
},
{
"tcp://localhost:4243",
"",
"localhost:4243",
host: "tcp://localhost:4243",
expectedHost: "",
expectedURLHost: "localhost:4243",
},
}

Expand Down
5 changes: 5 additions & 0 deletions integration-cli/docker_api_attach_test.go
View file
Open in desktop
Expand Up @@ -236,6 +236,11 @@ func requestHijack(method, endpoint string, data io.Reader, ct, daemon string, m
req.URL.Scheme = "http"
req.URL.Host = hostURL.Host

if hostURL.Scheme == "unix" || hostURL.Scheme == "npipe" {
// Override host header for non-tcp connections.
req.Host = client.DummyHost
}

for _, opt := range modifiers {
opt(req)
}
Expand Down
14 changes: 12 additions & 2 deletions pkg/plugins/client.go
View file
Open in desktop
Expand Up @@ -18,6 +18,12 @@ import (

const (
defaultTimeOut = 30

// dummyHost is a hostname used for local communication.
//
// For local communications (npipe://, unix://), the hostname is not used,
// but we need valid and meaningful hostname.
dummyHost = "plugin.moby.localhost"
)

func newTransport(addr string, tlsConfig *tlsconfig.Options) (transport.Transport, error) {
Expand All @@ -44,8 +50,12 @@ func newTransport(addr string, tlsConfig *tlsconfig.Options) (transport.Transpor
return nil, err
}
scheme := httpScheme(u)

return transport.NewHTTPTransport(tr, scheme, socket), nil
hostName := u.Host
if hostName == "" || u.Scheme == "unix" || u.Scheme == "npipe" {
// Override host header for non-tcp connections.
hostName = dummyHost
}
return transport.NewHTTPTransport(tr, scheme, hostName), nil
}

// NewClient creates a new plugin client (http).
Expand Down
5 changes: 5 additions & 0 deletions testutil/request/request.go
View file
Open in desktop
Expand Up @@ -125,6 +125,11 @@ func newRequest(endpoint string, opts *Options) (*http.Request, error) {
}
req.URL.Host = hostURL.Host

if hostURL.Scheme == "unix" || hostURL.Scheme == "npipe" {
// Override host header for non-tcp connections.
req.Host = client.DummyHost
}

for _, config := range opts.requestModifiers {
if err := config(req); err != nil {
return nil, err
Expand Down