Limit the maximum number of smbfs shares that can be mounted

Enforce a limit on the maximum number of smbfs shares that can be
mounted by request of the user. When the limit is reached, the
user will see the message "Error mounting share. Too many SMB
shares are already mounted." in the "Add file share" dialog box.

Fixed: 1060396
Change-Id: I21467250d7c273d34591f351bc955a4e7d041a45
Test: unit_tests --gtest_filter="SmbServiceWithSmbfsTest.MountExcessiveShares"
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2100052
Commit-Queue: Josh Simmons <simmonsjosh@google.com>
Reviewed-by: Anand Mistry <amistry@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750879}

chrome/app/os_settings_strings.grdp

@@ -357,6 +357,9 @@
   <message name="IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_MOUNT_EXISTS_MESSAGE" desc="The message shown when mounting a new SMB share fails because the share is already mounted.">
     Error mounting share. The specified share is already mounted.
   </message>
+  <message name="IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_TOO_MANY_MOUNTS_MESSAGE" desc="The message shown when mounting a new SMB share fails because too many SMB shares are already mounted.">
+    Error mounting share. Too many SMB shares are already mounted.
+  </message>
   <message name="IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_MOUNT_INVALID_URL_MESSAGE" desc="The message shown when mounting a new SMB share fails because the URL is an invalid format.">
     Invalid URL format. Supported formats are \\server\share and smb://server/share.
   </message>

chrome/app/os_settings_strings_grdp/IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_TOO_MANY_MOUNTS_MESSAGE.png.sha1

+660c1ecc83ae08f341828351b4732e548e469627
\ No newline at end of file

chrome/browser/chromeos/smb_client/smb_service.cc

@@ -57,6 +57,9 @@ const char kModeUnknownValue[] = "unknown";
 const base::TimeDelta kHostDiscoveryInterval = base::TimeDelta::FromSeconds(60);
 // -3 is chosen because -1 and -2 have special meaning in smbprovider.
 const int32_t kInvalidMountId = -3;
+// Maximum number of smbfs shares to be mounted at the same time, only enforced
+// on user-initiated mount requests.
+const size_t kMaxSmbFsShares = 16;
 
 net::NetworkInterfaceList GetInterfaces() {
   net::NetworkInterfaceList list;
@@ -273,6 +276,12 @@ void SmbService::Mount(const file_system_provider::MountOptions& options,
     return;
   }
 
+  if (IsSmbFsEnabled() && smbfs_shares_.size() >= kMaxSmbFsShares) {
+    // Prevent users from mounting an excessive number of shares.
+    std::move(callback).Run(SmbMountResult::kTooManyOpened);
+    return;
+  }
+
   std::string username;
   std::string password;
   std::string workgroup;

chrome/browser/chromeos/smb_client/smb_service_unittest.cc

@@ -16,6 +16,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
@@ -925,5 +926,103 @@ TEST_F(SmbServiceWithSmbfsTest, MountSaved) {
   EXPECT_TRUE(registry.GetAll().empty());
 }
 
+TEST_F(SmbServiceWithSmbfsTest, MountExcessiveShares) {
+  // The maxmium number of smbfs shares that can be mounted simultaneously.
+  // Should match the definition in smb_service.cc.
+  const size_t kMaxSmbFsShares = 16;
+  CreateService(profile_);
+  WaitForSetupComplete();
+
+  // Check: It is possible to mount the maximum number of shares.
+  for (size_t i = 0; i < kMaxSmbFsShares; ++i) {
+    mojo::Remote<smbfs::mojom::SmbFs> smbfs_remote;
+    MockSmbFsImpl smbfs_impl(smbfs_remote.BindNewPipeAndPassReceiver());
+    mojo::Remote<smbfs::mojom::SmbFsDelegate> smbfs_delegate_remote;
+
+    smbfs::SmbFsHost::Delegate* smbfs_host_delegate = nullptr;
+    std::unique_ptr<MockSmbFsMounter> mock_mounter =
+        std::make_unique<MockSmbFsMounter>();
+
+    smb_service_->SetSmbFsMounterCreationCallbackForTesting(
+        base::BindLambdaForTesting([&mock_mounter, &smbfs_host_delegate](
+                                       const std::string& share_path,
+                                       const std::string& mount_dir_name,
+                                       const SmbFsShare::MountOptions& options,
+                                       smbfs::SmbFsHost::Delegate* delegate)
+                                       -> std::unique_ptr<smbfs::SmbFsMounter> {
+          smbfs_host_delegate = delegate;
+          return std::move(mock_mounter);
+        }));
+
+    const std::string share_path =
+        std::string(kSharePath) + base::NumberToString(i);
+    const std::string mount_path =
+        std::string(kMountPath) + base::NumberToString(i);
+
+    EXPECT_CALL(*mock_mounter, Mount(_))
+        .WillOnce([this, &smbfs_host_delegate, &smbfs_remote,
+                   &smbfs_delegate_remote,
+                   &mount_path](smbfs::SmbFsMounter::DoneCallback callback) {
+          std::move(callback).Run(
+              smbfs::mojom::MountError::kOk,
+              std::make_unique<smbfs::SmbFsHost>(
+                  MakeMountPoint(base::FilePath(mount_path)),
+                  smbfs_host_delegate, std::move(smbfs_remote),
+                  smbfs_delegate_remote.BindNewPipeAndPassReceiver()));
+        });
+
+    base::RunLoop run_loop;
+    smb_service_->Mount(
+        mount_options_, base::FilePath(share_path), kTestUser, kTestPassword,
+        false /* use_chromad_kerberos */,
+        false /* should_open_file_manager_after_mount */,
+        false /* save_credentials */,
+        base::BindLambdaForTesting([&run_loop](SmbMountResult result) {
+          EXPECT_EQ(SmbMountResult::kSuccess, result);
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+  }
+
+  // Check: After mounting the maximum number of shares, requesting to mount an
+  // additional share should fail.
+  mojo::Remote<smbfs::mojom::SmbFs> smbfs_remote;
+  MockSmbFsImpl smbfs_impl(smbfs_remote.BindNewPipeAndPassReceiver());
+  mojo::Remote<smbfs::mojom::SmbFsDelegate> smbfs_delegate_remote;
+
+  smbfs::SmbFsHost::Delegate* smbfs_host_delegate = nullptr;
+  std::unique_ptr<MockSmbFsMounter> mock_mounter =
+      std::make_unique<MockSmbFsMounter>();
+
+  smb_service_->SetSmbFsMounterCreationCallbackForTesting(
+      base::BindLambdaForTesting([&mock_mounter, &smbfs_host_delegate](
+                                     const std::string& share_path,
+                                     const std::string& mount_dir_name,
+                                     const SmbFsShare::MountOptions& options,
+                                     smbfs::SmbFsHost::Delegate* delegate)
+                                     -> std::unique_ptr<smbfs::SmbFsMounter> {
+        smbfs_host_delegate = delegate;
+        return std::move(mock_mounter);
+      }));
+
+  const std::string share_path =
+      std::string(kSharePath) + base::NumberToString(kMaxSmbFsShares);
+  const std::string mount_path =
+      std::string(kMountPath) + base::NumberToString(kMaxSmbFsShares);
+
+  base::RunLoop run_loop;
+  smb_service_->Mount(
+      mount_options_, base::FilePath(share_path), kTestUser, kTestPassword,
+      false /* use_chromad_kerberos */,
+      false /* should_open_file_manager_after_mount */,
+      false /* save_credentials */,
+      base::BindLambdaForTesting([&run_loop](SmbMountResult result) {
+        EXPECT_EQ(SmbMountResult::kTooManyOpened, result);
+        run_loop.Quit();
+      }));
+
+  run_loop.Run();
+}
+
 }  // namespace smb_client
 }  // namespace chromeos

chrome/browser/ui/webui/chromeos/smb_shares/smb_shares_localized_strings_provider.cc

@@ -40,6 +40,8 @@ void AddLocalizedStrings(content::WebUIDataSource* html_source) {
        IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_UNSUPPORTED_DEVICE_MESSAGE},
       {"smbShareAddedMountExistsMessage",
        IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_MOUNT_EXISTS_MESSAGE},
+      {"smbShareAddedTooManyMountsMessage",
+       IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_TOO_MANY_MOUNTS_MESSAGE},
       {"smbShareAddedInvalidURLMessage",
        IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_MOUNT_INVALID_URL_MESSAGE},
       {"smbShareAddedInvalidSSOURLMessage",

chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider.cc

@@ -1221,6 +1221,8 @@ void AddFilesStrings(content::WebUIDataSource* html_source) {
        IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_UNSUPPORTED_DEVICE_MESSAGE},
       {"smbShareAddedMountExistsMessage",
        IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_MOUNT_EXISTS_MESSAGE},
+      {"smbShareAddedTooManyMountsMessage",
+       IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_TOO_MANY_MOUNTS_MESSAGE},
       {"smbShareAddedInvalidURLMessage",
        IDS_SETTINGS_DOWNLOADS_SHARE_ADDED_MOUNT_INVALID_URL_MESSAGE},
       {"smbShareAddedInvalidSSOURLMessage",

ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.js

@@ -265,6 +265,10 @@ Polymer({
         this.setGeneralError_(
             loadTimeData.getString('smbShareAddedMountExistsMessage'));
         break;
+      case SmbMountResult.TOO_MANY_OPENED:
+        this.setGeneralError_(
+            loadTimeData.getString('smbShareAddedTooManyMountsMessage'));
+        break;
       default:
         this.setGeneralError_(
             loadTimeData.getString('smbShareAddedErrorMessage'));