Remove associated keying material when unregistering from GCM

components/gcm_driver/gcm_driver_desktop_unittest.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_driver_desktop.h"
 
 #include <stdint.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 102 matching lines @@
   FakeGCMConnectionObserver* gcm_connection_observer() {
     return gcm_connection_observer_.get();
   }
   const std::string& registration_id() const { return registration_id_; }
   GCMClient::Result registration_result() const { return registration_result_; }
   const std::string& send_message_id() const { return send_message_id_; }
   GCMClient::Result send_result() const { return send_result_; }
   GCMClient::Result unregistration_result() const {
     return unregistration_result_;
   }
+  const std::string& p256dh() const { return p256dh_; }
+  const std::string& auth_secret() const { return auth_secret_; }
 
   void PumpIOLoop();
 
   void ClearResults();
 
   bool HasAppHandlers() const;
   FakeGCMClient* GetGCMClient();
 
   void CreateDriver();
   void ShutdownDriver();
   void AddAppHandlers();
   void RemoveAppHandlers();
 
   void Register(const std::string& app_id,
                 const std::vector<std::string>& sender_ids,
                 WaitToFinish wait_to_finish);
   void Send(const std::string& app_id,
             const std::string& receiver_id,
             const OutgoingMessage& message,
             WaitToFinish wait_to_finish);
+  void GetEncryptionInfo(const std::string& app_id,
+                         WaitToFinish wait_to_finish);
   void Unregister(const std::string& app_id, WaitToFinish wait_to_finish);
 
   void WaitForAsyncOperation();
 
   void RegisterCompleted(const std::string& registration_id,
                          GCMClient::Result result);
   void SendCompleted(const std::string& message_id, GCMClient::Result result);
+  void GetEncryptionInfoCompleted(const std::string& p256dh,
+                                  const std::string& auth_secret);
   void UnregisterCompleted(GCMClient::Result result);
 
   const base::Closure& async_operation_completed_callback() const {
     return async_operation_completed_callback_;
   }
   void set_async_operation_completed_callback(const base::Closure& callback) {
     async_operation_completed_callback_ = callback;
   }
 
  private:
   base::ScopedTempDir temp_dir_;
   TestingPrefServiceSimple prefs_;
-  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   base::MessageLoopForUI message_loop_;
   base::Thread io_thread_;
   base::FieldTrialList field_trial_list_;
   scoped_ptr<GCMDriverDesktop> driver_;
   scoped_ptr<FakeGCMAppHandler> gcm_app_handler_;
   scoped_ptr<FakeGCMConnectionObserver> gcm_connection_observer_;
 
   base::Closure async_operation_completed_callback_;
 
   std::string registration_id_;
   GCMClient::Result registration_result_;
   std::string send_message_id_;
   GCMClient::Result send_result_;
   GCMClient::Result unregistration_result_;
+  std::string p256dh_;
+  std::string auth_secret_;
 
   DISALLOW_COPY_AND_ASSIGN(GCMDriverTest);
 };
 
 GCMDriverTest::GCMDriverTest()
-    : task_runner_(new base::TestSimpleTaskRunner()),
-      io_thread_("IOThread"),
+    : io_thread_("IOThread"),
       field_trial_list_(NULL),
       registration_result_(GCMClient::UNKNOWN_ERROR),
       send_result_(GCMClient::UNKNOWN_ERROR),
       unregistration_result_(GCMClient::UNKNOWN_ERROR) {
 }
 
 GCMDriverTest::~GCMDriverTest() {
 }
 
 void GCMDriverTest::SetUp() {
@@ skipping 34 matching lines @@
   return !driver_->app_handlers().empty();
 }
 
 FakeGCMClient* GCMDriverTest::GetGCMClient() {
   return static_cast<FakeGCMClient*>(driver_->GetGCMClientForTesting());
 }
 
 void GCMDriverTest::CreateDriver() {
   scoped_refptr<net::URLRequestContextGetter> request_context =
       new net::TestURLRequestContextGetter(io_thread_.task_runner());
-  // TODO(johnme): Need equivalent test coverage of GCMDriverAndroid.
   driver_.reset(new GCMDriverDesktop(
       scoped_ptr<GCMClientFactory>(new FakeGCMClientFactory(
           base::ThreadTaskRunnerHandle::Get(), io_thread_.task_runner())),
       GCMClient::ChromeBuildInfo(), "http://channel.status.request.url",
       "user-agent-string", &prefs_, temp_dir_.path(), request_context,
       base::ThreadTaskRunnerHandle::Get(), io_thread_.task_runner(),
-      task_runner_));
+      message_loop_.task_runner()));
 
   gcm_app_handler_.reset(new FakeGCMAppHandler);
   gcm_connection_observer_.reset(new FakeGCMConnectionObserver);
 
   driver_->AddConnectionObserver(gcm_connection_observer_.get());
 }
 
 void GCMDriverTest::ShutdownDriver() {
   if (gcm_connection_observer())
     driver()->RemoveConnectionObserver(gcm_connection_observer());
@@ skipping 31 matching lines @@
   async_operation_completed_callback_ = run_loop.QuitClosure();
   driver_->Send(app_id,
                 receiver_id,
                 message,
                 base::Bind(&GCMDriverTest::SendCompleted,
                            base::Unretained(this)));
   if (wait_to_finish == WAIT)
     run_loop.Run();
 }
 
+void GCMDriverTest::GetEncryptionInfo(const std::string& app_id,
+                                      WaitToFinish wait_to_finish) {
+  base::RunLoop run_loop;
+  async_operation_completed_callback_ = run_loop.QuitClosure();
+  driver_->GetEncryptionInfo(
+      app_id, base::Bind(&GCMDriverTest::GetEncryptionInfoCompleted,
+                         base::Unretained(this)));
+  if (wait_to_finish == WAIT)
+    run_loop.Run();
+}
+
 void GCMDriverTest::Unregister(const std::string& app_id,
                                WaitToFinish wait_to_finish) {
   base::RunLoop run_loop;
   async_operation_completed_callback_ = run_loop.QuitClosure();
   driver_->Unregister(app_id,
                       base::Bind(&GCMDriverTest::UnregisterCompleted,
                                  base::Unretained(this)));
   if (wait_to_finish == WAIT)
     run_loop.Run();
 }
@@ skipping 13 matching lines @@
 }
 
 void GCMDriverTest::SendCompleted(const std::string& message_id,
                                   GCMClient::Result result) {
   send_message_id_ = message_id;
   send_result_ = result;
   if (!async_operation_completed_callback_.is_null())
     async_operation_completed_callback_.Run();
 }
 
+void GCMDriverTest::GetEncryptionInfoCompleted(const std::string& p256dh,
+                                               const std::string& auth_secret) {
+  p256dh_ = p256dh;
+  auth_secret_ = auth_secret;
+  if (!async_operation_completed_callback_.is_null())

[johnme, 2016/02/16 18:36:13]

FWIW, this can never be null, since you never reset
async_operation_completed_callback_. But I guess no harm in being defensive.

[Peter Beverloo, 2016/02/16 21:50:50]

Just being consistent with the rest of the file - we can clean up a bunch of
stuff in here in due course.

+    async_operation_completed_callback_.Run();
+}
+
 void GCMDriverTest::UnregisterCompleted(GCMClient::Result result) {
   unregistration_result_ = result;
   if (!async_operation_completed_callback_.is_null())
     async_operation_completed_callback_.Run();
 }
 
 TEST_F(GCMDriverTest, Create) {
   // Create GCMDriver first. By default GCM is set to delay start.
   CreateDriver();
   EXPECT_FALSE(driver()->IsStarted());
@@ skipping 311 matching lines @@
   Register(kTestAppID1, sender_ids, GCMDriverTest::WAIT);
 
   EXPECT_FALSE(registration_id().empty());
   EXPECT_EQ(GCMClient::SUCCESS, registration_result());
 
   Unregister(kTestAppID1, GCMDriverTest::WAIT);
 
   EXPECT_EQ(GCMClient::SUCCESS, unregistration_result());
 }
 
+TEST_F(GCMDriverFunctionalTest, UnregisterRemovesEncryptionInfo) {
+  std::vector<std::string> sender_ids;
+  sender_ids.push_back("sender1");
+  Register(kTestAppID1, sender_ids, GCMDriverTest::WAIT);
+
+  EXPECT_FALSE(registration_id().empty());
+  EXPECT_EQ(GCMClient::SUCCESS, registration_result());
+
+  GetEncryptionInfo(kTestAppID1, GCMDriverTest::WAIT);
+
+  EXPECT_FALSE(p256dh().empty());
+  EXPECT_FALSE(auth_secret().empty());
+
+  const std::string app_p256dh = p256dh();
+  const std::string app_auth_secret = auth_secret();
+
+  GetEncryptionInfo(kTestAppID1, GCMDriverTest::WAIT);
+
+  EXPECT_EQ(app_p256dh, p256dh());
+  EXPECT_EQ(app_auth_secret, auth_secret());
+
+  Unregister(kTestAppID1, GCMDriverTest::WAIT);
+
+  EXPECT_EQ(GCMClient::SUCCESS, unregistration_result());
+
+  GetEncryptionInfo(kTestAppID1, GCMDriverTest::WAIT);
+
+  // The GCMKeyStore eagerly creates new keying material for registrations that

[johnme, 2016/02/16 18:36:13]

It seems odd (and a potential security foot-gun for us) that you can generate
and persist encryption info without having a valid registration?

[Peter Beverloo, 2016/02/16 21:50:50]

There are two requirements that led to this -

  (1) The encryption code has to be shared between Android and desktop. While
desktop has a good database already (GCMStore down in the engine), Play Services
will do this for us on Android, but doesn't allow us to query state. As such, we
don't know, unless we want to do duplicated bookkeeping.

  (2) In order to give subscriptions that currently don't have encryption info
associated with them an upgrade path, we eagerly create this on request.

+  // don't have any associated with them, so the most appropriate check to do is
+  // to verify that the returned material is different from before.
+
+  EXPECT_NE(app_p256dh, p256dh());
+  EXPECT_NE(app_auth_secret, auth_secret());
+}
+
 TEST_F(GCMDriverFunctionalTest, UnregisterWhenAsyncOperationPending) {
   std::vector<std::string> sender_ids;
   sender_ids.push_back("sender1");
   // First start registration without waiting for it to complete.
   Register(kTestAppID1, sender_ids, GCMDriverTest::DO_NOT_WAIT);
 
   // Test that unregistration fails with async operation pending when there is a
   // registration already in progress.
   Unregister(kTestAppID1, GCMDriverTest::WAIT);
   EXPECT_EQ(GCMClient::ASYNC_OPERATION_PENDING,
@@ skipping 642 matching lines @@
   PumpUILoop();
   EXPECT_EQ(GCMClient::UNKNOWN_ERROR, unregistration_result());
 
   // DeleteToken operation will be invoked after GCMClient becomes ready.
   GetGCMClient()->PerformDelayedStart();
   WaitForAsyncOperation();
   EXPECT_EQ(GCMClient::SUCCESS, unregistration_result());
 }
 
 }  // namespace gcm