runsc metric-server: Double-check directory access permission.

Prior to this CL, pointing `runsc metric-server` to a `--root` directory
it didn't have access to didn't result in an error, because it uses a glob
pattern which wouldn't match anything (with no error) when pointed at a
directory with no access.

This CL fixes this behavior by checking for directory-listing capability
explicitly.

PiperOrigin-RevId: 504960517
This commit is contained in:
Etienne Perot
2023-01-26 15:44:59 -08:00
committed by gVisor bot
parent 880398c3c3
commit 6c6c73c6c4
2 changed files with 32 additions and 0 deletions
+6
View File
@@ -19,6 +19,7 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"math/rand"
"net"
"net/http"
@@ -743,6 +744,11 @@ func (m *MetricServer) Execute(ctx context.Context, f *flag.FlagSet, args ...any
if _, err := container.ListSandboxes(conf.RootDir); err != nil {
return util.Errorf("Invalid root directory %q: tried to list sandboxes within it and got: %v", conf.RootDir, err)
}
// container.ListSandboxes uses a glob pattern, which doesn't error out on
// permission errors. Double-check by actually listing the directory.
if _, err := ioutil.ReadDir(conf.RootDir); err != nil {
return util.Errorf("Invalid root directory %q: tried to list all entries within it and got: %v", conf.RootDir, err)
}
m.startTime = time.Now()
m.rand = rand.New(rand.NewSource(m.startTime.UnixNano()))
m.rootDir = conf.RootDir
+26
View File
@@ -17,6 +17,7 @@ package container
import (
"context"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"testing"
@@ -434,3 +435,28 @@ func TestContainerMetricsMultiple(t *testing.T) {
t.Errorf("Unexpectedly found testmetric_fs_opens metric data for no-metrics container %s: %v", noMetricsCont.ID, val)
}
}
func TestMetricServerChecksRootDirectoryAccess(t *testing.T) {
te, cleanup := setupMetrics(t)
defer cleanup()
if err := te.client.ShutdownServer(te.testCtx); err != nil {
t.Fatalf("Cannot stop metric server: %v", err)
}
prevStat, err := os.Lstat(te.sleepConf.RootDir)
if err != nil {
t.Fatalf("cannot stat %q: %v", te.sleepConf.RootDir, err)
}
if err := os.Chmod(te.sleepConf.RootDir, 0); err != nil {
t.Fatalf("cannot chmod %q as 000: %v", te.sleepConf.RootDir, err)
}
defer os.Chmod(te.sleepConf.RootDir, prevStat.Mode())
if _, err := ioutil.ReadDir(te.sleepConf.RootDir); err == nil {
t.Logf("Can still read directory %v despite chmodding it to 0. Maybe we are running as root? Skipping test.", te.sleepConf.RootDir)
return
}
shorterCtx, shorterCtxCancel := context.WithTimeout(te.testCtx, time.Second)
defer shorterCtxCancel()
if err := te.client.SpawnServer(shorterCtx, te.sleepConf); err == nil {
t.Error("Metric server was successfully able to be spawned despite not having access to the root directory")
}
}