arc: Change the way to generate ro.serialno for ARCVM

Previously, Chrome just stored a randomly generated serial number in
the user's prefs and passed it to ArcVmClientAdapter when it starts
the VM.

This CL changes the way of the serial number generation as follows:

1) Chrome no longer stores the serial number itself in the pref. It
   stores the "salt" instead.
2) The pref location is changed from the user's prefs to Local State.
3) When /var/lib/misc/arc_salt (which is for ARC container) exists,
   Chrome copies the file's content to the Local State pref.
4) Generate the serial number for the user in exactly the same was
   as ARC container which is sha256(user_id + salt_in_local_state).
5) Don't set the local state pref unless ARCVM is in use.

As a result, devices that are upgraded from ARC container to ARCVM
can continue to use the same ro.serialno on ARCVM.

This CL depends on
https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2271390
(platform2).

BUG=b:143976415
TEST=try, ARC container/VM still starts

Change-Id: Ia75e0e09d25d0eb3f806e865c1b19bca77507c9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2271534
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Long Cheng <lgcheng@google.com>
Cr-Commit-Position: refs/heads/master@{#787611}

chrome/browser/chromeos/arc/session/arc_service_launcher.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/arc/session/arc_service_launcher.h"
 
+#include <string>
 #include <utility>
 
 #include "ash/public/cpp/default_scale_factor_retriever.h"
@@ -120,7 +121,7 @@ void ArcServiceLauncher::Initialize(
     mojo::PendingRemote<ash::mojom::CrosDisplayConfigController>
         display_config) {
   default_scale_factor_retriever_.Start(std::move(display_config));
-  arc_session_manager_->ExpandPropertyFiles();
+  arc_session_manager_->ExpandPropertyFilesAndReadSalt();
 }
 
 void ArcServiceLauncher::MaybeSetProfile(Profile* profile) {

chrome/browser/chromeos/arc/session/arc_session_manager.h

@@ -7,6 +7,8 @@
 
 #include <memory>
 #include <ostream>
+#include <string>
+#include <utility>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -22,6 +24,7 @@
 #include "components/arc/session/arc_stop_reason.h"
 
 class ArcAppLauncher;
+class PrefService;
 class Profile;
 
 namespace arc {
@@ -105,6 +108,9 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
     STOPPING,
   };
 
+  using ExpansionResult = std::pair<std::string /* salt on disk */,
+                                    bool /* expansion successful */>;
+
   // Observer for those services outside of ARC which want to know ARC events.
   class Observer {
    public:
@@ -160,6 +166,15 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   static void SetUiEnabledForTesting(bool enabled);
   static void SetArcTermsOfServiceOobeNegotiatorEnabledForTesting(bool enabled);
   static void EnableCheckAndroidManagementForTesting(bool enable);
+  static std::string GenerateFakeSerialNumberForTesting(
+      const std::string& chromeos_user,
+      const std::string& salt);
+  static std::string GetOrCreateSerialNumberForTesting(
+      PrefService* local_state,
+      const std::string& chromeos_user,
+      const std::string& arc_salt_on_disk);
+  static bool ReadSaltOnDiskForTesting(const base::FilePath& salt_path,
+                                       std::string* out_salt);
 
   // Returns true if ARC is allowed to run for the current session.
   // TODO(hidehiko): The name is very close to IsArcAllowedForProfile(), but
@@ -167,8 +182,9 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   bool IsAllowed() const;
 
   // Start expanding the property files. Note that these property files are
-  // needed to start the mini instance.
-  void ExpandPropertyFiles();
+  // needed to start the mini instance. This function also tries to read
+  // /var/lib/misc/arc_salt when ARCVM is enabled.
+  void ExpandPropertyFilesAndReadSalt();
 
   // Initializes ArcSessionManager. Before this runs, Profile must be set
   // via SetProfile().
@@ -240,6 +256,9 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   void OnProvisioningFinished(ProvisioningResult result,
                               mojom::ArcSignInErrorPtr error);
 
+  // A helper function that calls ArcSessionRunner's SetUserInfo.
+  void SetUserInfo();
+
   // Returns the time when the sign in process started, or a null time if
   // signing in didn't happen during this session.
   base::TimeTicks sign_in_start_time() const { return sign_in_start_time_; }
@@ -289,9 +308,9 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
     OnTermsOfServiceNegotiated(accepted);
   }
 
-  // Invokes OnExpandPropertyFiles as if the expansion is done.
-  void OnExpandPropertyFilesForTesting(bool result) {
-    OnExpandPropertyFiles(result);
+  // Invokes OnExpandPropertyFilesAndReadSalt as if the expansion is done.
+  void OnExpandPropertyFilesAndReadSaltForTesting(bool result) {
+    OnExpandPropertyFilesAndReadSalt(ExpansionResult{{}, result});
   }
 
   void reset_property_files_expansion_result() {
@@ -391,8 +410,8 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   // chromeos::SessionManagerClient::Observer:
   void EmitLoginPromptVisibleCalled() override;
 
-  // Called when ExpandPropertyFiles is done.
-  void OnExpandPropertyFiles(bool result);
+  // Called when ExpandPropertyFilesAndReadSalt is done.
+  void OnExpandPropertyFilesAndReadSalt(ExpansionResult result);
 
   std::unique_ptr<ArcSessionRunner> arc_session_runner_;
 
@@ -433,6 +452,9 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
 
   ArcAppIdProviderImpl app_id_provider_;
 
+  // The content of /var/lib/misc/arc_salt. Empty if the file doesn't exist.
+  base::Optional<std::string> arc_salt_on_disk_;
+
   base::Optional<bool> property_files_expansion_result_;
   base::FilePath property_files_source_dir_;
   base::FilePath property_files_dest_dir_;

chrome/browser/chromeos/arc/test/test_arc_session_manager.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/chromeos/arc/test/test_arc_session_manager.h"
 
+#include <utility>
+
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
@@ -38,7 +40,7 @@ std::unique_ptr<ArcSessionManager> CreateTestArcSessionManager(
   // be automatically called. Because of that, we can call
   // OnExpandPropertyFilesForTesting() instead with |true| for easier unit
   // testing (without calling base::RunLoop().RunUntilIdle() here and there.)
-  manager->OnExpandPropertyFilesForTesting(true);
+  manager->OnExpandPropertyFilesAndReadSaltForTesting(true);
   return manager;
 }
 
@@ -53,7 +55,7 @@ bool ExpandPropertyFilesForTesting(ArcSessionManager* arc_session_manager,
     return false;
   arc_session_manager->set_property_files_source_dir_for_testing(source_dir);
   arc_session_manager->set_property_files_dest_dir_for_testing(dest_dir);
-  arc_session_manager->ExpandPropertyFiles();
+  arc_session_manager->ExpandPropertyFilesAndReadSalt();
   return true;
 }
 

components/arc/arc_prefs.cc

@@ -91,9 +91,6 @@ const char kArcPushInstallAppsRequested[] = "arc.push_install.requested";
 // A preference that holds the list of apps that the admin requested to be
 // push-installed, but which have not been successfully installed yet.
 const char kArcPushInstallAppsPending[] = "arc.push_install.pending";
-// A preference to keep the ro.serialno and ro.boot.serialno Android properties
-// used to start ARC.
-const char kArcSerialNumber[] = "arc.serialno";
 // A preference to keep deferred requests of setting notifications enabled flag.
 const char kArcSetNotificationsEnabledDeferred[] =
     "arc.set_notifications_enabled_deferred";
@@ -128,7 +125,13 @@ const char kNativeBridge64BitSupportExperimentEnabled[] =
 // crash.
 const char kStabilityMetrics[] = "arc.metrics.stability";
 
+// A preference to keep the salt for generating ro.serialno and ro.boot.serialno
+// Android properties. Used only in ARCVM.
+const char kArcSerialNumberSalt[] = "arc.serialno_salt";
+
 void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
+  // Sorted in lexicographical order.
+  registry->RegisterStringPref(kArcSerialNumberSalt, std::string());
   registry->RegisterBooleanPref(kNativeBridge64BitSupportExperimentEnabled,
                                 false);
   registry->RegisterDictionaryPref(kStabilityMetrics);
@@ -171,7 +174,6 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(kArcFastAppReinstallPackages);
   registry->RegisterBooleanPref(kArcPolicyComplianceReported, false);
   registry->RegisterBooleanPref(kArcProvisioningInitiatedFromOobe, false);
-  registry->RegisterStringPref(kArcSerialNumber, std::string());
   registry->RegisterBooleanPref(kArcSignedIn, false);
   registry->RegisterBooleanPref(kArcSkippedReportingNotice, false);
   registry->RegisterBooleanPref(kArcTermsAccepted, false);

components/arc/arc_prefs.h

@@ -33,7 +33,6 @@ ARC_EXPORT extern const char kArcPolicyComplianceReported[];
 ARC_EXPORT extern const char kArcProvisioningInitiatedFromOobe[];
 ARC_EXPORT extern const char kArcPushInstallAppsPending[];
 ARC_EXPORT extern const char kArcPushInstallAppsRequested[];
-ARC_EXPORT extern const char kArcSerialNumber[];
 ARC_EXPORT extern const char kArcSetNotificationsEnabledDeferred[];
 ARC_EXPORT extern const char kArcSignedIn[];
 ARC_EXPORT extern const char kArcSkippedReportingNotice[];
@@ -45,6 +44,7 @@ ARC_EXPORT extern const char kEcryptfsMigrationStrategy[];
 ARC_EXPORT extern const char kEngagementPrefsPrefix[];
 
 // Local state prefs in lexicographical order.
+ARC_EXPORT extern const char kArcSerialNumberSalt[];
 ARC_EXPORT extern const char kNativeBridge64BitSupportExperimentEnabled[];
 ARC_EXPORT extern const char kStabilityMetrics[];
 

components/arc/session/arc_session_runner.cc

@@ -243,7 +243,7 @@ void ArcSessionRunner::SetUserInfo(
   // can also be called multiple times in tests.
   // TODO(yusukes): Fix tests and add DCHECKs to make sure they are not empty
   // and the function is called only once.
-  DCHECK(!serial_number.empty());
+  DCHECK(!IsArcVmEnabled() || !serial_number.empty());
   cryptohome_id_ = cryptohome_id;
   user_id_hash_ = hash;
   serial_number_ = serial_number;