cros-disks: Remove unused class FUSEHelper
BUG=chromium:950442
BUG=chromium:933018
TEST=platform.CrosDisks*
Change-Id: I9d039ec4ca69723f515851456b4233a454c7cd14
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2577388
Tested-by: Sergei Datsenko <dats@chromium.org>
Auto-Submit: Sergei Datsenko <dats@chromium.org>
Reviewed-by: Austin Tankiang <austinct@chromium.org>
Reviewed-by: François Degros <fdegros@chromium.org>
Commit-Queue: Sergei Datsenko <dats@chromium.org>
diff --git a/cros-disks/BUILD.gn b/cros-disks/BUILD.gn
index b127a5c..35fcd86 100644
--- a/cros-disks/BUILD.gn
+++ b/cros-disks/BUILD.gn
@@ -58,7 +58,6 @@
"file_reader.cc",
"filesystem_label.cc",
"format_manager.cc",
- "fuse_helper.cc",
"fuse_mount_manager.cc",
"fuse_mounter.cc",
"metrics.cc",
@@ -135,7 +134,6 @@
"file_reader_test.cc",
"filesystem_label_test.cc",
"format_manager_test.cc",
- "fuse_helper_test.cc",
"fuse_mount_manager_test.cc",
"fuse_mounter_test.cc",
"metrics_test.cc",
diff --git a/cros-disks/archive_manager.cc b/cros-disks/archive_manager.cc
index abc03e5..0e413ec 100644
--- a/cros-disks/archive_manager.cc
+++ b/cros-disks/archive_manager.cc
@@ -10,8 +10,8 @@
#include <base/strings/string_util.h>
#include <brillo/cryptohome.h>
-#include "cros-disks/fuse_helper.h"
#include "cros-disks/platform.h"
+#include "cros-disks/user.h"
namespace cros_disks {
@@ -67,17 +67,11 @@
MountOptions* const options) const {
DCHECK(options);
- uid_t uid;
- gid_t gid;
- if (!platform()->GetUserAndGroupId(FUSEHelper::kFilesUser, &uid, nullptr) ||
- !platform()->GetGroupId(FUSEHelper::kFilesGroup, &gid))
- return MOUNT_ERROR_INTERNAL;
-
options->SetReadOnlyOption();
options->EnforceOption("umask=0222");
options->EnforceOption(MountOptions::kOptionNoSymFollow);
- options->Initialize({}, true, base::NumberToString(uid),
- base::NumberToString(gid));
+ options->Initialize({}, true, base::NumberToString(kChronosUID),
+ base::NumberToString(kChronosAccessGID));
return MOUNT_ERROR_NONE;
}
diff --git a/cros-disks/archive_manager_test.cc b/cros-disks/archive_manager_test.cc
index a49435e..9f2a00b 100644
--- a/cros-disks/archive_manager_test.cc
+++ b/cros-disks/archive_manager_test.cc
@@ -162,37 +162,10 @@
}
TEST_F(ArchiveManagerTest, GetMountOptions) {
- const uid_t uid = 687123;
- const gid_t gid = 932648;
- EXPECT_CALL(platform_, GetUserAndGroupId("chronos", _, nullptr))
- .WillOnce(DoAll(SetArgPointee<1>(uid), Return(true)));
- EXPECT_CALL(platform_, GetGroupId("chronos-access", _))
- .WillOnce(DoAll(SetArgPointee<1>(gid), Return(true)));
-
MountOptions options;
EXPECT_EQ(manager_.GetMountOptions(&options), MOUNT_ERROR_NONE);
- EXPECT_EQ(
- options.ToString(),
- "ro,uid=687123,gid=932648,nodev,noexec,nosuid,umask=0222,nosymfollow");
-}
-
-TEST_F(ArchiveManagerTest, GetMountOptionsCannotGetGroupId) {
- const uid_t uid = 687123;
- EXPECT_CALL(platform_, GetUserAndGroupId("chronos", _, nullptr))
- .WillOnce(DoAll(SetArgPointee<1>(uid), Return(true)));
- EXPECT_CALL(platform_, GetGroupId("chronos-access", _))
- .WillOnce(Return(false));
-
- MountOptions options;
- EXPECT_EQ(manager_.GetMountOptions(&options), MOUNT_ERROR_INTERNAL);
-}
-
-TEST_F(ArchiveManagerTest, GetMountOptionsCannotGetUserId) {
- EXPECT_CALL(platform_, GetUserAndGroupId("chronos", _, nullptr))
- .WillOnce(Return(false));
-
- MountOptions options;
- EXPECT_EQ(manager_.GetMountOptions(&options), MOUNT_ERROR_INTERNAL);
+ EXPECT_EQ(options.ToString(),
+ "ro,uid=1000,gid=1001,nodev,noexec,nosuid,umask=0222,nosymfollow");
}
} // namespace cros_disks
diff --git a/cros-disks/fuse_helper.cc b/cros-disks/fuse_helper.cc
deleted file mode 100644
index 2b3663d..0000000
--- a/cros-disks/fuse_helper.cc
+++ /dev/null
@@ -1,121 +0,0 @@
-// Copyright 2018 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "cros-disks/fuse_helper.h"
-
-#include <algorithm>
-#include <memory>
-#include <utility>
-
-#include <base/logging.h>
-#include <base/strings/strcat.h>
-#include <base/strings/string_util.h>
-
-#include "cros-disks/fuse_mounter.h"
-#include "cros-disks/mount_options.h"
-#include "cros-disks/platform.h"
-#include "cros-disks/system_mounter.h"
-#include "cros-disks/uri.h"
-
-namespace cros_disks {
-
-const char FUSEHelper::kFilesUser[] = "chronos";
-const char FUSEHelper::kFilesGroup[] = "chronos-access";
-const char FUSEHelper::kOptionAllowOther[] = "allow_other";
-const char FUSEHelper::kOptionDefaultPermissions[] = "default_permissions";
-
-constexpr char kFUSEHelperWorkingDirParam[] = "_fuse_working_dir=";
-
-FUSEHelper::FUSEHelper(const std::string& fuse_type,
- const Platform* platform,
- brillo::ProcessReaper* process_reaper,
- const base::FilePath& mount_program_path,
- const std::string& mount_user)
- : fuse_type_(fuse_type),
- platform_(platform),
- process_reaper_(process_reaper),
- mount_program_path_(mount_program_path),
- mount_user_(mount_user) {}
-
-FUSEHelper::~FUSEHelper() = default;
-
-bool FUSEHelper::CanMount(const Uri& source) const {
- return source.scheme() == type() && !source.path().empty();
-}
-
-std::string FUSEHelper::GetTargetSuffix(const Uri& source) const {
- std::string path = source.path();
- std::replace(path.begin(), path.end(), '/', '$');
- std::replace(path.begin(), path.end(), '.', '_');
- return path;
-}
-
-std::unique_ptr<MountPoint> FUSEHelper::MountWithDir(
- const Mounter& mounter,
- const base::FilePath& working_dir,
- const std::string& source,
- const base::FilePath& target_path,
- std::vector<std::string> params,
- MountErrorType* error) {
- params.push_back(
- base::StrCat({kFUSEHelperWorkingDirParam, working_dir.value()}));
- return mounter.Mount(source, target_path, std::move(params), error);
-}
-
-std::unique_ptr<MountPoint> FUSEHelper::Mount(const std::string& source,
- const base::FilePath& target_path,
- std::vector<std::string> params,
- MountErrorType* error) const {
- base::FilePath working_dir;
- for (auto it = params.begin(); it != params.end(); ++it) {
- if (base::StartsWith(*it, kFUSEHelperWorkingDirParam,
- base::CompareCase::SENSITIVE)) {
- working_dir =
- base::FilePath(it->substr(strlen(kFUSEHelperWorkingDirParam)));
- params.erase(it);
- break;
- }
- }
- CHECK(!working_dir.empty());
- CHECK(Uri::IsUri(source));
- const auto impl =
- CreateMounter(working_dir, Uri::Parse(source), target_path, params);
- if (!impl) {
- *error = MOUNT_ERROR_INVALID_MOUNT_OPTIONS;
- return nullptr;
- }
- return impl->Mount(source, target_path, std::move(params), error);
-}
-
-bool FUSEHelper::CanMount(const std::string& source,
- const std::vector<std::string>& params,
- base::FilePath* suggested_dir_name) const {
- const Uri uri = Uri::Parse(source);
- if (!uri.valid()) {
- return false;
- }
- if (!CanMount(uri)) {
- return false;
- }
- *suggested_dir_name = base::FilePath(GetTargetSuffix(uri));
- return true;
-}
-
-std::unique_ptr<FUSEMounter> FUSEHelper::CreateMounter(
- const base::FilePath& /*working_dir*/,
- const Uri& /*source*/,
- const base::FilePath& /*target_path*/,
- const std::vector<std::string>& options) const {
- MountOptions mount_options;
- mount_options.Initialize(options, false, "", "");
- return std::make_unique<FUSEMounterLegacy>(
- FUSEMounterLegacy::Params{.filesystem_type = type(),
- .mount_options = std::move(mount_options),
- .mount_program = program_path().value(),
- .mount_user = user(),
- .platform = platform(),
- .process_reaper = process_reaper()});
-}
-
-} // namespace cros_disks
diff --git a/cros-disks/fuse_helper.h b/cros-disks/fuse_helper.h
deleted file mode 100644
index 1093566..0000000
--- a/cros-disks/fuse_helper.h
+++ /dev/null
@@ -1,110 +0,0 @@
-// Copyright 2018 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CROS_DISKS_FUSE_HELPER_H_
-#define CROS_DISKS_FUSE_HELPER_H_
-
-#include <memory>
-#include <string>
-#include <vector>
-
-#include <base/files/file_path.h>
-#include <chromeos/dbus/service_constants.h>
-#include <gtest/gtest_prod.h>
-
-#include "cros-disks/fuse_mounter.h"
-
-namespace cros_disks {
-
-class Uri;
-
-// Base class to simplify calling of individual mounters based on
-// specific conventions for a particular userspace FUSE implementation.
-// TODO(dats): Remove this class. This should be in the corresponding
-// instance of FUSEMounter.
-class FUSEHelper : public Mounter {
- public:
- // OS user that will access files provided by this module.
- static const char kFilesUser[];
-
- // OS group that will access files provided by this module.
- static const char kFilesGroup[];
-
- // FUSE kernel-level option allowing access to the mount by UIDs different
- // from the one that FUSE helper is being run. Relevant if FUSE helper is
- // being run in a sandbox under different UID.
- static const char kOptionAllowOther[];
-
- // Enable permission checking by the kernel instead of FUSE helper itself.
- static const char kOptionDefaultPermissions[];
-
- FUSEHelper(const std::string& fuse_type,
- const Platform* platform,
- brillo::ProcessReaper* process_reaper,
- const base::FilePath& mount_program_path,
- const std::string& mount_user);
- FUSEHelper(const FUSEHelper&) = delete;
- FUSEHelper& operator=(const FUSEHelper&) = delete;
-
- virtual ~FUSEHelper();
-
- const std::string& type() const { return fuse_type_; }
- const std::string& user() const { return mount_user_; }
- const Platform* platform() const { return platform_; }
- const base::FilePath& program_path() const { return mount_program_path_; }
-
- // Whether this helper is able to handle this kind of source.
- // Default implementation just compares scheme of the URI with FUSE type
- // and checks that there is some path in the URI.
- virtual bool CanMount(const Uri& source) const;
-
- // Derives suggested directory name for the mount point from the source.
- // Default implementation takes path part of the URI and escapes path
- // characters..
- virtual std::string GetTargetSuffix(const Uri& source) const;
-
- static std::unique_ptr<MountPoint> MountWithDir(
- const Mounter& mounter,
- const base::FilePath& working_dir,
- const std::string& source,
- const base::FilePath& target_path,
- std::vector<std::string> params,
- MountErrorType* error);
-
- // Mounter overrides:
- bool CanMount(const std::string& source,
- const std::vector<std::string>& params,
- base::FilePath* suggested_dir_name) const override;
-
- protected:
- std::unique_ptr<MountPoint> Mount(const std::string& source,
- const base::FilePath& target_path,
- std::vector<std::string> params,
- MountErrorType* error) const override;
-
- brillo::ProcessReaper* process_reaper() const { return process_reaper_; }
-
- // Does preprocessing and conversion of options and source format to be
- // compatible with the FUSE mount program, and returns resulting Mounter.
- // |working_dir| is a temporary writable directory that may be used by
- // this invocation of the mounter process.
- virtual std::unique_ptr<FUSEMounter> CreateMounter(
- const base::FilePath& working_dir,
- const Uri& source,
- const base::FilePath& target_path,
- const std::vector<std::string>& options) const;
-
- private:
- const std::string fuse_type_;
- const Platform* const platform_;
- brillo::ProcessReaper* const process_reaper_;
- const base::FilePath mount_program_path_;
- const std::string mount_user_;
-
- FRIEND_TEST(FUSEHelperTest, PrepareMountOptions);
-};
-
-} // namespace cros_disks
-
-#endif // CROS_DISKS_FUSE_HELPER_H_
diff --git a/cros-disks/fuse_helper_test.cc b/cros-disks/fuse_helper_test.cc
deleted file mode 100644
index 54fc20f..0000000
--- a/cros-disks/fuse_helper_test.cc
+++ /dev/null
@@ -1,78 +0,0 @@
-// Copyright 2018 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "cros-disks/fuse_helper.h"
-
-#include <string>
-#include <vector>
-
-#include <base/strings/string_split.h>
-#include <base/strings/string_util.h>
-#include <brillo/process/process_reaper.h>
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-
-#include "cros-disks/fuse_mounter.h"
-#include "cros-disks/mount_options.h"
-#include "cros-disks/platform.h"
-#include "cros-disks/uri.h"
-
-namespace cros_disks {
-
-namespace {
-
-const char kFUSEType[] = "fuse";
-const char kMountProgram[] = "/bin/dummy";
-const char kMountUser[] = "nobody";
-const Uri kSomeUri("fuse", "some/src/path");
-const base::FilePath kWorkingDir("/wkdir");
-const base::FilePath kMountDir("/mnt");
-
-} // namespace
-
-class FUSEHelperTest : public ::testing::Test {
- public:
- FUSEHelperTest()
- : helper_(kFUSEType,
- &platform_,
- &process_reaper_,
- base::FilePath(kMountProgram),
- kMountUser) {}
-
- protected:
- Platform platform_;
- brillo::ProcessReaper process_reaper_;
- FUSEHelper helper_;
-};
-
-// Verifies that CanMount correctly identifies handleable URIs.
-TEST_F(FUSEHelperTest, CanMount) {
- EXPECT_TRUE(helper_.CanMount(Uri::Parse("fuse://foo")));
- EXPECT_FALSE(helper_.CanMount(Uri::Parse("boose://foo")));
- EXPECT_FALSE(helper_.CanMount(Uri::Parse("http://foo")));
- EXPECT_FALSE(helper_.CanMount(Uri::Parse("fuse://")));
-}
-
-// Verifies that GetTargetSuffix escapes unwanted chars in URI.
-TEST_F(FUSEHelperTest, GetTargetSuffix) {
- EXPECT_EQ("foo", helper_.GetTargetSuffix(Uri::Parse("fuse://foo")));
- EXPECT_EQ("", helper_.GetTargetSuffix(Uri::Parse("fuse://")));
- EXPECT_EQ("a:b@c:d$__$etc$",
- helper_.GetTargetSuffix(Uri::Parse("fuse://a:b@c:d/../etc/")));
-}
-
-// Verifies that generic implementation applies default rules to MountOptions.
-// TODO(dats): Remove when get rid of MountOptions.
-TEST_F(FUSEHelperTest, PrepareMountOptions) {
- std::vector<std::string> options = {"sync", "foo=bar", "baz", "dirsync"};
- auto mounter =
- helper_.CreateMounter(kWorkingDir, kSomeUri, kMountDir, options);
- const FUSEMounterLegacy* legacy =
- static_cast<FUSEMounterLegacy*>(mounter.get());
- std::string opts = legacy->mount_options().ToString();
- EXPECT_THAT(opts, testing::StartsWith("sync,dirsync,"));
- EXPECT_THAT(opts, testing::Not(testing::HasSubstr("uid=")));
-}
-
-} // namespace cros_disks
diff --git a/cros-disks/fuse_mount_manager.cc b/cros-disks/fuse_mount_manager.cc
index abd9532..ffb0ae1 100644
--- a/cros-disks/fuse_mount_manager.cc
+++ b/cros-disks/fuse_mount_manager.cc
@@ -13,7 +13,6 @@
#include <brillo/process/process_reaper.h>
#include "cros-disks/drivefs_helper.h"
-#include "cros-disks/fuse_helper.h"
#include "cros-disks/fuse_mounter.h"
#include "cros-disks/platform.h"
#include "cros-disks/quote.h"
@@ -87,20 +86,7 @@
return nullptr;
}
- // Make a temporary dir where the helper may keep stuff needed by the mounter
- // process.
- std::string path;
- if (!platform()->CreateTemporaryDirInDir(working_dirs_root_, ".", &path) ||
- !platform()->SetPermissions(path, 0755)) {
- LOG(ERROR) << "Cannot create working directory for FUSE module mounting "
- << quote(source);
- *error = MOUNT_ERROR_DIRECTORY_CREATION_FAILED;
- return nullptr;
- }
-
- auto mountpoint =
- FUSEHelper::MountWithDir(*selected_helper, base::FilePath(path), source,
- mount_path, options, error);
+ auto mountpoint = selected_helper->Mount(source, mount_path, options, error);
LOG_IF(ERROR, *error != MOUNT_ERROR_NONE)
<< "Mounting failed for source " << quote(source) << ": " << *error;
return mountpoint;
diff --git a/cros-disks/fuse_mount_manager_test.cc b/cros-disks/fuse_mount_manager_test.cc
index 34d3b42..70a490e 100644
--- a/cros-disks/fuse_mount_manager_test.cc
+++ b/cros-disks/fuse_mount_manager_test.cc
@@ -15,7 +15,6 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
-#include "cros-disks/fuse_helper.h"
#include "cros-disks/fuse_mounter.h"
#include "cros-disks/metrics.h"
#include "cros-disks/mount_options.h"
@@ -70,26 +69,21 @@
(const, override));
};
-// Mock implementation of FUSEHelper.
-class MockHelper : public FUSEHelper {
+// Mock implementation of a Mounter.
+class MockMounter : public Mounter {
public:
- MockHelper(const std::string& tag,
- const Platform* platform,
- brillo::ProcessReaper* process_reaper)
- : FUSEHelper(tag,
- platform,
- process_reaper,
- base::FilePath("/sbin/" + tag),
- "fuse-" + tag) {}
-
- MOCK_METHOD(bool, CanMount, (const Uri&), (const, override));
- MOCK_METHOD(std::string, GetTargetSuffix, (const Uri&), (const, override));
- MOCK_METHOD(std::unique_ptr<FUSEMounter>,
- CreateMounter,
- (const base::FilePath&,
- const Uri&,
- const base::FilePath&,
- const std::vector<std::string>&),
+ MOCK_METHOD(std::unique_ptr<MountPoint>,
+ Mount,
+ (const std::string& source,
+ const base::FilePath& target_path,
+ std::vector<std::string> params,
+ MountErrorType* error),
+ (const, override));
+ MOCK_METHOD(bool,
+ CanMount,
+ (const std::string& source,
+ const std::vector<std::string>& params,
+ base::FilePath* suggested_dir_name),
(const, override));
};
@@ -103,24 +97,6 @@
MOCK_METHOD(int, WaitNonBlockingImpl, (), (override));
};
-class MockMounter : public FUSEMounterLegacy {
- public:
- MockMounter(const Platform* platform, brillo::ProcessReaper* process_reaper)
- : FUSEMounterLegacy({.filesystem_type = "fuse",
- .mount_program = "/bin/sh",
- .mount_user = "root",
- .platform = platform,
- .process_reaper = process_reaper}) {}
-
- MOCK_METHOD(std::unique_ptr<SandboxedProcess>,
- PrepareSandbox,
- (const std::string&,
- const base::FilePath&,
- std::vector<std::string>,
- MountErrorType*),
- (const, override));
-};
-
} // namespace
class FUSEMountManagerTest : public ::testing::Test {
@@ -131,16 +107,16 @@
&platform_,
&metrics_,
&process_reaper_),
- foo_(new MockHelper("foo", &platform_, &process_reaper_)),
- bar_(new MockHelper("bar", &platform_, &process_reaper_)),
- baz_(new MockHelper("baz", &platform_, &process_reaper_)) {
+ foo_(new MockMounter()),
+ bar_(new MockMounter()),
+ baz_(new MockMounter()) {
ON_CALL(platform_, Unmount(_, _))
.WillByDefault(Return(MOUNT_ERROR_INVALID_ARGUMENT));
ON_CALL(platform_, DirectoryExists(_)).WillByDefault(Return(true));
}
protected:
- void RegisterHelper(std::unique_ptr<FUSEHelper> helper) {
+ void RegisterHelper(std::unique_ptr<Mounter> helper) {
manager_.RegisterHelper(std::move(helper));
}
@@ -162,9 +138,9 @@
MockPlatform platform_;
brillo::ProcessReaper process_reaper_;
FUSEMountManager manager_;
- std::unique_ptr<MockHelper> foo_;
- std::unique_ptr<MockHelper> bar_;
- std::unique_ptr<MockHelper> baz_;
+ std::unique_ptr<MockMounter> foo_;
+ std::unique_ptr<MockMounter> bar_;
+ std::unique_ptr<MockMounter> baz_;
};
// Verifies that CanMount returns false when there are no handlers registered.
@@ -174,9 +150,9 @@
// Verifies that CanMount returns false when known helpers can't handle that.
TEST_F(FUSEMountManagerTest, CanMount_NotHandled) {
- EXPECT_CALL(*foo_, CanMount(_)).WillOnce(Return(false));
- EXPECT_CALL(*bar_, CanMount(_)).WillOnce(Return(false));
- EXPECT_CALL(*baz_, CanMount(_)).WillOnce(Return(false));
+ EXPECT_CALL(*foo_, CanMount).WillOnce(Return(false));
+ EXPECT_CALL(*bar_, CanMount).WillOnce(Return(false));
+ EXPECT_CALL(*baz_, CanMount).WillOnce(Return(false));
RegisterHelper(std::move(foo_));
RegisterHelper(std::move(bar_));
RegisterHelper(std::move(baz_));
@@ -186,9 +162,9 @@
// Verify that CanMount returns true when there is a helper that can handle
// this source.
TEST_F(FUSEMountManagerTest, CanMount) {
- EXPECT_CALL(*foo_, CanMount(_)).WillOnce(Return(false));
- EXPECT_CALL(*bar_, CanMount(_)).WillOnce(Return(true));
- EXPECT_CALL(*baz_, CanMount(_)).Times(0);
+ EXPECT_CALL(*foo_, CanMount).WillOnce(Return(false));
+ EXPECT_CALL(*bar_, CanMount).WillOnce(Return(true));
+ EXPECT_CALL(*baz_, CanMount).Times(0);
RegisterHelper(std::move(foo_));
RegisterHelper(std::move(bar_));
RegisterHelper(std::move(baz_));
@@ -197,19 +173,18 @@
// Verify that SuggestMountPath dispatches query for name to the correct helper.
TEST_F(FUSEMountManagerTest, SuggestMountPath) {
- EXPECT_CALL(*foo_, CanMount(_)).WillOnce(Return(false));
- EXPECT_CALL(*foo_, GetTargetSuffix(_)).Times(0);
- EXPECT_CALL(*bar_, CanMount(_)).WillOnce(Return(true));
- EXPECT_CALL(*bar_, GetTargetSuffix(kSomeSource)).WillOnce(Return("suffix"));
- EXPECT_CALL(*baz_, CanMount(_)).Times(0);
- EXPECT_CALL(*baz_, GetTargetSuffix(_)).Times(0);
+ EXPECT_CALL(*foo_, CanMount).WillOnce(Return(false));
+ EXPECT_CALL(*bar_, CanMount)
+ .WillOnce(
+ DoAll(SetArgPointee<2>(base::FilePath("suffix")), Return(true)));
+ EXPECT_CALL(*baz_, CanMount).Times(0);
RegisterHelper(std::move(foo_));
RegisterHelper(std::move(bar_));
RegisterHelper(std::move(baz_));
EXPECT_EQ("/mntroot/suffix", manager_.SuggestMountPath(kSomeSource.value()));
}
-// Verify that DoMount fails when there are not helpers.
+// Verify that DoMount fails when there are no helpers.
TEST_F(FUSEMountManagerTest, DoMount_NoHandlers) {
MountErrorType mount_error;
std::unique_ptr<MountPoint> mount_point =
@@ -219,9 +194,9 @@
// Verify that DoMount fails when helpers don't handle this source.
TEST_F(FUSEMountManagerTest, DoMount_NotHandled) {
- EXPECT_CALL(*foo_, CanMount(_)).WillOnce(Return(false));
- EXPECT_CALL(*bar_, CanMount(_)).WillOnce(Return(false));
- EXPECT_CALL(*baz_, CanMount(_)).WillOnce(Return(false));
+ EXPECT_CALL(*foo_, CanMount).WillOnce(Return(false));
+ EXPECT_CALL(*bar_, CanMount).WillOnce(Return(false));
+ EXPECT_CALL(*baz_, CanMount).WillOnce(Return(false));
RegisterHelper(std::move(foo_));
RegisterHelper(std::move(bar_));
RegisterHelper(std::move(baz_));
@@ -234,26 +209,20 @@
// Verify that DoMount delegates mounting to the correct helpers when
// dispatching by source description.
TEST_F(FUSEMountManagerTest, DoMount_BySource) {
- EXPECT_CALL(*foo_, CanMount(_)).WillOnce(Return(false));
- EXPECT_CALL(*bar_, CanMount(_)).WillRepeatedly(Return(true));
- EXPECT_CALL(*baz_, CanMount(_)).Times(0);
- EXPECT_CALL(*foo_, CreateMounter(_, _, _, _)).Times(0);
- EXPECT_CALL(platform_, Mount(_, kSomeMountpoint, _, _, _))
- .WillOnce(Return(MOUNT_ERROR_NONE));
- EXPECT_CALL(platform_, CreateTemporaryDirInDir(kWorkingDirRoot, _, _))
- .WillOnce(DoAll(SetArgPointee<2>("/blah"), Return(true)));
- EXPECT_CALL(platform_, SetPermissions("/blah", 0755)).WillOnce(Return(true));
- MockMounter* mounter = new MockMounter(&platform_, &process_reaper_);
- EXPECT_CALL(*mounter, PrepareSandbox(kSomeSource.value(),
- base::FilePath(kSomeMountpoint), _, _))
+ EXPECT_CALL(*foo_, CanMount).WillOnce(Return(false));
+ EXPECT_CALL(*bar_, CanMount)
.WillOnce(
- DoAll(SetArgPointee<3>(MOUNT_ERROR_NONE),
- Return(ByMove(std::make_unique<MockSandboxedProcess>()))));
- std::unique_ptr<FUSEMounter> ptr(mounter);
- EXPECT_CALL(*bar_, CreateMounter(base::FilePath("/blah"), kSomeSource,
- base::FilePath(kSomeMountpoint), _))
- .WillOnce(Return(ByMove(std::move(ptr))));
- EXPECT_CALL(*baz_, CreateMounter(_, _, _, _)).Times(0);
+ DoAll(SetArgPointee<2>(base::FilePath("suffix")), Return(true)));
+ EXPECT_CALL(*baz_, CanMount).Times(0);
+
+ EXPECT_CALL(*foo_, Mount).Times(0);
+ EXPECT_CALL(*baz_, Mount).Times(0);
+
+ EXPECT_CALL(*bar_, Mount(kSomeSource.value(), _, _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(MOUNT_ERROR_NONE),
+ Return(ByMove(MountPoint::CreateLeaking(
+ base::FilePath(kSomeMountpoint))))));
+
RegisterHelper(std::move(foo_));
RegisterHelper(std::move(bar_));
RegisterHelper(std::move(baz_));
diff --git a/cros-disks/fuse_mounter.cc b/cros-disks/fuse_mounter.cc
index 0318175..bff5421 100644
--- a/cros-disks/fuse_mounter.cc
+++ b/cros-disks/fuse_mounter.cc
@@ -54,14 +54,10 @@
}
// TODO(dats): Remove when it's done beforehead by the caller.
-OwnerUser ResolveUserOrDie(const Platform* platform,
- const std::string& user,
- const std::string& group) {
+OwnerUser ResolveUserOrDie(const Platform* platform, const std::string& user) {
OwnerUser result;
PCHECK(platform->GetUserAndGroupId(user, &result.uid, &result.gid));
- if (!group.empty()) {
- PCHECK(platform->GetGroupId(group, &result.gid));
- }
+ result.gid = kChronosAccessGID;
return result;
}
@@ -482,7 +478,7 @@
platform(),
{base::FilePath(std::move(params.mount_program)),
UnsetIfEmpty(base::FilePath(std::move(params.seccomp_policy)))},
- ResolveUserOrDie(platform(), params.mount_user, params.mount_group),
+ ResolveUserOrDie(platform(), params.mount_user),
params.network_access,
std::move(params.supplementary_groups),
UnsetIfEmpty(base::FilePath(std::move(params.mount_namespace)))) {}
diff --git a/cros-disks/fuse_mounter.h b/cros-disks/fuse_mounter.h
index 40c0816..7da29bd 100644
--- a/cros-disks/fuse_mounter.h
+++ b/cros-disks/fuse_mounter.h
@@ -195,9 +195,6 @@
// Not recorded if empty or if metrics is null.
std::string metrics_name;
- // Optional group to run the FUSE mount program as.
- std::string mount_group;
-
// Optional mount namespace where the source path exists.
std::string mount_namespace;
diff --git a/cros-disks/fuse_mounter_test.cc b/cros-disks/fuse_mounter_test.cc
index 1ec694b..e4fc5aa 100644
--- a/cros-disks/fuse_mounter_test.cc
+++ b/cros-disks/fuse_mounter_test.cc
@@ -480,7 +480,8 @@
kMountProgram, "-o", MountOptions().ToString(),
kSomeSource, StartsWith("/dev/fd/"))))
.WillOnce(Return(0));
- EXPECT_CALL(platform_, SetOwnership(kSomeSource, getuid(), kMountGID))
+ EXPECT_CALL(platform_,
+ SetOwnership(kSomeSource, getuid(), kChronosAccessGID))
.WillOnce(Return(true));
EXPECT_CALL(platform_, SetPermissions(kSomeSource, S_IRUSR | S_IWUSR |
S_IRGRP | S_IWGRP))
diff --git a/cros-disks/rar_manager.cc b/cros-disks/rar_manager.cc
index da18f9c..f37d0dc 100644
--- a/cros-disks/rar_manager.cc
+++ b/cros-disks/rar_manager.cc
@@ -14,7 +14,6 @@
#include <base/strings/string_util.h>
#include "cros-disks/error_logger.h"
-#include "cros-disks/fuse_helper.h"
#include "cros-disks/fuse_mounter.h"
#include "cros-disks/metrics.h"
#include "cros-disks/platform.h"
@@ -57,7 +56,6 @@
.filesystem_type = "rarfs",
.metrics = metrics(),
.metrics_name = "Rar2fs",
- .mount_group = FUSEHelper::kFilesGroup,
.mount_namespace = std::move(mount_namespace.name),
.mount_program = "/usr/bin/rar2fs",
.mount_user = "fuse-rar2fs",
diff --git a/cros-disks/zip_manager.cc b/cros-disks/zip_manager.cc
index d848949..c5f78eb 100644
--- a/cros-disks/zip_manager.cc
+++ b/cros-disks/zip_manager.cc
@@ -12,7 +12,6 @@
#include <base/strings/string_util.h>
#include "cros-disks/error_logger.h"
-#include "cros-disks/fuse_helper.h"
#include "cros-disks/fuse_mounter.h"
#include "cros-disks/metrics.h"
#include "cros-disks/platform.h"
@@ -48,7 +47,6 @@
.filesystem_type = "zipfs",
.metrics = metrics(),
.metrics_name = "FuseZip",
- .mount_group = FUSEHelper::kFilesGroup,
.mount_namespace = GetMountNamespaceFor(source_path).name,
.mount_program = "/usr/bin/fuse-zip",
.mount_user = "fuse-zip",
