From 9edf80d9348bbcfea42f73258865d729d8074ab0 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 14 Jul 2020 16:20:22 -0400 Subject: [PATCH 1/5] Preserve passwd on container restart We added code to create a `/etc/passwd` file that we bind-mount into the container in some cases (most notably, `--userns=keep-id` containers). This, unfortunately, was not persistent, so user-added users would be dropped on container restart. Changing where we store the file should fix this. Further, we want to ensure that lookups of users in the container use the right /etc/passwd if we replaced it. There was already logic to do this, but it only worked for user-added mounts; it's easy enough to alter it to use our mounts as well. Signed-off-by: Matthew Heon --- libpod/container_internal.go | 37 ++++++++++++++++++------------ libpod/container_internal_linux.go | 13 ++++++++++- libpod/util.go | 26 +++++++++++++++++++++ 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 6753114617..95e0bb0d4f 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -27,7 +27,6 @@ import ( securejoin "github.com/cyphar/filepath-securejoin" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" - "github.com/opencontainers/selinux/go-selinux/label" "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -1754,32 +1753,40 @@ func (c *Container) postDeleteHooks(ctx context.Context) error { return nil } -// writeStringToRundir copies the provided file to the runtimedir -func (c *Container) writeStringToRundir(destFile, output string) (string, error) { +// writeStringToRundir writes the given string to a file with the given name in +// the container's temporary files directory. The file will be chown'd to the +// container's root user and have an appropriate SELinux label set. +// If a file with the same name already exists, it will be deleted and recreated +// with the new contents. +// Returns the full path to the new file. +func (c *Container) writeStringToRundir(destFile, contents string) (string, error) { destFileName := filepath.Join(c.state.RunDir, destFile) if err := os.Remove(destFileName); err != nil && !os.IsNotExist(err) { return "", errors.Wrapf(err, "error removing %s for container %s", destFile, c.ID()) } - f, err := os.Create(destFileName) - if err != nil { - return "", errors.Wrapf(err, "unable to create %s", destFileName) - } - defer f.Close() - if err := f.Chown(c.RootUID(), c.RootGID()); err != nil { + if err := writeStringToPath(destFileName, contents, c.config.MountLabel, c.RootUID(), c.RootGID()); err != nil { return "", err } - if _, err := f.WriteString(output); err != nil { - return "", errors.Wrapf(err, "unable to write %s", destFileName) - } - // Relabel runDirResolv for the container - if err := label.Relabel(destFileName, c.config.MountLabel, false); err != nil { + return destFileName, nil +} + +// writeStringToStaticDir writes the given string to a file with the given name +// in the container's permanent files directory. The file will be chown'd to the +// container's root user and have an appropriate SELinux label set. +// Unlike writeStringToRundir, will *not* delete and re-create if the file +// already exists (will instead error). +// Returns the full path to the new file. +func (c *Container) writeStringToStaticDir(filename, contents string) (string, error) { + destFileName := filepath.Join(c.config.StaticDir, filename) + + if err := writeStringToPath(destFileName, contents, c.config.MountLabel, c.RootUID(), c.RootGID()); err != nil { return "", err } - return filepath.Join(c.state.RunDir, destFile), nil + return destFileName, nil } // appendStringToRundir appends the provided string to the runtimedir file diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 0d9a1c8241..b790da4458 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -238,6 +238,9 @@ func (c *Container) getUserOverrides() *lookup.Overrides { } } } + if path, ok := c.state.BindMounts["/etc/passwd"]; ok { + overrides.ContainerEtcPasswdPath = path + } return &overrides } @@ -1523,6 +1526,14 @@ func (c *Container) generatePasswd() (string, error) { if !c.config.AddCurrentUserPasswdEntry && c.config.User == "" { return "", nil } + if MountExists(c.config.Spec.Mounts, "/etc/passwd") { + return "", nil + } + // Re-use passwd if possible + passwdPath := filepath.Join(c.config.StaticDir, "passwd") + if _, err := os.Stat(passwdPath); err == nil { + return passwdPath, nil + } pwd := "" if c.config.User != "" { entry, err := c.generateUserPasswdEntry() @@ -1572,7 +1583,7 @@ func (c *Container) generatePasswd() (string, error) { if err != nil && !os.IsNotExist(err) { return "", errors.Wrapf(err, "unable to read passwd file %s", originPasswdFile) } - passwdFile, err := c.writeStringToRundir("passwd", string(orig)+pwd) + passwdFile, err := c.writeStringToStaticDir("passwd", string(orig)+pwd) if err != nil { return "", errors.Wrapf(err, "failed to create temporary passwd file") } diff --git a/libpod/util.go b/libpod/util.go index 8c2d946bad..a8d405b5fe 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -18,6 +18,7 @@ import ( "github.com/cri-o/ocicni/pkg/ocicni" "github.com/fsnotify/fsnotify" spec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -273,3 +274,28 @@ func makeInspectPortBindings(bindings []ocicni.PortMapping) map[string][]define. } return portBindings } + +// Write a given string to a new file at a given path. +// Will error if a file with the given name already exists. +// Will be chown'd to the UID/GID provided and have the provided SELinux label +// set. +func writeStringToPath(path, contents, mountLabel string, uid, gid int) error { + f, err := os.Create(path) + if err != nil { + return errors.Wrapf(err, "unable to create %s", path) + } + defer f.Close() + if err := f.Chown(uid, gid); err != nil { + return err + } + + if _, err := f.WriteString(contents); err != nil { + return errors.Wrapf(err, "unable to write %s", path) + } + // Relabel runDirResolv for the container + if err := label.Relabel(path, mountLabel, false); err != nil { + return err + } + + return nil +} From d34b1e7ad51a8e3211a3ce82931a4e75369b69f8 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 31 Aug 2020 14:06:49 -0400 Subject: [PATCH 2/5] Ensure rootless containers without a passwd can start We want to modify /etc/passwd to add an entry for the user in question, but at the same time we don't want to require the container provide a /etc/passwd (a container with a single, statically linked binary and nothing else is perfectly fine and should be allowed, for example). We could create the passwd file if it does not exist, but if the container doesn't provide one, it's probably better not to make one at all. Gate changes to /etc/passwd behind a stat() of the file in the container returning cleanly. Fixes #7515 Signed-off-by: Matthew Heon --- libpod/container_internal_linux.go | 11 +++++++++++ test/e2e/run_passwd_test.go | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index b790da4458..1abfd2f6f0 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -1534,6 +1534,17 @@ func (c *Container) generatePasswd() (string, error) { if _, err := os.Stat(passwdPath); err == nil { return passwdPath, nil } + // Check if container has a /etc/passwd - if it doesn't do nothing. + passwdPath, err := securejoin.SecureJoin(c.state.Mountpoint, "/etc/passwd") + if err != nil { + return "", errors.Wrapf(err, "error creating path to container %s /etc/passwd", c.ID()) + } + if _, err := os.Stat(passwdPath); err != nil { + if os.IsNotExist(err) { + return "", nil + } + return "", errors.Wrapf(err, "unable to access container %s /etc/passwd", c.ID()) + } pwd := "" if c.config.User != "" { entry, err := c.generateUserPasswdEntry() diff --git a/test/e2e/run_passwd_test.go b/test/e2e/run_passwd_test.go index 6caf8a094a..4ef4156a4c 100644 --- a/test/e2e/run_passwd_test.go +++ b/test/e2e/run_passwd_test.go @@ -60,4 +60,17 @@ var _ = Describe("Podman run passwd", func() { Expect(session.ExitCode()).To(Equal(0)) Expect(session.LineInOutputContains("passwd")).To(BeTrue()) }) + + It("podman can run container without /etc/passwd", func() { + SkipIfRemote() + dockerfile := `FROM alpine +RUN rm -f /etc/passwd /etc/shadow /etc/group +USER 1000` + imgName := "testimg" + podmanTest.BuildImage(dockerfile, imgName, "false") + session := podmanTest.Podman([]string{"run", "--rm", imgName, "ls", "/etc/"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Not(ContainSubstring("passwd"))) + }) }) From 5843e417c9f8c7cd49f4ee841ed01a264bf78170 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 1 Sep 2020 11:03:13 -0400 Subject: [PATCH 3/5] Update release notes for v2.0.6 Signed-off-by: Matthew Heon --- RELEASE_NOTES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 814cfe4cdd..72c777d50d 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -3,6 +3,9 @@ ## 2.0.6 ### Bugfixes - Fixed a bug where running systemd in a container on a cgroups v1 system would fail. +- Fixed a bug where `/etc/passwd` could be re-created every time a container is restarted if the container's `/etc/passwd` did not contain an entry for the user the container was started as. +- Fixed a bug where containers without an `/etc/passwd` file specifying a non-root user would not start. +- Fixed a bug where the `--remote` flag would sometimes not make remote connections and would instead attempt to run Podman locally. ### Misc - Updated the containers/common library to v0.14.10 From 27362ba1ad8879ea71610fa68a651a1651e0180f Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 1 Sep 2020 11:05:40 -0400 Subject: [PATCH 4/5] Bump to v2.0.6 Signed-off-by: Matthew Heon --- changelog.txt | 9 +++++++++ version/version.go | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/changelog.txt b/changelog.txt index 9dd24a8c62..8610c0b78f 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,3 +1,12 @@ +- Changelog for v2.0.6 (2020-09-01): + * Update release notes for v2.0.6 + * Ensure rootless containers without a passwd can start + * Preserve passwd on container restart + * Remove help/usage from --remote pre-check + * system tests: podman-remote, image tree + * Refactor parsing to not require --remote to be first + * Enable --remote flag + - Changelog for v2.0.6-rc1 (2020-08-28): * Update release notes for v2.0.6 * Bump c/common to v0.14.10 and c/conmon to v2.0.20 diff --git a/version/version.go b/version/version.go index 8dd18e1002..a52f1a7ba7 100644 --- a/version/version.go +++ b/version/version.go @@ -4,7 +4,7 @@ package version // NOTE: remember to bump the version at the top // of the top-level README.md file when this is // bumped. -const Version = "2.0.6-dev" +const Version = "2.0.6" // APIVersion is the version for the remote // client API. It is used to determine compatibility From ce7b78a43a386d53a6e4a2688f7ce0aa99511498 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 1 Sep 2020 11:08:02 -0400 Subject: [PATCH 5/5] Bump to v2.0.7-dev Signed-off-by: Matthew Heon --- contrib/spec/podman.spec.in | 2 +- version/version.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/spec/podman.spec.in b/contrib/spec/podman.spec.in index 8414b70c96..4fe3a7cc89 100644 --- a/contrib/spec/podman.spec.in +++ b/contrib/spec/podman.spec.in @@ -42,7 +42,7 @@ Epoch: 99 %else Epoch: 0 %endif -Version: 2.0.6 +Version: 2.0.7 Release: #COMMITDATE#.git%{shortcommit0}%{?dist} Summary: Manage Pods, Containers and Container Images License: ASL 2.0 diff --git a/version/version.go b/version/version.go index a52f1a7ba7..68a2971e9b 100644 --- a/version/version.go +++ b/version/version.go @@ -4,7 +4,7 @@ package version // NOTE: remember to bump the version at the top // of the top-level README.md file when this is // bumped. -const Version = "2.0.6" +const Version = "2.0.7-dev" // APIVersion is the version for the remote // client API. It is used to determine compatibility