[EasyUnlock] Fixing the setup connection finder and message format.

BUG=

Review-Url: https://codereview.chromium.org/2849493002
Cr-Commit-Position: refs/heads/master@{#467937}

chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_api.cc

@@ -37,7 +37,7 @@
 #include "components/cryptauth/proto/cryptauth_api.pb.h"
 #include "components/cryptauth/remote_device.h"
 #include "components/cryptauth/secure_message_delegate.h"
-#include "components/proximity_auth/bluetooth_low_energy_connection_finder.h"
+#include "components/proximity_auth/bluetooth_low_energy_setup_connection_finder.h"
 #include "components/proximity_auth/bluetooth_util.h"
 #include "components/proximity_auth/logging/logging.h"
 #include "components/proximity_auth/proximity_auth_client.h"
@@ -1083,7 +1083,15 @@ bool EasyUnlockPrivateFindSetupConnectionFunction::RunAsync() {
       easy_unlock_private::FindSetupConnection::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
 
-  // TODO(tengs): Reimplement scanning for the remote device.
+  // Creates a BLE connection finder to look for any device advertising
+  // |params->setup_service_uuid|.
+  connection_finder_.reset(
+      new proximity_auth::BluetoothLowEnergySetupConnectionFinder(
+          params->setup_service_uuid,
+          cryptauth::BluetoothThrottlerImpl::GetInstance()));
+
+  connection_finder_->Find(base::Bind(
+      &EasyUnlockPrivateFindSetupConnectionFunction::OnConnectionFound, this));
 
   timer_.reset(new base::OneShotTimer());
   timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(params->time_out),

chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_connection_manager.cc

@@ -99,11 +99,10 @@ bool EasyUnlockPrivateConnectionManager::Disconnect(const Extension* extension,
 bool EasyUnlockPrivateConnectionManager::SendMessage(
     const Extension* extension,
     int connection_id,
-    const std::string& payload) {
+    const std::string& message_body) {
   Connection* connection = GetConnection(extension->id(), connection_id);
   if (connection && connection->IsConnected()) {
-    connection->SendMessage(base::MakeUnique<WireMessage>(
-        payload, std::string(kEasyUnlockFeatureName)));
+    connection->SendMessage(base::MakeUnique<WireMessage>(message_body));
     return true;
   }
   return false;
@@ -128,15 +127,10 @@ void EasyUnlockPrivateConnectionManager::OnConnectionStatusChanged(
 void EasyUnlockPrivateConnectionManager::OnMessageReceived(
     const Connection& connection,
     const WireMessage& message) {
-  if (message.feature() != std::string(kEasyUnlockFeatureName)) {
-    // Only process messages received as part of EasyUnlock.
-    return;
-  }
-
   std::string event_name = api::easy_unlock_private::OnDataReceived::kEventName;
   events::HistogramValue histogram_value =
       events::EASY_UNLOCK_PRIVATE_ON_DATA_RECEIVED;
-  std::vector<char> data(message.payload().begin(), message.payload().end());
+  std::vector<char> data(message.body().begin(), message.body().end());
   std::unique_ptr<base::ListValue> args =
       api::easy_unlock_private::OnDataReceived::Create(0, data);
   DispatchConnectionEvent(event_name, histogram_value, &connection,

chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_connection_manager.h

@@ -47,11 +47,11 @@ class EasyUnlockPrivateConnectionManager
   // |connection_id| is valid.
   bool Disconnect(const Extension* extension, int connection_id);
 
-  // Sends |payload| through the connection with |connection_id|. Returns true
-  // if |connection_id| is valid.
+  // Sends |message_body| through the connection with |connection_id|. Returns
+  // true if |connection_id| is valid.
   bool SendMessage(const Extension* extension,
                    int connection_id,
-                   const std::string& payload);
+                   const std::string& message_body);
 
   // Returns the Bluetooth address of the device connected with a given
   // |connection_id|, and an empty string if |connection_id| was not found.

components/cryptauth/wire_message.cc

@@ -20,8 +20,11 @@
 // The wire messages have a simple format:
 // [ message version ] [ body length ] [ JSON body ]
 //       1 byte            2 bytes      body length
-// The JSON body contains two fields: an optional permit_id field and a required
-// data field.
+// When sending encrypted messages, the JSON body contains two fields: an
+// optional |permit_id| field and a required |payload| field.
+//
+// For non-encrypted messages, the message itself is the JSON body, and it
+// doesn't have a |payload| field.
 
 namespace cryptauth {
 namespace {
@@ -104,8 +107,14 @@ std::unique_ptr<WireMessage> WireMessage::Deserialize(
   DCHECK(success);
 
   std::string payload_base64;
-  if (!body->GetString(kPayloadKey, &payload_base64) ||
-      payload_base64.empty()) {
+  if (!body->GetString(kPayloadKey, &payload_base64)) {
+    // The body is a valid JSON, but it doesn't contain a |payload| field. It
+    // must be a non-encrypted message.
+    return base::WrapUnique(
+        new WireMessage(serialized_message.substr(kHeaderLength)));
+  }
+
+  if (payload_base64.empty()) {
     PA_LOG(WARNING) << "Error: Missing payload.";
     return nullptr;
   }
@@ -127,24 +136,29 @@ std::unique_ptr<WireMessage> WireMessage::Deserialize(
 }
 
 std::string WireMessage::Serialize() const {
-  if (payload_.empty()) {
-    PA_LOG(ERROR) << "Failed to serialize empty wire message.";
-    return std::string();
-  }
-
-  // Create JSON body containing permit id and payload.
-  base::DictionaryValue body;
-
-  std::string base64_payload;
-  base::Base64UrlEncode(payload_, base::Base64UrlEncodePolicy::INCLUDE_PADDING,
-                        &base64_payload);
-  body.SetString(kPayloadKey, base64_payload);
-  body.SetString(kFeatureKey, feature_);
-
   std::string json_body;
-  if (!base::JSONWriter::Write(body, &json_body)) {
-    PA_LOG(ERROR) << "Failed to convert WireMessage body to JSON: " << body;
-    return std::string();
+  if (body_.empty()) {
+    if (payload_.empty()) {
+      PA_LOG(ERROR) << "Failed to serialize empty wire message.";
+      return std::string();
+    }
+
+    // Create JSON body containing permit id and payload.
+    base::DictionaryValue body;
+
+    std::string base64_payload;
+    base::Base64UrlEncode(payload_,
+                          base::Base64UrlEncodePolicy::INCLUDE_PADDING,
+                          &base64_payload);
+    body.SetString(kPayloadKey, base64_payload);
+    body.SetString(kFeatureKey, feature_);
+
+    if (!base::JSONWriter::Write(body, &json_body)) {
+      PA_LOG(ERROR) << "Failed to convert WireMessage body to JSON: " << body;
+      return std::string();
+    }
+  } else {
+    json_body = body_;
   }
 
   // Create header containing version and payload size.
@@ -170,4 +184,6 @@ std::string WireMessage::Serialize() const {
 WireMessage::WireMessage(const std::string& payload, const std::string& feature)
     : payload_(payload), feature_(feature) {}
 
+WireMessage::WireMessage(const std::string& body) : body_(body) {}
+
 }  // namespace cryptauth

components/cryptauth/wire_message.h

@@ -16,6 +16,11 @@ class WireMessage {
  public:
   // Creates a WireMessage containing |payload| for feature |feature|.
   explicit WireMessage(const std::string& payload, const std::string& feature);
+
+  // Creates a WireMessage containing |body| (a serialized JSON) as the message
+  // body.
+  explicit WireMessage(const std::string& body);
+
   virtual ~WireMessage();
 
   // Returns the deserialized message from |serialized_message|, or nullptr if
@@ -32,6 +37,7 @@ class WireMessage {
 
   const std::string& payload() const { return payload_; }
   const std::string& feature() const { return feature_; }
+  const std::string& body() const { return body_; }
 
  private:
   // The message payload.
@@ -41,6 +47,10 @@ class WireMessage {
   // EasyUnlock).
   const std::string feature_;
 
+  // The message body. When this is set |payload_| and |feature_| are empty, and
+  // vice-versa.
+  const std::string body_;
+
   DISALLOW_COPY_AND_ASSIGN(WireMessage);
 };
 

components/cryptauth/wire_message_unittest.cc

@@ -88,14 +88,15 @@ TEST(CryptAuthWireMessageTest, Deserialize_BodyIsNotADictionary) {
   EXPECT_FALSE(message);
 }
 
-TEST(CryptAuthWireMessageTest, Deserialize_BodyLacksPayload) {
+TEST(CryptAuthWireMessageTest, Deserialize_NonEncryptedMessage) {
   bool is_incomplete;
   std::string header("\3\0\x02", 3);
   std::string bytes = header + "{}";
   std::unique_ptr<WireMessage> message =
       WireMessage::Deserialize(bytes, &is_incomplete);
   EXPECT_FALSE(is_incomplete);
-  EXPECT_FALSE(message);
+  ASSERT_TRUE(message);
+  EXPECT_EQ("{}", message->body());
 }
 
 TEST(CryptAuthWireMessageTest, Deserialize_BodyHasEmptyPayload) {

components/proximity_auth/BUILD.gn

@@ -12,6 +12,8 @@ static_library("proximity_auth") {
     "bluetooth_connection_finder.h",
     "bluetooth_low_energy_connection_finder.cc",
     "bluetooth_low_energy_connection_finder.h",
+    "bluetooth_low_energy_setup_connection_finder.cc",
+    "bluetooth_low_energy_setup_connection_finder.h",
     "bluetooth_util.cc",
     "bluetooth_util.h",
     "bluetooth_util_chromeos.cc",

components/proximity_auth/bluetooth_low_energy_connection_finder.cc

@@ -43,16 +43,19 @@ BluetoothLowEnergyConnectionFinder::BluetoothLowEnergyConnectionFinder(
     cryptauth::BluetoothThrottler* bluetooth_throttler)
     : BluetoothLowEnergyConnectionFinder(
           remote_device,
+          kBLEGattServiceUUID,
           beacon_seeds,
           base::MakeUnique<cryptauth::BackgroundEidGenerator>(),
           bluetooth_throttler) {}
 
 BluetoothLowEnergyConnectionFinder::BluetoothLowEnergyConnectionFinder(
     const cryptauth::RemoteDevice remote_device,
+    const std::string& service_uuid,
     const std::vector<cryptauth::BeaconSeed>& beacon_seeds,
     std::unique_ptr<cryptauth::BackgroundEidGenerator> eid_generator,
     cryptauth::BluetoothThrottler* bluetooth_throttler)
     : remote_device_(remote_device),
+      service_uuid_(service_uuid),
       beacon_seeds_(beacon_seeds),
       eid_generator_(std::move(eid_generator)),
       bluetooth_throttler_(bluetooth_throttler),
@@ -231,8 +234,7 @@ BluetoothLowEnergyConnectionFinder::CreateConnection(
     const std::string& device_address) {
   return cryptauth::weave::BluetoothLowEnergyWeaveClientConnection::Factory::
       NewInstance(remote_device_, device_address, adapter_,
-                  device::BluetoothUUID(kBLEGattServiceUUID),
-                  bluetooth_throttler_);
+                  device::BluetoothUUID(service_uuid_), bluetooth_throttler_);
 }
 
 void BluetoothLowEnergyConnectionFinder::OnConnectionStatusChanged(

components/proximity_auth/bluetooth_low_energy_connection_finder.h

@@ -69,6 +69,7 @@ class BluetoothLowEnergyConnectionFinder
  protected:
   BluetoothLowEnergyConnectionFinder(
       const cryptauth::RemoteDevice remote_device,
+      const std::string& service_uuid,
       const std::vector<cryptauth::BeaconSeed>& beacon_seeds,
       std::unique_ptr<cryptauth::BackgroundEidGenerator> eid_generator,
       cryptauth::BluetoothThrottler* bluetooth_throttler);
@@ -78,6 +79,9 @@ class BluetoothLowEnergyConnectionFinder
   virtual std::unique_ptr<cryptauth::Connection> CreateConnection(
       const std::string& device_address);
 
+  // Checks if |device| is advertising the right EID.
+  virtual bool IsRightDevice(device::BluetoothDevice* device);
+
  private:
   // Callback to be called when the Bluetooth adapter is initialized.
   void OnAdapterInitialized(scoped_refptr<device::BluetoothAdapter> adapter);
@@ -99,10 +103,6 @@ class BluetoothLowEnergyConnectionFinder
   // Stops the discovery session given by |discovery_session_|.
   void StopDiscoverySession();
 
-  // Checks if |device| is the right device: (i) has the adversement data or
-  // (ii) is paired and is the same as |remote_device|.
-  bool IsRightDevice(device::BluetoothDevice* device);
-
   // Restarts the discovery session after creating |connection_| fails.
   void RestartDiscoverySessionAsync();
 
@@ -115,6 +115,9 @@ class BluetoothLowEnergyConnectionFinder
   // |advertised_name_|.
   cryptauth::RemoteDevice remote_device_;
 
+  // The UUID of the service used by the Weave socket.
+  std::string service_uuid_;
+
   // The BeaconSeeds of the |remote_device|.
   std::vector<cryptauth::BeaconSeed> beacon_seeds_;
 

components/proximity_auth/bluetooth_low_energy_connection_finder_unittest.cc

@@ -44,6 +44,7 @@ using device::MockBluetoothDevice;
 namespace proximity_auth {
 namespace {
 
+const char kBLEGattServiceUUID[] = "b3b7e28e-a000-3e17-bd86-6e97b9e28c11";
 const char kAdvertisementUUID[] = "0000fe50-0000-1000-8000-00805f9b34fb";
 const int8_t kRssi = -30;
 const char kEidForPreviousTimeQuantum[] = "\x12\x34";
@@ -83,6 +84,7 @@ class MockBluetoothLowEnergyConnectionFinder
   MockBluetoothLowEnergyConnectionFinder()
       : BluetoothLowEnergyConnectionFinder(
             cryptauth::CreateLERemoteDeviceForTest(),
+            kBLEGattServiceUUID,
             CreateBeaconSeeds(),
             base::MakeUnique<FakeEidGenerator>(this),
             nullptr) {}

components/proximity_auth/bluetooth_low_energy_setup_connection_finder.cc

+// Copyright 2017 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/proximity_auth/bluetooth_low_energy_setup_connection_finder.h"
+
+#include <string>
+
+#include "base/memory/ptr_util.h"
+#include "components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h"
+#include "components/cryptauth/bluetooth_throttler.h"
+#include "components/cryptauth/connection.h"
+#include "components/proximity_auth/logging/logging.h"
+#include "device/bluetooth/bluetooth_device.h"
+#include "device/bluetooth/bluetooth_uuid.h"
+
+using device::BluetoothDevice;
+
+namespace proximity_auth {
+
+BluetoothLowEnergySetupConnectionFinder::
+    BluetoothLowEnergySetupConnectionFinder(
+        const std::string& remote_service_uuid,
+        cryptauth::BluetoothThrottler* bluetooth_throttler)
+    : BluetoothLowEnergyConnectionFinder(
+          cryptauth::RemoteDevice(),
+          remote_service_uuid,
+          std::vector<cryptauth::BeaconSeed>(),
+          base::MakeUnique<cryptauth::BackgroundEidGenerator>(),
+          bluetooth_throttler),
+      remote_service_uuid_(device::BluetoothUUID(remote_service_uuid)) {}
+
+bool BluetoothLowEnergySetupConnectionFinder::IsRightDevice(
+    BluetoothDevice* device) {
+  if (!device)
+    return false;
+
+  BluetoothDevice::UUIDSet uuids = device->GetUUIDs();
+  PA_LOG(INFO) << "Device " << device->GetAddress() << " has " << uuids.size()
+               << " services.";
+  return base::ContainsKey(uuids, remote_service_uuid_);
+}
+
+}  // namespace proximity_auth

components/proximity_auth/bluetooth_low_energy_setup_connection_finder.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_PROXIMITY_AUTH_BLUETOOTH_LOW_ENERGY_SETUP_CONNECTION_FINDER_H
+#define COMPONENTS_PROXIMITY_AUTH_BLUETOOTH_LOW_ENERGY_SETUP_CONNECTION_FINDER_H
+
+#include <string>
+
+#include "base/macros.h"
+#include "components/cryptauth/bluetooth_throttler.h"
+#include "components/cryptauth/remote_device.h"
+#include "components/proximity_auth/bluetooth_low_energy_connection_finder.h"
+#include "device/bluetooth/bluetooth_device.h"
+
+namespace proximity_auth {
+
+// This cryptauth::ConnectionFinder implementation is specialized in finding a
+// Bluetooth Low Energy remote device based on the advertised service UUID.
+class BluetoothLowEnergySetupConnectionFinder
+    : public BluetoothLowEnergyConnectionFinder {
+ public:
+  // Finds (and connects) to a Bluetooth low energy device, based on the UUID
+  // advertised by the remote device.
+  //
+  // |remote_service_uuid|: The UUID of the service used to send/receive data in
+  // remote device.
+  // |bluetooth_throttler|: The reconnection throttler.
+  BluetoothLowEnergySetupConnectionFinder(
+      const std::string& remote_service_uuid,
+      cryptauth::BluetoothThrottler* bluetooth_throttler);
+
+ private:
+  // Checks if |device| is the right device, that is is adversing tthe right
+  // service UUID.
+  bool IsRightDevice(device::BluetoothDevice* device) override;
+
+  // The UUID of the service advertised by the remote device.
+  device::BluetoothUUID remote_service_uuid_;
+
+  DISALLOW_COPY_AND_ASSIGN(BluetoothLowEnergySetupConnectionFinder);
+};
+
+}  // namespace proximity_auth
+
+#endif  // COMPONENTS_PROXIMITY_AUTH_BLUETOOTH_LOW_ENERGY_SETUP_CONNECTION_FINDER_H