Moved the chaps database directory under /home/root.
This CL also includes code to migrate existing chaps data to the new
location.
BUG=chromium:212630
TEST=unit, manual
Change-Id: I026b21fc10d1df043e1478db9d7d66fc95116011
Reviewed-on: https://gerrit.chromium.org/gerrit/59961
Reviewed-by: Darren Krahn <dkrahn@chromium.org>
Tested-by: Darren Krahn <dkrahn@chromium.org>
Commit-Queue: Darren Krahn <dkrahn@chromium.org>
diff --git a/homedirs.cc b/homedirs.cc
index a794b99..8281a2d 100644
--- a/homedirs.cc
+++ b/homedirs.cc
@@ -555,11 +555,16 @@
}
namespace {
+ const char *kChapsDaemonName = "chaps";
const char *kChapsDirName = ".chaps";
const char *kChapsSaltName = "auth_data_salt";
}
FilePath HomeDirs::GetChapsTokenDir(const std::string& user) const {
+ return chromeos::cryptohome::home::GetDaemonPath(user, kChapsDaemonName);
+}
+
+FilePath HomeDirs::GetLegacyChapsTokenDir(const std::string& user) const {
return chromeos::cryptohome::home::GetUserPath(user).Append(kChapsDirName);
}
diff --git a/homedirs.h b/homedirs.h
index b4fda82..326567b 100644
--- a/homedirs.h
+++ b/homedirs.h
@@ -109,6 +109,9 @@
// Returns the path to the user's chaps token directory.
virtual FilePath GetChapsTokenDir(const std::string& username) const;
+ // Returns the path to the user's legacy chaps token directory.
+ virtual FilePath GetLegacyChapsTokenDir(const std::string& username) const;
+
// Returns the path to the user's token salt.
virtual FilePath GetChapsTokenSaltPath(const std::string& username) const;
diff --git a/mock_platform.cc b/mock_platform.cc
index cb2993a..defd540 100644
--- a/mock_platform.cc
+++ b/mock_platform.cc
@@ -6,7 +6,8 @@
namespace cryptohome {
-MockPlatform::MockPlatform() : mock_enumerator_(new MockFileEnumerator()) {
+MockPlatform::MockPlatform()
+ : mock_enumerator_(new NiceMock<MockFileEnumerator>()) {
ON_CALL(*this, GetOwnership(_, _, _))
.WillByDefault(Invoke(this, &MockPlatform::MockGetOwnership));
ON_CALL(*this, SetOwnership(_, _, _))
diff --git a/mock_platform.h b/mock_platform.h
index 48c8618..56c580a 100644
--- a/mock_platform.h
+++ b/mock_platform.h
@@ -14,6 +14,7 @@
namespace cryptohome {
using ::testing::_;
+using ::testing::NiceMock;
using ::testing::Invoke;
using ::testing::Return;
@@ -134,7 +135,7 @@
MOCK_METHOD0(FirmwareWriteProtected, bool(void));
MOCK_METHOD1(SyncFile, bool(const std::string&));
- MockFileEnumerator* get_mock_enumerator() { return mock_enumerator_.get(); }
+ MockFileEnumerator* mock_enumerator() { return mock_enumerator_.get(); }
private:
bool MockGetOwnership(const std::string& path, uid_t* user_id,
@@ -164,7 +165,7 @@
bool recursive,
int file_type) {
MockFileEnumerator* e = mock_enumerator_.release();
- mock_enumerator_.reset(new MockFileEnumerator());
+ mock_enumerator_.reset(new NiceMock<MockFileEnumerator>());
mock_enumerator_->entries_.assign(e->entries_.begin(), e->entries_.end());
return e;
}
diff --git a/mount.cc b/mount.cc
index b921f8f..75581d4 100644
--- a/mount.cc
+++ b/mount.cc
@@ -1245,28 +1245,44 @@
}
bool Mount::CheckChapsDirectory(const std::string& dir,
+ const std::string& legacy_dir,
bool* permissions_status) {
- // If the Chaps database directory does not exist, create it.
- DCHECK(permissions_status);
- *permissions_status = true;
// 0750: u + rwx, g + rx
const mode_t chaps_permissions = S_IRWXU | S_IRGRP | S_IXGRP;
const mode_t permissions_mask = S_IRWXU | S_IRWXG | S_IRWXO;
+
+ DCHECK(permissions_status);
+ *permissions_status = true;
+ // If the Chaps database directory does not exist, create it.
if (!platform_->DirectoryExists(dir)) {
- if (!platform_->CreateDirectory(dir)) {
- LOG(ERROR) << "Failed to create " << dir;
- *permissions_status = false;
- return false;
- }
- if (!platform_->SetOwnership(dir, chaps_user_, default_access_group_)) {
- LOG(ERROR) << "Couldn't set file ownership for " << dir;
- *permissions_status = false;
- return false;
- }
- if (!platform_->SetPermissions(dir, chaps_permissions)) {
- LOG(ERROR) << "Couldn't set permissions for " << dir;
- *permissions_status = false;
- return false;
+ if (platform_->DirectoryExists(legacy_dir)) {
+ LOG(INFO) << "Moving chaps directory from " << legacy_dir << " to "
+ << dir;
+ if (!platform_->CopyWithPermissions(legacy_dir, dir)) {
+ *permissions_status = false;
+ return false;
+ }
+ if (!platform_->DeleteFile(legacy_dir, true)) {
+ PLOG(WARNING) << "Failed to clean up " << legacy_dir;
+ *permissions_status = false;
+ return false;
+ }
+ } else {
+ if (!platform_->CreateDirectory(dir)) {
+ LOG(ERROR) << "Failed to create " << dir;
+ *permissions_status = false;
+ return false;
+ }
+ if (!platform_->SetOwnership(dir, chaps_user_, default_access_group_)) {
+ LOG(ERROR) << "Couldn't set file ownership for " << dir;
+ *permissions_status = false;
+ return false;
+ }
+ if (!platform_->SetPermissions(dir, chaps_permissions)) {
+ LOG(ERROR) << "Couldn't set permissions for " << dir;
+ *permissions_status = false;
+ return false;
+ }
}
return true;
}
@@ -1310,8 +1326,11 @@
bool permissions_status = false;
std::string username = current_user_->username();
FilePath token_dir = homedirs_->GetChapsTokenDir(username);
+ FilePath legacy_token_dir = homedirs_->GetLegacyChapsTokenDir(username);
- if (!CheckChapsDirectory(token_dir.value(), &permissions_status))
+ if (!CheckChapsDirectory(token_dir.value(),
+ legacy_token_dir.value(),
+ &permissions_status))
return false;
// We may create a salt file and, if so, we want to restrict access to it.
ScopedUmask scoped_umask(platform_, kDefaultUmask);
diff --git a/mount.h b/mount.h
index bd48670..779b010 100644
--- a/mount.h
+++ b/mount.h
@@ -507,12 +507,16 @@
// does not exist, it is created and initialzed with the correct
// values. If the directory or its attributes cannot be checked,
// set or created, a fatal error has occured and the function
- // returns false.
+ // returns false. If the directory does not exist and a legacy directory
+ // exists, the legacy directory will be moved to the new location.
//
// Parameters
// dir - directory to check
+ // legacy_dir - legacy directory location
// permissions_check - set to false if permissions, uid or gid is incorrect
- bool CheckChapsDirectory(const std::string& dir, bool* permissions_check);
+ bool CheckChapsDirectory(const std::string& dir,
+ const std::string& legacy_dir,
+ bool* permissions_check);
// Ensures that the device policy is loaded.
//
@@ -777,6 +781,7 @@
FRIEND_TEST(MountTest,
CheckChapsDirectoryCalledWithExistingDirWithFatalError);
FRIEND_TEST(MountTest, CheckChapsDirectoryCalledWithExistingDir);
+ FRIEND_TEST(MountTest, CheckChapsDirectoryMigration);
DISALLOW_COPY_AND_ASSIGN(Mount);
};
diff --git a/mount_unittest.cc b/mount_unittest.cc
index a45938d..a7562f5 100644
--- a/mount_unittest.cc
+++ b/mount_unittest.cc
@@ -300,7 +300,8 @@
SetArgumentPointee<2>(shared_gid_),
Return(true)));
bool permissions_status = false;
- EXPECT_TRUE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_TRUE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_TRUE(permissions_status);
}
@@ -327,7 +328,8 @@
EXPECT_CALL(platform, GetPermissions("/fake", _))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(bad_perms), Return(true)));
bool permissions_status = false;
- EXPECT_TRUE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_TRUE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
}
@@ -362,7 +364,8 @@
Return(true)));
bool permissions_status = false;
- EXPECT_TRUE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_TRUE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
}
@@ -393,7 +396,8 @@
SetArgumentPointee<2>(bad_gid),
Return(true)));
bool permissions_status = false;
- EXPECT_TRUE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_TRUE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
}
@@ -417,6 +421,8 @@
EXPECT_TRUE(mount->Init());
EXPECT_CALL(platform, DirectoryExists("/fake"))
.WillRepeatedly(Return(false));
+ EXPECT_CALL(platform, DirectoryExists("/fake_legacy"))
+ .WillRepeatedly(Return(false));
EXPECT_CALL(platform, CreateDirectory("/fake"))
.WillOnce(Return(false))
.WillRepeatedly(Return(true));
@@ -428,11 +434,14 @@
EXPECT_CALL(platform, GetPermissions("/fake", _))
.WillRepeatedly(Return(false));
bool permissions_status = false;
- EXPECT_FALSE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_FALSE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
- EXPECT_FALSE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_FALSE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
- EXPECT_FALSE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_FALSE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
}
@@ -462,12 +471,74 @@
EXPECT_CALL(platform, GetOwnership("/fake", _, _))
.WillRepeatedly(Return(false));
bool permissions_status = false;
- EXPECT_FALSE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_FALSE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
- EXPECT_FALSE(mount->CheckChapsDirectory("/fake", &permissions_status));
+ EXPECT_FALSE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
EXPECT_FALSE(permissions_status);
}
+TEST_F(MountTest, CheckChapsDirectoryMigration) {
+ scoped_refptr<Mount> mount = new Mount();
+ NiceMock<MockPlatform> platform;
+ NiceMock<MockTpm> tpm;
+ mount->crypto()->set_tpm(&tpm);
+ mount->crypto()->set_platform(&platform);
+ mount->set_use_tpm(false);
+ mount->set_platform(&platform);
+
+ // Configure stub methods.
+ EXPECT_CALL(platform, Copy(_, _))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(platform, DeleteFile(_, _))
+ .WillRepeatedly(Return(true));
+
+ // Stubs which will trigger the migration code path.
+ EXPECT_CALL(platform, DirectoryExists("/fake"))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(platform, DirectoryExists("/fake_legacy"))
+ .WillRepeatedly(Return(true));
+
+ // Configure stat for the base directory.
+ struct stat base_stat = {0};
+ base_stat.st_mode = 040123;
+ base_stat.st_uid = 1;
+ base_stat.st_gid = 2;
+ EXPECT_CALL(platform, Stat(_, _))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(base_stat), Return(true)));
+
+ // Configure a fake enumerator.
+ MockFileEnumerator* enumerator = platform.mock_enumerator();
+ enumerator->entries_.push_back("/fake_legacy/test_file1");
+ enumerator->entries_.push_back("test_file2");
+ FileEnumerator::FindInfo find_info1 = {{0}, "/fake_legacy/test_file1"};
+ find_info1.stat.st_mode = 0555;
+ find_info1.stat.st_uid = 3;
+ find_info1.stat.st_gid = 4;
+ FileEnumerator::FindInfo find_info2 = {{0}, "test_file2"};
+ find_info2.stat.st_mode = 0777;
+ find_info2.stat.st_uid = 5;
+ find_info2.stat.st_gid = 6;
+ EXPECT_CALL(*enumerator, GetFindInfo(_))
+ .WillOnce(SetArgumentPointee<0>(find_info1))
+ .WillOnce(SetArgumentPointee<0>(find_info2));
+
+ // These expectations will ensure the ownership and permissions are being
+ // correctly applied after the directory has been moved.
+ EXPECT_CALL(platform, SetOwnership("/fake/test_file1", 3, 4)).Times(1);
+ EXPECT_CALL(platform, SetPermissions("/fake/test_file1", 0555)).Times(1);
+ EXPECT_CALL(platform, SetOwnership("/fake/test_file2", 5, 6)).Times(1);
+ EXPECT_CALL(platform, SetPermissions("/fake/test_file2", 0777)).Times(1);
+ EXPECT_CALL(platform, SetOwnership("/fake", 1, 2)).Times(1);
+ EXPECT_CALL(platform, SetPermissions("/fake", 0123)).Times(1);
+
+ bool permissions_status = false;
+ EXPECT_TRUE(mount->CheckChapsDirectory("/fake", "/fake_legacy",
+ &permissions_status));
+ EXPECT_TRUE(permissions_status);
+}
+
TEST_F(MountTest, CreateCryptohomeTest) {
InsertTestUsers(&kDefaultUsers[5], 1);
// creates a cryptohome and tests credentials
diff --git a/platform.cc b/platform.cc
index 9b43439..95f9b55 100644
--- a/platform.cc
+++ b/platform.cc
@@ -43,6 +43,27 @@
using base::SplitString;
using std::string;
+namespace {
+
+class ScopedPath {
+ public:
+ ScopedPath(cryptohome::Platform* platform, const string& dir)
+ : platform_(platform), dir_(dir) {}
+ ~ScopedPath() {
+ if (!dir_.empty() && !platform_->DeleteFile(dir_, true)) {
+ PLOG(WARNING) << "Failed to clean up " << dir_;
+ }
+ }
+ void release() {
+ dir_.clear();
+ }
+ private:
+ cryptohome::Platform* platform_;
+ string dir_;
+};
+
+} // namespace
+
namespace cryptohome {
const int kDefaultMountOptions = MS_NOEXEC | MS_NOSUID | MS_NODEV;
@@ -544,6 +565,71 @@
return file_util::CopyDirectory(from_path, to_path, true);
}
+bool Platform::CopyWithPermissions(const std::string& from_path,
+ const std::string& to_path) {
+ // Save stat for every file.
+ std::vector<FileEnumerator::FindInfo> entry_list;
+ FileEnumerator::FindInfo base_entry_info;
+ base_entry_info.filename = from_path;
+ if (!Stat(from_path, &base_entry_info.stat)) {
+ PLOG(ERROR) << "Failed to stat " << from_path;
+ return false;
+ }
+ entry_list.push_back(base_entry_info);
+ if (FileEnumerator::IsDirectory(base_entry_info)) {
+ int file_types = FileEnumerator::FILES | FileEnumerator::DIRECTORIES;
+ scoped_ptr<FileEnumerator> file_enumerator(
+ GetFileEnumerator(from_path, true, file_types));
+ string entry_path;
+ while (!(entry_path = file_enumerator->Next()).empty()) {
+ FileEnumerator::FindInfo entry_info;
+ file_enumerator->GetFindInfo(&entry_info);
+ // Keep the full path.
+ entry_info.filename = entry_path;
+ entry_list.push_back(entry_info);
+ }
+ }
+
+ if (!Copy(from_path, to_path)) {
+ PLOG(ERROR) << "Failed to copy " << from_path;
+ return false;
+ }
+
+ // If something goes wrong we want to blow away the half-baked path.
+ ScopedPath scoped_new_path(this, to_path);
+
+ // Unfortunately, ownership and permissions are not always retained.
+ // Apply the old ownership / permissions on a per-file basis.
+ const FilePath new_base(to_path);
+ const FilePath old_base(from_path);
+ for (size_t i = 0; i < entry_list.size(); ++i) {
+ FilePath old_path(entry_list[i].filename);
+ FilePath new_path(new_base);
+ if (old_path != old_base) {
+ if (old_path.IsAbsolute()) {
+ DCHECK(old_base.AppendRelativePath(old_path, &new_path));
+ } else {
+ new_path = new_base.Append(old_path);
+ }
+ }
+ if (!SetOwnership(new_path.value(),
+ entry_list[i].stat.st_uid,
+ entry_list[i].stat.st_gid)) {
+ PLOG(ERROR) << "Failed to set ownership for " << new_path.value();
+ return false;
+ }
+ const mode_t permissions_mask = 07777;
+ if (!SetPermissions(new_path.value(),
+ entry_list[i].stat.st_mode & permissions_mask)) {
+ PLOG(ERROR) << "Failed to set permissions for " << new_path.value();
+ return false;
+ }
+ }
+ // The copy is done, keep the new path.
+ scoped_new_path.release();
+ return true;
+}
+
bool Platform::StatVFS(const std::string& path, struct statvfs* vfs) {
return statvfs(path.c_str(), vfs) == 0;
}
diff --git a/platform.h b/platform.h
index 3e317bd..36f987e 100644
--- a/platform.h
+++ b/platform.h
@@ -295,6 +295,10 @@
// Copies from to to.
virtual bool Copy(const std::string& from, const std::string& to);
+ // Copies and retains permissions and ownership.
+ virtual bool CopyWithPermissions(const std::string& from,
+ const std::string& to);
+
// Moves a given path on the filesystem
//
// Parameters
