From 2d20d2e5c6efad1241d51e08c10fadad8e67a11b Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Fri, 1 Dec 2023 12:19:27 -0500 Subject: [PATCH] Handle symlinks when checking DB vs runtime configs When Podman starts, it checks a number of critical runtime paths against stored values in the database to make sure that existing containers are not broken by a configuration change. We recently made some changes to this logic to make our handling of the some options more sane (StaticDir in particular was set based on other passed options in a way that was not particularly sane) which has made the logic more sensitive to paths with symlinks. As a simple fix, handle symlinks properly in our DB vs runtime comparisons. The BoltDB bits are uglier because very, very old Podman versions sometimes did not stuff a proper value in the database and instead used the empty string. SQLite is new enough that we don't have to worry about such things. Fixes #20872 Signed-off-by: Matt Heon --- libpod/boltdb_state_internal.go | 41 ++++++++++++++++++++++--- libpod/sqlite_state.go | 54 ++++++++++++++++++++++----------- test/system/005-info.bats | 15 +++++++++ 3 files changed, 89 insertions(+), 21 deletions(-) diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 388110f251..75888410ab 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -4,7 +4,9 @@ package libpod import ( + "errors" "fmt" + "io/fs" "os" "path/filepath" "runtime" @@ -94,6 +96,7 @@ type dbConfigValidation struct { runtimeValue string key []byte defaultValue string + isPath bool } // Check if the configuration of the database is compatible with the @@ -112,42 +115,49 @@ func checkRuntimeConfig(db *bolt.DB, rt *Runtime) error { runtime.GOOS, osKey, runtime.GOOS, + false, }, { "libpod root directory (staticdir)", filepath.Clean(rt.config.Engine.StaticDir), staticDirKey, "", + true, }, { "libpod temporary files directory (tmpdir)", filepath.Clean(rt.config.Engine.TmpDir), tmpDirKey, "", + true, }, { "storage temporary directory (runroot)", filepath.Clean(rt.StorageConfig().RunRoot), runRootKey, storeOpts.RunRoot, + true, }, { "storage graph root directory (graphroot)", filepath.Clean(rt.StorageConfig().GraphRoot), graphRootKey, storeOpts.GraphRoot, + true, }, { "storage graph driver", rt.StorageConfig().GraphDriverName, graphDriverKey, storeOpts.GraphDriverName, + false, }, { "volume path", rt.config.Engine.VolumePath, volPathKey, "", + true, }, } @@ -222,22 +232,45 @@ func readOnlyValidateConfig(bucket *bolt.Bucket, toCheck dbConfigValidation) (bo } dbValue := string(keyBytes) + ourValue := toCheck.runtimeValue + + // Tolerate symlinks when possible - most relevant for OStree systems + // and rootless containers, where we want to put containers in /home, + // which is symlinked to /var/home. + if toCheck.isPath { + if dbValue != "" { + // Ignore ENOENT on both, on a fresh system some paths + // may not exist this early in Libpod init. + dbVal, err := filepath.EvalSymlinks(dbValue) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return false, fmt.Errorf("evaluating symlinks on DB %s path %q: %w", toCheck.name, dbValue, err) + } + dbValue = dbVal + } + if ourValue != "" { + ourVal, err := filepath.EvalSymlinks(ourValue) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return false, fmt.Errorf("evaluating symlinks on configured %s path %q: %w", toCheck.name, ourValue, err) + } + ourValue = ourVal + } + } - if toCheck.runtimeValue != dbValue { + if ourValue != dbValue { // If the runtime value is the empty string and default is not, // check against default. - if toCheck.runtimeValue == "" && toCheck.defaultValue != "" && dbValue == toCheck.defaultValue { + if ourValue == "" && toCheck.defaultValue != "" && dbValue == toCheck.defaultValue { return true, nil } // If the DB value is the empty string, check that the runtime // value is the default. - if dbValue == "" && toCheck.defaultValue != "" && toCheck.runtimeValue == toCheck.defaultValue { + if dbValue == "" && toCheck.defaultValue != "" && ourValue == toCheck.defaultValue { return true, nil } return true, fmt.Errorf("database %s %q does not match our %s %q: %w", - toCheck.name, dbValue, toCheck.name, toCheck.runtimeValue, define.ErrDBBadConfig) + toCheck.name, dbValue, toCheck.name, ourValue, define.ErrDBBadConfig) } return true, nil diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index f3f6d7c307..b1d366aeac 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -7,6 +7,7 @@ import ( "database/sql" "errors" "fmt" + "io/fs" "os" "path/filepath" goruntime "runtime" @@ -316,14 +317,14 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) { );` var ( - os, staticDir, tmpDir, graphRoot, runRoot, graphDriver, volumePath string - runtimeOS = goruntime.GOOS - runtimeStaticDir = filepath.Clean(s.runtime.config.Engine.StaticDir) - runtimeTmpDir = filepath.Clean(s.runtime.config.Engine.TmpDir) - runtimeGraphRoot = filepath.Clean(s.runtime.StorageConfig().GraphRoot) - runtimeRunRoot = filepath.Clean(s.runtime.StorageConfig().RunRoot) - runtimeGraphDriver = s.runtime.StorageConfig().GraphDriverName - runtimeVolumePath = filepath.Clean(s.runtime.config.Engine.VolumePath) + dbOS, staticDir, tmpDir, graphRoot, runRoot, graphDriver, volumePath string + runtimeOS = goruntime.GOOS + runtimeStaticDir = filepath.Clean(s.runtime.config.Engine.StaticDir) + runtimeTmpDir = filepath.Clean(s.runtime.config.Engine.TmpDir) + runtimeGraphRoot = filepath.Clean(s.runtime.StorageConfig().GraphRoot) + runtimeRunRoot = filepath.Clean(s.runtime.StorageConfig().RunRoot) + runtimeGraphDriver = s.runtime.StorageConfig().GraphDriverName + runtimeVolumePath = filepath.Clean(s.runtime.config.Engine.VolumePath) ) // Some fields may be empty, indicating they are set to the default. @@ -360,7 +361,7 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) { row := tx.QueryRow("SELECT Os, StaticDir, TmpDir, GraphRoot, RunRoot, GraphDriver, VolumeDir FROM DBConfig;") - if err := row.Scan(&os, &staticDir, &tmpDir, &graphRoot, &runRoot, &graphDriver, &volumePath); err != nil { + if err := row.Scan(&dbOS, &staticDir, &tmpDir, &graphRoot, &runRoot, &graphDriver, &volumePath); err != nil { if errors.Is(err, sql.ErrNoRows) { if _, err := tx.Exec(createRow, 1, schemaVersion, runtimeOS, runtimeStaticDir, runtimeTmpDir, runtimeGraphRoot, @@ -378,7 +379,26 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) { return fmt.Errorf("retrieving DB config: %w", err) } - checkField := func(fieldName, dbVal, ourVal string) error { + checkField := func(fieldName, dbVal, ourVal string, isPath bool) error { + if isPath { + // Evaluate symlinks. Ignore ENOENT. No guarantee all + // directories exist this early in Libpod init. + if dbVal != "" { + dbValClean, err := filepath.EvalSymlinks(dbVal) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("cannot evaluate symlinks on DB %s path %q: %w", fieldName, dbVal, err) + } + dbVal = dbValClean + } + if ourVal != "" { + ourValClean, err := filepath.EvalSymlinks(ourVal) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("cannot evaluate symlinks on our %s path %q: %w", fieldName, ourVal, err) + } + ourVal = ourValClean + } + } + if dbVal != ourVal { return fmt.Errorf("database %s %q does not match our %s %q: %w", fieldName, dbVal, fieldName, ourVal, define.ErrDBBadConfig) } @@ -386,25 +406,25 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) { return nil } - if err := checkField("os", os, runtimeOS); err != nil { + if err := checkField("os", dbOS, runtimeOS, false); err != nil { return err } - if err := checkField("static dir", staticDir, runtimeStaticDir); err != nil { + if err := checkField("static dir", staticDir, runtimeStaticDir, true); err != nil { return err } - if err := checkField("tmp dir", tmpDir, runtimeTmpDir); err != nil { + if err := checkField("tmp dir", tmpDir, runtimeTmpDir, true); err != nil { return err } - if err := checkField("graph root", graphRoot, runtimeGraphRoot); err != nil { + if err := checkField("graph root", graphRoot, runtimeGraphRoot, true); err != nil { return err } - if err := checkField("run root", runRoot, runtimeRunRoot); err != nil { + if err := checkField("run root", runRoot, runtimeRunRoot, true); err != nil { return err } - if err := checkField("graph driver", graphDriver, runtimeGraphDriver); err != nil { + if err := checkField("graph driver", graphDriver, runtimeGraphDriver, false); err != nil { return err } - if err := checkField("volume path", volumePath, runtimeVolumePath); err != nil { + if err := checkField("volume path", volumePath, runtimeVolumePath, true); err != nil { return err } diff --git a/test/system/005-info.bats b/test/system/005-info.bats index c5fcc3b85c..9f499c620c 100644 --- a/test/system/005-info.bats +++ b/test/system/005-info.bats @@ -183,6 +183,21 @@ host.slirp4netns.executable | $expr_path fi } +@test "rootless podman with symlinked $HOME" { + # This is only needed as rootless, but we don't have a skip_if_root + # And it will not hurt to run as root. + skip_if_remote "path validation is only done in libpod, does not effect remote" + + new_home=$PODMAN_TMPDIR/home + + ln -s /home $new_home + + # Just need the command to run cleanly + HOME=$PODMAN_TMPDIR/$HOME run_podman info + + rm $new_home +} + @test "podman --root PATH --volumepath info - basic output" { volumePath=${PODMAN_TMPDIR}/volumesGoHere if ! is_remote; then