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;
+}
+
 }  // 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 407 matching lines @@
                                       std::string* extra_data) {
   DCHECK(instance_id && extra_data);
 
   auto iter = instance_id_data_.find(app_id);
   if (iter == instance_id_data_.end())
     return;
   *instance_id = iter->second.first;
   *extra_data = iter->second.second;
 }
 
+bool GCMClientImpl::CategoryHasSubtype(const std::string& category) {
+  // 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).
+  //
+  // So a given Chrome profile always passes the same string (e.g.
+  // "com.chrome.stable.macosx") as the category when registering with a
+  // subtypes, 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).
+
+  return category == chrome_build_info_.category_for_subtypes;
+}
+
 void GCMClientImpl::AddHeartbeatInterval(const std::string& scope,
                                          int interval_ms) {
   DCHECK(mcs_client_);
   mcs_client_->AddHeartbeatInterval(scope, interval_ms);
 }
 
 void GCMClientImpl::RemoveHeartbeatInterval(const std::string& scope) {
   DCHECK(mcs_client_);
   mcs_client_->RemoveHeartbeatInterval(scope);
 }
@@ skipping 165 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
+  // GCMClient::CategoryHasSubtype.
+  CHECK(!CategoryHasSubtype(registration_info->app_id));
+
   // 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 27 matching lines @@
           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;
   }
 
   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,
+      instance_id_token_info && instance_id_token_info->use_subtype,

[Peter Beverloo, 2016/07/22 12:17:03]

nit: extract to a local bool w/ an appropriate name

[johnme, 2016/07/26 17:11:55]

Done.

+      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()));
   }
 
   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,
+      instance_id_token_info && instance_id_token_info->use_subtype,

[Peter Beverloo, 2016/07/22 12:17:03]

nit: extract to a local bool w/ an appropriate name

[johnme, 2016/07/26 17:11:55]

Done.

+      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 = "";

[Peter Beverloo, 2016/07/22 12:17:03]

nit: no init

[johnme, 2016/07/26 17:11:55]

Done.

+  auto subtype_iter = message_data.find(kSubtypeKey);

[Peter Beverloo, 2016/07/22 12:17:03]

This is using "subtype", which is potentially untrusted, while we agreed to
start using "gcm.subtype", which is trusted. This is not yet supported by the
server, though, so we should probably keep this code, but include a TODO for
simply deferring to the "gcm.subtype" value when present?

[johnme, 2016/07/26 17:11:55]

Added TODO.

+  if (subtype_iter != message_data.end()) {
+    subtype = subtype_iter->second;
+    message_data.erase(subtype_iter);
+  }
+
+  // If CategoryHasSubtype returns false, the subtype is untrusted input set by
+  // the sender, and we should ignore it.
+  bool use_subtype =
+      !subtype.empty() && CategoryHasSubtype(data_message_stanza.category());
+  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

[Peter Beverloo, 2016/07/22 12:17:03]

Is this DLOG necessary? It would only happen when the server sends an invalid
message to a debug build of Chrome, so (a) how would anyone ever see this
message, and (b) what are they supposed to do when it happens?

[johnme, 2016/07/26 17:11:55]

a) it's most likely to happen if the value of use_subtype is incorrect when we
register (or if the GCM server swaps the fields round, but that's very
unlikely).

b) hopefully file a bug?

+                    << " 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