Add support for GCM subtypes to desktop Instance ID implementation

components/gcm_driver/gcm_client_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/gcm_driver/gcm_client_impl.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <utility>
@@ skipping 68 matching lines @@
 };
 
 const char kGCMScope[] = "GCM";
 const int kMaxRegistrationRetries = 5;
 const int kMaxUnregistrationRetries = 5;
 const char kMessageTypeDataMessage[] = "gcm";
 const char kMessageTypeDeletedMessagesKey[] = "deleted_messages";
 const char kMessageTypeKey[] = "message_type";
 const char kMessageTypeSendErrorKey[] = "send_error";
 const char kSendErrorMessageIdKey[] = "google.message_id";
+const char kSubtypeKey[] = "subtype";
 const char kSendMessageFromValue[] = "gcm@chrome.com";
 const int64_t kDefaultUserSerialNumber = 0LL;
 const int kDestroyGCMStoreDelayMS = 5 * 60 * 1000;  // 5 minutes.
 
 GCMClient::Result ToGCMClientResult(MCSClient::MessageSendStatus status) {
   switch (status) {
     case MCSClient::QUEUED:
       return GCMClient::SUCCESS;
     case MCSClient::QUEUE_SIZE_LIMIT_REACHED:
       return GCMClient::NETWORK_ERROR;
@@ skipping 132 matching lines @@
 
   UMA_HISTOGRAM_ENUMERATION("GCM.OutgoingMessageTTL",
                             ttl_category,
                             TTL_CATEGORY_COUNT);
 }
 
 void RecordResetStoreErrorToUMA(ResetStoreError error) {
   UMA_HISTOGRAM_ENUMERATION("GCM.ResetStore", error, RESET_STORE_ERROR_COUNT);
 }
 
+// Helper method for DCHECKs that prevent requesting the same token both with
+// and without subtypes.
+bool InstanceIDsDifferOnlyInUseSubtype(
+    const RegistrationInfo* registration_info_a,
+    const RegistrationInfo* registration_info_b) {
+  const InstanceIDTokenInfo* token_a =
+      InstanceIDTokenInfo::FromRegistrationInfo(registration_info_a);
+  const InstanceIDTokenInfo* token_b =
+      InstanceIDTokenInfo::FromRegistrationInfo(registration_info_b);
+  return token_a && token_b && token_a->use_subtype != token_b->use_subtype &&
+         token_a->app_id == token_b->app_id &&
+         token_a->authorized_entity == token_b->authorized_entity &&
+         token_a->scope == token_b->scope;

[Peter Beverloo, 2016/07/28 12:34:10]

nit: Could we logically separate these checks for readability?

====
  if (!token_a || !token_b)
    return false;

  if (token_a->app_id != token_b->app_id ||
      token_a->authorized_entity != token_b->authorized_entity ||
      token_a->scope != token_b->scope) {
    return false;
  }

  return token_a->use_subtype != token_b->use_subtype;
====

[johnme, 2016/08/04 17:47:14]

Done.

+}
+
 }  // namespace
 
 GCMInternalsBuilder::GCMInternalsBuilder() {}
 GCMInternalsBuilder::~GCMInternalsBuilder() {}
 
 std::unique_ptr<base::Clock> GCMInternalsBuilder::BuildClock() {
   return base::WrapUnique<base::Clock>(new base::DefaultClock());
 }
 
 std::unique_ptr<MCSClient> GCMInternalsBuilder::BuildMCSClient(
@@ skipping 592 matching lines @@
   checkin_request_.reset();
   // Delete all of the pending registration and unregistration requests.
   pending_registration_requests_.clear();
   pending_unregistration_requests_.clear();
 }
 
 void GCMClientImpl::Register(
     const linked_ptr<RegistrationInfo>& registration_info) {
   DCHECK_EQ(state_, READY);
 
+  // Registrations should never pass as an app_id the special category used
+  // internally when registering with a subtype. See security note in
+  // GCMClientImpl::HandleIncomingMessage.
+  CHECK_NE(registration_info->app_id, chrome_build_info_.category_for_subtypes);
+
   // Find and use the cached registration ID.
   RegistrationInfoMap::const_iterator registrations_iter =
       registrations_.find(registration_info);
   if (registrations_iter != registrations_.end()) {
     bool matched = true;
 
     // For GCM registration, we also match the sender IDs since multiple
     // registrations are not supported.
     const GCMRegistrationInfo* gcm_registration_info =
         GCMRegistrationInfo::FromRegistrationInfo(registration_info.get());
     if (gcm_registration_info) {
       const GCMRegistrationInfo* cached_gcm_registration_info =
           GCMRegistrationInfo::FromRegistrationInfo(
               registrations_iter->first.get());
       DCHECK(cached_gcm_registration_info);
       if (cached_gcm_registration_info &&
           gcm_registration_info->sender_ids !=
               cached_gcm_registration_info->sender_ids) {
         matched = false;
       }
+    } else {
+      DCHECK(!InstanceIDsDifferOnlyInUseSubtype(
+          registration_info.get(), registrations_iter->first.get()))
+          << "Registering the same Instance ID token both with and without "
+             "subtypes is not supported";
     }
 
     if (matched) {
       delegate_->OnRegisterFinished(
           registration_info, registrations_iter->second, SUCCESS);
       return;
     }
   }
 
   std::unique_ptr<RegistrationRequest::CustomRequestHandler> request_handler;
@@ skipping 26 matching lines @@
     request_handler.reset(new InstanceIDGetTokenRequestHandler(
           instance_id_iter->second.first,
           instance_id_token_info->authorized_entity,
           instance_id_token_info->scope,
           ConstructGCMVersion(chrome_build_info_.version),
           instance_id_token_info->options));
     source_to_record = instance_id_token_info->authorized_entity + "/" +
                        instance_id_token_info->scope;
   }
 
+  bool use_subtype =
+      instance_id_token_info && instance_id_token_info->use_subtype;
+
   RegistrationRequest::RequestInfo request_info(
-      device_checkin_info_.android_id,
-      device_checkin_info_.secret,
-      registration_info->app_id);
+      device_checkin_info_.android_id, device_checkin_info_.secret,
+      registration_info->app_id, use_subtype,
+      chrome_build_info_.category_for_subtypes);
 
   std::unique_ptr<RegistrationRequest> registration_request(
       new RegistrationRequest(
           gservices_settings_.GetRegistrationURL(), request_info,
           std::move(request_handler), GetGCMBackoffPolicy(),
           base::Bind(&GCMClientImpl::OnRegisterCompleted,
                      weak_ptr_factory_.GetWeakPtr(), registration_info),
           kMaxRegistrationRetries, url_request_context_getter_, &recorder_,
           source_to_record));
   registration_request->Start();
@@ skipping 62 matching lines @@
   const InstanceIDTokenInfo* instance_id_token_info =
       InstanceIDTokenInfo::FromRegistrationInfo(registration_info.get());
   if (instance_id_token_info) {
     auto instance_id_iter = instance_id_data_.find(registration_info->app_id);
     if (instance_id_iter == instance_id_data_.end()) {
       // This should not be reached since we should not delete tokens when
       // an InstanceID has not been created yet.
       NOTREACHED();
       return;
     }
+
     request_handler.reset(new InstanceIDDeleteTokenRequestHandler(
         instance_id_iter->second.first,
         instance_id_token_info->authorized_entity,
         instance_id_token_info->scope,
         ConstructGCMVersion(chrome_build_info_.version)));
     source_to_record = instance_id_token_info->authorized_entity + "/" +
                        instance_id_token_info->scope;
   }
 
   // Remove the registration/token(s) from the cache and the store.
@@ skipping 27 matching lines @@
       OnUnregisterCompleted(registration_info,
                             UnregistrationRequest::SUCCESS);
       return;
     }
   } else {
     auto iter = registrations_.find(registration_info);
     if (iter == registrations_.end()) {
       delegate_->OnUnregisterFinished(registration_info, INVALID_PARAMETER);
       return;
     }
+    DCHECK(!InstanceIDsDifferOnlyInUseSubtype(registration_info.get(),
+                                              iter->first.get()));
     registrations_.erase(iter);
 
     gcm_store_->RemoveRegistration(
       registration_info->GetSerializedKey(),
       base::Bind(&GCMClientImpl::UpdateRegistrationCallback,
                   weak_ptr_factory_.GetWeakPtr()));
   }
 
+  bool use_subtype =
+      instance_id_token_info && instance_id_token_info->use_subtype;
+
   UnregistrationRequest::RequestInfo request_info(
-      device_checkin_info_.android_id,
-      device_checkin_info_.secret,
-      registration_info->app_id);
+      device_checkin_info_.android_id, device_checkin_info_.secret,
+      registration_info->app_id, use_subtype,
+      chrome_build_info_.category_for_subtypes);
 
   std::unique_ptr<UnregistrationRequest> unregistration_request(
       new UnregistrationRequest(
           gservices_settings_.GetRegistrationURL(), request_info,
           std::move(request_handler), GetGCMBackoffPolicy(),
           base::Bind(&GCMClientImpl::OnUnregisterCompleted,
                      weak_ptr_factory_.GetWeakPtr(), registration_info),
           kMaxUnregistrationRetries, url_request_context_getter_, &recorder_,
           source_to_record));
   unregistration_request->Start();
@@ skipping 179 matching lines @@
 }
 
 void GCMClientImpl::HandleIncomingMessage(const gcm::MCSMessage& message) {
   DCHECK(delegate_);
 
   const mcs_proto::DataMessageStanza& data_message_stanza =
       reinterpret_cast<const mcs_proto::DataMessageStanza&>(
           message.GetProtobuf());
   DCHECK_EQ(data_message_stanza.device_user_id(), kDefaultUserSerialNumber);
 
-  // Copying all the data from the stanza to a MessageData object. When present,
-  // keys like kMessageTypeKey or kSendErrorMessageIdKey will be filtered out
-  // later.
+  // Copy all the data from the stanza to a MessageData object. When present,
+  // keys like kSubtypeKey, kMessageTypeKey or kSendErrorMessageIdKey will be
+  // filtered out later.
   MessageData message_data;
   for (int i = 0; i < data_message_stanza.app_data_size(); ++i) {
     std::string key = data_message_stanza.app_data(i).key();
     message_data[key] = data_message_stanza.app_data(i).value();
   }
 
+  std::string subtype;
+  auto subtype_iter = message_data.find(kSubtypeKey);
+  if (subtype_iter != message_data.end()) {
+    subtype = subtype_iter->second;
+    message_data.erase(subtype_iter);
+  }
+
+  // SECURITY NOTE: Subtypes that we receive from GCM *cannot* be trusted for
+  // registrations without a subtype (as the sender can pass any subtype they
+  // want). They can however be trusted for registrations that are known to have
+  // a subtype (as GCM overwrites anything passed by the sender).
+  //
+  // TODO(johnme): Get FCM to send the subtype as a trusted google.subtype key.

[Peter Beverloo, 2016/07/28 12:34:10]

This is rather passive-aggressive :-) What about:

  // TODO(johnme): Remove this limitation when GCM sends us the subtype in a
  // field that's guaranteed to be trusted. crbug.com/123456

[johnme, 2016/08/04 17:47:14]

I clicked on that crbug link! Done.

+  //
+  // So a given Chrome profile always passes a fixed string called
+  // |category_for_subtypes| (of the form "com.chrome.stable.macosx") as the
+  // category when registering with a subtype, and incoming subtypes are only
+  // trusted for that category.
+  //
+  // (On Android, all registrations made by Chrome on behalf of third-party
+  // apps/extensions/websites have always had a subtype, so we don't need such a
+  // check, indeed we cannot, since category is fixed to the true package name).

[Peter Beverloo, 2016/07/28 12:34:10]

nit: avoid use of "we" (3x) in comments... All of these can easily be rephrased,
making the comments shorter too.

[johnme, 2016/08/04 17:47:14]

Removed we's. I can't find ways to shorten these comments without losing clarity
though.

+  bool subtype_is_trusted = data_message_stanza.category() ==
+                            chrome_build_info_.category_for_subtypes;
+  bool use_subtype = subtype_is_trusted && !subtype.empty();
+  std::string app_id = use_subtype ? subtype : data_message_stanza.category();
+
   MessageType message_type = DATA_MESSAGE;
-  MessageData::iterator iter = message_data.find(kMessageTypeKey);
-  if (iter != message_data.end()) {
-    message_type = DecodeMessageType(iter->second);
-    message_data.erase(iter);
+  MessageData::iterator type_iter = message_data.find(kMessageTypeKey);
+  if (type_iter != message_data.end()) {
+    message_type = DecodeMessageType(type_iter->second);
+    message_data.erase(type_iter);
   }
 
   switch (message_type) {
     case DATA_MESSAGE:
-      HandleIncomingDataMessage(data_message_stanza, message_data);
+      HandleIncomingDataMessage(app_id, use_subtype, data_message_stanza,
+                                message_data);
       break;
     case DELETED_MESSAGES:
-      recorder_.RecordDataMessageReceived(data_message_stanza.category(),
-                                          data_message_stanza.from(),
-                                          data_message_stanza.ByteSize(),
-                                          true,
+      recorder_.RecordDataMessageReceived(app_id, data_message_stanza.from(),
+                                          data_message_stanza.ByteSize(), true,
                                           GCMStatsRecorder::DELETED_MESSAGES);
-      delegate_->OnMessagesDeleted(data_message_stanza.category());
+      delegate_->OnMessagesDeleted(app_id);
       break;
     case SEND_ERROR:
-      HandleIncomingSendError(data_message_stanza, message_data);
+      HandleIncomingSendError(app_id, data_message_stanza, message_data);
       break;
     case UNKNOWN:
     default:  // Treat default the same as UNKNOWN.
       DVLOG(1) << "Unknown message_type received. Message ignored. "
-               << "App ID: " << data_message_stanza.category() << ".";
+               << "App ID: " << app_id << ".";
       break;
   }
 }
 
 void GCMClientImpl::HandleIncomingDataMessage(
+    const std::string& app_id,
+    bool use_subtype,
     const mcs_proto::DataMessageStanza& data_message_stanza,
     MessageData& message_data) {
-  std::string app_id = data_message_stanza.category();
   std::string sender = data_message_stanza.from();
 
   // Drop the message when the app is not registered for the sender of the
   // message.
   bool registered = false;
 
   // First, find among all GCM registrations.
   std::unique_ptr<GCMRegistrationInfo> gcm_registration(
       new GCMRegistrationInfo);
   gcm_registration->app_id = app_id;
   auto gcm_registration_iter = registrations_.find(
       make_linked_ptr<RegistrationInfo>(gcm_registration.release()));
   if (gcm_registration_iter != registrations_.end()) {
     GCMRegistrationInfo* cached_gcm_registration =
         GCMRegistrationInfo::FromRegistrationInfo(
             gcm_registration_iter->first.get());
     if (cached_gcm_registration &&
         std::find(cached_gcm_registration->sender_ids.begin(),
                   cached_gcm_registration->sender_ids.end(),
                   sender) != cached_gcm_registration->sender_ids.end()) {
       registered = true;
     }
   }
 
   // Then, find among all InstanceID registrations.
   if (!registered) {
-    std::unique_ptr<InstanceIDTokenInfo> instance_id_token(
-        new InstanceIDTokenInfo);
+    linked_ptr<InstanceIDTokenInfo> instance_id_token(new InstanceIDTokenInfo);
     instance_id_token->app_id = app_id;
+    instance_id_token->use_subtype = use_subtype;
     instance_id_token->authorized_entity = sender;
     instance_id_token->scope = kGCMScope;
-    auto instance_id_token_iter = registrations_.find(
-        make_linked_ptr<RegistrationInfo>(instance_id_token.release()));
-    if (instance_id_token_iter != registrations_.end())
-      registered = true;
+
+    auto instance_id_token_iter = registrations_.find(instance_id_token);
+    if (instance_id_token_iter != registrations_.end()) {
+      if (InstanceIDsDifferOnlyInUseSubtype(
+              instance_id_token.get(), instance_id_token_iter->first.get())) {
+        DLOG(ERROR) << "Incoming GCM message for " << app_id
+                    << " had use_subtype = " << use_subtype
+                    << " which conflicts with stored registration";
+      } else {
+        registered = true;
+      }
+    }
   }
 
   recorder_.RecordDataMessageReceived(app_id, sender,
       data_message_stanza.ByteSize(), registered,
       GCMStatsRecorder::DATA_MESSAGE);
   if (!registered)
     return;
 
   IncomingMessage incoming_message;
   incoming_message.sender_id = data_message_stanza.from();
   if (data_message_stanza.has_token())
     incoming_message.collapse_key = data_message_stanza.token();
   incoming_message.data = message_data;
   incoming_message.raw_data = data_message_stanza.raw_data();
 
   delegate_->OnMessageReceived(app_id, incoming_message);
 }
 
 void GCMClientImpl::HandleIncomingSendError(
+    const std::string& app_id,
     const mcs_proto::DataMessageStanza& data_message_stanza,
     MessageData& message_data) {
   SendErrorDetails send_error_details;
   send_error_details.additional_data = message_data;
   send_error_details.result = SERVER_ERROR;
 
   MessageData::iterator iter =
       send_error_details.additional_data.find(kSendErrorMessageIdKey);
   if (iter != send_error_details.additional_data.end()) {
     send_error_details.message_id = iter->second;
     send_error_details.additional_data.erase(iter);
   }
 
-  recorder_.RecordIncomingSendError(
-      data_message_stanza.category(),
-      data_message_stanza.to(),
-      data_message_stanza.id());
-  delegate_->OnMessageSendError(data_message_stanza.category(),
-                                send_error_details);
+  recorder_.RecordIncomingSendError(app_id, data_message_stanza.to(),
+                                    data_message_stanza.id());
+  delegate_->OnMessageSendError(app_id, send_error_details);
 }
 
 bool GCMClientImpl::HasStandaloneRegisteredApp() const {
   if (registrations_.empty())
     return false;
   // Note that account mapper is not counted as a standalone app since it is
   // automatically started when other app uses GCM.
   return registrations_.size() > 1 ||
          !ExistsGCMRegistrationInMap(registrations_, kGCMAccountMapperAppId);
 }
 
 }  // namespace gcm