smbfs: Use stable mount path for smbfs shares

Uses a combination of share-specific values (user hash, URL, source,
authentication type, workgroup & username) as input to a SHA256 hash
that is then used to create a mount path that does not change across
mount / unmount cycles.

Test: unit_tests --gtest_filter="*SmbFs*"
Test: Deploy to DUT, ensure SMB share mounts successfully
Test: Mount share, unmount, remount, ensure mount path remains constant
Bug: 1130884
Change-Id: I24603cc9d09609e65c3b869c3c42142c30425788
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2430539
Commit-Queue: Josh Simmons <simmonsjosh@google.com>
Reviewed-by: Anand K Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815026}

chrome/browser/chromeos/smb_client/smbfs_share.cc

@@ -8,12 +8,15 @@
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/unguessable_token.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/smb_client/smb_service_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/chromeos/smb_shares/smb_credentials_dialog.h"
+#include "crypto/sha2.h"
 #include "storage/browser/file_system/external_mount_points.h"
 
 namespace chromeos {
@@ -22,6 +25,7 @@ namespace smb_client {
 namespace {
 
 constexpr char kMountDirPrefix[] = "smbfs-";
+constexpr char kMountIdHashSeparator[] = "#";
 constexpr base::TimeDelta kAllowCredentialsTimeout =
     base::TimeDelta::FromSeconds(5);
 
@@ -57,8 +61,7 @@ SmbFsShare::SmbFsShare(Profile* profile,
       share_url_(share_url),
       display_name_(display_name),
       options_(options),
-      mount_id_(
-          base::ToLowerASCII(base::UnguessableToken::Create().ToString())) {
+      mount_id_(GenerateStableMountId()) {
   DCHECK(share_url_.IsValid());
 }
 
@@ -305,5 +308,40 @@ void SmbFsShare::SetMounterCreationCallbackForTest(
   mounter_creation_callback_for_test_ = std::move(callback);
 }
 
+std::string SmbFsShare::GenerateStableMountId() const {
+  std::string hash_input = GenerateStableMountIdInput();
+  return base::ToLowerASCII(base::HexEncode(
+      crypto::SHA256HashString(hash_input).c_str(), crypto::kSHA256Length));
+}
+
+std::string SmbFsShare::GenerateStableMountIdInput() const {
+  std::vector<std::string> mount_id_hash_components;
+
+  // Shares are unique based on the user the profile is owned by.
+  mount_id_hash_components.push_back(
+      chromeos::ProfileHelper::Get()->GetUserIdHashFromProfile(profile_));
+
+  // The hostname in the URL should be that entered by the user or
+  // specified in the preconfigured share policy. It should not have
+  // been resolved to an IP address if a hostname was specified.
+  //
+  // This property is true implicitly due to the call path for
+  // SmbFsShare creation.
+  mount_id_hash_components.push_back(share_url_.ToString());
+
+  // Distinguish between Kerberos and user-supplied credentials.
+  mount_id_hash_components.push_back(
+      base::NumberToString(options_.kerberos_options ? 1 : 0));
+
+  // Distinguish between domains / workgroups (may be empty for guest or
+  // Kerberos).
+  mount_id_hash_components.push_back(options_.workgroup);
+
+  // Distinguish between usernames (may be empty for guest or Kerberos).
+  mount_id_hash_components.push_back(options_.username);
+
+  return base::JoinString(mount_id_hash_components, kMountIdHashSeparator);
+}
+
 }  // namespace smb_client
 }  // namespace chromeos

chrome/browser/chromeos/smb_client/smbfs_share.h

@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
+#include "base/gtest_prod_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/smb_client/smb_errors.h"
@@ -87,6 +88,9 @@ class SmbFsShare : public smbfs::SmbFsHost::Delegate {
   void SetMounterCreationCallbackForTest(MounterCreationCallback callback);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(SmbFsShareTest, GenerateStableMountId);
+  FRIEND_TEST_ALL_PREFIXES(SmbFsShareTest, GenerateStableMountIdInput);
+
   // Callback for smbfs::SmbFsMounter::Mount().
   void OnMountDone(MountCallback callback,
                    smbfs::mojom::MountError mount_error,
@@ -112,6 +116,21 @@ class SmbFsShare : public smbfs::SmbFsHost::Delegate {
   void OnDisconnected() override;
   void RequestCredentials(RequestCredentialsCallback callback) override;
 
+  // Generate a stable ID to uniquely identify the share across each
+  // mount / unmount cycle. This allows the share to have the same path
+  // on the filesystem each time it is mounted.
+  //
+  // This function creates uniqueness beyond that currently enforced by
+  // the system (which presently only allows one share per canonical URL
+  // to be mounted). IDs generated here will be forward compatible in a
+  // future where the same share could be mounted once (ie. read-only)
+  // by preconfigured policy and subsequently by the user but using
+  // read-write credentials).
+  std::string GenerateStableMountId() const;
+
+  // Generate the input for stable mount ID hash (simplifies testing).
+  std::string GenerateStableMountIdInput() const;
+
   Profile* const profile_;
   const SmbUrl share_url_;
   const std::string display_name_;

chrome/browser/chromeos/smb_client/smbfs_share_unittest.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/run_loop.h"
+#include "base/strings/string_split.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/gmock_callback_support.h"
 #include "base/test/task_environment.h"
@@ -14,6 +15,7 @@
 #include "chrome/browser/chromeos/file_manager/volume_manager.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager_factory.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager_observer.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/smb_client/smb_url.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/components/smbfs/smbfs_host.h"
@@ -35,9 +37,15 @@ namespace smb_client {
 namespace {
 
 constexpr char kSharePath[] = "smb://share/path";
+constexpr char kSharePath2[] = "smb://share/path2";
+constexpr char kShareUsername[] = "user";
+constexpr char kShareUsername2[] = "user2";
+constexpr char kShareWorkgroup[] = "workgroup";
+constexpr char kKerberosIdentity[] = "my-kerberos-identity";
 constexpr char kDisplayName[] = "Public";
 constexpr char kMountPath[] = "/share/mount/path";
 constexpr char kFileName[] = "file_name.ext";
+constexpr char kMountIdHashSeparator[] = "#";
 
 // Creates a new VolumeManager for tests.
 // By default, VolumeManager KeyedService is null for testing.
@@ -87,6 +95,8 @@ class TestSmbFsImpl : public smbfs::mojom::SmbFs {
               (override));
 };
 
+}  // namespace
+
 class SmbFsShareTest : public testing::Test {
  protected:
   void SetUp() override {
@@ -424,6 +434,66 @@ TEST_F(SmbFsShareTest, RemoveSavedCredentials_Disconnect) {
   }
 }
 
-}  // namespace
+TEST_F(SmbFsShareTest, GenerateStableMountIdInput) {
+  TestSmbFsImpl smbfs;
+
+  std::string profile_user_hash =
+      chromeos::ProfileHelper::Get()->GetUserIdHashFromProfile(&profile_);
+
+  smbfs::SmbFsMounter::MountOptions options1;
+  options1.username = kShareUsername;
+  options1.workgroup = kShareWorkgroup;
+  SmbFsShare share1(&profile_, SmbUrl(kSharePath), kDisplayName, options1);
+
+  std::string hash_input1 = share1.GenerateStableMountIdInput();
+  std::vector<std::string> tokens1 =
+      base::SplitString(hash_input1, kMountIdHashSeparator,
+                        base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+  EXPECT_EQ(tokens1.size(), 5);
+  EXPECT_EQ(tokens1[0], profile_user_hash);
+  EXPECT_EQ(tokens1[1], SmbUrl(kSharePath).ToString());
+  EXPECT_EQ(tokens1[2], "0" /* kerberos */);
+  EXPECT_EQ(tokens1[3], kShareWorkgroup);
+  EXPECT_EQ(tokens1[4], kShareUsername);
+
+  smbfs::SmbFsMounter::MountOptions options2;
+  options2.kerberos_options =
+      base::make_optional<smbfs::SmbFsMounter::KerberosOptions>(
+          smbfs::SmbFsMounter::KerberosOptions::Source::kKerberos,
+          kKerberosIdentity);
+  SmbFsShare share2(&profile_, SmbUrl(kSharePath2), kDisplayName, options2);
+
+  std::string hash_input2 = share2.GenerateStableMountIdInput();
+  std::vector<std::string> tokens2 =
+      base::SplitString(hash_input2, kMountIdHashSeparator,
+                        base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+  EXPECT_EQ(tokens2.size(), 5);
+  EXPECT_EQ(tokens2[0], profile_user_hash);
+  EXPECT_EQ(tokens2[1], SmbUrl(kSharePath2).ToString());
+  EXPECT_EQ(tokens2[2], "1" /* kerberos */);
+  EXPECT_EQ(tokens2[3], "");
+  EXPECT_EQ(tokens2[4], "");
+}
+
+TEST_F(SmbFsShareTest, GenerateStableMountId) {
+  TestSmbFsImpl smbfs;
+
+  smbfs::SmbFsMounter::MountOptions options1;
+  options1.username = kShareUsername;
+  SmbFsShare share1(&profile_, SmbUrl(kSharePath), kDisplayName, options1);
+  std::string mount_id1 = share1.GenerateStableMountId();
+
+  // Check: We get a different hash when options are varied.
+  smbfs::SmbFsMounter::MountOptions options2;
+  options2.username = kShareUsername2;
+  SmbFsShare share2(&profile_, SmbUrl(kSharePath), kDisplayName, options2);
+  std::string mount_id2 = share2.GenerateStableMountId();
+  EXPECT_TRUE(mount_id1.compare(mount_id2));
+
+  // Check: String is 64 characters long (SHA256 encoded as hex).
+  EXPECT_EQ(mount_id1.size(), 64);
+  EXPECT_EQ(mount_id2.size(), 64);
+}
+
 }  // namespace smb_client
 }  // namespace chromeos