Enable sharing / unsharing of smbfs paths into Crostini

Directories within smbfs shares, or the entire share, are mounted in
the VM (by seneschal) under the "SMB/<share-id>" directory. This allows
multiple smbfs shares, or multiple directories from a single share, to
be shared into the VM without encountering namespace conflicts.

Test: Share root and/or sub-folder of share into Crostini from Files.app
Bug: 962924
Change-Id: I8d2055603741e0b6197ca0d0543497eeca6776e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2434599
Commit-Queue: Josh Simmons <simmonsjosh@google.com>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816021}

chrome/browser/chromeos/file_manager/path_util.cc

@@ -57,6 +57,7 @@ constexpr char kCrostiniMapGoogleDrive[] = "GoogleDrive";
 constexpr char kCrostiniMapLinuxFiles[] = "LinuxFiles";
 constexpr char kCrostiniMapMyDrive[] = "MyDrive";
 constexpr char kCrostiniMapPlayFiles[] = "PlayFiles";
+constexpr char kCrostiniMapSmbFs[] = "SMB";
 constexpr char kCrostiniMapTeamDrives[] = "SharedDrives";
 constexpr char kFolderNameDownloads[] = "Downloads";
 constexpr char kFolderNameMyFiles[] = "MyFiles";
@@ -425,6 +426,13 @@ bool ConvertFileSystemURLToPathInsideVM(
     } else {
       *inside = vm_mount.Append(kCrostiniMapLinuxFiles);
     }
+  } else if (file_system_url.type() == storage::kFileSystemTypeSmbFs) {
+    // Do not assume the share is currently accessible via SmbService
+    // as this function is called during unmount when SmbFsShare is
+    // destroyed. The only information safely available is the stable
+    // mount ID.
+    *inside = vm_mount.Append(kCrostiniMapSmbFs);
+    *inside = inside->Append(id);
   } else {
     return false;
   }

chrome/browser/chromeos/file_manager/path_util_unittest.cc

@@ -362,6 +362,13 @@ TEST_F(FileManagerPathUtilTest, ConvertFileSystemURLToPathInsideVM) {
   mount_points->RegisterFileSystem(
       chromeos::kSystemMountNameArchive, storage::kFileSystemTypeNativeLocal,
       storage::FileSystemMountOption(), base::FilePath(kArchiveMountPath));
+  // SmbFsShare comes up with a unique stable ID for the share, it can
+  // just be faked here, the mount ID is expected to appear in the mount
+  // path, it's faked too.
+  mount_points->RegisterFileSystem(
+      "smbfsmountid" /* fake mount ID for mount name */,
+      storage::kFileSystemTypeSmbFs, storage::FileSystemMountOption(),
+      base::FilePath("/media/fuse/smbfs-smbfsmountid"));
 
   base::FilePath inside;
   base::FilePath vm_mount("/mnt/chromeos");
@@ -451,6 +458,13 @@ TEST_F(FileManagerPathUtilTest, ConvertFileSystemURLToPathInsideVM) {
           url::Origin(), "archive", base::FilePath("file.rar/path/in/archive")),
       vm_mount, &inside));
   EXPECT_EQ("/mnt/chromeos/archive/file.rar/path/in/archive", inside.value());
+
+  EXPECT_TRUE(ConvertFileSystemURLToPathInsideVM(
+      profile_.get(),
+      mount_points->CreateExternalFileSystemURL(
+          url::Origin(), "smbfsmountid", base::FilePath("path/in/smb/share")),
+      vm_mount, &inside));
+  EXPECT_EQ("/mnt/chromeos/SMB/smbfsmountid/path/in/smb/share", inside.value());
 }
 
 TEST_F(FileManagerPathUtilTest, ExtractMountNameFileSystemNameFullPath) {

chrome/browser/chromeos/guest_os/guest_os_share_path.cc

@@ -20,6 +20,9 @@
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager_factory.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
+#include "chrome/browser/chromeos/smb_client/smb_service.h"
+#include "chrome/browser/chromeos/smb_client/smb_service_factory.h"
+#include "chrome/browser/chromeos/smb_client/smbfs_share.h"
 #include "chromeos/components/drivefs/mojom/drivefs.mojom.h"
 #include "chromeos/dbus/concierge/concierge_service.pb.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -35,6 +38,10 @@
 
 namespace {
 
+// Root path under which FUSE filesystems such as DriveFS, SmbFs are mounted.
+constexpr base::FilePath::CharType kFuseFsRootPath[] =
+    FILE_PATH_LITERAL("/media/fuse");
+
 void OnSeneschalSharePathResponse(
     guest_os::GuestOsSharePath::SharePathCallback callback,
     base::Optional<vm_tools::seneschal::SharePathResponse> response) {
@@ -211,12 +218,18 @@ void GuestOsSharePath::CallSeneschalSharePath(const std::string& vm_name,
   }
 
   vm_tools::seneschal::SharePathRequest request;
+  base::FilePath fuse_fs_root_path(kFuseFsRootPath);
   base::FilePath drivefs_path;
   base::FilePath relative_path;
   drive::DriveIntegrationService* integration_service =
       drive::DriveIntegrationServiceFactory::GetForProfile(profile_);
   base::FilePath drivefs_mount_point_path;
   base::FilePath drivefs_mount_name;
+  chromeos::smb_client::SmbService* smb_service =
+      chromeos::smb_client::SmbServiceFactory::Get(profile_);
+  chromeos::smb_client::SmbFsShare* smb_share = nullptr;
+  base::FilePath smbfs_mount_point_path;
+  base::FilePath smbfs_mount_name;
 
   // Allow MyFiles directory and subdirs.
   bool allowed_path = false;
@@ -237,9 +250,8 @@ void GuestOsSharePath::CallSeneschalSharePath(const std::string& vm_name,
              (drivefs_mount_point_path =
                   integration_service->GetMountPointPath())
                  .AppendRelativePath(path, &drivefs_path) &&
-             base::FilePath("/media/fuse")
-                 .AppendRelativePath(drivefs_mount_point_path,
-                                     &drivefs_mount_name)) {
+             fuse_fs_root_path.AppendRelativePath(drivefs_mount_point_path,
+                                                  &drivefs_mount_name)) {
     // Allow subdirs of DriveFS except .Trash.
     request.set_drivefs_mount_name(drivefs_mount_name.value());
     base::FilePath root("root");
@@ -305,6 +317,16 @@ void GuestOsSharePath::CallSeneschalSharePath(const std::string& vm_name,
     allowed_path = true;
     request.set_storage_location(
         vm_tools::seneschal::SharePathRequest::ARCHIVE);
+  } else if (smb_service &&
+             (smb_share = smb_service->GetSmbFsShareForPath(path)) &&
+             ((smbfs_mount_point_path = smb_share->mount_path())
+                  .AppendRelativePath(path, &relative_path) ||
+              path == smbfs_mount_point_path /* sharing root of mount */) &&
+             fuse_fs_root_path.AppendRelativePath(smbfs_mount_point_path,
+                                                  &smbfs_mount_name)) {
+    allowed_path = true;
+    request.set_storage_location(vm_tools::seneschal::SharePathRequest::SMBFS);
+    request.set_smbfs_mount_name(smbfs_mount_name.value());
   }
 
   if (!allowed_path) {

chrome/browser/chromeos/smb_client/smbfs_share.cc

@@ -162,7 +162,11 @@ void SmbFsShare::Unmount(SmbFsShare::UnmountCallback callback) {
     return;
   }
 
-  // Remove volume from VolumeManager.
+  // Remove volume from VolumeManager. It's critical this is done before
+  // revoking the filesystem from ExternalMountPoints as some observers
+  // (ie. Crostini) need to create a cracked FileSystemURL (which
+  // requires the mount to still be registered with ExternalMountPoints)
+  // during the unmount process.
   file_manager::VolumeManager::Get(profile_)->RemoveSmbFsVolume(
       host_->mount_path());
 

ui/file_manager/file_manager/background/js/crostini.js

@@ -249,8 +249,6 @@ CrostiniImpl.DEFAULT_VM = 'termina';
 CrostiniImpl.PLUGIN_VM = 'PvmDefault';
 
 /**
- * Keep in sync with histograms.xml:FileBrowserCrostiniSharedPathsDepth
- * histogram_suffix.
  * @type {!Map<?VolumeManagerCommon.RootType, string>}
  * @const
  */
@@ -265,4 +263,5 @@ CrostiniImpl.VALID_ROOT_TYPES_FOR_SHARE = new Map([
   [VolumeManagerCommon.RootType.SHARED_DRIVE, 'TeamDrive'],
   [VolumeManagerCommon.RootType.CROSTINI, 'Crostini'],
   [VolumeManagerCommon.RootType.ARCHIVE, 'Archive'],
+  [VolumeManagerCommon.RootType.SMB, 'SMB'],
 ]);