From f6b1f01de365e5858575241e3e9cfd724fbb8909 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 15 Apr 2019 09:23:00 +0200 Subject: [PATCH] Remove hack MalformedHostHeaderOverride This hack was added to fix a compatibility with clients that were built using Go 1.5 and older (added in 3d6f5984f52802fe2f4af0dd2296c9e2e4a1e003) This hack causes some problems with current clients; with Go 1.5 and older no longer being supported for some time, and being several years old, it should now be ok to remove this hack altogether. People using tools that are built with those versions of Go wouldn't have updated those for years, and are probably out of date anyway; that's not something we can continue taking into account. This will affect docker clients (the docker cli) for docker 1.12 and older. Those versions have reached EOL a long time ago (and have known unpatched vulnerabilities), so should no longer be used anyway, but We should add a nebtuib in the release notes, just in case someone, somewhere, still has such old tools. For those affected, using a more recent client (and if needed, setting the DOCKER_API_VERSION environment variable to the needed API version) should provide a way out. This reverts the changes originally made in; #22000 and #22888, which were to address #20865. Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/daemon.go | 1 - cmd/dockerd/daemon_unix.go | 13 -- cmd/dockerd/daemon_windows.go | 5 - cmd/dockerd/hack/malformed_host_override.go | 121 ----------------- .../hack/malformed_host_override_test.go | 124 ------------------ 5 files changed, 264 deletions(-) delete mode 100644 cmd/dockerd/hack/malformed_host_override.go delete mode 100644 cmd/dockerd/hack/malformed_host_override_test.go diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index e53f05d15d..709f8efbad 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -641,7 +641,6 @@ func loadListeners(cli *DaemonCli, serverConfig *apiserver.Config) ([]string, er if err != nil { return nil, err } - ls = wrapListeners(proto, ls) // If we're binding to a TCP port, make sure that a container doesn't try to use it. if proto == "tcp" { if err := allocateDaemonPort(addr); err != nil { diff --git a/cmd/dockerd/daemon_unix.go b/cmd/dockerd/daemon_unix.go index 7b6214fc8f..2500260028 100644 --- a/cmd/dockerd/daemon_unix.go +++ b/cmd/dockerd/daemon_unix.go @@ -13,7 +13,6 @@ import ( "time" "github.com/containerd/containerd/runtime/v1/linux" - "github.com/docker/docker/cmd/dockerd/hack" "github.com/docker/docker/daemon" "github.com/docker/docker/daemon/config" "github.com/docker/docker/libcontainerd/supervisor" @@ -122,18 +121,6 @@ func allocateDaemonPort(addr string) error { return nil } -func wrapListeners(proto string, ls []net.Listener) []net.Listener { - switch proto { - case "unix": - ls[0] = &hack.MalformedHostHeaderOverride{Listener: ls[0]} - case "fd": - for i := range ls { - ls[i] = &hack.MalformedHostHeaderOverride{Listener: ls[i]} - } - } - return ls -} - func newCgroupParent(config *config.Config) string { cgroupParent := "docker" useSystemd := daemon.UsingSystemd(config) diff --git a/cmd/dockerd/daemon_windows.go b/cmd/dockerd/daemon_windows.go index 46932760cd..2d74531d46 100644 --- a/cmd/dockerd/daemon_windows.go +++ b/cmd/dockerd/daemon_windows.go @@ -3,7 +3,6 @@ package main import ( "context" "fmt" - "net" "os" "path/filepath" "time" @@ -86,10 +85,6 @@ func allocateDaemonPort(addr string) error { return nil } -func wrapListeners(proto string, ls []net.Listener) []net.Listener { - return ls -} - func newCgroupParent(config *config.Config) string { return "" } diff --git a/cmd/dockerd/hack/malformed_host_override.go b/cmd/dockerd/hack/malformed_host_override.go deleted file mode 100644 index ddd5eb9d8b..0000000000 --- a/cmd/dockerd/hack/malformed_host_override.go +++ /dev/null @@ -1,121 +0,0 @@ -// +build !windows - -package hack // import "github.com/docker/docker/cmd/dockerd/hack" - -import "net" - -// MalformedHostHeaderOverride is a wrapper to be able -// to overcome the 400 Bad request coming from old docker -// clients that send an invalid Host header. -type MalformedHostHeaderOverride struct { - net.Listener -} - -// MalformedHostHeaderOverrideConn wraps the underlying unix -// connection and keeps track of the first read from http.Server -// which just reads the headers. -type MalformedHostHeaderOverrideConn struct { - net.Conn - first bool -} - -var closeConnHeader = []byte("\r\nConnection: close\r") - -// Read reads the first *read* request from http.Server to inspect -// the Host header. If the Host starts with / then we're talking to -// an old docker client which send an invalid Host header. To not -// error out in http.Server we rewrite the first bytes of the request -// to sanitize the Host header itself. -// In case we're not dealing with old docker clients the data is just passed -// to the server w/o modification. -func (l *MalformedHostHeaderOverrideConn) Read(b []byte) (n int, err error) { - // http.Server uses a 4k buffer - if l.first && len(b) == 4096 { - // This keeps track of the first read from http.Server which just reads - // the headers - l.first = false - // The first read of the connection by http.Server is done limited to - // DefaultMaxHeaderBytes (usually 1 << 20) + 4096. - // Here we do the first read which gets us all the http headers to - // be inspected and modified below. - c, err := l.Conn.Read(b) - if err != nil { - return c, err - } - - var ( - start, end int - firstLineFeed = -1 - buf []byte - ) - for i := 0; i <= c-1-7; i++ { - if b[i] == '\n' && firstLineFeed == -1 { - firstLineFeed = i - } - if b[i] != '\n' { - continue - } - - if b[i+1] == '\r' && b[i+2] == '\n' { - return c, nil - } - - if b[i+1] != 'H' { - continue - } - if b[i+2] != 'o' { - continue - } - if b[i+3] != 's' { - continue - } - if b[i+4] != 't' { - continue - } - if b[i+5] != ':' { - continue - } - if b[i+6] != ' ' { - continue - } - if b[i+7] != '/' { - continue - } - // ensure clients other than the docker clients do not get this hack - if i != firstLineFeed { - return c, nil - } - start = i + 7 - // now find where the value ends - for ii, bbb := range b[start:c] { - if bbb == '\n' { - end = start + ii - break - } - } - buf = make([]byte, 0, c+len(closeConnHeader)-(end-start)) - // strip the value of the host header and - // inject `Connection: close` to ensure we don't reuse this connection - buf = append(buf, b[:start]...) - buf = append(buf, closeConnHeader...) - buf = append(buf, b[end:c]...) - copy(b, buf) - break - } - if len(buf) == 0 { - return c, nil - } - return len(buf), nil - } - return l.Conn.Read(b) -} - -// Accept makes the listener accepts connections and wraps the connection -// in a MalformedHostHeaderOverrideConn initializing first to true. -func (l *MalformedHostHeaderOverride) Accept() (net.Conn, error) { - c, err := l.Listener.Accept() - if err != nil { - return c, err - } - return &MalformedHostHeaderOverrideConn{c, true}, nil -} diff --git a/cmd/dockerd/hack/malformed_host_override_test.go b/cmd/dockerd/hack/malformed_host_override_test.go deleted file mode 100644 index 6874b059be..0000000000 --- a/cmd/dockerd/hack/malformed_host_override_test.go +++ /dev/null @@ -1,124 +0,0 @@ -// +build !windows - -package hack // import "github.com/docker/docker/cmd/dockerd/hack" - -import ( - "bytes" - "io" - "net" - "strings" - "testing" -) - -type bufConn struct { - net.Conn - buf *bytes.Buffer -} - -func (bc *bufConn) Read(b []byte) (int, error) { - return bc.buf.Read(b) -} - -func TestHeaderOverrideHack(t *testing.T) { - tests := [][2][]byte{ - { - []byte("GET /foo\nHost: /var/run/docker.sock\nUser-Agent: Docker\r\n\r\n"), - []byte("GET /foo\nHost: \r\nConnection: close\r\nUser-Agent: Docker\r\n\r\n"), - }, - { - []byte("GET /foo\nHost: /var/run/docker.sock\nUser-Agent: Docker\nFoo: Bar\r\n"), - []byte("GET /foo\nHost: \r\nConnection: close\r\nUser-Agent: Docker\nFoo: Bar\r\n"), - }, - { - []byte("GET /foo\nHost: /var/run/docker.sock\nUser-Agent: Docker\r\n\r\ntest something!"), - []byte("GET /foo\nHost: \r\nConnection: close\r\nUser-Agent: Docker\r\n\r\ntest something!"), - }, - { - []byte("GET /foo\nHost: /var/run/docker.sock\nUser-Agent: Docker\r\n\r\ntest something! " + strings.Repeat("test", 15000)), - []byte("GET /foo\nHost: \r\nConnection: close\r\nUser-Agent: Docker\r\n\r\ntest something! " + strings.Repeat("test", 15000)), - }, - { - []byte("GET /foo\nFoo: Bar\nHost: /var/run/docker.sock\nUser-Agent: Docker\r\n\r\n"), - []byte("GET /foo\nFoo: Bar\nHost: /var/run/docker.sock\nUser-Agent: Docker\r\n\r\n"), - }, - } - - // Test for https://github.com/docker/docker/issues/23045 - h0 := "GET /foo\nUser-Agent: Docker\r\n\r\n" - h0 = h0 + strings.Repeat("a", 4096-len(h0)-1) + "\n" - tests = append(tests, [2][]byte{[]byte(h0), []byte(h0)}) - - for _, pair := range tests { - read := make([]byte, 4096) - client := &bufConn{ - buf: bytes.NewBuffer(pair[0]), - } - l := MalformedHostHeaderOverrideConn{client, true} - - n, err := l.Read(read) - if err != nil && err != io.EOF { - t.Fatalf("read: %d - %d, err: %v\n%s", n, len(pair[0]), err, string(read[:n])) - } - if !bytes.Equal(read[:n], pair[1][:n]) { - t.Fatalf("\n%s\n%s\n", read[:n], pair[1][:n]) - } - } -} - -func BenchmarkWithHack(b *testing.B) { - client, srv := net.Pipe() - done := make(chan struct{}) - req := []byte("GET /foo\nHost: /var/run/docker.sock\nUser-Agent: Docker\n") - read := make([]byte, 4096) - b.SetBytes(int64(len(req) * 30)) - - l := MalformedHostHeaderOverrideConn{client, true} - go func() { - for { - if _, err := srv.Write(req); err != nil { - srv.Close() - break - } - l.first = true // make sure each subsequent run uses the hack parsing - } - close(done) - }() - - for i := 0; i < b.N; i++ { - for i := 0; i < 30; i++ { - if n, err := l.Read(read); err != nil && err != io.EOF { - b.Fatalf("read: %d - %d, err: %v\n%s", n, len(req), err, string(read[:n])) - } - } - } - l.Close() - <-done -} - -func BenchmarkNoHack(b *testing.B) { - client, srv := net.Pipe() - done := make(chan struct{}) - req := []byte("GET /foo\nHost: /var/run/docker.sock\nUser-Agent: Docker\n") - read := make([]byte, 4096) - b.SetBytes(int64(len(req) * 30)) - - go func() { - for { - if _, err := srv.Write(req); err != nil { - srv.Close() - break - } - } - close(done) - }() - - for i := 0; i < b.N; i++ { - for i := 0; i < 30; i++ { - if _, err := client.Read(read); err != nil && err != io.EOF { - b.Fatal(err) - } - } - } - client.Close() - <-done -}