Skip to content

Commit

Permalink
cmd/initContainer: Try to handle config files that're absolute symlinks
Browse files Browse the repository at this point in the history
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:
containers#380

containers#187
  • Loading branch information
HarryMichal authored and debarshiray committed Aug 28, 2020
1 parent 10b7de5 commit 88a95b0
Showing 1 changed file with 92 additions and 2 deletions.
94 changes: 92 additions & 2 deletions src/cmd/initContainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"
"os/exec"
"os/user"
"path/filepath"
"strings"
"syscall"

Expand Down Expand Up @@ -407,8 +408,47 @@ func mountBind(containerPath, source, flags string) error {
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 {
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)
if folder {
Expand All @@ -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 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
}

0 comments on commit 88a95b0

Please sign in to comment.