Skip to content

Commit

Permalink
libpod: Drop checks for paths in sqlite+boltdb
Browse files Browse the repository at this point in the history
The original logic here is old, dating to
7eb5ce9
and got inherited when the sqlite database was added.

Since then, various changes have landed here especially
around canonicalizing symbolic links.

However, this code *still* often causes problems; most recently
in https://gitlab.com/fedora/bootc/base-images/-/issues/20
where it seems like the way Anaconda has the system set up
trips this up again.

I can certainly believe that things can go wrong if one
overrides/reconfigures e.g. the runtime state dir to be
different. But there's also a lot of other ways to break
podman...and it's trivial to subvert this check with a
bind mount over the absolute path, pointing to some
arbitrary different place.

In general, encoding file names into files that are potentially
owned by the user is ugly...it can trip up basic things like
migrating a home directory, etc.

Since I am not aware of a common misconfiguration that these
checks block, and I am *very* aware of a lot of times they
have incorrectly blocked correct situations...just drop the
checks.

If we *do* need to do some more validation later, I think we
could say encode the directory inodes for at least the volume
dir. And the runtime dir could have the inode for the root,
but not the other way around.

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cgwalters committed Jul 30, 2024
1 parent b4d0c95 commit 1943115
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 86 deletions.
36 changes: 0 additions & 36 deletions libpod/boltdb_state_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package libpod
import (
"fmt"
"os"
"path/filepath"
"runtime"
"strings"

Expand Down Expand Up @@ -113,48 +112,13 @@ func checkRuntimeConfig(db *bolt.DB, rt *Runtime) error {
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,
},
}

// These fields were missing and will have to be recreated.
Expand Down
63 changes: 13 additions & 50 deletions libpod/sqlite_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,14 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
);`

var (
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)
dbOS, graphDriver 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.
Expand Down Expand Up @@ -363,9 +363,9 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
}
}()

row := tx.QueryRow("SELECT Os, StaticDir, TmpDir, GraphRoot, RunRoot, GraphDriver, VolumeDir FROM DBConfig;")
row := tx.QueryRow("SELECT Os, GraphDriver FROM DBConfig;")

if err := row.Scan(&dbOS, &staticDir, &tmpDir, &graphRoot, &runRoot, &graphDriver, &volumePath); err != nil {
if err := row.Scan(&dbOS, &graphDriver); err != nil {
if errors.Is(err, sql.ErrNoRows) {
if _, err := tx.Exec(createRow, 1, schemaVersion, runtimeOS,
runtimeStaticDir, runtimeTmpDir, runtimeGraphRoot,
Expand All @@ -383,55 +383,18 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
return fmt.Errorf("retrieving DB config: %w", err)
}

checkField := func(fieldName, dbVal, ourVal string, isPath bool) error {
if isPath {
// 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.
// Ignore ENOENT as reasonable, as some paths may not exist in early Libpod
// init.
if dbVal != "" {
checkedVal, err := evalSymlinksIfExists(dbVal)
if err != nil {
return fmt.Errorf("cannot evaluate symlinks on DB %s path %q: %w", fieldName, dbVal, err)
}
dbVal = checkedVal
}
if ourVal != "" {
checkedVal, err := evalSymlinksIfExists(ourVal)
if err != nil {
return fmt.Errorf("cannot evaluate symlinks on our %s path %q: %w", fieldName, ourVal, err)
}
ourVal = checkedVal
}
}

checkField := func(fieldName, dbVal, ourVal string) error {
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", dbOS, runtimeOS, false); err != nil {
return err
}
if err := checkField("static dir", staticDir, runtimeStaticDir, true); err != nil {
return err
}
if err := checkField("tmp dir", tmpDir, runtimeTmpDir, true); err != nil {
return err
}
if err := checkField("graph root", graphRoot, runtimeGraphRoot, true); err != nil {
return err
}
if err := checkField("run root", runRoot, runtimeRunRoot, true); err != nil {
return err
}
if err := checkField("graph driver", graphDriver, runtimeGraphDriver, false); err != nil {
if err := checkField("os", dbOS, runtimeOS); err != nil {
return err
}
if err := checkField("volume path", volumePath, runtimeVolumePath, true); err != nil {
if err := checkField("graph driver", graphDriver, runtimeGraphDriver); err != nil {
return err
}

Expand Down

0 comments on commit 1943115

Please sign in to comment.