Skip to content

Commit

Permalink
Handle symlinks when checking DB vs runtime configs
Browse files Browse the repository at this point in the history
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 containers#20872

Signed-off-by: Matt Heon <mheon@redhat.com>
  • Loading branch information
mheon committed Dec 1, 2023
1 parent 3b03e85 commit 2135949
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 14 deletions.
41 changes: 37 additions & 4 deletions libpod/boltdb_state_internal.go
Expand Up @@ -4,7 +4,9 @@
package libpod

import (
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -93,6 +95,7 @@ type dbConfigValidation struct {
runtimeValue string
key []byte
defaultValue string
isPath bool
}

// Check if the configuration of the database is compatible with the
Expand All @@ -111,42 +114,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,
},
}

Expand Down Expand Up @@ -221,22 +231,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
Expand Down
35 changes: 25 additions & 10 deletions libpod/sqlite_state.go
Expand Up @@ -7,6 +7,7 @@ import (
"database/sql"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
goruntime "runtime"
Expand Down Expand Up @@ -315,7 +316,7 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
);`

var (
os, staticDir, tmpDir, graphRoot, runRoot, graphDriver, volumePath string
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)
Expand Down Expand Up @@ -359,7 +360,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,
Expand All @@ -377,33 +378,47 @@ 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.
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
ourValClean, err := filepath.EvalSymlinks(dbVal)
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)
}

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
}

Expand Down
15 changes: 15 additions & 0 deletions test/system/005-info.bats
Expand Up @@ -187,6 +187,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=$new_home run_podman info

rm $new_home
}

@test "podman --root PATH --volumepath info - basic output" {
volumePath=${PODMAN_TMPDIR}/volumesGoHere
if ! is_remote; then
Expand Down

0 comments on commit 2135949

Please sign in to comment.