diff options
author | skerner@chromium.org <skerner@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-30 07:19:11 +0000 |
---|---|---|
committer | skerner@chromium.org <skerner@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-30 07:19:11 +0000 |
commit | 046062e82159edaeedbbf244731a4d306902d016 (patch) | |
tree | a08e8309b22aa1a45955735dda35ee7c83d2d9de /base | |
parent | 2082cb3dbf0e611b109909fca14be2e217087970 (diff) | |
download | chromium_src-046062e82159edaeedbbf244731a4d306902d016.zip chromium_src-046062e82159edaeedbbf244731a4d306902d016.tar.gz chromium_src-046062e82159edaeedbbf244731a4d306902d016.tar.bz2 |
Revert 51231 - Loosen permission on extension temp dir when a flag is used.
Issue 35198 can not be reproduced locally. To enable users to do experiments, three command line flags are added to chrome:
--issue35198-crxdir-browser: Have the browser process create the directory in which the extension will be unzipped.
--issue35198-logging: Enable log messages from directory creation in the utility process to be moved to the browser process.
--issue35198-permission: Use the most permissive file permissions possible on the extension unpack directory.
BUG=35198
TEST=manual
Review URL: http://codereview.chromium.org/2802018
TBR=skerner@chromium.org
Review URL: http://codereview.chromium.org/2861039
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51238 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base')
-rw-r--r-- | base/file_util.h | 3 | ||||
-rw-r--r-- | base/file_util_posix.cc | 10 | ||||
-rw-r--r-- | base/file_util_unittest.cc | 3 | ||||
-rw-r--r-- | base/file_util_win.cc | 73 | ||||
-rw-r--r-- | base/scoped_temp_dir.cc | 13 | ||||
-rw-r--r-- | base/scoped_temp_dir.h | 3 | ||||
-rw-r--r-- | base/scoped_temp_dir_unittest.cc | 2 |
7 files changed, 12 insertions, 95 deletions
diff --git a/base/file_util.h b/base/file_util.h index f33e78b..48f431f 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -267,8 +267,7 @@ bool CreateTemporaryFileInDir(const FilePath& dir, // new directory does not have the same name as an existing directory. bool CreateTemporaryDirInDir(const FilePath& base_dir, const FilePath::StringType& prefix, - FilePath* new_dir, - bool lossen_permissions); + FilePath* new_dir); // Create a new directory under TempPath. If prefix is provided, the new // directory name is in the format of prefixyyyy. diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc index e868a41..9d738da 100644 --- a/base/file_util_posix.cc +++ b/base/file_util_posix.cc @@ -428,15 +428,7 @@ static bool CreateTemporaryDirInDirImpl(const FilePath& base_dir, bool CreateTemporaryDirInDir(const FilePath& base_dir, const FilePath::StringType& prefix, - FilePath* new_dir, - bool loosen_permissions) { - // To understand crbug/35198, the ability to call this - // this function on windows while giving loose permissions - // to the resulting directory has been temporarily added. - // It should not be posible to call this function with - // loosen_permissions == true on non-windows plarforms. - CHECK(!loosen_permissions); - + FilePath* new_dir) { FilePath::StringType mkdtemp_template = prefix; mkdtemp_template.append(FILE_PATH_LITERAL("XXXXXX")); return CreateTemporaryDirInDirImpl(base_dir, mkdtemp_template, new_dir); diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc index b0a1a8e..39e5398 100644 --- a/base/file_util_unittest.cc +++ b/base/file_util_unittest.cc @@ -1569,8 +1569,7 @@ TEST_F(FileUtilTest, CreateNewTemporaryDirInDirTest) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( test_dir_, FILE_PATH_LITERAL("CreateNewTemporaryDirInDirTest"), - &new_dir, - false)); + &new_dir)); EXPECT_TRUE(file_util::PathExists(new_dir)); EXPECT_TRUE(test_dir_.IsParent(new_dir)); EXPECT_TRUE(file_util::Delete(new_dir, false)); diff --git a/base/file_util_win.cc b/base/file_util_win.cc index 74db90cd..8a15370 100644 --- a/base/file_util_win.cc +++ b/base/file_util_win.cc @@ -63,56 +63,6 @@ bool DevicePathToDriveLetterPath(const FilePath& device_path, return true; } -// Build a security descriptor with the weakest possible file permissions. -bool InitLooseSecurityDescriptor(SECURITY_ATTRIBUTES *sa, - SECURITY_DESCRIPTOR *sd) { - DWORD last_error; - - if (!InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION)) { - last_error = GetLastError(); - LOG(ERROR) << "InitializeSecurityDescriptor failed: GetLastError() = " - << last_error; - return false; - } - - if (!SetSecurityDescriptorDacl(sd, - TRUE, // bDaclPresent: Add one to |sd|. - NULL, // pDacl: NULL means allow all access. - FALSE // bDaclDefaulted: Not defaulted. - )) { - last_error = GetLastError(); - LOG(ERROR) << "SetSecurityDescriptorDacl() failed: GetLastError() = " - << last_error; - return false; - } - - if (!SetSecurityDescriptorGroup(sd, - NULL, // pGroup: No no primary group. - FALSE // bGroupDefaulted: Not defaulted. - )) { - last_error = GetLastError(); - LOG(ERROR) << "SetSecurityDescriptorGroup() failed: GetLastError() = " - << last_error; - return false; - } - - if (!SetSecurityDescriptorSacl(sd, - FALSE, // bSaclPresent: No SACL. - NULL, - FALSE - )) { - last_error = GetLastError(); - LOG(ERROR) << "SetSecurityDescriptorSacl() failed: GetLastError() = " - << last_error; - return false; - } - - sa->nLength = sizeof(SECURITY_ATTRIBUTES); - sa->lpSecurityDescriptor = sd; - sa->bInheritHandle = TRUE; - return true; -} - } // namespace std::wstring GetDirectoryFromPath(const std::wstring& path) { @@ -600,19 +550,7 @@ bool CreateTemporaryFileInDir(const FilePath& dir, bool CreateTemporaryDirInDir(const FilePath& base_dir, const FilePath::StringType& prefix, - FilePath* new_dir, - bool loosen_permissions) { - SECURITY_ATTRIBUTES sa; - SECURITY_DESCRIPTOR sd; - - LPSECURITY_ATTRIBUTES directory_security_attributes = NULL; - if (loosen_permissions) { - if (InitLooseSecurityDescriptor(&sa, &sd)) - directory_security_attributes = &sa; - else - LOG(ERROR) << "Failed to init security attributes, fall back to NULL."; - } - + FilePath* new_dir) { FilePath path_to_create; srand(static_cast<uint32>(time(NULL))); @@ -627,8 +565,7 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir, new_dir_name.append(IntToWString(rand() % kint16max)); path_to_create = path_to_create.Append(new_dir_name); - if (::CreateDirectory(path_to_create.value().c_str(), - directory_security_attributes)) + if (::CreateDirectory(path_to_create.value().c_str(), NULL)) break; count++; } @@ -638,7 +575,6 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir, } *new_dir = path_to_create; - return true; } @@ -648,10 +584,7 @@ bool CreateNewTempDirectory(const FilePath::StringType& prefix, if (!GetTempDir(&system_temp_dir)) return false; - return CreateTemporaryDirInDir(system_temp_dir, - prefix, - new_temp_path, - false); + return CreateTemporaryDirInDir(system_temp_dir, prefix, new_temp_path); } bool CreateDirectory(const FilePath& full_path) { diff --git a/base/scoped_temp_dir.cc b/base/scoped_temp_dir.cc index c3fb00a..c8ed9c6 100644 --- a/base/scoped_temp_dir.cc +++ b/base/scoped_temp_dir.cc @@ -4,7 +4,6 @@ #include "base/scoped_temp_dir.h" -#include "base/command_line.h" #include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" @@ -27,20 +26,16 @@ bool ScopedTempDir::CreateUniqueTempDir() { return true; } -bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path, - bool loose_permissions) { +bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path) { // If |path| does not exist, create it. - if (!file_util::CreateDirectory(base_path)) { - LOG(ERROR) << "Failed to create base directory " << base_path.value(); + if (!file_util::CreateDirectory(base_path)) return false; - } - // Create a new, uniquely named directory under |base_path|. + // Create a new, uniquly named directory under |base_path|. if (!file_util::CreateTemporaryDirInDir( base_path, FILE_PATH_LITERAL("scoped_dir_"), - &path_, - loose_permissions)) { + &path_)) { return false; } return true; diff --git a/base/scoped_temp_dir.h b/base/scoped_temp_dir.h index a0708dc3..a5dca1e 100644 --- a/base/scoped_temp_dir.h +++ b/base/scoped_temp_dir.h @@ -26,8 +26,7 @@ class ScopedTempDir { bool CreateUniqueTempDir(); // Creates a unique directory under a given path, and takes ownership of it. - bool CreateUniqueTempDirUnderPath(const FilePath& path, - bool loose_permissions); + bool CreateUniqueTempDirUnderPath(const FilePath& path); // Takes ownership of directory at |path|, creating it if necessary. // Don't call multiple times unless Take() has been called first. diff --git a/base/scoped_temp_dir_unittest.cc b/base/scoped_temp_dir_unittest.cc index e180119..4be0d07 100644 --- a/base/scoped_temp_dir_unittest.cc +++ b/base/scoped_temp_dir_unittest.cc @@ -65,7 +65,7 @@ TEST(ScopedTempDir, UniqueTempDirUnderPath) { FilePath test_path; { ScopedTempDir dir; - EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path, false)); + EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path)); test_path = dir.path(); EXPECT_TRUE(file_util::DirectoryExists(test_path)); EXPECT_TRUE(base_path.IsParent(test_path)); |