cmd/initContainer: Try to handle config files that're absolute symlinks

Currently toolbox containers can get misconfigured if some
configuration files on the host are absolute symbolic links to some
other location.

For example, when systemd-resolved is used to manage /etc/resolv.conf
on the host, and if the file is an absolute link to
/run/systemd/resolve/stub-resolv.conf, then /etc/resolv.conf ends up
being invalid inside the container. This happens because the
container's /etc/resolv.conf points to /run/host/etc/resolv.conf, which
in turn points to /run/systemd/resolve/stub-resolv.conf, but that's
absent from the container.

This is, of course, not a problem with relative symbolic links. If the
host had a relative link to ../run/systemd/resolve/stub-resolv.conf,
then it would continue to work inside the container.

One solution would have been to use flatpak-session-helper to maintain
a copy of the host's /etc/resolv.conf in
$XDG_RUNTIME_DIR/.flatpak-helper/monitor. However, that doesn't work
when toolbox(1) is run as root.

The other option is to prepend the destination of the absolute symbolic
link with /run/host to make it resolve inside the container. It might
not work with funky links, but it's enough for the common case where a
an administrator changed the host's /etc/resolv.conf into a sane, but
absolute, symbolic link.

Properly configured hosts should anyway use relative symbolic links,
because they don't need to be adjusted in such scenarios. That's also
what Fedora and Ubuntu do, by default.

Thanks to Tudor Roman for raising a concern about relative symbolic
links.

Based on Martin Pitt's work on the POSIX shell implementation:
https://github.com/containers/toolbox/pull/380

https://github.com/containers/toolbox/issues/187
This commit is contained in:
Ondřej Míchal 2020-05-27 15:57:11 +02:00 committed by Debarshi Ray
parent 10b7de5e6c
commit 88a95b07af
1 changed files with 92 additions and 2 deletions

View File

@ -23,6 +23,7 @@ import (
"os" "os"
"os/exec" "os/exec"
"os/user" "os/user"
"path/filepath"
"strings" "strings"
"syscall" "syscall"
@ -407,8 +408,47 @@ func mountBind(containerPath, source, flags string) error {
return nil return nil
} }
// redirectPath serves for creating symbolic links for crucial system
// configuration files to their counterparts on the host's filesystem.
//
// containerPath and target must be absolute paths
//
// If the target itself is a symbolic link, redirectPath will evaluate the
// link. If it's valid then redirectPath will link containerPath to target.
// If it's not, then redirectPath will still proceed with the linking in two
// different ways depending whether target is an absolute or a relative link:
//
// * absolute - target's destination will be prepended with /run/host, and
// containerPath will be linked to the resulting path. This is
// an attempt to unbreak things, but if it doesn't work then
// it's the user's responsibility to fix it up.
//
// This is meant to address the common case where
// /etc/resolv.conf on the host (ie., /run/host/etc/resolv.conf
// inside the container) is an absolute symbolic link to
// something like /run/systemd/resolve/stub-resolv.conf. The
// container's /etc/resolv.conf will then get linked to
// /run/host/run/systemd/resolved/resolv-stub.conf.
//
// This is why properly configured hosts should use relative
// symbolic links, because they don't need to be adjusted in
// such scenarios.
//
// * relative - containerPath will be linked to the invalid target, and it's
// the user's responsibility to fix it up.
//
// folder signifies if the target is a folder
func redirectPath(containerPath, target string, folder bool) error { func redirectPath(containerPath, target string, folder bool) error {
logrus.Debugf("Redirecting %s to %s", containerPath, target) if !filepath.IsAbs(containerPath) {
panic("containerPath must be an absolute path")
}
if !filepath.IsAbs(target) {
panic("target must be an absolute path")
}
logrus.Debugf("Preparing to redirect %s to %s", containerPath, target)
targetSanitized := sanitizeRedirectionTarget(target)
err := os.Remove(containerPath) err := os.Remove(containerPath)
if folder { if folder {
@ -421,9 +461,59 @@ func redirectPath(containerPath, target string, folder bool) error {
} }
} }
if err := os.Symlink(target, containerPath); err != nil { logrus.Debugf("Redirecting %s to %s", containerPath, targetSanitized)
if err := os.Symlink(targetSanitized, containerPath); err != nil {
return fmt.Errorf("failed to redirect %s to %s: %w", containerPath, target, err) return fmt.Errorf("failed to redirect %s to %s: %w", containerPath, target, err)
} }
return nil return nil
} }
func sanitizeRedirectionTarget(target string) string {
if !filepath.IsAbs(target) {
panic("target must be an absolute path")
}
fileInfo, err := os.Lstat(target)
if err != nil {
if os.IsNotExist(err) {
logrus.Warnf("%s not found", target)
} else {
logrus.Warnf("Failed to lstat %s: %v", target, err)
}
return target
}
fileMode := fileInfo.Mode()
if fileMode&os.ModeSymlink == 0 {
logrus.Debugf("%s isn't a symbolic link", target)
return target
}
logrus.Debugf("%s is a symbolic link", target)
_, err = filepath.EvalSymlinks(target)
if err == nil {
return target
}
logrus.Warnf("Failed to resolve %s: %v", target, err)
targetDestination, err := os.Readlink(target)
if err != nil {
logrus.Warnf("Failed to get the destination of %s: %v", target, err)
return target
}
logrus.Debugf("%s points to %s", target, targetDestination)
if filepath.IsAbs(targetDestination) {
logrus.Debugf("Prepending /run/host to %s", targetDestination)
targetGuess := filepath.Join("/run/host", targetDestination)
return targetGuess
}
return target
}