logging: new mode -l passthrough

.github/workflows/integration.yml

@@ -42,7 +42,8 @@ jobs:
       - name: Run conmon integration tests
         run: |
           make vendor
-          sudo make test
+          sudo mkdir -p /var/run/crio
+          sudo make test-binary
 
   cri-o:
     runs-on: ubuntu-latest

runner/conmon/conmon.go

@@ -22,6 +22,7 @@ type ConmonInstance struct {
 	pidFile string
 	stdout  io.Writer
 	stderr  io.Writer
+	stdin   io.Reader
 
 	parentStartPipe  *os.File
 	parentAttachPipe *os.File
@@ -61,6 +62,7 @@ func NewConmonInstance(options ...ConmonOption) (*ConmonInstance, error) {
 
 	ci.cmd.Stdout = ci.stdout
 	ci.cmd.Stderr = ci.stderr
+	ci.cmd.Stdin = ci.stdin
 	return ci, nil
 }
 

runner/conmon/options.go

@@ -30,6 +30,13 @@ func WithStderr(stderr io.Writer) ConmonOption {
 	}
 }
 
+func WithStdin(stdin io.Reader) ConmonOption {
+	return func(ci *ConmonInstance) error {
+		ci.stdin = stdin
+		return nil
+	}
+}
+
 func WithPath(path string) ConmonOption {
 	return func(ci *ConmonInstance) error {
 		ci.path = path

runner/conmon_test/conmon_test.go

@@ -9,6 +9,8 @@ import (
 	"github.com/containers/conmon/runner/conmon"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/pkg/errors"
+	"golang.org/x/sys/unix"
 )
 
 var _ = Describe("conmon", func() {
@@ -71,6 +73,18 @@ var _ = Describe("conmon", func() {
 			Expect(err).To(BeNil())
 		})
 		AfterEach(func() {
+			for {
+				// There is a race condition on the directory deletion
+				// as conmon could still be running and creating files
+				// under tmpDir.  Attempt rmdir again if it fails with
+				// ENOTEMPTY.
+				err := os.RemoveAll(tmpDir)
+				if err != nil && errors.Is(err, unix.ENOTEMPTY) {
+					continue
+				}
+				Expect(err).To(BeNil())
+				break
+			}
 			Expect(os.RemoveAll(tmpDir)).To(BeNil())
 			err := os.Chdir(origCwd)
 			Expect(err).To(BeNil())

runner/conmon_test/ctr_logs_test.go

@@ -55,6 +55,11 @@ var _ = Describe("conmon ctr logs", func() {
 		_, err := os.Stat(tmpLogPath)
 		Expect(err).To(BeNil())
 	})
+	It("log driver as passthrough should pass", func() {
+		stdout, stderr := getConmonOutputGivenLogOpts(conmon.WithLogDriver("passthrough", ""))
+		Expect(stdout).To(BeEmpty())
+		Expect(stderr).To(BeEmpty())
+	})
 	It("log driver as k8s-file with invalid path should fail", func() {
 		_, stderr := getConmonOutputGivenLogOpts(conmon.WithLogDriver("k8s-file", invalidPath))
 		Expect(stderr).To(ContainSubstring("Failed to open log file"))

runner/conmon_test/suite_test.go

@@ -37,8 +37,9 @@ func TestConmon(t *testing.T) {
 func getConmonOutputGivenOptions(options ...conmon.ConmonOption) (string, string) {
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
+	var stdin bytes.Buffer
 
-	options = append(options, conmon.WithStdout(&stdout), conmon.WithStderr(&stderr))
+	options = append(options, conmon.WithStdout(&stdout), conmon.WithStderr(&stderr), conmon.WithStdin(&stdin))
 
 	ci, err := conmon.CreateAndExecConmon(options...)
 	Expect(err).To(BeNil())

src/conmon.c

@@ -25,6 +25,16 @@
 #include <sys/stat.h>
 #include <locale.h>
 
+static void disconnect_std_streams(int dev_null_r, int dev_null_w)
+{
+	if (dup2(dev_null_r, STDIN_FILENO) < 0)
+		pexit("Failed to dup over stdin");
+	if (dup2(dev_null_w, STDOUT_FILENO) < 0)
+		pexit("Failed to dup over stdout");
+	if (dup2(dev_null_w, STDERR_FILENO) < 0)
+		pexit("Failed to dup over stderr");
+}
+
 int main(int argc, char *argv[])
 {
 	setlocale(LC_ALL, "");
@@ -114,13 +124,8 @@ int main(int argc, char *argv[])
 	/* Disconnect stdio from parent. We need to do this, because
 	   the parent is waiting for the stdout to end when the intermediate
 	   child dies */
-	if (dup2(dev_null_r, STDIN_FILENO) < 0)
-		pexit("Failed to dup over stdin");
-	if (dup2(dev_null_w, STDOUT_FILENO) < 0)
-		pexit("Failed to dup over stdout");
-	if (dup2(dev_null_w, STDERR_FILENO) < 0)
-		pexit("Failed to dup over stderr");
-
+	if (!logging_is_passthrough())
+		disconnect_std_streams(dev_null_r, dev_null_w);
 	/* Create a new session group */
 	setsid();
 
@@ -193,7 +198,7 @@ int main(int argc, char *argv[])
 
 	/* Setup endpoint for attach */
 	_cleanup_free_ char *attach_symlink_dir_path = NULL;
-	if (opt_bundle_path != NULL) {
+	if (opt_bundle_path != NULL && !logging_is_passthrough()) {
 		attach_symlink_dir_path = setup_attach_socket();
 		dummyfd = setup_terminal_control_fifo();
 		setup_console_fifo();
@@ -227,27 +232,28 @@ int main(int argc, char *argv[])
 		if (sigprocmask(SIG_SETMASK, &oldmask, NULL) < 0)
 			_pexit("Failed to unblock signals");
 
-		if (workerfd_stdin < 0)
-			workerfd_stdin = dev_null_r;
-		if (dup2(workerfd_stdin, STDIN_FILENO) < 0)
-			_pexit("Failed to dup over stdin");
-		if (workerfd_stdin != dev_null_r && fchmod(STDIN_FILENO, 0777) < 0)
-			nwarn("Failed to chown stdin");
-
-		if (workerfd_stdout < 0)
-			workerfd_stdout = dev_null_w;
-		if (dup2(workerfd_stdout, STDOUT_FILENO) < 0)
-			_pexit("Failed to dup over stdout");
-		if (workerfd_stdout != dev_null_w && fchmod(STDOUT_FILENO, 0777) < 0)
-			nwarn("Failed to chown stdout");
-
-		if (workerfd_stderr < 0)
-			workerfd_stderr = workerfd_stdout;
-		if (dup2(workerfd_stderr, STDERR_FILENO) < 0)
-			_pexit("Failed to dup over stderr");
-		if (workerfd_stderr != dev_null_w && fchmod(STDERR_FILENO, 0777) < 0)
-			nwarn("Failed to chown stderr");
-
+		if (!logging_is_passthrough()) {
+			if (workerfd_stdin < 0)
+				workerfd_stdin = dev_null_r;
+			if (dup2(workerfd_stdin, STDIN_FILENO) < 0)
+				_pexit("Failed to dup over stdin");
+			if (workerfd_stdin != dev_null_r && fchmod(STDIN_FILENO, 0777) < 0)
+				nwarn("Failed to chown stdin");
+
+			if (workerfd_stdout < 0)
+				workerfd_stdout = dev_null_w;
+			if (dup2(workerfd_stdout, STDOUT_FILENO) < 0)
+				_pexit("Failed to dup over stdout");
+			if (workerfd_stdout != dev_null_w && fchmod(STDOUT_FILENO, 0777) < 0)
+				nwarn("Failed to chown stdout");
+
+			if (workerfd_stderr < 0)
+				workerfd_stderr = workerfd_stdout;
+			if (dup2(workerfd_stderr, STDERR_FILENO) < 0)
+				_pexit("Failed to dup over stderr");
+			if (workerfd_stderr != dev_null_w && fchmod(STDERR_FILENO, 0777) < 0)
+				nwarn("Failed to chown stderr");
+		}
 		/* If LISTEN_PID env is set, we need to set the LISTEN_PID
 		   it to the new child process */
 		char *listenpid = getenv("LISTEN_PID");
@@ -287,6 +293,9 @@ int main(int argc, char *argv[])
 		exit(127);
 	}
 
+	if (logging_is_passthrough())
+		disconnect_std_streams(dev_null_r, dev_null_w);
+
 	if ((signal(SIGTERM, on_sig_exit) == SIG_ERR) || (signal(SIGQUIT, on_sig_exit) == SIG_ERR)
 	    || (signal(SIGINT, on_sig_exit) == SIG_ERR))
 		pexit("Failed to register the signal handler");

src/ctr_logging.c

@@ -24,6 +24,7 @@ static inline int sd_journal_sendv(G_GNUC_UNUSED const struct iovec *iov, G_GNUC
 /* Different types of container logging */
 static gboolean use_journald_logging = FALSE;
 static gboolean use_k8s_logging = FALSE;
+static gboolean use_logging_passthrough = FALSE;
 
 /* Value the user must input for each log driver */
 static const char *const K8S_FILE_STRING = "k8s-file";
@@ -71,6 +72,11 @@ static int set_k8s_timestamp(char *buf, ssize_t buflen, const char *pipename);
 static void reopen_k8s_file(void);
 
 
+gboolean logging_is_passthrough(void)
+{
+	return use_logging_passthrough;
+}
+
 /*
  * configures container log specific information, such as the drivers the user
  * called with and the max log size for log file types. For the log file types
@@ -160,6 +166,14 @@ static void parse_log_path(char *log_config)
 		return;
 	}
 
+	if (!strcmp(driver, "passthrough")) {
+		if (isatty(STDIN_FILENO) || isatty(STDOUT_FILENO) || isatty(STDERR_FILENO))
+			nexitf("cannot use a tty with passthrough logging mode to prevent attacks via TIOCSTI");
+
+		use_logging_passthrough = TRUE;
+		return;
+	}
+
 	if (!strcmp(driver, JOURNALD_FILE_STRING)) {
 		use_journald_logging = TRUE;
 		return;

src/ctr_logging.h

@@ -9,5 +9,6 @@ void reopen_log_files(void);
 bool write_to_logs(stdpipe_t pipe, char *buf, ssize_t num_read);
 void configure_log_drivers(gchar **log_drivers, int64_t log_size_max_, char *cuuid_, char *name_, char *tag);
 void sync_logs(void);
+gboolean logging_is_passthrough(void);
 
 #endif /* !defined(CTR_LOGGING_H) */

src/ctrl.c

@@ -252,8 +252,13 @@ static void setup_fifo(int *fifo_r, int *fifo_w, char *filename, char *error_var
 	if (!fifo_r || !fifo_w)
 		pexitf("setup fifo was passed a NULL pointer");
 
-	if (mkfifo(fifo_path, 0666) == -1)
-		pexitf("Failed to mkfifo at %s", fifo_path);
+	if (mkfifo(fifo_path, 0666) == -1) {
+		if (errno == EEXIST) {
+			unlink(fifo_path);
+			if (mkfifo(fifo_path, 0666) == -1)
+				pexitf("Failed to mkfifo at %s", fifo_path);
+		}
+	}
 
 	if ((*fifo_r = open(fifo_path, O_RDONLY | O_NONBLOCK | O_CLOEXEC)) == -1)
 		pexitf("Failed to open %s read half", error_var_name);