[sync] Add obfuscatedGaiaId along with encryption keys on ChromeOS

The field is somewhat redundant because the signin flow gets the user
identifier (gaia ID) by other means. However, to prevent bugs that can
lead to mixing up encryption keys across users, this patch bundles the
gaia ID together with encryption keys and verifies the integrity of
the information when the signin procedure completes, prior to the
actual processing of the keys.

Change-Id: I75f834c2f5e6cfccca752ea2d517bb8256ef9aa6
Fixed: 1081651
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2897569
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#885039}

chrome/browser/resources/gaia_auth_host/authenticator.js

@@ -50,6 +50,7 @@ cr.define('cr.login', function() {
    * Sync trusted vault encryption keys optionally passed with 'authCompleted'
    * message.
    * @typedef {{
+   *   obfuscatedGaiaId: string,
    *   encryptionKeys: Array<SyncTrustedVaultKey>,
    *   trustedRecoveryMethods: Array<SyncTrustedRecoveryMethod>
    * }}

chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc

@@ -116,9 +116,24 @@ const char kAuthIframeParentName[] = "signin-frame";
 
 const char kEndpointGen[] = "1.0";
 
-bool IsSyncTrustedVaultKeysEnabled() {
-  return base::FeatureList::IsEnabled(
-      ::switches::kSyncSupportTrustedVaultPassphraseRecovery);
+absl::optional<SyncTrustedVaultKeys> GetSyncTrustedVaultKeysForUserContext(
+    const base::DictionaryValue* js_object,
+    const std::string& gaia_id) {
+  if (!base::FeatureList::IsEnabled(
+          ::switches::kSyncSupportTrustedVaultPassphraseRecovery)) {
+    return absl::nullopt;
+  }
+
+  // |js_object| is not expected to be null, but as extra precaution, guard
+  // against crashes.
+  if (!js_object)
+    return absl::nullopt;
+
+  SyncTrustedVaultKeys parsed_keys = SyncTrustedVaultKeys::FromJs(*js_object);
+  if (parsed_keys.gaia_id() != gaia_id)
+    return absl::nullopt;
+
+  return absl::make_optional(std::move(parsed_keys));
 }
 
 // Must be kept consistent with ChromeOSSamlApiUsed in enums.xml
@@ -801,10 +816,8 @@ void GaiaScreenHandler::HandleCompleteAuthentication(
           GetAccountId(email, gaia_id, AccountType::GOOGLE), using_saml,
           using_saml_api_, password,
           SamlPasswordAttributes::FromJs(*password_attributes),
-          IsSyncTrustedVaultKeysEnabled()
-              ? absl::make_optional(
-                    SyncTrustedVaultKeys::FromJs(*sync_trusted_vault_keys))
-              : absl::nullopt,
+          GetSyncTrustedVaultKeysForUserContext(sync_trusted_vault_keys,
+                                                gaia_id),
           *extension_provided_client_cert_usage_observer_,
           pending_user_context_.get(), &error)) {
     LoginDisplayHost::default_host()->GetSigninUI()->ShowSigninError(

chromeos/login/auth/sync_trusted_vault_keys.cc

@@ -15,6 +15,7 @@ namespace {
 
 // Keys in base::Value dictionaries.
 const char kEncryptionKeysDictKey[] = "encryptionKeys";
+const char kGaiaIdDictKey[] = "obfuscatedGaiaId";
 const char kKeyMaterialDictKey[] = "keyMaterial";
 const char kMethodTypeHintDictKey[] = "type";
 const char kPublicKeyDictKey[] = "publicKey";
@@ -103,11 +104,16 @@ SyncTrustedVaultKeys::~SyncTrustedVaultKeys() = default;
 // static
 SyncTrustedVaultKeys SyncTrustedVaultKeys::FromJs(
     const base::DictionaryValue& js_object) {
+  SyncTrustedVaultKeys result;
+  const std::string* gaia_id = js_object.FindStringKey(kGaiaIdDictKey);
+  if (gaia_id) {
+    result.gaia_id_ = *gaia_id;
+  }
+
   const std::vector<KeyMaterialAndVersion> encryption_keys =
       ParseList(js_object.FindListKey(kEncryptionKeysDictKey),
                 base::BindRepeating(&ParseSingleEncryptionKey));
 
-  SyncTrustedVaultKeys result;
   for (const KeyMaterialAndVersion& key : encryption_keys) {
     if (key.version != 0) {
       result.encryption_keys_.push_back(key.key_material);
@@ -122,6 +128,10 @@ SyncTrustedVaultKeys SyncTrustedVaultKeys::FromJs(
   return result;
 }
 
+const std::string& SyncTrustedVaultKeys::gaia_id() const {
+  return gaia_id_;
+}
+
 const std::vector<std::vector<uint8_t>>& SyncTrustedVaultKeys::encryption_keys()
     const {
   return encryption_keys_;

chromeos/login/auth/sync_trusted_vault_keys.h

@@ -31,6 +31,8 @@ class COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH) SyncTrustedVaultKeys {
   // authenticator.js.
   static SyncTrustedVaultKeys FromJs(const base::DictionaryValue& js_object);
 
+  const std::string& gaia_id() const;
+
   const std::vector<std::vector<uint8_t>>& encryption_keys() const;
   int last_encryption_key_version() const;
 
@@ -46,6 +48,7 @@ class COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH) SyncTrustedVaultKeys {
   const std::vector<TrustedRecoveryMethod>& trusted_recovery_methods() const;
 
  private:
+  std::string gaia_id_;
   std::vector<std::vector<uint8_t>> encryption_keys_;
   int last_encryption_key_version_ = 0;
   std::vector<TrustedRecoveryMethod> trusted_recovery_methods_;

chromeos/login/auth/sync_trusted_vault_keys_unittest.cc

@@ -67,6 +67,13 @@ TEST(SyncTrustedVaultKeysTest, FromJsWithInvalidDictionary) {
   EXPECT_THAT(SyncTrustedVaultKeys::FromJs(value), HasEmptyValue());
 }
 
+TEST(SyncTrustedVaultKeysTest, FromJsWithGaiaId) {
+  const std::string kGaiaId = "user1";
+  base::DictionaryValue value;
+  value.SetStringKey("obfuscatedGaiaId", kGaiaId);
+  EXPECT_THAT(SyncTrustedVaultKeys::FromJs(value).gaia_id(), Eq(kGaiaId));
+}
+
 TEST(SyncTrustedVaultKeysTest, FromJsWithEncryptionKeys) {
   const std::vector<uint8_t> kEncryptionKeyMaterial1 = {1, 2, 3, 4};
   const std::vector<uint8_t> kEncryptionKeyMaterial2 = {5, 6, 7, 8};

google_apis/gaia/fake_gaia.cc

@@ -140,6 +140,7 @@ std::string FormatSyncTrustedRecoveryMethods(
 }
 
 std::string FormatSyncTrustedVaultKeysHeader(
+    const std::string& gaia_id,
     const FakeGaia::SyncTrustedVaultKeys& sync_trusted_vault_keys) {
   // Single line used because this string populates HTTP headers. Similarly,
   // base64 encoding is used to avoid line breaks and meanwhile adopt JSON
@@ -147,12 +148,13 @@ std::string FormatSyncTrustedVaultKeysHeader(
   // embedded_setup_chromeos.html.
   const char format[] =
       "{"
+      "\"obfuscatedGaiaId\":\"%s\","
       "\"fakeEncryptionKeyMaterial\":\"%s\","
       "\"fakeEncryptionKeyVersion\":%d,"
       "\"fakeTrustedRecoveryMethods\":[%s]"
       "}";
   return base::StringPrintf(
-      format,
+      format, gaia_id.c_str(),
       base::Base64Encode(sync_trusted_vault_keys.encryption_key).c_str(),
       sync_trusted_vault_keys.encryption_key_version,
       FormatSyncTrustedRecoveryMethods(
@@ -288,6 +290,7 @@ void FakeGaia::AddSyncTrustedKeysHeader(BasicHttpResponse* http_response,
   http_response->AddCustomHeader(
       "fake-sync-trusted-vault-keys",
       FormatSyncTrustedVaultKeysHeader(
+          GetGaiaIdOfEmail(email),
           email_to_sync_trusted_vault_keys_map_.at(email)));
 }
 

google_apis/test/embedded_setup_chromeos.html

@@ -223,6 +223,7 @@ function base64DecodeToArrayBuffer(encoded) {
 // posting to authenticator.js, due to the use of binary blobs in the API.
 function convertSyncTrustedVaultKeys(fakeKeys) {
   var keys = {
+    obfuscatedGaiaId: fakeKeys.obfuscatedGaiaId,
     encryptionKeys: [
       {keyMaterial: base64DecodeToArrayBuffer(fakeKeys.fakeEncryptionKeyMaterial),
        version: fakeKeys.fakeEncryptionKeyVersion}