From 67e3ddb75ff27b8de0022e330413b4308ec5b010 Mon Sep 17 00:00:00 2001
From: Arnaud Porterie <arnaud.porterie@docker.com>
Date: Fri, 5 Dec 2014 16:50:56 -0800
Subject: [PATCH] Forbid client piping to tty enabled container
Forbid `docker run -t` with a redirected stdin (such as `echo test |
docker run -ti busybox cat`). Forbid `docker exec -t` with a redirected
stdin. Forbid `docker attach` with a redirect stdin toward a tty enabled
container.
Signed-off-by: Arnaud Porterie <arnaud.porterie@docker.com>
---
api/client/cli.go | 11 ++
api/client/commands.go | 16 ++-
docs/man/docker-attach.1.md | 3 +
docs/man/docker-exec.1.md | 3 +
docs/man/docker-run.1.md | 3 +
docs/sources/reference/run.md | 7 +-
integration-cli/docker_cli_attach_test.go | 47 ++++++++
integration-cli/docker_cli_exec_test.go | 49 +++++++-
integration-cli/docker_cli_run_test.go | 29 +++++
integration/commands_test.go | 133 +++++++++-------------
10 files changed, 213 insertions(+), 88 deletions(-)
diff --git a/api/client/cli.go b/api/client/cli.go
index a477d0b3a9..e54eb8056e 100644
--- a/api/client/cli.go
+++ b/api/client/cli.go
@@ -3,6 +3,7 @@ package client
import (
"crypto/tls"
"encoding/json"
+ "errors"
"fmt"
"io"
"net"
@@ -104,6 +105,16 @@ func (cli *DockerCli) LoadConfigFile() (err error) {
return err
}
+func (cli *DockerCli) CheckTtyInput(attachStdin, ttyMode bool) error {
+ // In order to attach to a container tty, input stream for the client must
+ // be a tty itself: redirecting or piping the client standard input is
+ // incompatible with `docker run -t`, `docker exec -t` or `docker attach`.
+ if ttyMode && attachStdin && !cli.isTerminalIn {
+ return errors.New("cannot enable tty mode on non tty input")
+ }
+ return nil
+}
+
func NewDockerCli(in io.ReadCloser, out, err io.Writer, key libtrust.PrivateKey, proto, addr string, tlsConfig *tls.Config) *DockerCli {
var (
inFd uintptr
diff --git a/api/client/commands.go b/api/client/commands.go
index 5203513d90..89e5796bbb 100644
--- a/api/client/commands.go
+++ b/api/client/commands.go
@@ -1974,6 +1974,10 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
tty = config.GetBool("Tty")
)
+ if err := cli.CheckTtyInput(!*noStdin, tty); err != nil {
+ return err
+ }
+
if tty && cli.isTerminalOut {
if err := cli.monitorTtySize(cmd.Arg(0), false); err != nil {
log.Debugf("Error monitoring TTY size: %s", err)
@@ -2288,7 +2292,11 @@ func (cli *DockerCli) CmdRun(args ...string) error {
return nil
}
- if *flDetach {
+ if !*flDetach {
+ if err := cli.CheckTtyInput(config.AttachStdin, config.Tty); err != nil {
+ return err
+ }
+ } else {
if fl := cmd.Lookup("attach"); fl != nil {
flAttach = fl.Value.(*opts.ListOpts)
if flAttach.Len() != 0 {
@@ -2600,7 +2608,11 @@ func (cli *DockerCli) CmdExec(args ...string) error {
return nil
}
- if execConfig.Detach {
+ if !execConfig.Detach {
+ if err := cli.CheckTtyInput(execConfig.AttachStdin, execConfig.Tty); err != nil {
+ return err
+ }
+ } else {
if _, _, err := readBody(cli.call("POST", "/exec/"+execID+"/start", execConfig, false)); err != nil {
return err
}
diff --git a/docs/man/docker-attach.1.md b/docs/man/docker-attach.1.md
index 21bd566406..19fbaceb4a 100644
--- a/docs/man/docker-attach.1.md
+++ b/docs/man/docker-attach.1.md
@@ -20,6 +20,9 @@ container, or `CTRL-\` to get a stacktrace of the Docker client when it quits.
When you detach from a container the exit code will be returned to
the client.
+It is forbidden to redirect the standard input of a docker attach command while
+attaching to a tty-enabled container (i.e.: launched with -t`).
+
# OPTIONS
**--no-stdin**=*true*|*false*
Do not attach STDIN. The default is *false*.
diff --git a/docs/man/docker-exec.1.md b/docs/man/docker-exec.1.md
index c4e649016a..3db296ed76 100644
--- a/docs/man/docker-exec.1.md
+++ b/docs/man/docker-exec.1.md
@@ -31,5 +31,8 @@ container is unpaused, and then run
**-t**, **--tty**=*true*|*false*
Allocate a pseudo-TTY. The default is *false*.
+The **-t** option is incompatible with a redirection of the docker client
+standard input.
+
# HISTORY
November 2014, updated by Sven Dowideit <SvenDowideit@home.org.au>
diff --git a/docs/man/docker-run.1.md b/docs/man/docker-run.1.md
index f0129bedc9..44c5545084 100644
--- a/docs/man/docker-run.1.md
+++ b/docs/man/docker-run.1.md
@@ -267,6 +267,9 @@ outside of a container on the host.
input of any container. This can be used, for example, to run a throwaway
interactive shell. The default is value is false.
+The **-t** option is incompatible with a redirection of the docker client
+standard input.
+
**-u**, **--user**=""
Username or UID
diff --git a/docs/sources/reference/run.md b/docs/sources/reference/run.md
index 8ac9f9d789..74a567c00b 100644
--- a/docs/sources/reference/run.md
+++ b/docs/sources/reference/run.md
@@ -94,9 +94,10 @@ specify to which of the three standard streams (`STDIN`, `STDOUT`,
$ sudo docker run -a stdin -a stdout -i -t ubuntu /bin/bash
-For interactive processes (like a shell) you will typically want a tty
-as well as persistent standard input (`STDIN`), so you'll use `-i -t`
-together in most interactive cases.
+For interactive processes (like a shell), you must use `-i -t` together in
+order to allocate a tty for the container process. Specifying `-t` is however
+forbidden when the client standard output is redirected or pipe, such as in:
+`echo test | docker run -i busybox cat`.
## Container identification
diff --git a/integration-cli/docker_cli_attach_test.go b/integration-cli/docker_cli_attach_test.go
index d03a986e48..0530d3896e 100644
--- a/integration-cli/docker_cli_attach_test.go
+++ b/integration-cli/docker_cli_attach_test.go
@@ -87,3 +87,50 @@ func TestAttachMultipleAndRestart(t *testing.T) {
logDone("attach - multiple attach")
}
+
+func TestAttachTtyWithoutStdin(t *testing.T) {
+ defer deleteAllContainers()
+
+ cmd := exec.Command(dockerBinary, "run", "-d", "-ti", "busybox")
+ out, _, err := runCommandWithOutput(cmd)
+ if err != nil {
+ t.Fatalf("failed to start container: %v (%v)", out, err)
+ }
+
+ id := strings.TrimSpace(out)
+ if err := waitRun(id); err != nil {
+ t.Fatal(err)
+ }
+
+ defer func() {
+ cmd := exec.Command(dockerBinary, "kill", id)
+ if out, _, err := runCommandWithOutput(cmd); err != nil {
+ t.Fatalf("failed to kill container: %v (%v)", out, err)
+ }
+ }()
+
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+
+ cmd := exec.Command(dockerBinary, "attach", id)
+ if _, err := cmd.StdinPipe(); err != nil {
+ t.Fatal(err)
+ }
+
+ expected := "cannot enable tty mode"
+ if out, _, err := runCommandWithOutput(cmd); err == nil {
+ t.Fatal("attach should have failed")
+ } else if !strings.Contains(out, expected) {
+ t.Fatal("attach failed with error %q: expected %q", out, expected)
+ }
+ }()
+
+ select {
+ case <-done:
+ case <-time.After(attachWait):
+ t.Fatal("attach is running but should have failed")
+ }
+
+ logDone("attach - forbid piped stdin to tty enabled container")
+}
diff --git a/integration-cli/docker_cli_exec_test.go b/integration-cli/docker_cli_exec_test.go
index 3c11a87808..b07f215a36 100644
--- a/integration-cli/docker_cli_exec_test.go
+++ b/integration-cli/docker_cli_exec_test.go
@@ -273,7 +273,7 @@ func TestExecTtyCloseStdin(t *testing.T) {
t.Fatal(out, err)
}
- cmd = exec.Command(dockerBinary, "exec", "-it", "exec_tty_stdin", "cat")
+ cmd = exec.Command(dockerBinary, "exec", "-i", "exec_tty_stdin", "cat")
stdinRw, err := cmd.StdinPipe()
if err != nil {
t.Fatal(err)
@@ -304,3 +304,50 @@ func TestExecTtyCloseStdin(t *testing.T) {
logDone("exec - stdin is closed properly with tty enabled")
}
+
+func TestExecTtyWithoutStdin(t *testing.T) {
+ defer deleteAllContainers()
+
+ cmd := exec.Command(dockerBinary, "run", "-d", "-ti", "busybox")
+ out, _, err := runCommandWithOutput(cmd)
+ if err != nil {
+ t.Fatalf("failed to start container: %v (%v)", out, err)
+ }
+
+ id := strings.TrimSpace(out)
+ if err := waitRun(id); err != nil {
+ t.Fatal(err)
+ }
+
+ defer func() {
+ cmd := exec.Command(dockerBinary, "kill", id)
+ if out, _, err := runCommandWithOutput(cmd); err != nil {
+ t.Fatalf("failed to kill container: %v (%v)", out, err)
+ }
+ }()
+
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+
+ cmd := exec.Command(dockerBinary, "exec", "-ti", id, "true")
+ if _, err := cmd.StdinPipe(); err != nil {
+ t.Fatal(err)
+ }
+
+ expected := "cannot enable tty mode"
+ if out, _, err := runCommandWithOutput(cmd); err == nil {
+ t.Fatal("exec should have failed")
+ } else if !strings.Contains(out, expected) {
+ t.Fatal("exec failed with error %q: expected %q", out, expected)
+ }
+ }()
+
+ select {
+ case <-done:
+ case <-time.After(3 * time.Second):
+ t.Fatal("exec is running but should have failed")
+ }
+
+ logDone("exec - forbid piped stdin to tty enabled container")
+}
diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go
index 20a096f196..0b56f235fe 100644
--- a/integration-cli/docker_cli_run_test.go
+++ b/integration-cli/docker_cli_run_test.go
@@ -2742,3 +2742,32 @@ func TestRunPortFromDockerRangeInUse(t *testing.T) {
logDone("run - find another port if port from autorange already bound")
}
+
+func TestRunTtyWithPipe(t *testing.T) {
+ defer deleteAllContainers()
+
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+
+ cmd := exec.Command(dockerBinary, "run", "-ti", "busybox", "true")
+ if _, err := cmd.StdinPipe(); err != nil {
+ t.Fatal(err)
+ }
+
+ expected := "cannot enable tty mode"
+ if out, _, err := runCommandWithOutput(cmd); err == nil {
+ t.Fatal("run should have failed")
+ } else if !strings.Contains(out, expected) {
+ t.Fatal("run failed with error %q: expected %q", out, expected)
+ }
+ }()
+
+ select {
+ case <-done:
+ case <-time.After(3 * time.Second):
+ t.Fatal("container is running but should have failed")
+ }
+
+ logDone("run - forbid piped stdin with tty")
+}
diff --git a/integration/commands_test.go b/integration/commands_test.go
index b00c68641e..aa21791b50 100644
--- a/integration/commands_test.go
+++ b/integration/commands_test.go
@@ -15,6 +15,7 @@ import (
"github.com/docker/docker/pkg/term"
"github.com/docker/docker/utils"
"github.com/docker/libtrust"
+ "github.com/kr/pty"
)
func closeWrap(args ...io.Closer) error {
@@ -162,72 +163,20 @@ func TestRunDisconnect(t *testing.T) {
})
}
-// Expected behaviour: the process stay alive when the client disconnects
-// but the client detaches.
-func TestRunDisconnectTty(t *testing.T) {
-
- stdin, stdinPipe := io.Pipe()
- stdout, stdoutPipe := io.Pipe()
- key, err := libtrust.GenerateECP256PrivateKey()
- if err != nil {
- t.Fatal(err)
- }
-
- cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
- defer cleanup(globalEngine, t)
-
- c1 := make(chan struct{})
- go func() {
- defer close(c1)
- // We're simulating a disconnect so the return value doesn't matter. What matters is the
- // fact that CmdRun returns.
- if err := cli.CmdRun("-i", "-t", unitTestImageID, "/bin/cat"); err != nil {
- log.Debugf("Error CmdRun: %s", err)
- }
- }()
-
- container := waitContainerStart(t, 10*time.Second)
-
- state := setRaw(t, container)
- defer unsetRaw(t, container, state)
-
- // Client disconnect after run -i should keep stdin out in TTY mode
- setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() {
- if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil {
- t.Fatal(err)
- }
- })
-
- // Close pipes (simulate disconnect)
- if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil {
- t.Fatal(err)
- }
-
- // wait for CmdRun to return
- setTimeout(t, "Waiting for CmdRun timed out", 5*time.Second, func() {
- <-c1
- })
-
- // In tty mode, we expect the process to stay alive even after client's stdin closes.
-
- // Give some time to monitor to do his thing
- container.WaitStop(500 * time.Millisecond)
- if !container.IsRunning() {
- t.Fatalf("/bin/cat should still be running after closing stdin (tty mode)")
- }
-}
-
// TestRunDetach checks attaching and detaching with the escape sequence.
func TestRunDetach(t *testing.T) {
-
- stdin, stdinPipe := io.Pipe()
stdout, stdoutPipe := io.Pipe()
+ cpty, tty, err := pty.Open()
+ if err != nil {
+ t.Fatal(err)
+ }
+
key, err := libtrust.GenerateECP256PrivateKey()
if err != nil {
t.Fatal(err)
}
- cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
+ cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
defer cleanup(globalEngine, t)
ch := make(chan struct{})
@@ -242,22 +191,22 @@ func TestRunDetach(t *testing.T) {
defer unsetRaw(t, container, state)
setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() {
- if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil {
+ if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil {
t.Fatal(err)
}
})
setTimeout(t, "Escape sequence timeout", 5*time.Second, func() {
- stdinPipe.Write([]byte{16})
+ cpty.Write([]byte{16})
time.Sleep(100 * time.Millisecond)
- stdinPipe.Write([]byte{17})
+ cpty.Write([]byte{17})
})
// wait for CmdRun to return
setTimeout(t, "Waiting for CmdRun timed out", 15*time.Second, func() {
<-ch
})
- closeWrap(stdin, stdinPipe, stdout, stdoutPipe)
+ closeWrap(cpty, stdout, stdoutPipe)
time.Sleep(500 * time.Millisecond)
if !container.IsRunning() {
@@ -271,14 +220,18 @@ func TestRunDetach(t *testing.T) {
// TestAttachDetach checks that attach in tty mode can be detached using the long container ID
func TestAttachDetach(t *testing.T) {
- stdin, stdinPipe := io.Pipe()
stdout, stdoutPipe := io.Pipe()
+ cpty, tty, err := pty.Open()
+ if err != nil {
+ t.Fatal(err)
+ }
+
key, err := libtrust.GenerateECP256PrivateKey()
if err != nil {
t.Fatal(err)
}
- cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
+ cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
defer cleanup(globalEngine, t)
ch := make(chan struct{})
@@ -309,9 +262,13 @@ func TestAttachDetach(t *testing.T) {
state := setRaw(t, container)
defer unsetRaw(t, container, state)
- stdin, stdinPipe = io.Pipe()
stdout, stdoutPipe = io.Pipe()
- cli = client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
+ cpty, tty, err = pty.Open()
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ cli = client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
ch = make(chan struct{})
go func() {
@@ -324,7 +281,7 @@ func TestAttachDetach(t *testing.T) {
}()
setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() {
- if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil {
+ if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil {
if err != io.ErrClosedPipe {
t.Fatal(err)
}
@@ -332,9 +289,9 @@ func TestAttachDetach(t *testing.T) {
})
setTimeout(t, "Escape sequence timeout", 5*time.Second, func() {
- stdinPipe.Write([]byte{16})
+ cpty.Write([]byte{16})
time.Sleep(100 * time.Millisecond)
- stdinPipe.Write([]byte{17})
+ cpty.Write([]byte{17})
})
// wait for CmdRun to return
@@ -342,7 +299,7 @@ func TestAttachDetach(t *testing.T) {
<-ch
})
- closeWrap(stdin, stdinPipe, stdout, stdoutPipe)
+ closeWrap(cpty, stdout, stdoutPipe)
time.Sleep(500 * time.Millisecond)
if !container.IsRunning() {
@@ -356,14 +313,18 @@ func TestAttachDetach(t *testing.T) {
// TestAttachDetachTruncatedID checks that attach in tty mode can be detached
func TestAttachDetachTruncatedID(t *testing.T) {
- stdin, stdinPipe := io.Pipe()
stdout, stdoutPipe := io.Pipe()
+ cpty, tty, err := pty.Open()
+ if err != nil {
+ t.Fatal(err)
+ }
+
key, err := libtrust.GenerateECP256PrivateKey()
if err != nil {
t.Fatal(err)
}
- cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
+ cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
defer cleanup(globalEngine, t)
// Discard the CmdRun output
@@ -379,9 +340,13 @@ func TestAttachDetachTruncatedID(t *testing.T) {
state := setRaw(t, container)
defer unsetRaw(t, container, state)
- stdin, stdinPipe = io.Pipe()
stdout, stdoutPipe = io.Pipe()
- cli = client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
+ cpty, tty, err = pty.Open()
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ cli = client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
ch := make(chan struct{})
go func() {
@@ -394,7 +359,7 @@ func TestAttachDetachTruncatedID(t *testing.T) {
}()
setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() {
- if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil {
+ if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil {
if err != io.ErrClosedPipe {
t.Fatal(err)
}
@@ -402,16 +367,16 @@ func TestAttachDetachTruncatedID(t *testing.T) {
})
setTimeout(t, "Escape sequence timeout", 5*time.Second, func() {
- stdinPipe.Write([]byte{16})
+ cpty.Write([]byte{16})
time.Sleep(100 * time.Millisecond)
- stdinPipe.Write([]byte{17})
+ cpty.Write([]byte{17})
})
// wait for CmdRun to return
setTimeout(t, "Waiting for CmdAttach timed out", 15*time.Second, func() {
<-ch
})
- closeWrap(stdin, stdinPipe, stdout, stdoutPipe)
+ closeWrap(cpty, stdout, stdoutPipe)
time.Sleep(500 * time.Millisecond)
if !container.IsRunning() {
@@ -425,14 +390,18 @@ func TestAttachDetachTruncatedID(t *testing.T) {
// Expected behaviour, the process stays alive when the client disconnects
func TestAttachDisconnect(t *testing.T) {
- stdin, stdinPipe := io.Pipe()
stdout, stdoutPipe := io.Pipe()
+ cpty, tty, err := pty.Open()
+ if err != nil {
+ t.Fatal(err)
+ }
+
key, err := libtrust.GenerateECP256PrivateKey()
if err != nil {
t.Fatal(err)
}
- cli := client.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
+ cli := client.NewDockerCli(tty, stdoutPipe, ioutil.Discard, key, testDaemonProto, testDaemonAddr, nil)
defer cleanup(globalEngine, t)
go func() {
@@ -470,12 +439,12 @@ func TestAttachDisconnect(t *testing.T) {
}()
setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() {
- if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil {
+ if err := assertPipe("hello\n", "hello", stdout, cpty, 150); err != nil {
t.Fatal(err)
}
})
// Close pipes (client disconnects)
- if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil {
+ if err := closeWrap(cpty, stdout, stdoutPipe); err != nil {
t.Fatal(err)
}