List message decryption failures on chrome://gcm-internals

This CL is basically just plumbing of a result code to an internal
diagnosis page. Decryption is done at the level of the GCM Driver,
while, for desktop, the GCMStatsRecorder is owned by the GCMClient
several layers down, on another thread.

This is a re-land of the following CL, which got reverted due to
flakiness on iOS: https://codereview.chromium.org/1616113003/

The flakiness is addresses by pumping the loops in the order in
which messages are posted to the other threads.

TBR=jianli
BUG=569127

Review URL: https://codereview.chromium.org/1641993004

Cr-Commit-Position: refs/heads/master@{#372644}

components/gcm_driver/BUILD.gn

@@ -160,6 +160,7 @@ source_set("unit_tests") {
     ":test_support",
     "//base",
     "//base/test:test_support",
+    "//components/gcm_driver/crypto",
     "//components/prefs:test_support",
     "//google_apis:test_support",
     "//google_apis/gcm:test_support",

components/gcm_driver/crypto/gcm_encryption_provider.cc

@@ -29,6 +29,27 @@ const base::FilePath::CharType kEncryptionDirectoryName[] =
 
 }  // namespace
 
+std::string GCMEncryptionProvider::ToDecryptionFailureDetailsString(
+    DecryptionFailure reason) {
+  switch(reason) {
+    case DECRYPTION_FAILURE_UNKNOWN:
+      return "Unknown failure";
+    case DECRYPTION_FAILURE_INVALID_ENCRYPTION_HEADER:
+      return "Invalid format for the Encryption header";
+    case DECRYPTION_FAILURE_INVALID_CRYPTO_KEY_HEADER:
+      return "Invalid format for the Crypto-Key header";
+    case DECRYPTION_FAILURE_NO_KEYS:
+      return "There are no associated keys with the subscription";
+    case DECRYPTION_FAILURE_INVALID_PUBLIC_KEY:
+      return "The public key in the Crypto-Key header is invalid";
+    case DECRYPTION_FAILURE_INVALID_PAYLOAD:
+      return "AES-GCM decryption failed";
+  }
+
+  NOTREACHED();
+  return "(invalid reason)";
+}
+
 GCMEncryptionProvider::GCMEncryptionProvider()
     : weak_ptr_factory_(this) {
 }

components/gcm_driver/crypto/gcm_encryption_provider.h

@@ -58,6 +58,9 @@ class GCMEncryptionProvider {
   // Callback to be invoked when a message cannot be decoded.
   using DecryptionFailedCallback = base::Callback<void(DecryptionFailure)>;
 
+  // Converts |reason| to a string describing the details of said reason.
+  static std::string ToDecryptionFailureDetailsString(DecryptionFailure reason);
+
   GCMEncryptionProvider();
   ~GCMEncryptionProvider();
 

components/gcm_driver/fake_gcm_client.cc

@@ -158,14 +158,26 @@ void FakeGCMClient::Send(const std::string& app_id,
                             weak_ptr_factory_.GetWeakPtr(), app_id, message));
 }
 
+void FakeGCMClient::RecordDecryptionFailure(
+    const std::string& app_id,
+    GCMEncryptionProvider::DecryptionFailure reason) {
+  recorder_.RecordDecryptionFailure(app_id, reason);
+}
+
 void FakeGCMClient::SetRecording(bool recording) {
+  recorder_.set_is_recording(recording);
 }
 
 void FakeGCMClient::ClearActivityLogs() {
+  recorder_.Clear();
 }
 
 GCMClient::GCMStatistics FakeGCMClient::GetStatistics() const {
-  return GCMClient::GCMStatistics();
+  GCMClient::GCMStatistics statistics;
+  statistics.is_recording = recorder_.is_recording();
+
+  recorder_.CollectActivities(&statistics.recorded_activities);
+  return statistics;
 }
 
 void FakeGCMClient::SetAccountTokens(

components/gcm_driver/fake_gcm_client.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/gcm_driver/gcm_client.h"
+#include "components/gcm_driver/gcm_stats_recorder_impl.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -57,6 +58,9 @@ class FakeGCMClient : public GCMClient {
   void Send(const std::string& app_id,
             const std::string& receiver_id,
             const OutgoingMessage& message) override;
+  void RecordDecryptionFailure(const std::string& app_id,
+                               GCMEncryptionProvider::DecryptionFailure reason)
+      override;
   void SetRecording(bool recording) override;
   void ClearActivityLogs() override;
   GCMStatistics GetStatistics() const override;
@@ -115,6 +119,7 @@ class FakeGCMClient : public GCMClient {
   scoped_refptr<base::SequencedTaskRunner> ui_thread_;
   scoped_refptr<base::SequencedTaskRunner> io_thread_;
   std::map<std::string, std::pair<std::string, std::string>> instance_id_data_;
+  GCMStatsRecorderImpl recorder_;
   base::WeakPtrFactory<FakeGCMClient> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeGCMClient);

components/gcm_driver/fake_gcm_driver.cc

@@ -74,6 +74,11 @@ void FakeGCMDriver::SendImpl(const std::string& app_id,
                              const OutgoingMessage& message) {
 }
 
+void FakeGCMDriver::RecordDecryptionFailure(
+    const std::string& app_id,
+    GCMEncryptionProvider::DecryptionFailure reason) {
+}
+
 void FakeGCMDriver::SetAccountTokens(
     const std::vector<GCMClient::AccountTokenInfo>& account_tokens) {
 }

components/gcm_driver/fake_gcm_driver.h

@@ -54,6 +54,9 @@ class FakeGCMDriver : public GCMDriver {
   void SendImpl(const std::string& app_id,
                 const std::string& receiver_id,
                 const OutgoingMessage& message) override;
+  void RecordDecryptionFailure(const std::string& app_id,
+                               GCMEncryptionProvider::DecryptionFailure reason)
+      override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FakeGCMDriver);

components/gcm_driver/gcm_activity.cc

@@ -44,6 +44,12 @@ SendingActivity::SendingActivity() {
 SendingActivity::~SendingActivity() {
 }
 
+DecryptionFailureActivity::DecryptionFailureActivity() {
+}
+
+DecryptionFailureActivity::~DecryptionFailureActivity() {
+}
+
 RecordedActivities::RecordedActivities() {
 }
 

components/gcm_driver/gcm_activity.h

@@ -64,6 +64,14 @@ struct SendingActivity : Activity {
   std::string message_id;
 };
 
+// Contains relevant data of a message decryption failure.
+struct DecryptionFailureActivity : Activity {
+  DecryptionFailureActivity();
+  ~DecryptionFailureActivity() override;
+
+  std::string app_id;
+};
+
 struct RecordedActivities {
   RecordedActivities();
   virtual ~RecordedActivities();
@@ -73,6 +81,7 @@ struct RecordedActivities {
   std::vector<RegistrationActivity> registration_activities;
   std::vector<ReceivingActivity> receiving_activities;
   std::vector<SendingActivity> sending_activities;
+  std::vector<DecryptionFailureActivity> decryption_failure_activities;
 };
 
 }  // namespace gcm

components/gcm_driver/gcm_client.h

@@ -14,6 +14,7 @@
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/gcm_driver/common/gcm_messages.h"
+#include "components/gcm_driver/crypto/gcm_encryption_provider.h"
 #include "components/gcm_driver/gcm_activity.h"
 #include "components/gcm_driver/registration_info.h"
 
@@ -268,6 +269,11 @@ class GCMClient {
                     const std::string& receiver_id,
                     const OutgoingMessage& message) = 0;
 
+  // Records a decryption failure due to |reason| for the |app_id|.
+  virtual void RecordDecryptionFailure(
+      const std::string& app_id,
+      GCMEncryptionProvider::DecryptionFailure reason) = 0;
+
   // Enables or disables internal activity recording.
   virtual void SetRecording(bool recording) = 0;
 

components/gcm_driver/gcm_client_impl.cc

@@ -1152,6 +1152,12 @@ std::string GCMClientImpl::GetStateString() const {
   }
 }
 
+void GCMClientImpl::RecordDecryptionFailure(
+    const std::string& app_id,
+    GCMEncryptionProvider::DecryptionFailure reason) {
+  recorder_.RecordDecryptionFailure(app_id, reason);
+}
+
 void GCMClientImpl::SetRecording(bool recording) {
   recorder_.set_is_recording(recording);
 }

components/gcm_driver/gcm_client_impl.h

@@ -119,6 +119,9 @@ class GCMClientImpl
   void Send(const std::string& app_id,
             const std::string& receiver_id,
             const OutgoingMessage& message) override;
+  void RecordDecryptionFailure(const std::string& app_id,
+                               GCMEncryptionProvider::DecryptionFailure reason)
+      override;
   void SetRecording(bool recording) override;
   void ClearActivityLogs() override;
   GCMStatistics GetStatistics() const override;

components/gcm_driver/gcm_driver.cc

@@ -19,11 +19,6 @@ namespace {
 
 const size_t kMaxSenders = 100;
 
-// TODO(peter): Implement an event for GCMAppHandlers that should be called
-// when decryption of an incoming message has failed.
-void DecryptionFailedCallback(
-    GCMEncryptionProvider::DecryptionFailure reason) {}
-
 }  // namespace
 
 InstanceIDHandler::InstanceIDHandler() {
@@ -280,7 +275,8 @@ void GCMDriver::DispatchMessage(const std::string& app_id,
       app_id, message,
       base::Bind(&GCMDriver::DispatchMessage,
                  weak_ptr_factory_.GetWeakPtr(), app_id),
-      base::Bind(&DecryptionFailedCallback));
+      base::Bind(&GCMDriver::RecordDecryptionFailure,
+                 weak_ptr_factory_.GetWeakPtr(), app_id));
 }
 
 void GCMDriver::RegisterAfterUnregister(

components/gcm_driver/gcm_driver.h

@@ -256,6 +256,11 @@ class GCMDriver {
                         const std::string& receiver_id,
                         const OutgoingMessage& message) = 0;
 
+  // Platform-specific implementation of recording message decryption failures.
+  virtual void RecordDecryptionFailure(
+      const std::string& app_id,
+      GCMEncryptionProvider::DecryptionFailure reason) = 0;
+
   // Runs the Register callback.
   void RegisterFinished(const std::string& app_id,
                         const std::string& registration_id,

components/gcm_driver/gcm_driver_android.cc

@@ -240,7 +240,8 @@ void GCMDriverAndroid::UnregisterImpl(const std::string& app_id) {
 }
 
 void GCMDriverAndroid::UnregisterWithSenderIdImpl(
-    const std::string& app_id, const std::string& sender_id) {
+    const std::string& app_id,
+    const std::string& sender_id) {
   JNIEnv* env = AttachCurrentThread();
 
   recorder_.RecordUnregistrationSent(app_id);
@@ -256,4 +257,10 @@ void GCMDriverAndroid::SendImpl(const std::string& app_id,
   NOTIMPLEMENTED();
 }
 
+void GCMDriverAndroid::RecordDecryptionFailure(
+    const std::string& app_id,
+    GCMEncryptionProvider::DecryptionFailure reason) {
+  recorder_.RecordDecryptionFailure(app_id, reason);
+}
+
 }  // namespace gcm

components/gcm_driver/gcm_driver_android.h

@@ -92,6 +92,9 @@ class GCMDriverAndroid : public GCMDriver,
   void SendImpl(const std::string& app_id,
                 const std::string& receiver_id,
                 const OutgoingMessage& message) override;
+  void RecordDecryptionFailure(const std::string& app_id,
+                               GCMEncryptionProvider::DecryptionFailure reason)
+      override;
 
  private:
   base::android::ScopedJavaGlobalRef<jobject> java_ref_;

components/gcm_driver/gcm_driver_desktop.cc

@@ -107,6 +107,9 @@ class GCMDriverDesktop::IOWorker : public GCMClient::Delegate {
                    const std::string& authorized_entity,
                    const std::string& scope);
 
+  void RecordDecryptionFailure(const std::string& app_id,
+                               GCMEncryptionProvider::DecryptionFailure reason);
+
   // For testing purpose. Can be called from UI thread. Use with care.
   GCMClient* gcm_client_for_testing() const { return gcm_client_.get(); }
 
@@ -496,6 +499,13 @@ void GCMDriverDesktop::IOWorker::RemoveHeartbeatInterval(
   gcm_client_->RemoveHeartbeatInterval(scope);
 }
 
+void GCMDriverDesktop::IOWorker::RecordDecryptionFailure(
+    const std::string& app_id,
+    GCMEncryptionProvider::DecryptionFailure reason) {
+  DCHECK(io_thread_->RunsTasksOnCurrentThread());
+  gcm_client_->RecordDecryptionFailure(app_id, reason);
+}
+
 GCMDriverDesktop::GCMDriverDesktop(
     scoped_ptr<GCMClientFactory> gcm_client_factory,
     const GCMClient::ChromeBuildInfo& chrome_build_info,
@@ -722,6 +732,17 @@ void GCMDriverDesktop::DoSend(const std::string& app_id,
                  message));
 }
 
+void GCMDriverDesktop::RecordDecryptionFailure(
+    const std::string& app_id,
+    GCMEncryptionProvider::DecryptionFailure reason) {
+  DCHECK(ui_thread_->RunsTasksOnCurrentThread());
+  io_thread_->PostTask(
+      FROM_HERE,
+      base::Bind(&GCMDriverDesktop::IOWorker::RecordDecryptionFailure,
+                 base::Unretained(io_worker_.get()),
+                 app_id, reason));
+}
+
 GCMClient* GCMDriverDesktop::GetGCMClientForTesting() const {
   DCHECK(ui_thread_->RunsTasksOnCurrentThread());
   return io_worker_ ? io_worker_->gcm_client_for_testing() : NULL;

components/gcm_driver/gcm_driver_desktop.h

@@ -121,6 +121,9 @@ class GCMDriverDesktop : public GCMDriver,
   void SendImpl(const std::string& app_id,
                 const std::string& receiver_id,
                 const OutgoingMessage& message) override;
+  void RecordDecryptionFailure(const std::string& app_id,
+                               GCMEncryptionProvider::DecryptionFailure reason)
+      override;
 
  private:
   class IOWorker;

components/gcm_driver/gcm_driver_desktop_unittest.cc

@@ -19,6 +19,7 @@
 #include "base/test/test_simple_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/thread.h"
+#include "components/gcm_driver/crypto/gcm_encryption_provider.h"
 #include "components/gcm_driver/fake_gcm_app_handler.h"
 #include "components/gcm_driver/fake_gcm_client.h"
 #include "components/gcm_driver/fake_gcm_client_factory.h"
@@ -810,6 +811,32 @@ TEST_F(GCMDriverFunctionalTest, MessageWithCollapseKeyReceived) {
             gcm_app_handler()->message().collapse_key);
 }
 
+TEST_F(GCMDriverFunctionalTest, EncryptedMessageReceivedError) {
+  // GCM registration has to be performed otherwise GCM will not be started.
+  Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT);
+
+  IncomingMessage message;
+
+  // All required information to trigger the encryption path, but with an
+  // invalid Crypto-Key header value to trigger an error.
+  message.data["encryption"] = "salt=ysyxqlYTgE0WvcZrmHbUbg";
+  message.data["crypto-key"] = "hey=thereisnopublickey";
+  message.sender_id = "sender";
+  message.raw_data = "foobar";
+
+  GetGCMClient()->SetRecording(true);
+  GetGCMClient()->ReceiveMessage(kTestAppID1, message);
+
+  PumpIOLoop();
+  PumpUILoop();
+  PumpIOLoop();
+
+  GCMClient::GCMStatistics statistics = GetGCMClient()->GetStatistics();
+  EXPECT_TRUE(statistics.is_recording);
+  EXPECT_EQ(
+      1u, statistics.recorded_activities.decryption_failure_activities.size());
+}
+
 TEST_F(GCMDriverFunctionalTest, MessagesDeleted) {
   // GCM registration has to be performed otherwise GCM will not be started.
   Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT);

components/gcm_driver/gcm_internals_constants.cc

@@ -33,5 +33,6 @@ const char kRegistrationInfo[] = "registrationInfo";
 const char kResendQueueSize[] = "resendQueueSize";
 const char kSendInfo[] = "sendInfo";
 const char kSendQueueSize[] = "sendQueueSize";
+const char kDecryptionFailureInfo[] = "decryptionFailureInfo";
 
 }  // namespace gcm_driver

components/gcm_driver/gcm_internals_constants.h

@@ -37,6 +37,7 @@ extern const char kRegistrationInfo[];
 extern const char kResendQueueSize[];
 extern const char kSendInfo[];
 extern const char kSendQueueSize[];
+extern const char kDecryptionFailureInfo[];
 
 }  // namespace gcm_driver