From ad8648c6343cf2cf3e51a0f58cb053ee303f6ffb Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Tue, 4 Sep 2018 20:08:41 -0700 Subject: runsc: Pass log and config files to sandbox process by FD. This is a prereq for running the sandbox process as user "nobody", when it may not have permissions to open these files. Instead, we must open then before starting the sandbox process, and pass them by FD. The specutils.ReadSpecFromFile method was fixed to always seek to the beginning of the file before reading. This allows Files from the same FD to be read multiple times, as we do in the boot command when the apply-caps flag is set. Tested with --network=host. PiperOrigin-RevId: 211570647 Change-Id: I685be0a290aa7f70731ebdce82ebc0ebcc9d475c --- runsc/BUILD | 2 ++ runsc/boot/config.go | 3 ++ runsc/cmd/boot.go | 19 +++++++--- runsc/cmd/create.go | 3 ++ runsc/cmd/run.go | 3 ++ runsc/main.go | 25 +++++++------ runsc/sandbox/sandbox.go | 79 +++++++++++++++++++++++++++++++---------- runsc/specutils/specutils.go | 30 +++++++++++++--- runsc/test/testutil/testutil.go | 1 + 9 files changed, 127 insertions(+), 38 deletions(-) (limited to 'runsc') diff --git a/runsc/BUILD b/runsc/BUILD index 660cb2a06..e390b7bae 100644 --- a/runsc/BUILD +++ b/runsc/BUILD @@ -16,6 +16,7 @@ go_binary( "//pkg/log", "//runsc/boot", "//runsc/cmd", + "//runsc/specutils", "@com_github_google_subcommands//:go_default_library", ], ) @@ -45,6 +46,7 @@ go_binary( "//pkg/log", "//runsc/boot", "//runsc/cmd", + "//runsc/specutils", "@com_github_google_subcommands//:go_default_library", ], ) diff --git a/runsc/boot/config.go b/runsc/boot/config.go index efb8563ea..212f5b003 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -204,6 +204,9 @@ type Config struct { // TODO: Remove this when multiple container is fully supported. MultiContainer bool + // SpecFile is the file containing the OCI spec. + SpecFile string + // WatchdogAction sets what action the watchdog takes when triggered. WatchdogAction watchdog.Action diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index 666be902a..784baf23b 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -32,9 +32,12 @@ import ( // Boot implements subcommands.Command for the "boot" command which starts a // new sandbox. It should not be called directly. type Boot struct { - // bundleDir is the path to the bundle directory. + // bundleDir is the directory containing the OCI spec. bundleDir string + // specFD is the file descriptor that the spec will be read from. + specFD int + // controllerFD is the file descriptor of a stream socket for the // control server that is donated to this process. controllerFD int @@ -69,6 +72,7 @@ func (*Boot) Usage() string { // SetFlags implements subcommands.Command.SetFlags. func (b *Boot) SetFlags(f *flag.FlagSet) { f.StringVar(&b.bundleDir, "bundle", "", "required path to the root of the bundle directory") + f.IntVar(&b.specFD, "spec-fd", -1, "required fd with the container spec") f.IntVar(&b.controllerFD, "controller-fd", -1, "required FD of a stream socket for the control server that must be donated to this process") f.Var(&b.ioFDs, "io-fds", "list of FDs to connect 9P clients. They must follow this order: root first, then mounts as defined in the spec") f.BoolVar(&b.console, "console", false, "set to true if the sandbox should allow terminal ioctl(2) syscalls") @@ -78,7 +82,7 @@ func (b *Boot) SetFlags(f *flag.FlagSet) { // Execute implements subcommands.Command.Execute. It starts a sandbox in a // waiting state. func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus { - if b.bundleDir == "" || b.controllerFD == -1 || f.NArg() != 0 { + if b.specFD == -1 || b.controllerFD == -1 || f.NArg() != 0 { f.Usage() return subcommands.ExitUsageError } @@ -86,8 +90,10 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) // Ensure that if there is a panic, all goroutine stacks are printed. debug.SetTraceback("all") - // Get the spec from the bundleDir. - spec, err := specutils.ReadSpec(b.bundleDir) + // Get the spec from the specFD. + specFile := os.NewFile(uintptr(b.specFD), "spec file") + defer specFile.Close() + spec, err := specutils.ReadSpecFromFile(b.bundleDir, specFile) if err != nil { Fatalf("error reading spec: %v", err) } @@ -123,6 +129,11 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) args = append(args, arg) } } + + // Note that we've already read the spec from the spec FD, and + // we will read it again after the exec call. This works + // because the ReadSpecFromFile function seeks to the beginning + // of the file before reading. if err := setCapsAndCallSelf(args, caps); err != nil { Fatalf("%v", err) } diff --git a/runsc/cmd/create.go b/runsc/cmd/create.go index 94a889077..38ae03e7a 100644 --- a/runsc/cmd/create.go +++ b/runsc/cmd/create.go @@ -15,6 +15,8 @@ package cmd import ( + "path/filepath" + "context" "flag" "github.com/google/subcommands" @@ -83,6 +85,7 @@ func (c *Create) Execute(_ context.Context, f *flag.FlagSet, args ...interface{} Fatalf("error reading spec: %v", err) } specutils.LogSpec(spec) + conf.SpecFile = filepath.Join(bundleDir, "config.json") // Create the container. A new sandbox will be created for the // container unless the metadata specifies that it should be run in an diff --git a/runsc/cmd/run.go b/runsc/cmd/run.go index 681112f30..92aa6bc40 100644 --- a/runsc/cmd/run.go +++ b/runsc/cmd/run.go @@ -15,6 +15,7 @@ package cmd import ( + "path/filepath" "syscall" "context" @@ -71,6 +72,8 @@ func (r *Run) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) s if err != nil { Fatalf("error reading spec: %v", err) } + specutils.LogSpec(spec) + conf.SpecFile = filepath.Join(bundleDir, "config.json") ws, err := container.Run(id, spec, conf, bundleDir, r.consoleSocket, r.pidFile) if err != nil { diff --git a/runsc/main.go b/runsc/main.go index 773ec6486..0c9b9af78 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -17,13 +17,11 @@ package main import ( - "fmt" "io" "os" "path/filepath" "strings" "syscall" - "time" "context" "flag" @@ -32,6 +30,7 @@ import ( "gvisor.googlesource.com/gvisor/pkg/log" "gvisor.googlesource.com/gvisor/runsc/boot" "gvisor.googlesource.com/gvisor/runsc/cmd" + "gvisor.googlesource.com/gvisor/runsc/specutils" ) var ( @@ -48,6 +47,8 @@ var ( // Debugging flags. debugLogDir = flag.String("debug-log-dir", "", "additional location for logs. It creates individual log files per command") logPackets = flag.Bool("log-packets", false, "enable network packet logging") + logFD = flag.Int("log-fd", -1, "file descriptor to log to. If set, the 'log' flag is ignored.") + debugLogFD = flag.Int("debug-log-fd", -1, "file descriptor to write debug logs to. If set, the 'debug-log-dir' flag is ignored.") // Debugging flags: strace related strace = flag.Bool("strace", false, "enable strace") @@ -64,6 +65,7 @@ var ( panicSignal = flag.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.") ) +// gitRevision is set during linking. var gitRevision = "" func main() { @@ -152,7 +154,9 @@ func main() { } var logFile io.Writer = os.Stderr - if *logFilename != "" { + if *logFD > -1 { + logFile = os.NewFile(uintptr(*logFD), "log file") + } else if *logFilename != "" { // We must set O_APPEND and not O_TRUNC because Docker passes // the same log file for all commands (and also parses these // log files), so we can't destroy them on each command. @@ -173,18 +177,17 @@ func main() { cmd.Fatalf("invalid log format %q, must be 'json' or 'text'", *logFormat) } - if *debugLogDir != "" { + if *debugLogFD > -1 { + f := os.NewFile(uintptr(*debugLogFD), "debug log file") + e = log.MultiEmitter{e, log.GoogleEmitter{&log.Writer{Next: f}}} + } else if *debugLogDir != "" { if err := os.MkdirAll(*debugLogDir, 0775); err != nil { cmd.Fatalf("error creating dir %q: %v", *debugLogDir, err) } - - // Format: /runsc.log.. - scmd := flag.CommandLine.Arg(0) - filename := fmt.Sprintf("runsc.log.%s.%s", time.Now().Format("20060102-150405.000000"), scmd) - path := filepath.Join(*debugLogDir, filename) - f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664) + subcommand := flag.CommandLine.Arg(0) + f, err := specutils.DebugLogFile(*debugLogDir, subcommand) if err != nil { - cmd.Fatalf("error opening log file %q: %v", filename, err) + cmd.Fatalf("error opening debug log file in %q: %v", *debugLogDir, err) } e = log.MultiEmitter{e, log.GoogleEmitter{&log.Writer{Next: f}}} } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index f14a2f8c9..e0fadefcd 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -233,34 +233,70 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund // starts at 3 because 0, 1, and 2 are taken by stdin/out/err. nextFD := 3 - // Create control server socket here and donate FD to child process because - // it may be in a different network namespace and won't be reachable from - // outside. - addr := boot.ControlSocketAddr(s.ID) - fd, err := server.CreateSocket(addr) - log.Infof("Creating sandbox process with addr: %s", addr[1:]) // skip "\00". - if err != nil { - return fmt.Errorf("error creating control server socket for sandbox %q: %v", s.ID, err) - } - - consoleEnabled := consoleSocket != "" - binPath, err := specutils.BinPath() if err != nil { return err } cmd := exec.Command(binPath, conf.ToFlags()...) cmd.SysProcAttr = &syscall.SysProcAttr{} - cmd.Args = append(cmd.Args, - "boot", - "--bundle", bundleDir, - "--controller-fd="+strconv.Itoa(nextFD), - "--console="+strconv.FormatBool(consoleEnabled)) - nextFD++ - controllerFile := os.NewFile(uintptr(fd), "control_server_socket") + // Open the log files to pass to the sandbox as FDs. + // + // These flags must come BEFORE the "boot" command in cmd.Args. + if conf.LogFilename != "" { + logFile, err := os.OpenFile(conf.LogFilename, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return fmt.Errorf("error opening log file %q: %v", conf.LogFilename, err) + } + defer logFile.Close() + cmd.ExtraFiles = append(cmd.ExtraFiles, logFile) + cmd.Args = append(cmd.Args, "--log-fd="+strconv.Itoa(nextFD)) + nextFD++ + } + if conf.DebugLogDir != "" { + debugLogFile, err := specutils.DebugLogFile(conf.DebugLogDir, "boot") + if err != nil { + return fmt.Errorf("error opening debug log file in %q: %v", conf.DebugLogDir, err) + } + defer debugLogFile.Close() + cmd.ExtraFiles = append(cmd.ExtraFiles, debugLogFile) + cmd.Args = append(cmd.Args, "--debug-log-fd="+strconv.Itoa(nextFD)) + nextFD++ + } + + // Add the "boot" command to the args. + // + // All flags after this must be for the boot command + cmd.Args = append(cmd.Args, "boot", "--bundle="+bundleDir) + + consoleEnabled := consoleSocket != "" + cmd.Args = append(cmd.Args, "--console="+strconv.FormatBool(consoleEnabled)) + + // Create a socket for the control server and donate it to the sandbox. + addr := boot.ControlSocketAddr(s.ID) + sockFD, err := server.CreateSocket(addr) + log.Infof("Creating sandbox process with addr: %s", addr[1:]) // skip "\00". + if err != nil { + return fmt.Errorf("error creating control server socket for sandbox %q: %v", s.ID, err) + } + controllerFile := os.NewFile(uintptr(sockFD), "control_server_socket") defer controllerFile.Close() cmd.ExtraFiles = append(cmd.ExtraFiles, controllerFile) + cmd.Args = append(cmd.Args, "--controller-fd="+strconv.Itoa(nextFD)) + nextFD++ + + // Open the spec file to donate to the sandbox. + if conf.SpecFile == "" { + return fmt.Errorf("conf.SpecFile must be set") + } + specFile, err := os.Open(conf.SpecFile) + if err != nil { + return fmt.Errorf("error opening spec file %q: %v", conf.SpecFile, err) + } + defer specFile.Close() + cmd.ExtraFiles = append(cmd.ExtraFiles, specFile) + cmd.Args = append(cmd.Args, "--spec-fd="+strconv.Itoa(nextFD)) + nextFD++ // If there is a gofer, sends all socket ends to the sandbox. for _, f := range ioFiles { @@ -357,6 +393,11 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund nss = append(nss, specs.LinuxNamespace{Type: specs.UserNamespace}) } + // Log the fds we are donating to the sandbox process. + for i, f := range cmd.ExtraFiles { + log.Debugf("Donating FD %d: %q", i+3, f.Name()) + } + log.Debugf("Starting sandbox: %s %v", binPath, cmd.Args) if err := specutils.StartInNS(cmd, nss); err != nil { return err diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 6c1ac56c3..3234cc088 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -126,14 +126,28 @@ func absPath(base, rel string) string { // path, e.g. spec.Root.Path, mount.Source. func ReadSpec(bundleDir string) (*specs.Spec, error) { // The spec file must be in "config.json" inside the bundle directory. - specFile := filepath.Join(bundleDir, "config.json") - specBytes, err := ioutil.ReadFile(specFile) + specPath := filepath.Join(bundleDir, "config.json") + specFile, err := os.Open(specPath) if err != nil { - return nil, fmt.Errorf("error reading spec from file %q: %v", specFile, err) + return nil, fmt.Errorf("error opening spec file %q: %v", specPath, err) + } + defer specFile.Close() + return ReadSpecFromFile(bundleDir, specFile) +} + +// ReadSpecFromFile reads an OCI runtime spec from the given File, and +// normalizes all relative paths into absolute by prepending the bundle dir. +func ReadSpecFromFile(bundleDir string, specFile *os.File) (*specs.Spec, error) { + if _, err := specFile.Seek(0, os.SEEK_SET); err != nil { + return nil, fmt.Errorf("error seeking to beginning of file %q: %v", specFile.Name(), err) + } + specBytes, err := ioutil.ReadAll(specFile) + if err != nil { + return nil, fmt.Errorf("error reading spec from file %q: %v", specFile.Name(), err) } var spec specs.Spec if err := json.Unmarshal(specBytes, &spec); err != nil { - return nil, fmt.Errorf("error unmarshaling spec from file %q: %v\n %s", specFile, err, string(specBytes)) + return nil, fmt.Errorf("error unmarshaling spec from file %q: %v\n %s", specFile.Name(), err, string(specBytes)) } if err := ValidateSpec(&spec); err != nil { return nil, err @@ -372,3 +386,11 @@ func WaitForReady(pid int, timeout time.Duration, ready func() (bool, error)) er } return backoff.Retry(op, b) } + +// DebugLogFile opens a file in logDir based on the timestamp and subcommand +// for writing. +func DebugLogFile(logDir, subcommand string) (*os.File, error) { + // Format: /runsc.log.. + filename := fmt.Sprintf("runsc.log.%s.%s", time.Now().Format("20060102-150405.000000"), subcommand) + return os.OpenFile(filepath.Join(logDir, filename), os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664) +} diff --git a/runsc/test/testutil/testutil.go b/runsc/test/testutil/testutil.go index 25987d040..4429b981b 100644 --- a/runsc/test/testutil/testutil.go +++ b/runsc/test/testutil/testutil.go @@ -176,6 +176,7 @@ func SetupContainerInRoot(rootDir string, spec *specs.Spec, conf *boot.Config) ( } conf.RootDir = rootDir + conf.SpecFile = filepath.Join(bundleDir, "config.json") return bundleDir, nil } -- cgit v1.2.3