diff options
author | jln@chromium.org <jln@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-26 04:14:23 +0000 |
---|---|---|
committer | jln@chromium.org <jln@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-26 04:14:23 +0000 |
commit | 24f70c9cf5f021b74f26b89ca547e778356d0aee (patch) | |
tree | 06b4fee522caeba5e535fc23b45f955c173e27da /sandbox | |
parent | 08d0e483998bb878934a8108956614cd4195de51 (diff) | |
download | chromium_src-24f70c9cf5f021b74f26b89ca547e778356d0aee.zip chromium_src-24f70c9cf5f021b74f26b89ca547e778356d0aee.tar.gz chromium_src-24f70c9cf5f021b74f26b89ca547e778356d0aee.tar.bz2 |
Revert 237242 "Linux sandbox: move CurrentProcessHasOpenDirectories"
> Linux sandbox: move CurrentProcessHasOpenDirectories
>
> Move CurrentProcessHasOpenDirectories() to the Credentials class and rename
> it to HasOpenDirectory().
> Also add some unittests.
>
> BUG=312380
> R=jorgelo@chromium.org
>
> Review URL: https://codereview.chromium.org/85403011
TBR=jln@chromium.org
Review URL: https://codereview.chromium.org/85343005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237251 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sandbox')
-rw-r--r-- | sandbox/linux/services/credentials.cc | 61 | ||||
-rw-r--r-- | sandbox/linux/services/credentials.h | 19 | ||||
-rw-r--r-- | sandbox/linux/services/credentials_unittest.cc | 44 |
3 files changed, 3 insertions, 121 deletions
diff --git a/sandbox/linux/services/credentials.cc b/sandbox/linux/services/credentials.cc index cea757c..0af5a42b 100644 --- a/sandbox/linux/services/credentials.cc +++ b/sandbox/linux/services/credentials.cc @@ -4,13 +4,9 @@ #include "sandbox/linux/services/credentials.h" -#include <dirent.h> #include <errno.h> -#include <fcntl.h> #include <stdio.h> #include <sys/capability.h> -#include <sys/stat.h> -#include <sys/types.h> #include <unistd.h> #include "base/basictypes.h" @@ -53,15 +49,6 @@ struct FILECloser { // TODO(jln): fix base/. typedef scoped_ptr<FILE, FILECloser> ScopedFILE; -struct DIRCloser { - void operator()(DIR* d) const { - DCHECK(d); - PCHECK(0 == closedir(d)); - } -}; - -typedef scoped_ptr<DIR, DIRCloser> ScopedDIR; - COMPILE_ASSERT((base::is_same<uid_t, gid_t>::value), UidAndGidAreSameType); // generic_id_t can be used for either uid_t or gid_t. typedef uid_t generic_id_t; @@ -156,51 +143,6 @@ Credentials::Credentials() { Credentials::~Credentials() { } -bool Credentials::HasOpenDirectory(int proc_fd) { - int proc_self_fd = -1; - if (proc_fd >= 0) { - proc_self_fd = openat(proc_fd, "self/fd", O_DIRECTORY | O_RDONLY); - } else { - proc_self_fd = openat(AT_FDCWD, "/proc/self/fd", O_DIRECTORY | O_RDONLY); - if (proc_self_fd < 0) { - // If not available, guess false. - // TODO(mostynb@opera.com): add a CHECK_EQ(ENOENT, errno); Figure out what - // other situations are here. http://crbug.com/314985 - return false; - } - } - CHECK_GE(proc_self_fd, 0); - - // Ownership of proc_self_fd is transferred here, it must not be closed - // or modified afterwards except via dir. - ScopedDIR dir(fdopendir(proc_self_fd)); - CHECK(dir); - - struct dirent e; - struct dirent* de; - while (!readdir_r(dir.get(), &e, &de) && de) { - if (strcmp(e.d_name, ".") == 0 || strcmp(e.d_name, "..") == 0) { - continue; - } - - int fd_num; - CHECK(base::StringToInt(e.d_name, &fd_num)); - if (fd_num == proc_fd || fd_num == proc_self_fd) { - continue; - } - - struct stat s; - // It's OK to use proc_self_fd here, fstatat won't modify it. - CHECK(fstatat(proc_self_fd, e.d_name, &s, 0) == 0); - if (S_ISDIR(s.st_mode)) { - return true; - } - } - - // No open unmanaged directories found. - return false; -} - bool Credentials::DropAllCapabilities() { ScopedCap cap(cap_init()); CHECK(cap); @@ -257,9 +199,6 @@ bool Credentials::MoveToNewUserNS() { } bool Credentials::DropFileSystemAccess() { - // Chrooting to a safe empty dir will only be safe if no directory file - // descriptor is available to the process. - DCHECK(!HasOpenDirectory(-1)); return ChrootToSafeEmptyDir(); } diff --git a/sandbox/linux/services/credentials.h b/sandbox/linux/services/credentials.h index c23db93..80b2ec1 100644 --- a/sandbox/linux/services/credentials.h +++ b/sandbox/linux/services/credentials.h @@ -26,20 +26,6 @@ class Credentials { Credentials(); ~Credentials(); - // Checks whether the current process has any directory file descriptor open. - // Directory file descriptors are "capabilities" that would let a process use - // system calls such as openat() to bypass restrictions such as - // DropFileSystemAccess(). - // Sometimes it's useful to call HasOpenDirectory() after file system access - // has been dropped. In this case, |proc_fd| should be a file descriptor to - // /proc. The file descriptor in |proc_fd| will be ignored by - // HasOpenDirectory() and remains owned by the caller. It is very important - // for the caller to close it. - // If /proc is available, |proc_fd| can be passed as -1. - // If |proc_fd| is -1 and /proc is not available, this function will return - // false. - bool HasOpenDirectory(int proc_fd); - // Drop all capabilities in the effective, inheritable and permitted sets for // the current process. bool DropAllCapabilities(); @@ -66,8 +52,9 @@ class Credentials { // CAP_SYS_CHROOT can be acquired by using the MoveToNewUserNS() API. // Make sure to call DropAllCapabilities() after this call to prevent // escapes. - // To be secure, it's very important for this API to not be called while the - // process has any directory file descriptor open. + // To be secure, it's very important for this API to not be called with any + // directory file descriptor present. TODO(jln): integrate with + // crbug.com/269806 when available. bool DropFileSystemAccess(); private: diff --git a/sandbox/linux/services/credentials_unittest.cc b/sandbox/linux/services/credentials_unittest.cc index 9160bf7..da61cd5 100644 --- a/sandbox/linux/services/credentials_unittest.cc +++ b/sandbox/linux/services/credentials_unittest.cc @@ -5,20 +5,14 @@ #include "sandbox/linux/services/credentials.h" #include <errno.h> -#include <fcntl.h> #include <stdio.h> -#include <sys/stat.h> -#include <sys/types.h> #include <unistd.h> -#include "base/file_util.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "sandbox/linux/tests/unit_tests.h" #include "testing/gtest/include/gtest/gtest.h" -using file_util::ScopedFD; - namespace sandbox { namespace { @@ -58,44 +52,6 @@ TEST(Credentials, CreateAndDestroy) { scoped_ptr<Credentials> cred2(new Credentials); } -TEST(Credentials, HasOpenDirectory) { - Credentials creds; - // No open directory should exist at startup. - EXPECT_FALSE(creds.HasOpenDirectory(-1)); - { - // Have a "/dev" file descriptor around. - int dev_fd = open("/dev", O_RDONLY | O_DIRECTORY); - ScopedFD dev_fd_closer(&dev_fd); - EXPECT_TRUE(creds.HasOpenDirectory(-1)); - } - EXPECT_FALSE(creds.HasOpenDirectory(-1)); -} - -TEST(Credentials, HasOpenDirectoryWithFD) { - Credentials creds; - - int proc_fd = open("/proc", O_RDONLY | O_DIRECTORY); - ScopedFD proc_fd_closer(&proc_fd); - ASSERT_LE(0, proc_fd); - - // Don't pass |proc_fd|, an open directory (proc_fd) should - // be detected. - EXPECT_TRUE(creds.HasOpenDirectory(-1)); - // Pass |proc_fd| and no open directory should be detected. - EXPECT_FALSE(creds.HasOpenDirectory(proc_fd)); - - { - // Have a "/dev" file descriptor around. - int dev_fd = open("/dev", O_RDONLY | O_DIRECTORY); - ScopedFD dev_fd_closer(&dev_fd); - EXPECT_TRUE(creds.HasOpenDirectory(proc_fd)); - } - - // The "/dev" file descriptor should now be closed, |proc_fd| is the only - // directory file descriptor open. - EXPECT_FALSE(creds.HasOpenDirectory(proc_fd)); -} - SANDBOX_TEST(Credentials, DropAllCaps) { Credentials creds; CHECK(creds.DropAllCapabilities()); |