summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbaude <bbaude@redhat.com>2018-10-24 10:39:12 -0500
committerbaude <bbaude@redhat.com>2018-10-25 06:42:43 -0500
commit6246942d377bd9ed665a4ac448120352454dd83d (patch)
treef99794cbb171220c8ac6ff7c0008381062a6b6df
parent57f778aed93efc0961b1335bcd07c3c82a11da0a (diff)
downloadpodman-6246942d377bd9ed665a4ac448120352454dd83d.tar.gz
podman-6246942d377bd9ed665a4ac448120352454dd83d.tar.bz2
podman-6246942d377bd9ed665a4ac448120352454dd83d.zip
Increase security and performance when looking up groups
We implement the securejoin method to make sure the paths to /etc/passwd and /etc/group are not symlinks to something naughty or outside the container image. And then instead of actually chrooting, we use the runc functions to get information about a user. The net result is increased security and a a performance gain from 41ms to 100us. Signed-off-by: baude <bbaude@redhat.com>
-rw-r--r--libpod/container_internal_linux.go26
-rw-r--r--vendor/github.com/cyphar/filepath-securejoin/LICENSE28
-rw-r--r--vendor/github.com/cyphar/filepath-securejoin/README.md65
-rw-r--r--vendor/github.com/cyphar/filepath-securejoin/join.go134
-rw-r--r--vendor/github.com/cyphar/filepath-securejoin/vendor.conf1
-rw-r--r--vendor/github.com/cyphar/filepath-securejoin/vfs.go41
6 files changed, 291 insertions, 4 deletions
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index b25645e5c..5a6b72580 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -21,6 +21,8 @@ import (
"github.com/containers/libpod/pkg/criu"
"github.com/containers/libpod/pkg/rootless"
"github.com/containers/storage/pkg/idtools"
+ "github.com/cyphar/filepath-securejoin"
+ "github.com/opencontainers/runc/libcontainer/user"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/runtime-tools/generate"
"github.com/opencontainers/selinux/go-selinux/label"
@@ -197,12 +199,28 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
// Look up and add groups the user belongs to, if a group wasn't directly specified
if !rootless.IsRootless() && !strings.Contains(c.config.User, ":") {
- groups, err := chrootuser.GetAdditionalGroupsForUser(c.state.Mountpoint, uint64(g.Config.Process.User.UID))
- if err != nil && errors.Cause(err) != chrootuser.ErrNoSuchUser {
+ var groupDest, passwdDest string
+ defaultExecUser := user.ExecUser{
+ Uid: 0,
+ Gid: 0,
+ Home: "/",
+ }
+
+ // Make sure the /etc/group and /etc/passwd destinations are not a symlink to something naughty
+ if groupDest, err = securejoin.SecureJoin(c.state.Mountpoint, "/etc/group"); err != nil {
+ logrus.Debug(err)
return nil, err
}
- for _, gid := range groups {
- g.AddProcessAdditionalGid(gid)
+ if passwdDest, err = securejoin.SecureJoin(c.state.Mountpoint, "/etc/passwd"); err != nil {
+ logrus.Debug(err)
+ return nil, err
+ }
+ execUser, err := user.GetExecUserPath(c.config.User, &defaultExecUser, passwdDest, groupDest)
+ if err != nil {
+ return nil, err
+ }
+ for _, gid := range execUser.Sgids {
+ g.AddProcessAdditionalGid(uint32(gid))
}
}
diff --git a/vendor/github.com/cyphar/filepath-securejoin/LICENSE b/vendor/github.com/cyphar/filepath-securejoin/LICENSE
new file mode 100644
index 000000000..bec842f29
--- /dev/null
+++ b/vendor/github.com/cyphar/filepath-securejoin/LICENSE
@@ -0,0 +1,28 @@
+Copyright (C) 2014-2015 Docker Inc & Go Authors. All rights reserved.
+Copyright (C) 2017 SUSE LLC. All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+ * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+ * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+ * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
diff --git a/vendor/github.com/cyphar/filepath-securejoin/README.md b/vendor/github.com/cyphar/filepath-securejoin/README.md
new file mode 100644
index 000000000..49b2baa9f
--- /dev/null
+++ b/vendor/github.com/cyphar/filepath-securejoin/README.md
@@ -0,0 +1,65 @@
+## `filepath-securejoin` ##
+
+[![Build Status](https://travis-ci.org/cyphar/filepath-securejoin.svg?branch=master)](https://travis-ci.org/cyphar/filepath-securejoin)
+
+An implementation of `SecureJoin`, a [candidate for inclusion in the Go
+standard library][go#20126]. The purpose of this function is to be a "secure"
+alternative to `filepath.Join`, and in particular it provides certain
+guarantees that are not provided by `filepath.Join`.
+
+This is the function prototype:
+
+```go
+func SecureJoin(root, unsafePath string) (string, error)
+```
+
+This library **guarantees** the following:
+
+* If no error is set, the resulting string **must** be a child path of
+ `SecureJoin` and will not contain any symlink path components (they will all
+ be expanded).
+
+* When expanding symlinks, all symlink path components **must** be resolved
+ relative to the provided root. In particular, this can be considered a
+ userspace implementation of how `chroot(2)` operates on file paths. Note that
+ these symlinks will **not** be expanded lexically (`filepath.Clean` is not
+ called on the input before processing).
+
+* Non-existant path components are unaffected by `SecureJoin` (similar to
+ `filepath.EvalSymlinks`'s semantics).
+
+* The returned path will always be `filepath.Clean`ed and thus not contain any
+ `..` components.
+
+A (trivial) implementation of this function on GNU/Linux systems could be done
+with the following (note that this requires root privileges and is far more
+opaque than the implementation in this library, and also requires that
+`readlink` is inside the `root` path):
+
+```go
+package securejoin
+
+import (
+ "os/exec"
+ "path/filepath"
+)
+
+func SecureJoin(root, unsafePath string) (string, error) {
+ unsafePath = string(filepath.Separator) + unsafePath
+ cmd := exec.Command("chroot", root,
+ "readlink", "--canonicalize-missing", "--no-newline", unsafePath)
+ output, err := cmd.CombinedOutput()
+ if err != nil {
+ return "", err
+ }
+ expanded := string(output)
+ return filepath.Join(root, expanded), nil
+}
+```
+
+[go#20126]: https://github.com/golang/go/issues/20126
+
+### License ###
+
+The license of this project is the same as Go, which is a BSD 3-clause license
+available in the `LICENSE` file.
diff --git a/vendor/github.com/cyphar/filepath-securejoin/join.go b/vendor/github.com/cyphar/filepath-securejoin/join.go
new file mode 100644
index 000000000..c4ca3d713
--- /dev/null
+++ b/vendor/github.com/cyphar/filepath-securejoin/join.go
@@ -0,0 +1,134 @@
+// Copyright (C) 2014-2015 Docker Inc & Go Authors. All rights reserved.
+// Copyright (C) 2017 SUSE LLC. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package securejoin is an implementation of the hopefully-soon-to-be-included
+// SecureJoin helper that is meant to be part of the "path/filepath" package.
+// The purpose of this project is to provide a PoC implementation to make the
+// SecureJoin proposal (https://github.com/golang/go/issues/20126) more
+// tangible.
+package securejoin
+
+import (
+ "bytes"
+ "os"
+ "path/filepath"
+ "strings"
+ "syscall"
+
+ "github.com/pkg/errors"
+)
+
+// ErrSymlinkLoop is returned by SecureJoinVFS when too many symlinks have been
+// evaluated in attempting to securely join the two given paths.
+var ErrSymlinkLoop = errors.Wrap(syscall.ELOOP, "secure join")
+
+// IsNotExist tells you if err is an error that implies that either the path
+// accessed does not exist (or path components don't exist). This is
+// effectively a more broad version of os.IsNotExist.
+func IsNotExist(err error) bool {
+ // If it's a bone-fide ENOENT just bail.
+ if os.IsNotExist(errors.Cause(err)) {
+ return true
+ }
+
+ // Check that it's not actually an ENOTDIR, which in some cases is a more
+ // convoluted case of ENOENT (usually involving weird paths).
+ var errno error
+ switch err := errors.Cause(err).(type) {
+ case *os.PathError:
+ errno = err.Err
+ case *os.LinkError:
+ errno = err.Err
+ case *os.SyscallError:
+ errno = err.Err
+ }
+ return errno == syscall.ENOTDIR || errno == syscall.ENOENT
+}
+
+// SecureJoinVFS joins the two given path components (similar to Join) except
+// that the returned path is guaranteed to be scoped inside the provided root
+// path (when evaluated). Any symbolic links in the path are evaluated with the
+// given root treated as the root of the filesystem, similar to a chroot. The
+// filesystem state is evaluated through the given VFS interface (if nil, the
+// standard os.* family of functions are used).
+//
+// Note that the guarantees provided by this function only apply if the path
+// components in the returned string are not modified (in other words are not
+// replaced with symlinks on the filesystem) after this function has returned.
+// Such a symlink race is necessarily out-of-scope of SecureJoin.
+func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
+ // Use the os.* VFS implementation if none was specified.
+ if vfs == nil {
+ vfs = osVFS{}
+ }
+
+ var path bytes.Buffer
+ n := 0
+ for unsafePath != "" {
+ if n > 255 {
+ return "", ErrSymlinkLoop
+ }
+
+ // Next path component, p.
+ i := strings.IndexRune(unsafePath, filepath.Separator)
+ var p string
+ if i == -1 {
+ p, unsafePath = unsafePath, ""
+ } else {
+ p, unsafePath = unsafePath[:i], unsafePath[i+1:]
+ }
+
+ // Create a cleaned path, using the lexical semantics of /../a, to
+ // create a "scoped" path component which can safely be joined to fullP
+ // for evaluation. At this point, path.String() doesn't contain any
+ // symlink components.
+ cleanP := filepath.Clean(string(filepath.Separator) + path.String() + p)
+ if cleanP == string(filepath.Separator) {
+ path.Reset()
+ continue
+ }
+ fullP := filepath.Clean(root + cleanP)
+
+ // Figure out whether the path is a symlink.
+ fi, err := vfs.Lstat(fullP)
+ if err != nil && !IsNotExist(err) {
+ return "", err
+ }
+ // Treat non-existent path components the same as non-symlinks (we
+ // can't do any better here).
+ if IsNotExist(err) || fi.Mode()&os.ModeSymlink == 0 {
+ path.WriteString(p)
+ path.WriteRune(filepath.Separator)
+ continue
+ }
+
+ // Only increment when we actually dereference a link.
+ n++
+
+ // It's a symlink, expand it by prepending it to the yet-unparsed path.
+ dest, err := vfs.Readlink(fullP)
+ if err != nil {
+ return "", err
+ }
+ // Absolute symlinks reset any work we've already done.
+ if filepath.IsAbs(dest) {
+ path.Reset()
+ }
+ unsafePath = dest + string(filepath.Separator) + unsafePath
+ }
+
+ // We have to clean path.String() here because it may contain '..'
+ // components that are entirely lexical, but would be misleading otherwise.
+ // And finally do a final clean to ensure that root is also lexically
+ // clean.
+ fullP := filepath.Clean(string(filepath.Separator) + path.String())
+ return filepath.Clean(root + fullP), nil
+}
+
+// SecureJoin is a wrapper around SecureJoinVFS that just uses the os.* library
+// of functions as the VFS. If in doubt, use this function over SecureJoinVFS.
+func SecureJoin(root, unsafePath string) (string, error) {
+ return SecureJoinVFS(root, unsafePath, nil)
+}
diff --git a/vendor/github.com/cyphar/filepath-securejoin/vendor.conf b/vendor/github.com/cyphar/filepath-securejoin/vendor.conf
new file mode 100644
index 000000000..66bb574b9
--- /dev/null
+++ b/vendor/github.com/cyphar/filepath-securejoin/vendor.conf
@@ -0,0 +1 @@
+github.com/pkg/errors v0.8.0
diff --git a/vendor/github.com/cyphar/filepath-securejoin/vfs.go b/vendor/github.com/cyphar/filepath-securejoin/vfs.go
new file mode 100644
index 000000000..a82a5eae1
--- /dev/null
+++ b/vendor/github.com/cyphar/filepath-securejoin/vfs.go
@@ -0,0 +1,41 @@
+// Copyright (C) 2017 SUSE LLC. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package securejoin
+
+import "os"
+
+// In future this should be moved into a separate package, because now there
+// are several projects (umoci and go-mtree) that are using this sort of
+// interface.
+
+// VFS is the minimal interface necessary to use SecureJoinVFS. A nil VFS is
+// equivalent to using the standard os.* family of functions. This is mainly
+// used for the purposes of mock testing, but also can be used to otherwise use
+// SecureJoin with VFS-like system.
+type VFS interface {
+ // Lstat returns a FileInfo describing the named file. If the file is a
+ // symbolic link, the returned FileInfo describes the symbolic link. Lstat
+ // makes no attempt to follow the link. These semantics are identical to
+ // os.Lstat.
+ Lstat(name string) (os.FileInfo, error)
+
+ // Readlink returns the destination of the named symbolic link. These
+ // semantics are identical to os.Readlink.
+ Readlink(name string) (string, error)
+}
+
+// osVFS is the "nil" VFS, in that it just passes everything through to the os
+// module.
+type osVFS struct{}
+
+// Lstat returns a FileInfo describing the named file. If the file is a
+// symbolic link, the returned FileInfo describes the symbolic link. Lstat
+// makes no attempt to follow the link. These semantics are identical to
+// os.Lstat.
+func (o osVFS) Lstat(name string) (os.FileInfo, error) { return os.Lstat(name) }
+
+// Readlink returns the destination of the named symbolic link. These
+// semantics are identical to os.Readlink.
+func (o osVFS) Readlink(name string) (string, error) { return os.Readlink(name) }