diff options
author | Ed Santiago <santiago@redhat.com> | 2020-08-27 12:57:33 -0600 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-08-27 12:57:33 -0600 |
commit | b13af4537fea0647c029067c664f1e79e0a6c3e6 (patch) | |
tree | 98163cb6467b5742ed73e2dbbc8be77b884fbce2 /libpod | |
parent | 7d3cadcc54cbad9a109471f586c10541544bc7db (diff) | |
parent | 2ea9dac5e1d00b2820bd7156e3bea4b9fd98c1e6 (diff) | |
download | podman-b13af4537fea0647c029067c664f1e79e0a6c3e6.tar.gz podman-b13af4537fea0647c029067c664f1e79e0a6c3e6.tar.bz2 podman-b13af4537fea0647c029067c664f1e79e0a6c3e6.zip |
Merge pull request #7451 from mheon/fix_7195
Send HTTP Hijack headers after successful attach
Diffstat (limited to 'libpod')
-rw-r--r-- | libpod/container_api.go | 85 | ||||
-rw-r--r-- | libpod/container_exec.go | 24 | ||||
-rw-r--r-- | libpod/oci.go | 7 | ||||
-rw-r--r-- | libpod/oci_conmon_exec_linux.go | 39 | ||||
-rw-r--r-- | libpod/oci_conmon_linux.go | 135 | ||||
-rw-r--r-- | libpod/oci_missing.go | 7 | ||||
-rw-r--r-- | libpod/util.go | 22 |
7 files changed, 204 insertions, 115 deletions
diff --git a/libpod/container_api.go b/libpod/container_api.go index c44e89042..0d7bbacd0 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -1,18 +1,14 @@ package libpod import ( - "bufio" "context" "io/ioutil" - "net" + "net/http" "os" - "strings" - "sync" "time" "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/libpod/events" - "github.com/containers/podman/v2/libpod/logs" "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -267,15 +263,10 @@ func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <- // over the socket; if this is not set, but streamLogs is, only the logs will be // sent. // At least one of streamAttach and streamLogs must be set. -func (c *Container) HTTPAttach(httpCon net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, streamAttach, streamLogs bool) (deferredErr error) { - isTerminal := false - if c.config.Spec.Process != nil { - isTerminal = c.config.Spec.Process.Terminal - } - // Ensure our contract of writing errors to and closing the HTTP conn is - // honored. +func (c *Container) HTTPAttach(r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, streamAttach, streamLogs bool, hijackDone chan<- bool) error { + // Ensure we don't leak a goroutine if we exit before hijack completes. defer func() { - hijackWriteErrorAndClose(deferredErr, c.ID(), isTerminal, httpCon, httpBuf) + close(hijackDone) }() if !c.batched { @@ -299,74 +290,8 @@ func (c *Container) HTTPAttach(httpCon net.Conn, httpBuf *bufio.ReadWriter, stre logrus.Infof("Performing HTTP Hijack attach to container %s", c.ID()) - logSize := 0 - if streamLogs { - // Get all logs for the container - logChan := make(chan *logs.LogLine) - logOpts := new(logs.LogOptions) - logOpts.Tail = -1 - logOpts.WaitGroup = new(sync.WaitGroup) - errChan := make(chan error) - go func() { - var err error - // In non-terminal mode we need to prepend with the - // stream header. - logrus.Debugf("Writing logs for container %s to HTTP attach", c.ID()) - for logLine := range logChan { - if !isTerminal { - device := logLine.Device - var header []byte - headerLen := uint32(len(logLine.Msg)) - logSize += len(logLine.Msg) - switch strings.ToLower(device) { - case "stdin": - header = makeHTTPAttachHeader(0, headerLen) - case "stdout": - header = makeHTTPAttachHeader(1, headerLen) - case "stderr": - header = makeHTTPAttachHeader(2, headerLen) - default: - logrus.Errorf("Unknown device for log line: %s", device) - header = makeHTTPAttachHeader(1, headerLen) - } - _, err = httpBuf.Write(header) - if err != nil { - break - } - } - _, err = httpBuf.Write([]byte(logLine.Msg)) - if err != nil { - break - } - _, err = httpBuf.Write([]byte("\n")) - if err != nil { - break - } - err = httpBuf.Flush() - if err != nil { - break - } - } - errChan <- err - }() - go func() { - logOpts.WaitGroup.Wait() - close(logChan) - }() - if err := c.ReadLog(context.Background(), logOpts, logChan); err != nil { - return err - } - logrus.Debugf("Done reading logs for container %s, %d bytes", c.ID(), logSize) - if err := <-errChan; err != nil { - return err - } - } - if !streamAttach { - return nil - } - c.newContainerEvent(events.Attach) - return c.ociRuntime.HTTPAttach(c, httpCon, httpBuf, streams, detachKeys, cancel) + return c.ociRuntime.HTTPAttach(c, r, w, streams, detachKeys, cancel, hijackDone, streamAttach, streamLogs) } // AttachResize resizes the container's terminal, which is displayed by Attach diff --git a/libpod/container_exec.go b/libpod/container_exec.go index bfeae0a11..2a852ab81 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -1,9 +1,8 @@ package libpod import ( - "bufio" "io/ioutil" - "net" + "net/http" "os" "path/filepath" "strconv" @@ -373,17 +372,12 @@ func (c *Container) ExecStartAndAttach(sessionID string, streams *define.AttachS } // ExecHTTPStartAndAttach starts and performs an HTTP attach to an exec session. -func (c *Container) ExecHTTPStartAndAttach(sessionID string, httpCon net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool) (deferredErr error) { +func (c *Container) ExecHTTPStartAndAttach(sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool) error { // TODO: How do we combine streams with the default streams set in the exec session? - // The flow here is somewhat strange, because we need to determine if - // there's a terminal ASAP (for error handling). - // Until we know, assume it's true (don't add standard stream headers). - // Add a defer to ensure our invariant (HTTP session is closed) is - // maintained. - isTerminal := true + // Ensure that we don't leak a goroutine here defer func() { - hijackWriteErrorAndClose(deferredErr, c.ID(), isTerminal, httpCon, httpBuf) + close(hijackDone) }() if !c.batched { @@ -399,8 +393,6 @@ func (c *Container) ExecHTTPStartAndAttach(sessionID string, httpCon net.Conn, h if !ok { return errors.Wrapf(define.ErrNoSuchExecSession, "container %s has no exec session with ID %s", c.ID(), sessionID) } - // We can now finally get the real value of isTerminal. - isTerminal = session.Config.Terminal // Verify that we are in a good state to continue if !c.ensureState(define.ContainerStateRunning) { @@ -432,7 +424,13 @@ func (c *Container) ExecHTTPStartAndAttach(sessionID string, httpCon net.Conn, h streams.Stderr = session.Config.AttachStderr } - pid, attachChan, err := c.ociRuntime.ExecContainerHTTP(c, session.ID(), execOpts, httpCon, httpBuf, streams, cancel) + holdConnOpen := make(chan bool) + + defer func() { + close(holdConnOpen) + }() + + pid, attachChan, err := c.ociRuntime.ExecContainerHTTP(c, session.ID(), execOpts, r, w, streams, cancel, hijackDone, holdConnOpen) if err != nil { session.State = define.ExecStateStopped session.ExitCode = define.TranslateExecErrorToExitCode(define.ExecErrorCodeGeneric, err) diff --git a/libpod/oci.go b/libpod/oci.go index 89850affc..924c32510 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -1,8 +1,7 @@ package libpod import ( - "bufio" - "net" + "net/http" "github.com/containers/podman/v2/libpod/define" "k8s.io/client-go/tools/remotecommand" @@ -63,7 +62,7 @@ type OCIRuntime interface { // used instead. Detach keys of "" will disable detaching via keyboard. // The streams parameter will determine which streams to forward to the // client. - HTTPAttach(ctr *Container, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool) error + HTTPAttach(ctr *Container, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool, streamAttach, streamLogs bool) error // AttachResize resizes the terminal in use by the given container. AttachResize(ctr *Container, newSize remotecommand.TerminalSize) error @@ -80,7 +79,7 @@ type OCIRuntime interface { // Maintains the same invariants as ExecContainer (returns on session // start, with a goroutine running in the background to handle attach). // The HTTP attach itself maintains the same invariants as HTTPAttach. - ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, cancel <-chan bool) (int, chan error, error) + ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (int, chan error, error) // ExecContainerDetached executes a command in a running container, but // does not attach to it. Returns the PID of the exec session and an // error (if starting the exec session failed) diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index cfe3745fa..c18da68fe 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -1,9 +1,9 @@ package libpod import ( - "bufio" "fmt" "net" + "net/http" "os" "os/exec" "path/filepath" @@ -80,7 +80,7 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options // ExecContainerHTTP executes a new command in an existing container and // forwards its standard streams over an attach -func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, cancel <-chan bool) (int, chan error, error) { +func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (int, chan error, error) { if streams != nil { if !streams.Stdin && !streams.Stdout && !streams.Stderr { return -1, nil, errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to") @@ -129,7 +129,7 @@ func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, o attachChan := make(chan error) go func() { // attachToExec is responsible for closing pipes - attachChan <- attachExecHTTP(ctr, sessionID, httpBuf, streams, pipes, detachKeys, options.Terminal, cancel) + attachChan <- attachExecHTTP(ctr, sessionID, req, w, streams, pipes, detachKeys, options.Terminal, cancel, hijackDone, holdConnOpen) close(attachChan) }() @@ -496,7 +496,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex } // Attach to a container over HTTP -func attachExecHTTP(c *Container, sessionID string, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool) error { +func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (deferredErr error) { if pipes == nil || pipes.startPipe == nil || pipes.attachPipe == nil { return errors.Wrapf(define.ErrInvalidArg, "must provide a start and attach pipe to finish an exec attach") } @@ -549,6 +549,37 @@ func attachExecHTTP(c *Container, sessionID string, httpBuf *bufio.ReadWriter, s attachStdin = streams.Stdin } + // Perform hijack + hijacker, ok := w.(http.Hijacker) + if !ok { + return errors.Errorf("unable to hijack connection") + } + + httpCon, httpBuf, err := hijacker.Hijack() + if err != nil { + return errors.Wrapf(err, "error hijacking connection") + } + + hijackDone <- true + + // Write a header to let the client know what happened + writeHijackHeader(r, httpBuf) + + // Force a flush after the header is written. + if err := httpBuf.Flush(); err != nil { + return errors.Wrapf(err, "error flushing HTTP hijack header") + } + + go func() { + // We need to hold the connection open until the complete exec + // function has finished. This channel will be closed in a defer + // in that function, so we can wait for it here. + // Can't be a defer, because this would block the function from + // returning. + <-holdConnOpen + hijackWriteErrorAndClose(deferredErr, c.ID(), isTerminal, httpCon, httpBuf) + }() + // Next, STDIN. Avoid entirely if attachStdin unset. if attachStdin { go func() { diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 67593a68b..82d91c3f6 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -5,16 +5,19 @@ package libpod import ( "bufio" "bytes" + "context" "fmt" "io" "io/ioutil" "net" + "net/http" "os" "os/exec" "path/filepath" "runtime" "strconv" "strings" + "sync" "syscall" "text/template" "time" @@ -22,6 +25,7 @@ import ( "github.com/containers/common/pkg/config" conmonConfig "github.com/containers/conmon/runner/config" "github.com/containers/podman/v2/libpod/define" + "github.com/containers/podman/v2/libpod/logs" "github.com/containers/podman/v2/pkg/cgroups" "github.com/containers/podman/v2/pkg/errorhandling" "github.com/containers/podman/v2/pkg/lookup" @@ -503,7 +507,9 @@ func (r *ConmonOCIRuntime) UnpauseContainer(ctr *Container) error { // this function returns. // If this is a container with a terminal, we will stream raw. If it is not, we // will stream with an 8-byte header to multiplex STDOUT and STDERR. -func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool) (deferredErr error) { +// Returns any errors that occurred, and whether the connection was successfully +// hijacked before that error occurred. +func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool, streamAttach, streamLogs bool) (deferredErr error) { isTerminal := false if ctr.config.Spec.Process != nil { isTerminal = ctr.config.Spec.Process.Terminal @@ -521,17 +527,21 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, httpConn net.Conn, httpBuf } socketPath := buildSocketPath(attachSock) - conn, err := net.DialUnix("unixpacket", nil, &net.UnixAddr{Name: socketPath, Net: "unixpacket"}) - if err != nil { - return errors.Wrapf(err, "failed to connect to container's attach socket: %v", socketPath) - } - defer func() { - if err := conn.Close(); err != nil { - logrus.Errorf("unable to close container %s attach socket: %q", ctr.ID(), err) + var conn *net.UnixConn + if streamAttach { + newConn, err := net.DialUnix("unixpacket", nil, &net.UnixAddr{Name: socketPath, Net: "unixpacket"}) + if err != nil { + return errors.Wrapf(err, "failed to connect to container's attach socket: %v", socketPath) } - }() + conn = newConn + defer func() { + if err := conn.Close(); err != nil { + logrus.Errorf("unable to close container %s attach socket: %q", ctr.ID(), err) + } + }() - logrus.Debugf("Successfully connected to container %s attach socket %s", ctr.ID(), socketPath) + logrus.Debugf("Successfully connected to container %s attach socket %s", ctr.ID(), socketPath) + } detachString := ctr.runtime.config.Engine.DetachKeys if detachKeys != nil { @@ -554,6 +564,111 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, httpConn net.Conn, httpBuf attachStdin = streams.Stdin } + logrus.Debugf("Going to hijack container %s attach connection", ctr.ID()) + + // Alright, let's hijack. + hijacker, ok := w.(http.Hijacker) + if !ok { + return errors.Errorf("unable to hijack connection") + } + + httpCon, httpBuf, err := hijacker.Hijack() + if err != nil { + return errors.Wrapf(err, "error hijacking connection") + } + + hijackDone <- true + + writeHijackHeader(req, httpBuf) + + // Force a flush after the header is written. + if err := httpBuf.Flush(); err != nil { + return errors.Wrapf(err, "error flushing HTTP hijack header") + } + + defer func() { + hijackWriteErrorAndClose(deferredErr, ctr.ID(), isTerminal, httpCon, httpBuf) + }() + + logrus.Debugf("Hijack for container %s attach session done, ready to stream", ctr.ID()) + + // TODO: This is gross. Really, really gross. + // I want to say we should read all the logs into an array before + // calling this, in container_api.go, but that could take a lot of + // memory... + // On the whole, we need to figure out a better way of doing this, + // though. + logSize := 0 + if streamLogs { + logrus.Debugf("Will stream logs for container %s attach session", ctr.ID()) + + // Get all logs for the container + logChan := make(chan *logs.LogLine) + logOpts := new(logs.LogOptions) + logOpts.Tail = -1 + logOpts.WaitGroup = new(sync.WaitGroup) + errChan := make(chan error) + go func() { + var err error + // In non-terminal mode we need to prepend with the + // stream header. + logrus.Debugf("Writing logs for container %s to HTTP attach", ctr.ID()) + for logLine := range logChan { + if !isTerminal { + device := logLine.Device + var header []byte + headerLen := uint32(len(logLine.Msg)) + logSize += len(logLine.Msg) + switch strings.ToLower(device) { + case "stdin": + header = makeHTTPAttachHeader(0, headerLen) + case "stdout": + header = makeHTTPAttachHeader(1, headerLen) + case "stderr": + header = makeHTTPAttachHeader(2, headerLen) + default: + logrus.Errorf("Unknown device for log line: %s", device) + header = makeHTTPAttachHeader(1, headerLen) + } + _, err = httpBuf.Write(header) + if err != nil { + break + } + } + _, err = httpBuf.Write([]byte(logLine.Msg)) + if err != nil { + break + } + _, err = httpBuf.Write([]byte("\n")) + if err != nil { + break + } + err = httpBuf.Flush() + if err != nil { + break + } + } + errChan <- err + }() + go func() { + logOpts.WaitGroup.Wait() + close(logChan) + }() + if err := ctr.ReadLog(context.Background(), logOpts, logChan); err != nil { + return err + } + logrus.Debugf("Done reading logs for container %s, %d bytes", ctr.ID(), logSize) + if err := <-errChan; err != nil { + return err + } + } + if !streamAttach { + logrus.Debugf("Done streaming logs for container %s attach, exiting as attach streaming not requested", ctr.ID()) + return nil + } + + logrus.Debugf("Forwarding attach output for container %s", ctr.ID()) + // Handle STDOUT/STDERR go func() { var err error diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go index 83a6aaf90..9d12972d4 100644 --- a/libpod/oci_missing.go +++ b/libpod/oci_missing.go @@ -1,9 +1,8 @@ package libpod import ( - "bufio" "fmt" - "net" + "net/http" "path/filepath" "sync" @@ -111,7 +110,7 @@ func (r *MissingRuntime) UnpauseContainer(ctr *Container) error { } // HTTPAttach is not available as the runtime is missing -func (r *MissingRuntime) HTTPAttach(ctr *Container, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool) error { +func (r *MissingRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool, streamAttach, streamLogs bool) error { return r.printError() } @@ -126,7 +125,7 @@ func (r *MissingRuntime) ExecContainer(ctr *Container, sessionID string, options } // ExecContainerHTTP is not available as the runtime is missing -func (r *MissingRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, cancel <-chan bool) (int, chan error, error) { +func (r *MissingRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (int, chan error, error) { return -1, nil, r.printError() } diff --git a/libpod/util.go b/libpod/util.go index c93ba7919..585b07aca 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "fmt" "io" + "net/http" "os" "os/exec" "path/filepath" @@ -257,6 +258,27 @@ func makeHTTPAttachHeader(stream byte, length uint32) []byte { return header } +// writeHijackHeader writes a header appropriate for the type of HTTP Hijack +// that occurred in a hijacked HTTP connection used for attach. +func writeHijackHeader(r *http.Request, conn io.Writer) { + // AttachHeader is the literal header sent for upgraded/hijacked connections for + // attach, sourced from Docker at: + // https://raw.githubusercontent.com/moby/moby/b95fad8e51bd064be4f4e58a996924f343846c85/api/server/router/container/container_routes.go + // Using literally to ensure compatibility with existing clients. + c := r.Header.Get("Connection") + proto := r.Header.Get("Upgrade") + if len(proto) == 0 || !strings.EqualFold(c, "Upgrade") { + // OK - can't upgrade if not requested or protocol is not specified + fmt.Fprintf(conn, + "HTTP/1.1 200 OK\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n") + } else { + // Upraded + fmt.Fprintf(conn, + "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: %s\r\n\r\n", + proto) + } +} + // Convert OCICNI port bindings into Inspect-formatted port bindings. func makeInspectPortBindings(bindings []ocicni.PortMapping) map[string][]define.InspectHostPort { portBindings := make(map[string][]define.InspectHostPort) |