diff options
Diffstat (limited to 'chromium/components/cryptauth')
56 files changed, 2393 insertions, 1643 deletions
diff --git a/chromium/components/cryptauth/BUILD.gn b/chromium/components/cryptauth/BUILD.gn index b89eaaa91ae..dacb5510543 100644 --- a/chromium/components/cryptauth/BUILD.gn +++ b/chromium/components/cryptauth/BUILD.gn @@ -12,9 +12,6 @@ static_library("cryptauth") { "authenticator.h", "background_eid_generator.cc", "background_eid_generator.h", - "bluetooth_throttler.h", - "bluetooth_throttler_impl.cc", - "bluetooth_throttler_impl.h", "connection.cc", "connection.h", "cryptauth_access_token_fetcher.h", @@ -44,12 +41,14 @@ static_library("cryptauth") { "data_with_timestamp.h", "device_to_device_authenticator.cc", "device_to_device_authenticator.h", - "device_to_device_initiator_operations.cc", - "device_to_device_initiator_operations.h", + "device_to_device_initiator_helper.cc", + "device_to_device_initiator_helper.h", "device_to_device_secure_context.cc", "device_to_device_secure_context.h", "foreground_eid_generator.cc", "foreground_eid_generator.h", + "local_device_data_provider.cc", + "local_device_data_provider.h", "pref_names.cc", "pref_names.h", "raw_eid_generator.h", @@ -124,6 +123,8 @@ static_library("test_support") { "mock_cryptauth_client.h", "mock_foreground_eid_generator.cc", "mock_foreground_eid_generator.h", + "mock_local_device_data_provider.cc", + "mock_local_device_data_provider.h", "mock_remote_beacon_seed_fetcher.cc", "mock_remote_beacon_seed_fetcher.h", "mock_sync_scheduler.cc", @@ -148,7 +149,6 @@ source_set("unit_tests") { testonly = true sources = [ "background_eid_generator_unittest.cc", - "bluetooth_throttler_impl_unittest.cc", "connection_unittest.cc", "cryptauth_access_token_fetcher_impl_unittest.cc", "cryptauth_api_call_flow_unittest.cc", @@ -162,6 +162,7 @@ source_set("unit_tests") { "device_to_device_secure_context_unittest.cc", "fake_secure_message_delegate_unittest.cc", "foreground_eid_generator_unittest.cc", + "local_device_data_provider_unittest.cc", "raw_eid_generator_impl_unittest.cc", "remote_beacon_seed_fetcher_unittest.cc", "remote_device_loader_unittest.cc", diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.cc b/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.cc index be036e45e61..710c057a0b6 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.cc +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.cc @@ -35,7 +35,6 @@ BluetoothLowEnergyCharacteristicsFinder:: error_callback_(error_callback) { if (!adapter_) { error_callback_.Run(to_peripheral_char_, from_peripheral_char_); - ResetCallbacks(); return; } @@ -50,93 +49,78 @@ BluetoothLowEnergyCharacteristicsFinder:: BluetoothLowEnergyCharacteristicsFinder:: ~BluetoothLowEnergyCharacteristicsFinder() { - ResetCallbacks(); if (adapter_) { adapter_->RemoveObserver(this); - adapter_ = NULL; } } void BluetoothLowEnergyCharacteristicsFinder::GattCharacteristicAdded( BluetoothAdapter* adapter, BluetoothRemoteGattCharacteristic* characteristic) { - PA_LOG(INFO) << "New char found: " - << characteristic->GetUUID().canonical_value(); HandleCharacteristicUpdate(characteristic); } void BluetoothLowEnergyCharacteristicsFinder::GattDiscoveryCompleteForService( BluetoothAdapter* adapter, BluetoothRemoteGattService* service) { - if (service && service->GetUUID() == remote_service_.uuid) { - PA_LOG(INFO) << "All characteristics discovered for " - << remote_service_.uuid.canonical_value(); - - if (to_peripheral_char_.id.empty() || from_peripheral_char_.id.empty()) { - if (!error_callback_.is_null()) { - error_callback_.Run(to_peripheral_char_, from_peripheral_char_); - ResetCallbacks(); - } - } - } + if (!service || service->GetUUID() != remote_service_.uuid) + return; + + if (!to_peripheral_char_.id.empty() && !from_peripheral_char_.id.empty()) + return; + + error_callback_.Run(to_peripheral_char_, from_peripheral_char_); } void BluetoothLowEnergyCharacteristicsFinder::ScanRemoteCharacteristics( BluetoothDevice* device, const BluetoothUUID& service_uuid) { - PA_LOG(INFO) << "Scanning remote characteristics."; - if (device) { - std::vector<BluetoothRemoteGattService*> services = - device->GetGattServices(); - PA_LOG(INFO) << device->GetAddress() << " has " << services.size() - << " services."; - for (const auto* service : services) { - if (service->GetUUID() == service_uuid) { - // Right service found, now scaning its characteristics. - std::vector<device::BluetoothRemoteGattCharacteristic*> - characteristics = service->GetCharacteristics(); - PA_LOG(INFO) << "Service " << service_uuid.canonical_value() << " has " - << characteristics.size() << " characteristics."; - for (auto* characteristic : characteristics) { - HandleCharacteristicUpdate(characteristic); - } - break; - } + if (!device) + return; + + for (const auto* service : device->GetGattServices()) { + if (service->GetUUID() != service_uuid) + continue; + + // Right service found, now scaning its characteristics. + std::vector<device::BluetoothRemoteGattCharacteristic*> characteristics = + service->GetCharacteristics(); + for (auto* characteristic : characteristics) { + if (HandleCharacteristicUpdate(characteristic)) + return; } + break; } } -void BluetoothLowEnergyCharacteristicsFinder::HandleCharacteristicUpdate( +bool BluetoothLowEnergyCharacteristicsFinder::HandleCharacteristicUpdate( BluetoothRemoteGattCharacteristic* characteristic) { UpdateCharacteristicsStatus(characteristic); - if (!to_peripheral_char_.id.empty() && !from_peripheral_char_.id.empty() && - !success_callback_.is_null()) { - PA_LOG(INFO) << "Found write and read characteristics on remote device."; - success_callback_.Run(remote_service_, to_peripheral_char_, - from_peripheral_char_); - ResetCallbacks(); - } + if (to_peripheral_char_.id.empty() || from_peripheral_char_.id.empty()) + return false; + + success_callback_.Run(remote_service_, to_peripheral_char_, + from_peripheral_char_); + return true; } void BluetoothLowEnergyCharacteristicsFinder::UpdateCharacteristicsStatus( BluetoothRemoteGattCharacteristic* characteristic) { - if (characteristic && - characteristic->GetService()->GetUUID() == remote_service_.uuid) { - BluetoothUUID uuid = characteristic->GetUUID(); - if (to_peripheral_char_.uuid == uuid) - to_peripheral_char_.id = characteristic->GetIdentifier(); - if (from_peripheral_char_.uuid == uuid) - from_peripheral_char_.id = characteristic->GetIdentifier(); - - BluetoothRemoteGattService* service = characteristic->GetService(); - remote_service_.id = service->GetIdentifier(); + if (!characteristic || + characteristic->GetService()->GetUUID() != remote_service_.uuid) { + return; } -} -void BluetoothLowEnergyCharacteristicsFinder::ResetCallbacks() { - success_callback_.Reset(); - error_callback_.Reset(); + BluetoothUUID uuid = characteristic->GetUUID(); + if (to_peripheral_char_.uuid == uuid) + to_peripheral_char_.id = characteristic->GetIdentifier(); + if (from_peripheral_char_.uuid == uuid) + from_peripheral_char_.id = characteristic->GetIdentifier(); + + BluetoothRemoteGattService* service = characteristic->GetService(); + if (service) + remote_service_.id = service->GetIdentifier(); } } // namespace cryptauth diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.h b/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.h index 8741765a2d8..35afee241ff 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.h +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.h @@ -74,8 +74,9 @@ class BluetoothLowEnergyCharacteristicsFinder BluetoothLowEnergyCharacteristicsFinder(); private: - // Handles the discovery of a new characteristic. - void HandleCharacteristicUpdate( + // Handles the discovery of a new characteristic. Returns whether all + // characteristics were found. + bool HandleCharacteristicUpdate( device::BluetoothRemoteGattCharacteristic* characteristic); // Scans the remote chracteristics of the service with |uuid| in |device| @@ -89,10 +90,6 @@ class BluetoothLowEnergyCharacteristicsFinder void UpdateCharacteristicsStatus( device::BluetoothRemoteGattCharacteristic* characteristic); - // Resets |success_callback_| and |success_callback_|. This should be called - // whenever a callback is called to avoid multiple callbacks calls. - void ResetCallbacks(); - // The Bluetooth adapter where the connection was established. scoped_refptr<device::BluetoothAdapter> adapter_; diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc index d9d959aee0a..d0f0466642a 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc @@ -4,14 +4,16 @@ #include "components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h" +#include <sstream> #include <utility> #include "base/bind.h" #include "base/location.h" #include "base/memory/ptr_util.h" +#include "base/stl_util.h" #include "base/task_runner.h" #include "base/threading/thread_task_runner_handle.h" -#include "components/cryptauth/bluetooth_throttler.h" +#include "base/timer/timer.h" #include "components/cryptauth/connection_finder.h" #include "components/cryptauth/wire_message.h" #include "components/proximity_auth/logging/logging.h" @@ -35,6 +37,13 @@ const char kRXCharacteristicUUID[] = "00000100-0004-1000-8000-001A11000102"; // each message gets 3 attempts: the first one, and 2 retries. const int kMaxNumberOfRetryAttempts = 2; +// Timeouts for various status types. +const int kConnectionLatencyTimeoutSeconds = 2; +const int kGattConnectionTimeoutSeconds = 15; +const int kGattCharacteristicsTimeoutSeconds = 10; +const int kNotifySessionTimeoutSeconds = 5; +const int kConnectionResponseTimeoutSeconds = 2; + } // namespace // static @@ -48,17 +57,12 @@ BluetoothLowEnergyWeaveClientConnection::Factory::NewInstance( const RemoteDevice& remote_device, const std::string& device_address, scoped_refptr<device::BluetoothAdapter> adapter, - const device::BluetoothUUID remote_service_uuid, - BluetoothThrottler* bluetooth_throttler) { + const device::BluetoothUUID remote_service_uuid) { if (!factory_instance_) { factory_instance_ = new Factory(); } - return factory_instance_->BuildInstance( - remote_device, - device_address, - adapter, - remote_service_uuid, - bluetooth_throttler); + return factory_instance_->BuildInstance(remote_device, device_address, + adapter, remote_service_uuid); } // static @@ -72,11 +76,56 @@ BluetoothLowEnergyWeaveClientConnection::Factory::BuildInstance( const RemoteDevice& remote_device, const std::string& device_address, scoped_refptr<device::BluetoothAdapter> adapter, - const device::BluetoothUUID remote_service_uuid, - BluetoothThrottler* bluetooth_throttler) { + const device::BluetoothUUID remote_service_uuid) { return base::MakeUnique<BluetoothLowEnergyWeaveClientConnection>( - remote_device, device_address, adapter, remote_service_uuid, - bluetooth_throttler); + remote_device, device_address, adapter, remote_service_uuid); +} + +// static +base::TimeDelta BluetoothLowEnergyWeaveClientConnection::GetTimeoutForSubStatus( + SubStatus sub_status) { + switch (sub_status) { + case SubStatus::WAITING_CONNECTION_RESPONSE: + return base::TimeDelta::FromSeconds(kConnectionResponseTimeoutSeconds); + case SubStatus::WAITING_CONNECTION_LATENCY: + return base::TimeDelta::FromSeconds(kConnectionLatencyTimeoutSeconds); + case SubStatus::WAITING_GATT_CONNECTION: + return base::TimeDelta::FromSeconds(kGattConnectionTimeoutSeconds); + case SubStatus::WAITING_CHARACTERISTICS: + return base::TimeDelta::FromSeconds(kGattCharacteristicsTimeoutSeconds); + case SubStatus::WAITING_NOTIFY_SESSION: + return base::TimeDelta::FromSeconds(kNotifySessionTimeoutSeconds); + default: + // Max signifies that there should be no timeout. + return base::TimeDelta::Max(); + } +} + +// static +std::string BluetoothLowEnergyWeaveClientConnection::SubStatusToString( + SubStatus sub_status) { + switch (sub_status) { + case SubStatus::DISCONNECTED: + return "[disconnected]"; + case SubStatus::WAITING_CONNECTION_LATENCY: + return "[waiting to set connection latency]"; + case SubStatus::WAITING_GATT_CONNECTION: + return "[waiting for GATT connection to be created]"; + case SubStatus::WAITING_CHARACTERISTICS: + return "[waiting for GATT characteristics to be found]"; + case SubStatus::CHARACTERISTICS_FOUND: + return "[GATT characteristics have been found]"; + case SubStatus::WAITING_NOTIFY_SESSION: + return "[waiting for notify session to begin]"; + case SubStatus::NOTIFY_SESSION_READY: + return "[notify session is ready]"; + case SubStatus::WAITING_CONNECTION_RESPONSE: + return "[waiting for \"connection response\" uWeave packet]"; + case SubStatus::CONNECTED: + return "[connected]"; + default: + return "[invalid state]"; + } } BluetoothLowEnergyWeaveClientConnection:: @@ -84,81 +133,45 @@ BluetoothLowEnergyWeaveClientConnection:: const RemoteDevice& device, const std::string& device_address, scoped_refptr<device::BluetoothAdapter> adapter, - const device::BluetoothUUID remote_service_uuid, - BluetoothThrottler* bluetooth_throttler) + const device::BluetoothUUID remote_service_uuid) : Connection(device), device_address_(device_address), adapter_(adapter), - remote_service_({remote_service_uuid, ""}), + remote_service_({remote_service_uuid, std::string()}), packet_generator_( - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance()), - packet_receiver_( - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( - BluetoothLowEnergyWeavePacketReceiver::ReceiverType::CLIENT)), - tx_characteristic_({device::BluetoothUUID(kTXCharacteristicUUID), ""}), - rx_characteristic_({device::BluetoothUUID(kRXCharacteristicUUID), ""}), - bluetooth_throttler_(bluetooth_throttler), + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>()), + packet_receiver_(base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( + BluetoothLowEnergyWeavePacketReceiver::ReceiverType::CLIENT)), + tx_characteristic_( + {device::BluetoothUUID(kTXCharacteristicUUID), std::string()}), + rx_characteristic_( + {device::BluetoothUUID(kRXCharacteristicUUID), std::string()}), task_runner_(base::ThreadTaskRunnerHandle::Get()), + timer_(base::MakeUnique<base::OneShotTimer>()), sub_status_(SubStatus::DISCONNECTED), - write_remote_characteristic_pending_(false), weak_ptr_factory_(this) { - DCHECK(adapter_); - DCHECK(adapter_->IsInitialized()); - adapter_->AddObserver(this); } BluetoothLowEnergyWeaveClientConnection:: ~BluetoothLowEnergyWeaveClientConnection() { - // Since the destructor can be called at anytime, it would be unwise to send a - // connection close since we might not be connected at all. DestroyConnection(); - - if (adapter_) { - adapter_->RemoveObserver(this); - adapter_ = nullptr; - } } void BluetoothLowEnergyWeaveClientConnection::Connect() { DCHECK(sub_status() == SubStatus::DISCONNECTED); SetSubStatus(SubStatus::WAITING_CONNECTION_LATENCY); - base::TimeDelta throttler_delay = bluetooth_throttler_->GetDelay(); - PA_LOG(INFO) << "Connecting in " << throttler_delay; - - start_time_ = base::TimeTicks::Now(); - - // If necessary, wait to create a new GATT connection. - // - // Avoid creating a new GATT connection immediately after a given device was - // disconnected. This is a workaround for crbug.com/508919. - if (!throttler_delay.is_zero()) { - task_runner_->PostDelayedTask( - FROM_HERE, - base::Bind( - &BluetoothLowEnergyWeaveClientConnection::SetConnectionLatency, - weak_ptr_factory_.GetWeakPtr()), - throttler_delay); - return; - } - - SetConnectionLatency(); -} - -void BluetoothLowEnergyWeaveClientConnection::SetConnectionLatency() { - DCHECK(sub_status() == SubStatus::WAITING_CONNECTION_LATENCY); - - PA_LOG(INFO) << "Setting connection latency for " << device_address_; - device::BluetoothDevice* bluetooth_device = GetBluetoothDevice(); if (!bluetooth_device) { - PA_LOG(WARNING) << "Could not create GATT connection with " - << device_address_ << " because the device could not be " - << "found."; + PA_LOG(WARNING) << "Device not found; cannot set connection latency for " + << GetDeviceInfoLogString() << "."; + DestroyConnection(); return; } + PA_LOG(INFO) << "Setting connection latency for " << GetDeviceInfoLogString() + << "."; bluetooth_device->SetConnectionLatency( device::BluetoothDevice::ConnectionLatency::CONNECTION_LATENCY_LOW, base::Bind(&BluetoothLowEnergyWeaveClientConnection::CreateGattConnection, @@ -172,16 +185,16 @@ void BluetoothLowEnergyWeaveClientConnection::CreateGattConnection() { DCHECK(sub_status() == SubStatus::WAITING_CONNECTION_LATENCY); SetSubStatus(SubStatus::WAITING_GATT_CONNECTION); - PA_LOG(INFO) << "Creating GATT connection with " << device_address_; - device::BluetoothDevice* bluetooth_device = GetBluetoothDevice(); if (!bluetooth_device) { - PA_LOG(WARNING) << "Could not create GATT connection with " - << device_address_ << " because the device could not be " - << "found."; + PA_LOG(WARNING) << "Device not found; cannot create GATT connection to " + << GetDeviceInfoLogString() << "."; + DestroyConnection(); return; } + PA_LOG(INFO) << "Creating GATT connection with " << GetDeviceInfoLogString() + << "."; bluetooth_device->CreateGattConnection( base::Bind( &BluetoothLowEnergyWeaveClientConnection::OnGattConnectionCreated, @@ -193,107 +206,137 @@ void BluetoothLowEnergyWeaveClientConnection::CreateGattConnection() { void BluetoothLowEnergyWeaveClientConnection::Disconnect() { if (sub_status_ == SubStatus::CONNECTED) { - // Friendly disconnect by sending a connection close and then destroy the - // the connection once the connection close has been sent. - WriteRequest request(packet_generator_->CreateConnectionClose( - ReasonForClose::CLOSE_WITHOUT_ERROR), - WriteRequestType::CONNECTION_CLOSE); - WriteRemoteCharacteristic(request); - } else { - DestroyConnection(); + PA_LOG(INFO) << "Disconnection requested; sending \"connection close\" " + << "uWeave packet to " << GetDeviceInfoLogString() << "."; + + // Send a "connection close" uWeave packet. After the send has completed, + // the connection will disconnect automatically. + ClearQueueAndSendConnectionClose(); + return; } + + DestroyConnection(); } void BluetoothLowEnergyWeaveClientConnection::DestroyConnection() { - if (sub_status() != SubStatus::DISCONNECTED) { - weak_ptr_factory_.InvalidateWeakPtrs(); - StopNotifySession(); - characteristic_finder_.reset(); - if (gatt_connection_) { - PA_LOG(INFO) << "Disconnect from device " - << gatt_connection_->GetDeviceAddress(); - - // Destroying BluetoothGattConnection also disconnects it. - gatt_connection_.reset(); - } + if (adapter_) { + adapter_->RemoveObserver(this); + adapter_ = nullptr; + } + + weak_ptr_factory_.InvalidateWeakPtrs(); + notify_session_.reset(); + characteristic_finder_.reset(); - // Only transition to the DISCONNECTED state after perfoming all necessary - // operations. Otherwise, it'll trigger observers that can pontentially - // destroy the current instance (causing a crash). - SetSubStatus(SubStatus::DISCONNECTED); + if (gatt_connection_) { + PA_LOG(INFO) << "Disconnecting from " << GetDeviceInfoLogString(); + gatt_connection_.reset(); } + + SetSubStatus(SubStatus::DISCONNECTED); } void BluetoothLowEnergyWeaveClientConnection::SetSubStatus( SubStatus new_sub_status) { sub_status_ = new_sub_status; + timer_->Stop(); + + base::TimeDelta timeout_for_sub_status = GetTimeoutForSubStatus(sub_status_); + if (!timeout_for_sub_status.is_max()) { + timer_->Start( + FROM_HERE, timeout_for_sub_status, + base::Bind( + &BluetoothLowEnergyWeaveClientConnection::OnTimeoutForSubStatus, + weak_ptr_factory_.GetWeakPtr(), sub_status_)); + } - // Sets the status of parent class Connection accordingly. - if (new_sub_status == SubStatus::CONNECTED) { + // Sets the status of base class Connection. + if (new_sub_status == SubStatus::CONNECTED) SetStatus(Status::CONNECTED); - } else if (new_sub_status == SubStatus::DISCONNECTED) { + else if (new_sub_status == SubStatus::DISCONNECTED) SetStatus(Status::DISCONNECTED); - } else { + else SetStatus(Status::IN_PROGRESS); - } } -void BluetoothLowEnergyWeaveClientConnection::SetTaskRunnerForTesting( - scoped_refptr<base::TaskRunner> task_runner) { - task_runner_ = task_runner; +void BluetoothLowEnergyWeaveClientConnection::OnTimeoutForSubStatus( + SubStatus status) { + // Ensure that |sub_status| is still the active status. + DCHECK(status == sub_status()); + + PA_LOG(ERROR) << "Timed out waiting during SubStatus " + << SubStatusToString(status) << ". Destroying connection."; + DestroyConnection(); +} + +void BluetoothLowEnergyWeaveClientConnection::SetupTestDoubles( + scoped_refptr<base::TaskRunner> test_task_runner, + std::unique_ptr<base::Timer> test_timer, + std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> test_generator, + std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> test_receiver) { + task_runner_ = test_task_runner; + timer_ = std::move(test_timer); + packet_generator_ = std::move(test_generator); + packet_receiver_ = std::move(test_receiver); } void BluetoothLowEnergyWeaveClientConnection::SendMessageImpl( std::unique_ptr<WireMessage> message) { - PA_LOG(INFO) << "Sending message " << message->Serialize(); - std::string serialized_msg = message->Serialize(); - - std::vector<Packet> packets = - packet_generator_->EncodeDataMessage(serialized_msg); + DCHECK(sub_status() == SubStatus::CONNECTED); + + // Split |message| up into multiple packets which can be sent as one uWeave + // message. + std::vector<Packet> weave_packets = + packet_generator_->EncodeDataMessage(message->Serialize()); + + // For each packet, create a WriteRequest and add it to the queue. + for (uint32_t i = 0; i < weave_packets.size(); ++i) { + WriteRequestType request_type = (i != weave_packets.size() - 1) + ? WriteRequestType::REGULAR + : WriteRequestType::MESSAGE_COMPLETE; + queued_write_requests_.emplace(base::MakeUnique<WriteRequest>( + weave_packets[i], request_type, message.get())); + } - std::shared_ptr<WireMessage> request_message(message.release()); + // Add |message| to the queue of WireMessages. + queued_wire_messages_.emplace(std::move(message)); - for (uint32_t i = 0; i < packets.size(); ++i) { - WriteRequestType request_type = (i == packets.size() - 1) - ? WriteRequestType::MESSAGE_COMPLETE - : WriteRequestType::REGULAR; - WriteRequest request(packets[i], request_type, request_message); - WriteRemoteCharacteristic(request); - } + ProcessNextWriteRequest(); } -// Changes in the GATT connection with the remote device should be observed -// here. If the GATT connection is dropped, we should call DestroyConnection() -// anyway, so the object can notify its observers. void BluetoothLowEnergyWeaveClientConnection::DeviceChanged( device::BluetoothAdapter* adapter, device::BluetoothDevice* device) { - DCHECK(device); - if (sub_status() == SubStatus::DISCONNECTED || - device->GetAddress() != GetDeviceAddress()) + // Ignore updates about other devices. + if (device->GetAddress() != GetDeviceAddress()) return; - if (sub_status() != SubStatus::WAITING_CONNECTION_LATENCY && - sub_status() != SubStatus::WAITING_GATT_CONNECTION && - !device->IsConnected()) { - PA_LOG(INFO) << "GATT connection dropped " << GetDeviceAddress() - << "\ndevice connected: " << device->IsConnected() - << "\ngatt connection: " - << (gatt_connection_ ? gatt_connection_->IsConnected() - : false); - DestroyConnection(); + if (sub_status() == SubStatus::DISCONNECTED || + sub_status() == SubStatus::WAITING_CONNECTION_LATENCY || + sub_status() == SubStatus::WAITING_GATT_CONNECTION) { + // Ignore status change events if a connection has not yet occurred. + return; } + + // If a connection has already occurred and |device| is still connected, there + // is nothing to do. + if (device->IsConnected()) + return; + + PA_LOG(WARNING) << "GATT connection to " << GetDeviceInfoLogString() + << " has been dropped."; + DestroyConnection(); } void BluetoothLowEnergyWeaveClientConnection::DeviceRemoved( device::BluetoothAdapter* adapter, device::BluetoothDevice* device) { - DCHECK(device); - if (sub_status_ == SubStatus::DISCONNECTED || - device->GetAddress() != GetDeviceAddress()) + // Ignore updates about other devices. + if (device->GetAddress() != GetDeviceAddress()) return; - PA_LOG(INFO) << "Device removed " << GetDeviceAddress(); + PA_LOG(WARNING) << "Device has been lost: " << GetDeviceInfoLogString() + << "."; DestroyConnection(); } @@ -302,58 +345,69 @@ void BluetoothLowEnergyWeaveClientConnection::GattCharacteristicValueChanged( device::BluetoothRemoteGattCharacteristic* characteristic, const Packet& value) { DCHECK_EQ(adapter, adapter_.get()); - if (sub_status() != SubStatus::WAITING_CONNECTION_RESPONSE && - sub_status() != SubStatus::CONNECTED) - return; - PA_LOG(INFO) << "Characteristic value changed: " - << characteristic->GetUUID().canonical_value(); + // Ignore characteristics which do not apply to this connection. + if (characteristic->GetIdentifier() != rx_characteristic_.id) + return; - if (characteristic->GetIdentifier() == rx_characteristic_.id) { - ReceiverState state = packet_receiver_->ReceivePacket(value); + if (sub_status() != SubStatus::WAITING_CONNECTION_RESPONSE && + sub_status() != SubStatus::CONNECTED) { + PA_LOG(WARNING) << "Received message from " << GetDeviceInfoLogString() + << ", but was not expecting one. sub_status() = " + << sub_status(); + return; + } - PA_LOG(INFO) << "\nReceiver State: " << state; - switch (state) { - case ReceiverState::DATA_READY: - OnBytesReceived(packet_receiver_->GetDataMessage()); - break; - case ReceiverState::CONNECTION_CLOSED: - PA_LOG(ERROR) << "Connection closed due to: " << GetReasonForClose(); - DestroyConnection(); - break; - case ReceiverState::ERROR_DETECTED: - OnPacketReceiverError(); - break; - case ReceiverState::WAITING: - // Receiver state should have changed from CONNECTING to WAITING if - // a proper connection response had been received. - // The max packet size selected from the connection response will be - // used to generate future data packets. - packet_generator_->SetMaxPacketSize( - packet_receiver_->GetMaxPacketSize()); - DCHECK(sub_status() == SubStatus::WAITING_CONNECTION_RESPONSE); - CompleteConnection(); - break; - case ReceiverState::RECEIVING_DATA: - // Normal in between states, so do nothing. - break; - default: - NOTREACHED(); - } + switch (packet_receiver_->ReceivePacket(value)) { + case ReceiverState::DATA_READY: + OnBytesReceived(packet_receiver_->GetDataMessage()); + break; + case ReceiverState::CONNECTION_CLOSED: + PA_LOG(INFO) << "Received \"connection close\" uWeave packet from " + << GetDeviceInfoLogString() + << ". Reason: " << GetReasonForClose() << "."; + DestroyConnection(); + return; + case ReceiverState::ERROR_DETECTED: + PA_LOG(ERROR) << "Received invalid packet from " + << GetDeviceInfoLogString() << ". "; + ClearQueueAndSendConnectionClose(); + break; + case ReceiverState::WAITING: + CompleteConnection(); + break; + case ReceiverState::RECEIVING_DATA: + // Continue to wait for more packets to arrive; once the rest of the + // packets for this message are received, |packet_receiver_| will + // transition to the DATA_READY state. + break; + default: + NOTREACHED(); } } void BluetoothLowEnergyWeaveClientConnection::CompleteConnection() { - PA_LOG(INFO) << "Connection completed. Time elapsed: " - << base::TimeTicks::Now() - start_time_; + DCHECK(sub_status() == SubStatus::WAITING_CONNECTION_RESPONSE); SetSubStatus(SubStatus::CONNECTED); + + uint16_t max_packet_size = packet_receiver_->GetMaxPacketSize(); + PA_LOG(INFO) << "Received uWeave \"connection response\" packet; connection " + << "is now fully initialized for " << GetDeviceInfoLogString() + << ". Max packet size: " << max_packet_size; + + // Now that the "connection close" uWeave packet has been received, + // |packet_receiver_| should have received a max packet size from the GATT + // server. + packet_generator_->SetMaxPacketSize(max_packet_size); } void BluetoothLowEnergyWeaveClientConnection::OnSetConnectionLatencyError() { DCHECK(sub_status_ == SubStatus::WAITING_CONNECTION_LATENCY); - PA_LOG(WARNING) << "Error setting connection latency."; - // Even if setting the connection latency fails, we should continue with the - // connection anyways. + PA_LOG(WARNING) << "Error setting connection latency for connection to " + << GetDeviceInfoLogString() << "."; + + // Even if setting the connection latency fails, continue with the + // connection. This is unfortunate but should not be considered a fatal error. CreateGattConnection(); } @@ -361,23 +415,19 @@ void BluetoothLowEnergyWeaveClientConnection::OnCreateGattConnectionError( device::BluetoothDevice::ConnectErrorCode error_code) { DCHECK(sub_status_ == SubStatus::WAITING_GATT_CONNECTION); PA_LOG(WARNING) << "Error creating GATT connection to " - << remote_device().bluetooth_address - << " error code: " << error_code; + << GetDeviceInfoLogString() << ". Error code: " << error_code; DestroyConnection(); } void BluetoothLowEnergyWeaveClientConnection::OnGattConnectionCreated( std::unique_ptr<device::BluetoothGattConnection> gatt_connection) { DCHECK(sub_status() == SubStatus::WAITING_GATT_CONNECTION); - PA_LOG(INFO) << "GATT connection with " << gatt_connection->GetDeviceAddress() - << " created."; - PrintTimeElapsed(); - - // Informing |bluetooth_trottler_| a new connection was established. - bluetooth_throttler_->OnConnection(this); gatt_connection_ = std::move(gatt_connection); SetSubStatus(SubStatus::WAITING_CHARACTERISTICS); + + PA_LOG(INFO) << "Finding GATT characteristics for " + << GetDeviceInfoLogString() << "."; characteristic_finder_.reset(CreateCharacteristicsFinder( base::Bind( &BluetoothLowEnergyWeaveClientConnection::OnCharacteristicsFound, @@ -402,15 +452,15 @@ void BluetoothLowEnergyWeaveClientConnection::OnCharacteristicsFound( const RemoteAttribute& service, const RemoteAttribute& tx_characteristic, const RemoteAttribute& rx_characteristic) { - PA_LOG(INFO) << "Remote chacteristics found."; - PrintTimeElapsed(); - DCHECK(sub_status() == SubStatus::WAITING_CHARACTERISTICS); + remote_service_ = service; tx_characteristic_ = tx_characteristic; rx_characteristic_ = rx_characteristic; + characteristic_finder_.reset(); SetSubStatus(SubStatus::CHARACTERISTICS_FOUND); + StartNotifySession(); } @@ -418,38 +468,48 @@ void BluetoothLowEnergyWeaveClientConnection::OnCharacteristicsFinderError( const RemoteAttribute& tx_characteristic, const RemoteAttribute& rx_characteristic) { DCHECK(sub_status() == SubStatus::WAITING_CHARACTERISTICS); - PA_LOG(WARNING) << "Connection error, missing characteristics for SmartLock " - "service.\n" - << (tx_characteristic.id.empty() - ? tx_characteristic.uuid.canonical_value() - : "") - << ", " - << (rx_characteristic.id.empty() - ? ", " + rx_characteristic.uuid.canonical_value() - : "") - << " not found."; + + std::stringstream ss; + ss << "Could not find GATT characteristics for " << GetDeviceInfoLogString() + << ": "; + if (tx_characteristic.id.empty()) { + ss << "[TX: " << tx_characteristic.uuid.canonical_value() << "]"; + if (!rx_characteristic.id.empty()) + ss << ", "; + } + if (rx_characteristic.id.empty()) + ss << "[RX: " << rx_characteristic.uuid.canonical_value() << "]"; + PA_LOG(ERROR) << ss.str(); + + characteristic_finder_.reset(); DestroyConnection(); } void BluetoothLowEnergyWeaveClientConnection::StartNotifySession() { DCHECK(sub_status() == SubStatus::CHARACTERISTICS_FOUND); + device::BluetoothRemoteGattCharacteristic* characteristic = GetGattCharacteristic(rx_characteristic_.id); - CHECK(characteristic); + if (!characteristic) { + PA_LOG(ERROR) << "Characteristic no longer available after it was found. " + << "Cannot start notification session for " + << GetDeviceInfoLogString() << "."; + DestroyConnection(); + return; + } - // This is a workaround for crbug.com/507325. If |characteristic| is already - // notifying |characteristic->StartNotifySession()| will fail with - // GATT_ERROR_FAILED. + // Workaround for crbug.com/507325. If |characteristic| is already notifying, + // characteristic->StartNotifySession() fails with GATT_ERROR_FAILED. if (characteristic->IsNotifying()) { - PA_LOG(INFO) << characteristic->GetUUID().canonical_value() - << " already notifying."; SetSubStatus(SubStatus::NOTIFY_SESSION_READY); SendConnectionRequest(); return; } SetSubStatus(SubStatus::WAITING_NOTIFY_SESSION); + PA_LOG(INFO) << "Starting notification session for " + << GetDeviceInfoLogString() << "."; characteristic->StartNotifySession( base::Bind( &BluetoothLowEnergyWeaveClientConnection::OnNotifySessionStarted, @@ -461,193 +521,228 @@ void BluetoothLowEnergyWeaveClientConnection::StartNotifySession() { void BluetoothLowEnergyWeaveClientConnection::OnNotifySessionStarted( std::unique_ptr<device::BluetoothGattNotifySession> notify_session) { DCHECK(sub_status() == SubStatus::WAITING_NOTIFY_SESSION); - PA_LOG(INFO) << "Notification session started " - << notify_session->GetCharacteristicIdentifier(); - PrintTimeElapsed(); - - SetSubStatus(SubStatus::NOTIFY_SESSION_READY); notify_session_ = std::move(notify_session); - + SetSubStatus(SubStatus::NOTIFY_SESSION_READY); SendConnectionRequest(); } void BluetoothLowEnergyWeaveClientConnection::OnNotifySessionError( device::BluetoothRemoteGattService::GattErrorCode error) { DCHECK(sub_status() == SubStatus::WAITING_NOTIFY_SESSION); - PA_LOG(WARNING) << "Error starting notification session: " << error; + PA_LOG(ERROR) << "Cannot start notification session for " + << GetDeviceInfoLogString() << ". Error: " << error << "."; DestroyConnection(); } -void BluetoothLowEnergyWeaveClientConnection::StopNotifySession() { - if (notify_session_) { - notify_session_->Stop(base::Bind(&base::DoNothing)); - notify_session_.reset(); - } -} - void BluetoothLowEnergyWeaveClientConnection::SendConnectionRequest() { - if (sub_status() == SubStatus::NOTIFY_SESSION_READY) { - PA_LOG(INFO) << "Sending connection request to the server"; - SetSubStatus(SubStatus::WAITING_CONNECTION_RESPONSE); + DCHECK(sub_status() == SubStatus::NOTIFY_SESSION_READY); + SetSubStatus(SubStatus::WAITING_CONNECTION_RESPONSE); - WriteRequest write_request(packet_generator_->CreateConnectionRequest(), - WriteRequestType::CONNECTION_REQUEST); + PA_LOG(INFO) << "Sending \"connection request\" uWeave packet to " + << GetDeviceInfoLogString(); - WriteRemoteCharacteristic(write_request); - } -} - -void BluetoothLowEnergyWeaveClientConnection::WriteRemoteCharacteristic( - const WriteRequest& request) { - write_requests_queue_.push(request); + queued_write_requests_.emplace(base::MakeUnique<WriteRequest>( + packet_generator_->CreateConnectionRequest(), + WriteRequestType::CONNECTION_REQUEST)); ProcessNextWriteRequest(); } void BluetoothLowEnergyWeaveClientConnection::ProcessNextWriteRequest() { + // If there is already an in-progress write or there are no pending + // WriteRequests, there is nothing to do. + if (pending_write_request_ || queued_write_requests_.empty()) + return; + + pending_write_request_ = std::move(queued_write_requests_.front()); + queued_write_requests_.pop(); + + PA_LOG(INFO) << "Writing " << pending_write_request_->value.size() << " " + << "bytes to " << GetDeviceInfoLogString() << "."; + SendPendingWriteRequest(); +} + +void BluetoothLowEnergyWeaveClientConnection::SendPendingWriteRequest() { + DCHECK(pending_write_request_); + device::BluetoothRemoteGattCharacteristic* characteristic = GetGattCharacteristic(tx_characteristic_.id); - if (!write_requests_queue_.empty() && !write_remote_characteristic_pending_ && - characteristic) { - write_remote_characteristic_pending_ = true; - const WriteRequest& next_request = write_requests_queue_.front(); - - PA_LOG(INFO) << "Writing to characteristic " << next_request.value.size() - << " bytes"; - characteristic->WriteRemoteCharacteristic( - next_request.value, - base::Bind(&BluetoothLowEnergyWeaveClientConnection:: - OnRemoteCharacteristicWritten, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&BluetoothLowEnergyWeaveClientConnection:: - OnWriteRemoteCharacteristicError, - weak_ptr_factory_.GetWeakPtr())); + if (!characteristic) { + PA_LOG(ERROR) << "Characteristic no longer available after it was found. " + << "Cannot process write request for " + << GetDeviceInfoLogString() << "."; + DestroyConnection(); + return; } + + characteristic->WriteRemoteCharacteristic( + pending_write_request_->value, + base::Bind(&BluetoothLowEnergyWeaveClientConnection:: + OnRemoteCharacteristicWritten, + weak_ptr_factory_.GetWeakPtr()), + base::Bind(&BluetoothLowEnergyWeaveClientConnection:: + OnWriteRemoteCharacteristicError, + weak_ptr_factory_.GetWeakPtr())); } void BluetoothLowEnergyWeaveClientConnection::OnRemoteCharacteristicWritten() { - PA_LOG(INFO) << "Characteristic written."; + DCHECK(sub_status() == SubStatus::WAITING_CONNECTION_RESPONSE || + sub_status() == SubStatus::CONNECTED); - DCHECK(!write_requests_queue_.empty()); + if (!pending_write_request_) { + PA_LOG(ERROR) << "OnRemoteCharacteristicWritten() called, but no pending " + << "WriteRequest. Stopping connection to " + << GetDeviceInfoLogString(); + DestroyConnection(); + return; + } - const WriteRequest& request = write_requests_queue_.front(); + if (pending_write_request_->request_type == + WriteRequestType::CONNECTION_CLOSE) { + // Once a "connection close" uWeave packet has been sent, the connection + // is ready to be disconnected. + DestroyConnection(); + return; + } - switch (request.request_type) { - case WriteRequestType::REGULAR: - case WriteRequestType::CONNECTION_REQUEST: - break; - case WriteRequestType::MESSAGE_COMPLETE: - OnDidSendMessage(*request.message, true); - break; - case WriteRequestType::CONNECTION_CLOSE: + if (pending_write_request_->request_type == + WriteRequestType::MESSAGE_COMPLETE) { + if (queued_wire_messages_.empty()) { + PA_LOG(ERROR) << "Sent a WriteRequest with type == MESSAGE_COMPLETE, but " + << "there were no queued WireMessages. Cannot process " + << "completed write to " << GetDeviceInfoLogString(); DestroyConnection(); return; - default: - NOTREACHED(); + } + + std::unique_ptr<WireMessage> sent_message = + std::move(queued_wire_messages_.front()); + queued_wire_messages_.pop(); + DCHECK_EQ(sent_message.get(), + pending_write_request_->associated_wire_message); + + // Notify observers of the message being sent via a task on the run loop to + // ensure that if an observer deletes this object in response to receiving + // the OnSendCompleted() callback, a null pointer is not deferenced. + task_runner_->PostTask( + FROM_HERE, + base::Bind(&BluetoothLowEnergyWeaveClientConnection::OnDidSendMessage, + weak_ptr_factory_.GetWeakPtr(), *sent_message, + true /* success */)); } - // Removes the top of queue (already processed) and process the next request. - write_requests_queue_.pop(); - write_remote_characteristic_pending_ = false; + pending_write_request_.reset(); ProcessNextWriteRequest(); } void BluetoothLowEnergyWeaveClientConnection::OnWriteRemoteCharacteristicError( device::BluetoothRemoteGattService::GattErrorCode error) { - PA_LOG(WARNING) << "Error " << error << " writing characteristic: " - << tx_characteristic_.uuid.canonical_value(); - - write_remote_characteristic_pending_ = false; + DCHECK(sub_status() == SubStatus::WAITING_CONNECTION_RESPONSE || + sub_status() == SubStatus::CONNECTED); - DCHECK(!write_requests_queue_.empty()); - WriteRequest* request = &write_requests_queue_.front(); + if (!pending_write_request_) { + PA_LOG(ERROR) << "OnWriteRemoteCharacteristicError() called, but no " + << "pending WriteRequest. Stopping connection to " + << GetDeviceInfoLogString(); + DestroyConnection(); + return; + } - ++request->number_of_failed_attempts; + ++pending_write_request_->number_of_failed_attempts; + if (pending_write_request_->number_of_failed_attempts < + kMaxNumberOfRetryAttempts + 1) { + PA_LOG(WARNING) << "Error sending WriteRequest to " + << GetDeviceInfoLogString() << "; failure number " + << pending_write_request_->number_of_failed_attempts + << ". Retrying."; + SendPendingWriteRequest(); + return; + } - // If the message has failed to send the first time and up to - // |kMaxNumberOfRetryAttempts| retries, give up. - if (request->number_of_failed_attempts >= kMaxNumberOfRetryAttempts + 1) { - switch (request->request_type) { - case WriteRequestType::REGULAR: - case WriteRequestType::MESSAGE_COMPLETE: - OnDidSendMessage(*request->message, false); - break; - case WriteRequestType::CONNECTION_CLOSE: - case WriteRequestType::CONNECTION_REQUEST: - break; - default: - NOTREACHED(); - } + if (pending_write_request_->request_type == WriteRequestType::REGULAR || + pending_write_request_->request_type == + WriteRequestType::MESSAGE_COMPLETE) { + std::unique_ptr<WireMessage> failed_message = + std::move(queued_wire_messages_.front()); + queued_wire_messages_.pop(); + DCHECK_EQ(failed_message.get(), + pending_write_request_->associated_wire_message); - // If the previous write has failed each retry attempt up to the maximum - // number allowed, a "connection close" message cannot be sent since the - // connection is not working. Destroy the connection. - DestroyConnection(); - } else { - ProcessNextWriteRequest(); + OnDidSendMessage(*failed_message, false /* success */); } + + // Since the try limit has been hit, this is a fatal error. Destroy the + // connection, but post it as a new task to ensure that observers have a + // chance to process the OnSendCompleted() call. + task_runner_->PostTask( + FROM_HERE, + base::Bind(&BluetoothLowEnergyWeaveClientConnection::DestroyConnection, + weak_ptr_factory_.GetWeakPtr())); } -void BluetoothLowEnergyWeaveClientConnection::OnPacketReceiverError() { - PA_LOG(ERROR) << "Received erroneous packet. Closing connection."; +void BluetoothLowEnergyWeaveClientConnection::OnDidSendMessage( + const WireMessage& message, + bool success) { + Connection::OnDidSendMessage(message, success); +} - WriteRequest request(packet_generator_->CreateConnectionClose( - packet_receiver_->GetReasonToClose()), - WriteRequestType::CONNECTION_CLOSE); +void BluetoothLowEnergyWeaveClientConnection:: + ClearQueueAndSendConnectionClose() { + DCHECK(sub_status() == SubStatus::WAITING_CONNECTION_RESPONSE || + sub_status() == SubStatus::CONNECTED); - // Skip all other writes that's not in progress. + // The connection is now in an invalid state. Clear queued writes. + while (!queued_write_requests_.empty()) + queued_write_requests_.pop(); - // C++ queue does not have a clear method. - // According to stackoverflow - // "http://stackoverflow.com/questions/709146/how-do-i-clear-the-stdqueue- - // efficiently" - std::queue<WriteRequest> empty; - std::swap(write_requests_queue_, empty); + // Now, queue up a "connection close" uWeave packet. If there was a pending + // write, we must wait for it to complete before the "connection close" can + // be sent. + queued_write_requests_.emplace( + base::MakeUnique<WriteRequest>(packet_generator_->CreateConnectionClose( + packet_receiver_->GetReasonToClose()), + WriteRequestType::CONNECTION_CLOSE)); - if (write_remote_characteristic_pending_) { - // Add the in progress write back to the queue. - write_requests_queue_.push(empty.front()); + if (pending_write_request_) { + PA_LOG(WARNING) << "Waiting for current write to complete, then will send " + << "a \"connection close\" uWeave packet."; + } else { + PA_LOG(INFO) << "Sending a \"connection close\" uWeave packet."; } - WriteRemoteCharacteristic(request); -} - -void BluetoothLowEnergyWeaveClientConnection::PrintTimeElapsed() { - PA_LOG(INFO) << "Time elapsed: " << base::TimeTicks::Now() - start_time_; + ProcessNextWriteRequest(); } std::string BluetoothLowEnergyWeaveClientConnection::GetDeviceAddress() { - // When the remote device is connected we should rely on the address given by - // |gatt_connection_|. As the device address may change if the device is - // paired. The address in |gatt_connection_| is automatically updated in this - // case. + // When the remote device is connected, rely on the address given by + // |gatt_connection_|. Unpaired BLE device addresses are ephemeral and are + // expected to change periodically. return gatt_connection_ ? gatt_connection_->GetDeviceAddress() : device_address_; } device::BluetoothDevice* BluetoothLowEnergyWeaveClientConnection::GetBluetoothDevice() { - return adapter_->GetDevice(device_address_); + return adapter_->GetDevice(GetDeviceAddress()); } device::BluetoothRemoteGattService* BluetoothLowEnergyWeaveClientConnection::GetRemoteService() { device::BluetoothDevice* bluetooth_device = GetBluetoothDevice(); if (!bluetooth_device) { - PA_LOG(WARNING) << "Could not create GATT connection with " - << device_address_ << " because the device could not be " - << "found."; + PA_LOG(WARNING) << "Cannot find Bluetooth device for " + << GetDeviceInfoLogString(); return nullptr; } if (remote_service_.id.empty()) { - std::vector<device::BluetoothRemoteGattService*> services = - bluetooth_device->GetGattServices(); - for (auto* service : services) + for (auto* service : bluetooth_device->GetGattServices()) { if (service->GetUUID() == remote_service_.uuid) { remote_service_.id = service->GetIdentifier(); break; } + } } + return bluetooth_device->GetGattService(remote_service_.id); } @@ -656,7 +751,8 @@ BluetoothLowEnergyWeaveClientConnection::GetGattCharacteristic( const std::string& gatt_characteristic) { device::BluetoothRemoteGattService* remote_service = GetRemoteService(); if (!remote_service) { - PA_LOG(WARNING) << "Remote service not found."; + PA_LOG(WARNING) << "Cannot find GATT service for " + << GetDeviceInfoLogString(); return nullptr; } return remote_service->GetCharacteristic(gatt_characteristic); @@ -683,22 +779,15 @@ std::string BluetoothLowEnergyWeaveClientConnection::GetReasonForClose() { BluetoothLowEnergyWeaveClientConnection::WriteRequest::WriteRequest( const Packet& val, WriteRequestType request_type, - std::shared_ptr<WireMessage> message) + WireMessage* associated_wire_message) : value(val), request_type(request_type), - message(message), - number_of_failed_attempts(0) {} + associated_wire_message(associated_wire_message) {} BluetoothLowEnergyWeaveClientConnection::WriteRequest::WriteRequest( const Packet& val, WriteRequestType request_type) - : value(val), - request_type(request_type), - message(nullptr), - number_of_failed_attempts(0) {} - -BluetoothLowEnergyWeaveClientConnection::WriteRequest::WriteRequest( - const WriteRequest& other) = default; + : WriteRequest(val, request_type, nullptr /* associated_wire_message */) {} BluetoothLowEnergyWeaveClientConnection::WriteRequest::~WriteRequest() {} diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h index e86990c4526..1a74e45c6a9 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h @@ -1,7 +1,6 @@ // Copyright 2015 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_CRYPTAUTH_BLE_BLUETOOTH_LOW_ENERGY_WEAVE_CLIENT_CONNECTION_H_ #define COMPONENTS_CRYPTAUTH_BLE_BLUETOOTH_LOW_ENERGY_WEAVE_CLIENT_CONNECTION_H_ @@ -17,12 +16,12 @@ #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "base/time/time.h" +#include "base/timer/timer.h" #include "components/cryptauth/ble/bluetooth_low_energy_characteristics_finder.h" #include "components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.h" #include "components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.h" #include "components/cryptauth/ble/fake_wire_message.h" #include "components/cryptauth/ble/remote_attribute.h" -#include "components/cryptauth/bluetooth_throttler.h" #include "components/cryptauth/connection.h" #include "device/bluetooth/bluetooth_adapter.h" #include "device/bluetooth/bluetooth_device.h" @@ -40,6 +39,7 @@ namespace weave { // Creates GATT connection on top of the BLE connection and act as a Client. // uWeave communication follows the flow: +// // Client | Server // ---------------------------------|-------------------------------- // send connection request | @@ -59,27 +59,20 @@ class BluetoothLowEnergyWeaveClientConnection const RemoteDevice& remote_device, const std::string& device_address, scoped_refptr<device::BluetoothAdapter> adapter, - const device::BluetoothUUID remote_service_uuid, - BluetoothThrottler* bluetooth_throttler); - - // Exposed for testing. + const device::BluetoothUUID remote_service_uuid); static void SetInstanceForTesting(Factory* factory); protected: - // Exposed for testing. virtual std::unique_ptr<Connection> BuildInstance( const RemoteDevice& remote_device, const std::string& device_address, scoped_refptr<device::BluetoothAdapter> adapter, - const device::BluetoothUUID remote_service_uuid, - BluetoothThrottler* bluetooth_throttler); + const device::BluetoothUUID remote_service_uuid); private: static Factory* factory_instance_; }; - // The sub-state of a BluetoothLowEnergyWeaveClientConnection - // extends the IN_PROGRESS state of Connection::Status. enum SubStatus { DISCONNECTED, WAITING_CONNECTION_LATENCY, @@ -92,47 +85,44 @@ class BluetoothLowEnergyWeaveClientConnection CONNECTED, }; - // Constructs a Bluetooth low energy connection to the service with - // |remote_service_| on the |remote_device|. The |adapter| must be already - // initialized and ready. The GATT connection may alreaady be established and - // pass through |gatt_connection|. A subsequent call to Connect() must be - // made. + // Constructs the Connection object; a subsequent call to Connect() is + // necessary to initiate the BLE connection. BluetoothLowEnergyWeaveClientConnection( const RemoteDevice& remote_device, const std::string& device_address, scoped_refptr<device::BluetoothAdapter> adapter, - const device::BluetoothUUID remote_service_uuid, - BluetoothThrottler* bluetooth_throttler); + const device::BluetoothUUID remote_service_uuid); ~BluetoothLowEnergyWeaveClientConnection() override; - // namespace Connection: + // Connection: void Connect() override; void Disconnect() override; std::string GetDeviceAddress() override; protected: - // Exposed for testing. - // NOTE: This method may indirectly cause this object's destructor to be - // called. Do not perform any operations that touch the internals of this - // class after calling this method. + // Destroys the connection immediately; if there was an active connection, it + // will be disconnected after this call. Note that this function may notify + // observers of a connection status change. void DestroyConnection(); - // Exposed for testing. SubStatus sub_status() { return sub_status_; } - // Sets |task_runner_| for testing. - void SetTaskRunnerForTesting(scoped_refptr<base::TaskRunner> task_runner); + void SetupTestDoubles( + scoped_refptr<base::TaskRunner> test_task_runner, + std::unique_ptr<base::Timer> test_timer, + std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> test_generator, + std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> test_receiver); - // Virtual for testing. virtual BluetoothLowEnergyCharacteristicsFinder* CreateCharacteristicsFinder( const BluetoothLowEnergyCharacteristicsFinder::SuccessCallback& success_callback, const BluetoothLowEnergyCharacteristicsFinder::ErrorCallback& error_callback); - // namespace Connection: + // Connection: void SendMessageImpl(std::unique_ptr<WireMessage> message) override; + void OnDidSendMessage(const WireMessage& message, bool success) override; // device::BluetoothAdapter::Observer: void DeviceChanged(device::BluetoothAdapter* adapter, @@ -158,169 +148,107 @@ class BluetoothLowEnergyWeaveClientConnection struct WriteRequest { WriteRequest(const Packet& val, WriteRequestType request_type, - std::shared_ptr<WireMessage> message); + WireMessage* associated_wire_message); WriteRequest(const Packet& val, WriteRequestType request_type); - WriteRequest(const WriteRequest& other); - ~WriteRequest(); + WriteRequest(const WireMessage& other); + virtual ~WriteRequest(); Packet value; WriteRequestType request_type; - std::shared_ptr<WireMessage> message; - int number_of_failed_attempts; + WireMessage* associated_wire_message; + int number_of_failed_attempts = 0; }; + static std::string SubStatusToString(SubStatus sub_status); + + // Returns the timeout for the given SubStatus. If no timeout is needed for + // |sub_status|, base::TimeDelta::Max() is returned. + static base::TimeDelta GetTimeoutForSubStatus(SubStatus sub_status); + + // Sets the current status; if |status| corresponds to one of Connection's + // Status types, observers will be notified of the change. void SetSubStatus(SubStatus status); + void OnTimeoutForSubStatus(SubStatus status); - // Sets the connection interval before connecting. + // These functions are used to set up the connection so that it is ready to + // send/receive data. void SetConnectionLatency(); - - // Creates the GATT connection with |remote_device|. void CreateGattConnection(); - - // Called when a GATT connection is created. void OnGattConnectionCreated( std::unique_ptr<device::BluetoothGattConnection> gatt_connection); - - // Callback when there is an error setting the connection interval. void OnSetConnectionLatencyError(); - - // Callback called when there is an error creating the GATT connection. void OnCreateGattConnectionError( device::BluetoothDevice::ConnectErrorCode error_code); - - // Callback called when |tx_characteristic_| and |rx_characteristic_| were - // found. void OnCharacteristicsFound(const RemoteAttribute& service, const RemoteAttribute& tx_characteristic, const RemoteAttribute& rx_characteristic); - - // Callback called there was an error finding the characteristics. void OnCharacteristicsFinderError(const RemoteAttribute& tx_characteristic, const RemoteAttribute& rx_characteristic); - - // Starts a notify session for |rx_characteristic_| when ready - // (SubStatus::CHARACTERISTICS_FOUND). void StartNotifySession(); - - // Called when a notification session is successfully started for - // |rx_characteristic_| characteristic. void OnNotifySessionStarted( std::unique_ptr<device::BluetoothGattNotifySession> notify_session); - - // Called when there is an error starting a notification session for - // |rx_characteristic_| characteristic. void OnNotifySessionError(device::BluetoothGattService::GattErrorCode); - // Stops |notify_session_|. - void StopNotifySession(); - - // Sends a connection request to server if ready - // (SubStatus::NOTIFY_SESSION_READY). + // Sends the connection request message (the first message in the uWeave + // handshake). void SendConnectionRequest(); // Completes and updates the status accordingly. void CompleteConnection(); - // This is the only entry point for WriteRequests, which are processed - // accordingly the following flow: - // 1) |request| is enqueued; - // 2) |request| will be processed by ProcessNextWriteRequest() when there is - // no pending write request; - // 3) |request| will be dequeued when it's successfully processed - // (OnRemoteCharacteristicWritten()); - // 4) |request| is not dequeued if it fails - // (OnWriteRemoteCharacteristicError()), it remains on the queue and will be - // retried. |request| will remain on the queue until it succeeds or it - // triggers a Disconnect() call (after |max_number_of_tries_|). - void WriteRemoteCharacteristic(const WriteRequest& request); - - // Processes the next request in |write_requests_queue_|. + // If no write is in progress and there are queued packets, sends the next + // packet; if there is already a write in progress or there are no queued + // packets, this function is a no-op. void ProcessNextWriteRequest(); - // Called when the - // BluetoothRemoteGattCharacteristic::RemoteCharacteristicWrite() is - // successfully complete. + void SendPendingWriteRequest(); void OnRemoteCharacteristicWritten(); - - // Called when there is an error writing to the remote characteristic - // |tx_characteristic_|. void OnWriteRemoteCharacteristicError( device::BluetoothRemoteGattService::GattErrorCode error); + void ClearQueueAndSendConnectionClose(); - void OnPacketReceiverError(); - - // Prints the time elapsed since |Connect()| was called. - void PrintTimeElapsed(); - - // Returns the service corresponding to |remote_service_| in the current - // device. + // Private getters for the Bluetooth classes corresponding to this connection. device::BluetoothRemoteGattService* GetRemoteService(); - - // Returns the characteristic corresponding to |identifier| in the current - // service. device::BluetoothRemoteGattCharacteristic* GetGattCharacteristic( const std::string& identifier); + device::BluetoothDevice* GetBluetoothDevice(); // Get the reason that the other side of the connection decided to close the // connection. - // Will crash if the receiver is not in CONNECTION_CLOSED state. std::string GetReasonForClose(); - // Returns the device corresponding to |device_address_|. - device::BluetoothDevice* GetBluetoothDevice(); - - // The device to which to connect. + // The device to which to connect. This is the starting value, but the device + // address may change during the connection because BLE addresses are + // ephemeral. Use GetDeviceAddress() to get the most up-to-date address. const std::string device_address_; - // The Bluetooth adapter over which the Bluetooth connection will be made. scoped_refptr<device::BluetoothAdapter> adapter_; - - // Remote service the |gatt_connection_| was established with. RemoteAttribute remote_service_; - - // uWeave packet generator. std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> packet_generator_; - - // uWeave packet receiver. std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> packet_receiver_; - - // Characteristic used to send data to the remote device. RemoteAttribute tx_characteristic_; - - // Characteristic used to receive data from the remote device. RemoteAttribute rx_characteristic_; - - // Throttles repeated connection attempts to the same device. This is a - // workaround for crbug.com/508919. Not owned, must outlive this instance. - BluetoothThrottler* bluetooth_throttler_; - scoped_refptr<base::TaskRunner> task_runner_; + std::unique_ptr<base::Timer> timer_; - // The GATT connection with the remote device. + // These pointers start out null and are created during the connection + // process. std::unique_ptr<device::BluetoothGattConnection> gatt_connection_; - - // The characteristics finder for remote device. std::unique_ptr<BluetoothLowEnergyCharacteristicsFinder> characteristic_finder_; - - // The notify session for |rx_characteristic|. std::unique_ptr<device::BluetoothGattNotifySession> notify_session_; - // Internal connection status SubStatus sub_status_; - // Bytes already received for the current receive operation. - std::string incoming_bytes_buffer_; - - // Indicates there is a - // BluetoothRemoteGattCharacteristic::WriteRemoteCharacteristic - // operation pending. - bool write_remote_characteristic_pending_; - - std::queue<WriteRequest> write_requests_queue_; + // The WriteRequest that is currently being sent as well as those queued to be + // sent. Each WriteRequest corresponds to one uWeave packet to be sent. + std::unique_ptr<WriteRequest> pending_write_request_; + std::queue<std::unique_ptr<WriteRequest>> queued_write_requests_; - // Stores when the instace was created. - base::TimeTicks start_time_; + // WireMessages queued to be sent. Each WireMessage correponds to one or more + // WriteRequests. WireMessages remain in this queue until the last + // corresponding WriteRequest has been sent. + std::queue<std::unique_ptr<WireMessage>> queued_wire_messages_; base::WeakPtrFactory<BluetoothLowEnergyWeaveClientConnection> weak_ptr_factory_; diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc index 894860c383a..87333d3ddb7 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc @@ -14,7 +14,8 @@ #include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/test/test_simple_task_runner.h" -#include "components/cryptauth/bluetooth_throttler.h" +#include "base/timer/mock_timer.h" +#include "base/timer/timer.h" #include "components/cryptauth/connection_finder.h" #include "components/cryptauth/connection_observer.h" #include "components/cryptauth/cryptauth_test_util.h" @@ -39,78 +40,9 @@ using testing::StrictMock; using testing::SaveArg; namespace cryptauth { -namespace { - -const std::string kTestFeature = "testFeature"; - -class MockBluetoothThrottler : public BluetoothThrottler { - public: - MockBluetoothThrottler() {} - ~MockBluetoothThrottler() override {} - - MOCK_CONST_METHOD0(GetDelay, base::TimeDelta()); - MOCK_METHOD1(OnConnection, void(Connection* connection)); - - private: - DISALLOW_COPY_AND_ASSIGN(MockBluetoothThrottler); -}; - -class MockBluetoothLowEnergyCharacteristicsFinder - : public BluetoothLowEnergyCharacteristicsFinder { - public: - MockBluetoothLowEnergyCharacteristicsFinder() {} - ~MockBluetoothLowEnergyCharacteristicsFinder() override {} - - private: - DISALLOW_COPY_AND_ASSIGN(MockBluetoothLowEnergyCharacteristicsFinder); -}; - -class MockConnectionObserver : public ConnectionObserver { - public: - MockConnectionObserver() - : num_send_completed_(0), delete_on_disconnect_(false) {} - - void OnConnectionStatusChanged(Connection* connection, - Connection::Status old_status, - Connection::Status new_status) override { - if (new_status == Connection::Status::DISCONNECTED && delete_on_disconnect_) - delete connection; - } - - void OnMessageReceived(const Connection& connection, - const WireMessage& message) override {} - - void OnSendCompleted(const Connection& conenction, - const WireMessage& message, - bool success) override { - last_deserialized_message_ = message.payload(); - last_send_success_ = success; - num_send_completed_++; - } - - std::string GetLastDeserializedMessage() { - return last_deserialized_message_; - } - - bool GetLastSendSuccess() { return last_send_success_; } - - int GetNumSendCompleted() { return num_send_completed_; } - - bool delete_on_disconnect() { return delete_on_disconnect_; } - void set_delete_on_disconnect(bool delete_on_disconnect) { - delete_on_disconnect_ = delete_on_disconnect; - } - - private: - std::string last_deserialized_message_; - bool last_send_success_; - int num_send_completed_; - bool delete_on_disconnect_; -}; - -} // namespace namespace weave { + namespace { typedef BluetoothLowEnergyWeaveClientConnection::SubStatus SubStatus; @@ -118,6 +50,8 @@ typedef BluetoothLowEnergyWeavePacketReceiver::State ReceiverState; typedef BluetoothLowEnergyWeavePacketReceiver::ReceiverError ReceiverError; typedef BluetoothLowEnergyWeavePacketReceiver::ReceiverType ReceiverType; +const char kTestFeature[] = "testFeature"; + const char kServiceUUID[] = "DEADBEEF-CAFE-FEED-FOOD-D15EA5EBEEEF"; const char kTXCharacteristicUUID[] = "977c6674-1239-4e72-993b-502369b8bb5a"; const char kRXCharacteristicUUID[] = "f4b904a2-a030-43b3-98a8-221c536c03cb"; @@ -188,11 +122,11 @@ class MockBluetoothLowEnergyWeavePacketGenerator void SetMaxPacketSize(uint16_t size) override { max_packet_size_ = size; } std::vector<Packet> EncodeDataMessage(std::string message) override { - if (message == (kTestFeature + "," + kSmallMessage) - && max_packet_size_ == kDefaultMaxPacketSize) { + if (message == (std::string(kTestFeature) + "," + kSmallMessage) && + max_packet_size_ == kDefaultMaxPacketSize) { return kSmallPackets; - } else if (message == (kTestFeature + "," + kLargeMessage) - && max_packet_size_ == kLargeMaxPacketSize) { + } else if (message == (std::string(kTestFeature) + "," + kLargeMessage) && + max_packet_size_ == kLargeMaxPacketSize) { return kLargePackets; } else { NOTREACHED(); @@ -271,46 +205,6 @@ class MockBluetoothLowEnergyWeavePacketReceiver ReasonForClose reason_to_close_; }; -class MockBluetoothLowEnergyWeavePacketGeneratorFactory - : public BluetoothLowEnergyWeavePacketGenerator::Factory { - public: - // most_recent_instance_ will be obsolete after the connection class - // destructs. Do not use if that's the case. - MockBluetoothLowEnergyWeavePacketGenerator* GetMostRecentInstance() { - return most_recent_instance_; - } - - private: - std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> BuildInstance() - override { - most_recent_instance_ = new MockBluetoothLowEnergyWeavePacketGenerator(); - return std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator>( - most_recent_instance_); - } - - MockBluetoothLowEnergyWeavePacketGenerator* most_recent_instance_; -}; - -class MockBluetoothLowEnergyWeavePacketReceiverFactory - : public BluetoothLowEnergyWeavePacketReceiver::Factory { - public: - // most_recent_instance_ will be obsolete after the connection class - // destructs. Do not use if that's the case. - MockBluetoothLowEnergyWeavePacketReceiver* GetMostRecentInstance() { - return most_recent_instance_; - } - - private: - std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> BuildInstance( - ReceiverType receiver_type) override { - most_recent_instance_ = new MockBluetoothLowEnergyWeavePacketReceiver(); - return std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver>( - most_recent_instance_); - } - - MockBluetoothLowEnergyWeavePacketReceiver* most_recent_instance_; -}; - class TestBluetoothLowEnergyWeaveClientConnection : public BluetoothLowEnergyWeaveClientConnection { public: @@ -318,13 +212,11 @@ class TestBluetoothLowEnergyWeaveClientConnection const RemoteDevice& remote_device, const std::string& device_address, scoped_refptr<device::BluetoothAdapter> adapter, - const device::BluetoothUUID remote_service_uuid, - BluetoothThrottler* bluetooth_throttler) + const device::BluetoothUUID remote_service_uuid) : BluetoothLowEnergyWeaveClientConnection(remote_device, device_address, adapter, - remote_service_uuid, - bluetooth_throttler) {} + remote_service_uuid) {} ~TestBluetoothLowEnergyWeaveClientConnection() override {} @@ -339,7 +231,7 @@ class TestBluetoothLowEnergyWeaveClientConnection // Exposing inherited protected methods for testing. using BluetoothLowEnergyWeaveClientConnection::GattCharacteristicValueChanged; - using BluetoothLowEnergyWeaveClientConnection::SetTaskRunnerForTesting; + using BluetoothLowEnergyWeaveClientConnection::SetupTestDoubles; using BluetoothLowEnergyWeaveClientConnection::DestroyConnection; // Exposing inherited protected fields for testing. @@ -350,43 +242,96 @@ class TestBluetoothLowEnergyWeaveClientConnection DISALLOW_COPY_AND_ASSIGN(TestBluetoothLowEnergyWeaveClientConnection); }; +class MockBluetoothLowEnergyCharacteristicsFinder + : public BluetoothLowEnergyCharacteristicsFinder { + public: + MockBluetoothLowEnergyCharacteristicsFinder() {} + ~MockBluetoothLowEnergyCharacteristicsFinder() override {} + + private: + DISALLOW_COPY_AND_ASSIGN(MockBluetoothLowEnergyCharacteristicsFinder); +}; + +class MockConnectionObserver : public ConnectionObserver { + public: + MockConnectionObserver(Connection* connection) + : connection_(connection), + num_send_completed_(0), + delete_on_disconnect_(false), + delete_on_message_sent_(false) {} + + void OnConnectionStatusChanged(Connection* connection, + Connection::Status old_status, + Connection::Status new_status) override { + if (new_status == Connection::Status::DISCONNECTED && delete_on_disconnect_) + delete connection_; + } + + void OnMessageReceived(const Connection& connection, + const WireMessage& message) override {} + + void OnSendCompleted(const Connection& conenction, + const WireMessage& message, + bool success) override { + last_deserialized_message_ = message.payload(); + last_send_success_ = success; + num_send_completed_++; + + if (delete_on_message_sent_) + delete connection_; + } + + std::string GetLastDeserializedMessage() { + return last_deserialized_message_; + } + + bool GetLastSendSuccess() { return last_send_success_; } + + int GetNumSendCompleted() { return num_send_completed_; } + + bool delete_on_disconnect() { return delete_on_disconnect_; } + + void set_delete_on_disconnect(bool delete_on_disconnect) { + delete_on_disconnect_ = delete_on_disconnect; + } + + void set_delete_on_message_sent(bool delete_on_message_sent) { + delete_on_message_sent_ = delete_on_message_sent; + } + + private: + Connection* connection_; + std::string last_deserialized_message_; + bool last_send_success_; + int num_send_completed_; + bool delete_on_disconnect_; + bool delete_on_message_sent_; +}; + } // namespace class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest : public testing::Test { public: CryptAuthBluetoothLowEnergyWeaveClientConnectionTest() - : adapter_(new NiceMock<device::MockBluetoothAdapter>), - remote_device_(CreateLERemoteDeviceForTest()), + : remote_device_(CreateLERemoteDeviceForTest()), service_uuid_(device::BluetoothUUID(kServiceUUID)), tx_characteristic_uuid_(device::BluetoothUUID(kTXCharacteristicUUID)), - rx_characteristic_uuid_(device::BluetoothUUID(kRXCharacteristicUUID)), - notify_session_alias_(NULL), - bluetooth_throttler_(new NiceMock<MockBluetoothThrottler>), - task_runner_(new base::TestSimpleTaskRunner), - generator_factory_( - new MockBluetoothLowEnergyWeavePacketGeneratorFactory()), - receiver_factory_( - new MockBluetoothLowEnergyWeavePacketReceiverFactory()) { - BluetoothLowEnergyWeavePacketGenerator::Factory::SetInstanceForTesting( - generator_factory_.get()); - BluetoothLowEnergyWeavePacketReceiver::Factory::SetInstanceForTesting( - receiver_factory_.get()); - } - - ~CryptAuthBluetoothLowEnergyWeaveClientConnectionTest() override { - BluetoothLowEnergyWeavePacketGenerator::Factory::SetInstanceForTesting( - nullptr); - BluetoothLowEnergyWeavePacketReceiver::Factory::SetInstanceForTesting( - nullptr); - } + rx_characteristic_uuid_(device::BluetoothUUID(kRXCharacteristicUUID)) {} + ~CryptAuthBluetoothLowEnergyWeaveClientConnectionTest() override {} void SetUp() override { + test_timer_ = nullptr; + generator_ = nullptr; + receiver_ = nullptr; + + adapter_ = make_scoped_refptr(new NiceMock<device::MockBluetoothAdapter>()); + task_runner_ = make_scoped_refptr(new base::TestSimpleTaskRunner()); + mock_bluetooth_device_ = base::MakeUnique<NiceMock<device::MockBluetoothDevice>>( adapter_.get(), 0, kTestRemoteDeviceName, kTestRemoteDeviceBluetoothAddress, false, false); - service_ = base::MakeUnique<NiceMock<device::MockBluetoothGattService>>( mock_bluetooth_device_.get(), kServiceID, service_uuid_, true, false); tx_characteristic_ = @@ -394,15 +339,12 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest service_.get(), kTXCharacteristicID, tx_characteristic_uuid_, false, kCharacteristicProperties, device::BluetoothRemoteGattCharacteristic::PERMISSION_NONE); - rx_characteristic_ = base::MakeUnique<NiceMock<device::MockBluetoothGattCharacteristic>>( service_.get(), kRXCharacteristicID, rx_characteristic_uuid_, false, kCharacteristicProperties, device::BluetoothRemoteGattCharacteristic::PERMISSION_NONE); - device::BluetoothAdapterFactory::SetAdapterForTesting(adapter_); - std::vector<const device::BluetoothDevice*> devices; devices.push_back(mock_bluetooth_device_.get()); ON_CALL(*adapter_, GetDevices()).WillByDefault(Return(devices)); @@ -414,8 +356,12 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest .WillByDefault(Return(rx_characteristic_.get())); ON_CALL(*service_, GetCharacteristic(kTXCharacteristicID)) .WillByDefault(Return(tx_characteristic_.get())); + + device::BluetoothAdapterFactory::SetAdapterForTesting(adapter_); } + void TearDown() override { connection_observer_.reset(); } + // Creates a BluetoothLowEnergyWeaveClientConnection and verifies it's in // DISCONNECTED state. std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> @@ -426,15 +372,23 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( new TestBluetoothLowEnergyWeaveClientConnection( remote_device_, kTestRemoteDeviceBluetoothAddress, adapter_, - service_uuid_, bluetooth_throttler_.get())); + service_uuid_)); EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); EXPECT_EQ(connection->status(), Connection::DISCONNECTED); // Add the mock observer to observe on OnDidMessageSend. - connection->AddObserver(&connection_observer_); - - connection->SetTaskRunnerForTesting(task_runner_); + connection_observer_ = + base::WrapUnique(new MockConnectionObserver(connection.get())); + connection->AddObserver(connection_observer_.get()); + + test_timer_ = new base::MockTimer(false /* retains_user_task */, + false /* is_repeating */); + generator_ = new NiceMock<MockBluetoothLowEnergyWeavePacketGenerator>(); + receiver_ = new NiceMock<MockBluetoothLowEnergyWeavePacketReceiver>(); + connection->SetupTestDoubles(task_runner_, base::WrapUnique(test_timer_), + base::WrapUnique(generator_), + base::WrapUnique(receiver_)); return connection; } @@ -453,10 +407,6 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest .WillOnce(DoAll(SaveArg<0>(&create_gatt_connection_success_callback_), SaveArg<1>(&create_gatt_connection_error_callback_))); - // No throttling by default - EXPECT_CALL(*bluetooth_throttler_, GetDelay()) - .WillOnce(Return(base::TimeDelta())); - connection->Connect(); // Handle setting the connection latency. @@ -521,7 +471,6 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest std::unique_ptr<device::MockBluetoothGattNotifySession> notify_session( new NiceMock<device::MockBluetoothGattNotifySession>( tx_characteristic_->GetWeakPtr())); - notify_session_alias_ = notify_session.get(); notify_session_success_callback_.Run(std::move(notify_session)); task_runner_->RunUntilIdle(); @@ -541,7 +490,7 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest EXPECT_EQ(last_value_written_on_tx_characteristic_, kConnectionRequest); // OnDidSendMessage is not called. - EXPECT_EQ(0, connection_observer_.GetNumSendCompleted()); + EXPECT_EQ(0, connection_observer_->GetNumSendCompleted()); RunWriteCharacteristicSuccessCallback(); @@ -549,17 +498,13 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest if (selected_packet_size == kDefaultMaxPacketSize) { connection->GattCharacteristicValueChanged( adapter_.get(), rx_characteristic_.get(), kSmallConnectionResponse); - EXPECT_EQ(receiver_factory_->GetMostRecentInstance()->GetMaxPacketSize(), - kDefaultMaxPacketSize); - EXPECT_EQ(generator_factory_->GetMostRecentInstance()->GetMaxPacketSize(), - kDefaultMaxPacketSize); + EXPECT_EQ(receiver_->GetMaxPacketSize(), kDefaultMaxPacketSize); + EXPECT_EQ(generator_->GetMaxPacketSize(), kDefaultMaxPacketSize); } else if (selected_packet_size == kLargeMaxPacketSize) { connection->GattCharacteristicValueChanged( adapter_.get(), rx_characteristic_.get(), kLargeConnectionResponse); - EXPECT_EQ(receiver_factory_->GetMostRecentInstance()->GetMaxPacketSize(), - kLargeMaxPacketSize); - EXPECT_EQ(generator_factory_->GetMostRecentInstance()->GetMaxPacketSize(), - kLargeMaxPacketSize); + EXPECT_EQ(receiver_->GetMaxPacketSize(), kLargeMaxPacketSize); + EXPECT_EQ(generator_->GetMaxPacketSize(), kLargeMaxPacketSize); } else { NOTREACHED(); } @@ -571,10 +516,6 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest // Transitions |connection| to a DISCONNECTED state regardless of its initial // state. void Disconnect(TestBluetoothLowEnergyWeaveClientConnection* connection) { - // A notify session was previously set. - if (notify_session_alias_) - EXPECT_CALL(*notify_session_alias_, Stop(_)); - if (connection->sub_status() == SubStatus::CONNECTED) { EXPECT_CALL(*tx_characteristic_, WriteRemoteCharacteristic(_, _, _)) .WillOnce( @@ -609,29 +550,30 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest EXPECT_FALSE(write_remote_characteristic_error_callback_.is_null()); ASSERT_FALSE(write_remote_characteristic_success_callback_.is_null()); write_remote_characteristic_success_callback_.Run(); + task_runner_->RunUntilIdle(); } protected: + const RemoteDevice remote_device_; + const device::BluetoothUUID service_uuid_; + const device::BluetoothUUID tx_characteristic_uuid_; + const device::BluetoothUUID rx_characteristic_uuid_; + const proximity_auth::ScopedDisableLoggingForTesting disable_logging_; + scoped_refptr<device::MockBluetoothAdapter> adapter_; - RemoteDevice remote_device_; - device::BluetoothUUID service_uuid_; - device::BluetoothUUID tx_characteristic_uuid_; - device::BluetoothUUID rx_characteristic_uuid_; + base::MockTimer* test_timer_; + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + std::unique_ptr<device::MockBluetoothDevice> mock_bluetooth_device_; std::unique_ptr<device::MockBluetoothGattService> service_; std::unique_ptr<device::MockBluetoothGattCharacteristic> tx_characteristic_; std::unique_ptr<device::MockBluetoothGattCharacteristic> rx_characteristic_; std::vector<uint8_t> last_value_written_on_tx_characteristic_; - device::MockBluetoothGattNotifySession* notify_session_alias_; - std::unique_ptr<MockBluetoothThrottler> bluetooth_throttler_; - scoped_refptr<base::TestSimpleTaskRunner> task_runner_; base::MessageLoop message_loop_; bool last_wire_message_success_; - std::unique_ptr<MockBluetoothLowEnergyWeavePacketGeneratorFactory> - generator_factory_; - std::unique_ptr<MockBluetoothLowEnergyWeavePacketReceiverFactory> - receiver_factory_; - MockConnectionObserver connection_observer_; + NiceMock<MockBluetoothLowEnergyWeavePacketGenerator>* generator_; + NiceMock<MockBluetoothLowEnergyWeavePacketReceiver>* receiver_; + std::unique_ptr<MockConnectionObserver> connection_observer_; // Callbacks base::Closure connection_latency_callback_; @@ -655,14 +597,16 @@ class CryptAuthBluetoothLowEnergyWeaveClientConnectionTest device::BluetoothRemoteGattCharacteristic::ErrorCallback write_remote_characteristic_error_callback_; - proximity_auth::ScopedDisableLoggingForTesting disable_logging_; + private: + DISALLOW_COPY_AND_ASSIGN( + CryptAuthBluetoothLowEnergyWeaveClientConnectionTest); }; TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, CreateAndDestroyWithoutConnectCallDoesntCrash) { BluetoothLowEnergyWeaveClientConnection connection( remote_device_, kTestRemoteDeviceBluetoothAddress, adapter_, - service_uuid_, bluetooth_throttler_.get()); + service_uuid_); } TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, @@ -737,6 +681,28 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, } TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, + ConnectFailsCharacteristicsFoundThenUnavailable) { + std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( + CreateConnection()); + ConnectGatt(connection.get()); + + // Simulate the inability to fetch the characteristic after it was received. + // This would most likely be due to the Bluetooth device or service being + // removed during a connection attempt. See crbug.com/756174. + EXPECT_CALL(*service_, GetCharacteristic(_)).WillOnce(Return(nullptr)); + + EXPECT_FALSE(characteristics_finder_error_callback_.is_null()); + ASSERT_FALSE(characteristics_finder_success_callback_.is_null()); + characteristics_finder_success_callback_.Run( + {service_uuid_, kServiceID}, + {tx_characteristic_uuid_, kTXCharacteristicID}, + {rx_characteristic_uuid_, kRXCharacteristicID}); + + EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); + EXPECT_EQ(connection->status(), Connection::DISCONNECTED); +} + +TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, ConnectFailsNotifySessionError) { std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( CreateConnection()); @@ -766,7 +732,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, // message |kMaxNumberOfTries| times. There is alredy one EXPECT_CALL for // WriteRemoteCharacteristic(_,_,_) in NotifySessionStated, that's why we use // |kMaxNumberOfTries-1| in the EXPECT_CALL statement. - EXPECT_EQ(0, connection_observer_.GetNumSendCompleted()); + EXPECT_EQ(0, connection_observer_->GetNumSendCompleted()); EXPECT_CALL(*tx_characteristic_, WriteRemoteCharacteristic(_, _, _)) .Times(kMaxNumberOfTries - 1) .WillRepeatedly( @@ -780,6 +746,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, EXPECT_FALSE(write_remote_characteristic_success_callback_.is_null()); write_remote_characteristic_error_callback_.Run( device::BluetoothRemoteGattService::GATT_ERROR_UNKNOWN); + task_runner_->RunUntilIdle(); } EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); @@ -843,9 +810,9 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, RunWriteCharacteristicSuccessCallback(); - EXPECT_EQ(1, connection_observer_.GetNumSendCompleted()); - EXPECT_EQ(kSmallMessage, connection_observer_.GetLastDeserializedMessage()); - EXPECT_TRUE(connection_observer_.GetLastSendSuccess()); + EXPECT_EQ(1, connection_observer_->GetNumSendCompleted()); + EXPECT_EQ(kSmallMessage, connection_observer_->GetLastDeserializedMessage()); + EXPECT_TRUE(connection_observer_->GetLastSendSuccess()); } TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, @@ -887,9 +854,9 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, RunWriteCharacteristicSuccessCallback(); - EXPECT_EQ(1, connection_observer_.GetNumSendCompleted()); - EXPECT_EQ(kLargeMessage, connection_observer_.GetLastDeserializedMessage()); - EXPECT_TRUE(connection_observer_.GetLastSendSuccess()); + EXPECT_EQ(1, connection_observer_->GetNumSendCompleted()); + EXPECT_EQ(kLargeMessage, connection_observer_->GetLastDeserializedMessage()); + EXPECT_TRUE(connection_observer_->GetLastSendSuccess()); } TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, @@ -914,13 +881,14 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, EXPECT_FALSE(write_remote_characteristic_success_callback_.is_null()); write_remote_characteristic_error_callback_.Run( device::BluetoothRemoteGattService::GATT_ERROR_UNKNOWN); + task_runner_->RunUntilIdle(); if (i == kMaxNumberOfTries - 1) { - EXPECT_EQ(1, connection_observer_.GetNumSendCompleted()); + EXPECT_EQ(1, connection_observer_->GetNumSendCompleted()); EXPECT_EQ(kSmallMessage, - connection_observer_.GetLastDeserializedMessage()); - EXPECT_FALSE(connection_observer_.GetLastSendSuccess()); + connection_observer_->GetLastDeserializedMessage()); + EXPECT_FALSE(connection_observer_->GetLastSendSuccess()); } else { - EXPECT_EQ(0, connection_observer_.GetNumSendCompleted()); + EXPECT_EQ(0, connection_observer_->GetNumSendCompleted()); } } @@ -937,8 +905,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, connection->GattCharacteristicValueChanged( adapter_.get(), rx_characteristic_.get(), kConnectionCloseUnknownError); - EXPECT_EQ(receiver_factory_->GetMostRecentInstance()->GetReasonForClose(), - ReasonForClose::UNKNOWN_ERROR); + EXPECT_EQ(receiver_->GetReasonForClose(), ReasonForClose::UNKNOWN_ERROR); EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); EXPECT_EQ(connection->status(), Connection::DISCONNECTED); } @@ -961,8 +928,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, EXPECT_EQ(last_value_written_on_tx_characteristic_, kConnectionCloseApplicationError); - EXPECT_EQ(receiver_factory_->GetMostRecentInstance()->GetReasonToClose(), - ReasonForClose::APPLICATION_ERROR); + EXPECT_EQ(receiver_->GetReasonToClose(), ReasonForClose::APPLICATION_ERROR); RunWriteCharacteristicSuccessCallback(); EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); @@ -1000,8 +966,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, EXPECT_EQ(last_value_written_on_tx_characteristic_, kConnectionCloseApplicationError); - EXPECT_EQ(receiver_factory_->GetMostRecentInstance()->GetReasonToClose(), - ReasonForClose::APPLICATION_ERROR); + EXPECT_EQ(receiver_->GetReasonToClose(), ReasonForClose::APPLICATION_ERROR); RunWriteCharacteristicSuccessCallback(); EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); @@ -1010,11 +975,10 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, // Test for fix to crbug.com/708744. Without the fix, this test will crash. TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, - ReceiverErrorAndConnectionDeletedTest) { - connection_observer_.set_delete_on_disconnect(true); - + ObserverDeletesConnectionOnDisconnect) { TestBluetoothLowEnergyWeaveClientConnection* connection = CreateConnection().release(); + connection_observer_->set_delete_on_disconnect(true); InitializeConnection(connection, kDefaultMaxPacketSize); @@ -1029,10 +993,38 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, EXPECT_EQ(last_value_written_on_tx_characteristic_, kConnectionCloseApplicationError); - EXPECT_EQ(receiver_factory_->GetMostRecentInstance()->GetReasonToClose(), - ReasonForClose::APPLICATION_ERROR); + EXPECT_EQ(receiver_->GetReasonToClose(), ReasonForClose::APPLICATION_ERROR); + + RunWriteCharacteristicSuccessCallback(); + + // We cannot check if connection's status and sub_status are DISCONNECTED + // because it has been deleted. +} + +// Test for fix to crbug.com/ 751884. Without the fix, this test will crash. +TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, + ObserverDeletesConnectionOnMessageSent) { + TestBluetoothLowEnergyWeaveClientConnection* connection = + CreateConnection().release(); + connection_observer_->set_delete_on_message_sent(true); + + InitializeConnection(connection, kDefaultMaxPacketSize); + + EXPECT_CALL(*tx_characteristic_, WriteRemoteCharacteristic(_, _, _)) + .WillOnce( + DoAll(SaveArg<0>(&last_value_written_on_tx_characteristic_), + SaveArg<1>(&write_remote_characteristic_success_callback_), + SaveArg<2>(&write_remote_characteristic_error_callback_))); + + connection->SendMessage( + base::MakeUnique<FakeWireMessage>(kSmallMessage, kTestFeature)); + EXPECT_EQ(last_value_written_on_tx_characteristic_, kSmallPackets0); RunWriteCharacteristicSuccessCallback(); + task_runner_->RunUntilIdle(); + EXPECT_EQ(1, connection_observer_->GetNumSendCompleted()); + EXPECT_EQ(kSmallMessage, connection_observer_->GetLastDeserializedMessage()); + EXPECT_TRUE(connection_observer_->GetLastSendSuccess()); // We cannot check if connection's status and sub_status are DISCONNECTED // because it has been deleted. @@ -1070,6 +1062,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, write_remote_characteristic_error_callback_.Run( device::BluetoothRemoteGattService::GATT_ERROR_UNKNOWN); + task_runner_->RunUntilIdle(); } EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); @@ -1081,8 +1074,6 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( CreateConnection()); - EXPECT_CALL(*bluetooth_throttler_, GetDelay()) - .WillOnce(Return(base::TimeDelta(base::TimeDelta::FromSeconds(1)))); EXPECT_CALL(*mock_bluetooth_device_, SetConnectionLatency( device::BluetoothDevice::CONNECTION_LATENCY_LOW, _, _)) @@ -1162,6 +1153,112 @@ TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, ConnectionResponseReceived(connection.get(), kDefaultMaxPacketSize); } +TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, + Timeout_ConnectionLatency) { + std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( + CreateConnection()); + + EXPECT_CALL(*mock_bluetooth_device_, + SetConnectionLatency( + device::BluetoothDevice::CONNECTION_LATENCY_LOW, _, _)) + .WillOnce(DoAll(SaveArg<1>(&connection_latency_callback_), + SaveArg<2>(&connection_latency_error_callback_))); + + // Call Connect(), which should set the connection latency. + connection->Connect(); + EXPECT_EQ(connection->sub_status(), SubStatus::WAITING_CONNECTION_LATENCY); + EXPECT_EQ(connection->status(), Connection::IN_PROGRESS); + ASSERT_FALSE(connection_latency_callback_.is_null()); + ASSERT_FALSE(connection_latency_error_callback_.is_null()); + + // Simulate a timeout. + test_timer_->Fire(); + + EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); + EXPECT_EQ(connection->status(), Connection::DISCONNECTED); +} + +TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, + Timeout_GattConnection) { + std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( + CreateConnection()); + + EXPECT_CALL(*mock_bluetooth_device_, + SetConnectionLatency( + device::BluetoothDevice::CONNECTION_LATENCY_LOW, _, _)) + .WillOnce(DoAll(SaveArg<1>(&connection_latency_callback_), + SaveArg<2>(&connection_latency_error_callback_))); + + // Preparing |connection| for a CreateGattConnection call. + EXPECT_CALL(*mock_bluetooth_device_, CreateGattConnection(_, _)) + .WillOnce(DoAll(SaveArg<0>(&create_gatt_connection_success_callback_), + SaveArg<1>(&create_gatt_connection_error_callback_))); + + connection->Connect(); + + // Handle setting the connection latency. + EXPECT_EQ(connection->sub_status(), SubStatus::WAITING_CONNECTION_LATENCY); + EXPECT_EQ(connection->status(), Connection::IN_PROGRESS); + ASSERT_FALSE(connection_latency_callback_.is_null()); + ASSERT_FALSE(connection_latency_error_callback_.is_null()); + connection_latency_callback_.Run(); + + EXPECT_EQ(connection->sub_status(), SubStatus::WAITING_GATT_CONNECTION); + EXPECT_EQ(connection->status(), Connection::IN_PROGRESS); + + // Simulate a timeout. + test_timer_->Fire(); + + EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); + EXPECT_EQ(connection->status(), Connection::DISCONNECTED); +} + +TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, + Timeout_GattCharacteristics) { + std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( + CreateConnection()); + ConnectGatt(connection.get()); + EXPECT_EQ(connection->sub_status(), SubStatus::WAITING_CHARACTERISTICS); + EXPECT_EQ(connection->status(), Connection::IN_PROGRESS); + + // Simulate a timeout. + test_timer_->Fire(); + + EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); + EXPECT_EQ(connection->status(), Connection::DISCONNECTED); +} + +TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, + Timeout_NotifySession) { + std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( + CreateConnection()); + ConnectGatt(connection.get()); + CharacteristicsFound(connection.get()); + EXPECT_EQ(connection->sub_status(), SubStatus::WAITING_NOTIFY_SESSION); + EXPECT_EQ(connection->status(), Connection::IN_PROGRESS); + + // Simulate a timeout. + test_timer_->Fire(); + + EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); + EXPECT_EQ(connection->status(), Connection::DISCONNECTED); +} + +TEST_F(CryptAuthBluetoothLowEnergyWeaveClientConnectionTest, + Timeout_ConnectionResponse) { + std::unique_ptr<TestBluetoothLowEnergyWeaveClientConnection> connection( + CreateConnection()); + ConnectGatt(connection.get()); + CharacteristicsFound(connection.get()); + NotifySessionStarted(connection.get()); + + // Simulate a timeout. + test_timer_->Fire(); + + EXPECT_EQ(connection->sub_status(), SubStatus::DISCONNECTED); + EXPECT_EQ(connection->status(), Connection::DISCONNECTED); +} + } // namespace weave } // namespace cryptauth diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.cc b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.cc index cba14631aaf..f4b0205f4b9 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.cc +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.cc @@ -4,50 +4,25 @@ #include "components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.h" +#include <string.h> +#include <algorithm> #ifdef OS_WIN #include <winsock2.h> #else #include <netinet/in.h> #endif -#include <string.h> - -#include <algorithm> - #include "base/logging.h" namespace cryptauth { namespace weave { -// static. -BluetoothLowEnergyWeavePacketGenerator::Factory* - BluetoothLowEnergyWeavePacketGenerator::Factory::factory_instance_ = - nullptr; - -// static. -std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> -BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance() { - if (!factory_instance_) { - factory_instance_ = new Factory(); - } - return factory_instance_->BuildInstance(); -} - -// static. -void BluetoothLowEnergyWeavePacketGenerator::Factory::SetInstanceForTesting( - Factory* factory) { - factory_instance_ = factory; -} - -std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> -BluetoothLowEnergyWeavePacketGenerator::Factory::BuildInstance() { - return std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator>( - new BluetoothLowEnergyWeavePacketGenerator()); -} - BluetoothLowEnergyWeavePacketGenerator::BluetoothLowEnergyWeavePacketGenerator() : max_packet_size_(kDefaultMaxPacketSize), next_packet_counter_(0) {} +BluetoothLowEnergyWeavePacketGenerator:: + ~BluetoothLowEnergyWeavePacketGenerator() {} + Packet BluetoothLowEnergyWeavePacketGenerator::CreateConnectionRequest() { Packet packet(kMinConnectionRequestSize, 0); diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.h b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.h index 2cee2b8f164..e6aea4387f0 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.h +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator.h @@ -21,22 +21,8 @@ namespace weave { // Generates the messages sent using the uWeave protocol. class BluetoothLowEnergyWeavePacketGenerator { public: - class Factory { - public: - static std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> - NewInstance(); - - // Exposed for testing. - static void SetInstanceForTesting(Factory* factory); - - protected: - // Exposed for testing. - virtual std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> - BuildInstance(); - - private: - static Factory* factory_instance_; - }; + BluetoothLowEnergyWeavePacketGenerator(); + virtual ~BluetoothLowEnergyWeavePacketGenerator(); virtual Packet CreateConnectionRequest(); virtual Packet CreateConnectionResponse(); @@ -48,9 +34,6 @@ class BluetoothLowEnergyWeavePacketGenerator { // Will crash if message is empty. virtual std::vector<Packet> EncodeDataMessage(std::string message); - protected: - BluetoothLowEnergyWeavePacketGenerator(); - private: void SetShortField(uint32_t byte_offset, uint16_t val, Packet* packet); void SetPacketTypeBit(PacketType val, Packet* packet); diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator_unittest.cc b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator_unittest.cc index 7e1b4ed56ae..f9bf22575f4 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator_unittest.cc +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_generator_unittest.cc @@ -9,6 +9,7 @@ #include "base/logging.h" #include "base/macros.h" +#include "base/memory/ptr_util.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -23,7 +24,7 @@ class CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest void TestConnectionCloseWithReason(ReasonForClose reason_for_close, uint8_t expected_reason_for_close) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); Packet packet = generator->CreateConnectionClose(reason_for_close); @@ -51,7 +52,7 @@ class CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, CreateConnectionRequestTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); Packet packet = generator->CreateConnectionRequest(); @@ -76,7 +77,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, CreateConnectionResponseWithDefaultPacketSizeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); Packet packet = generator->CreateConnectionResponse(); @@ -96,7 +97,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, CreateConnectionResponseWithSelectedPacketSizeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); const uint8_t kSelectedPacketSize = 30; const uint16_t kResponseSize = 5; @@ -137,7 +138,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, EncodeDataMessageWithDefaultPacketSizeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); std::string data = "abcdefghijklmnopqrstuvwxyz"; @@ -170,7 +171,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, EncodeDataMessageWithSelectedPacketSizeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); const uint32_t packet_size = 30; const uint32_t residual_packet_size = 2; @@ -222,7 +223,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, PacketCounterForMixedPacketTypesTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); Packet packet = generator->CreateConnectionRequest(); @@ -241,7 +242,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketGeneratorTest, PacketCounterWrappedAroundTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketGenerator> generator = - BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance(); + base::MakeUnique<BluetoothLowEnergyWeavePacketGenerator>(); const uint8_t kNumPackets = 100; std::string data(kNumPackets * kByteDefaultMaxPacketSize, 'a'); diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.cc b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.cc index b1055e86ba3..b90cdad4c40 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.cc +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.cc @@ -21,32 +21,6 @@ const uint16_t kMaxPacketSizeLowerBound = 20; } // namespace -BluetoothLowEnergyWeavePacketReceiver::Factory* - BluetoothLowEnergyWeavePacketReceiver::Factory::factory_instance_ = nullptr; - -// static -std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> -BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( - ReceiverType receiver_type) { - if (!factory_instance_) { - factory_instance_ = new Factory(); - } - return factory_instance_->BuildInstance(receiver_type); -} - -// static -void BluetoothLowEnergyWeavePacketReceiver::Factory::SetInstanceForTesting( - Factory* factory) { - factory_instance_ = factory; -} - -std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> -BluetoothLowEnergyWeavePacketReceiver::Factory::BuildInstance( - ReceiverType receiver_type) { - return std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver>( - new BluetoothLowEnergyWeavePacketReceiver(receiver_type)); -} - BluetoothLowEnergyWeavePacketReceiver::BluetoothLowEnergyWeavePacketReceiver( ReceiverType receiver_type) : receiver_type_(receiver_type), diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.h b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.h index e369f540366..c3f68e6ae08 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.h +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver.h @@ -108,23 +108,7 @@ class BluetoothLowEnergyWeavePacketReceiver { PACKET_OUT_OF_SEQUENCE }; - class Factory { - public: - static std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> NewInstance( - ReceiverType receiver_type); - - // Exposed for testing. - static void SetInstanceForTesting(Factory* factory); - - protected: - // Exposed for testing. - virtual std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> - BuildInstance(ReceiverType receiver_type); - - private: - static Factory* factory_instance_; - }; - + explicit BluetoothLowEnergyWeavePacketReceiver(ReceiverType receiver_type); ~BluetoothLowEnergyWeavePacketReceiver(); typedef std::vector<uint8_t> Packet; @@ -159,9 +143,6 @@ class BluetoothLowEnergyWeavePacketReceiver { // Add a packet that's just been received over Connection to the receiver. virtual State ReceivePacket(const Packet& packet); - protected: - explicit BluetoothLowEnergyWeavePacketReceiver(ReceiverType receiver_type); - private: void ReceiveFirstPacket(const Packet& packet); void ReceiveNonFirstPacket(const Packet& packet); diff --git a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver_unittest.cc b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver_unittest.cc index 827e88b5916..a76d19bd2fb 100644 --- a/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver_unittest.cc +++ b/chromium/components/cryptauth/ble/bluetooth_low_energy_weave_packet_receiver_unittest.cc @@ -8,6 +8,7 @@ #include <string> #include "base/logging.h" +#include "base/memory/ptr_util.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -49,7 +50,7 @@ class CryptAuthBluetoothLowEnergyWeavePacketReceiverTest TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, WellBehavingServerPacketsNoControlDataTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -110,7 +111,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, WellBehavingServerPacketsWithFullControlDataTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, @@ -176,7 +177,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, WellBehavingServerPacketsWithSomeControlDataTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -226,7 +227,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, WellBehavingClientPacketsNoControlDataTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); const uint8_t kSelectedPacketSize = 30; @@ -265,7 +266,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, WellBehavingClientPacketsWithFullControlDataTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, @@ -318,7 +319,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, WellBehavingClientPacketsWithSomeControlDataTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, @@ -359,7 +360,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, LegacyCloseWithoutReasonTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -383,7 +384,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, OneBytePacketTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, kEmptyUpperByte, @@ -408,7 +409,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, EmptyPacketTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0; @@ -421,7 +422,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ServerReceivingConnectionResponseTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlResponseHeader, kEmptyUpperByte, kByteWeaveVersion, kEmptyUpperByte, @@ -436,7 +437,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ClientReceivingConnectionRequestTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, kByteWeaveVersion, kEmptyUpperByte, @@ -452,7 +453,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ReceiveConnectionCloseInConnecting) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); // uWeave Header: @@ -471,7 +472,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ReceiveDataInConnecting) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); // uWeave Header: @@ -492,7 +493,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ConnectionRequestTooSmallTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, kByteWeaveVersion, kEmptyUpperByte, @@ -507,7 +508,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ConnectionRequestTooLargeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0(kByteDefaultMaxPacketSize + 1, 0); @@ -524,7 +525,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ConnectionResponseTooSmallTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, kEmptyUpperByte, @@ -539,7 +540,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ConnectionResponseTooLargeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0(kByteDefaultMaxPacketSize + 1, 0); @@ -556,7 +557,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ConnectionCloseTooLargeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -581,7 +582,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, DataPacketTooLargeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -610,7 +611,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, FirstPacketNoFirstNorLastBitTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -639,7 +640,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, FirstPacketNoFirstYesLastBitTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -668,7 +669,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, NonFirstPacketYesFirstBitTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -708,7 +709,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, OutOfOrderPacketTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -737,7 +738,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, InvalidVersionInConnectionRequestTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); const uint8_t kWrongVersion = 2; @@ -758,7 +759,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, const uint8_t kSmallMaxPacketSize = 19; std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -775,7 +776,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, InvalidSelectedVersionInConnectionResponseTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, kByteWeaveVersion, @@ -792,7 +793,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, InvalidSelectedMaxPacketSizeInConnectionResponseTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); const uint8_t kSmallMaxPacketSize = 19; @@ -809,7 +810,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, UnrecognizedReasonForCloseInConnectionCloseTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, kEmptyUpperByte, @@ -835,7 +836,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, UnrecognizedControlCommandBitTwoTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); // uWeave Header: @@ -860,7 +861,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, InvalidControlCommandBitThreeTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); // uWeave Header: @@ -880,7 +881,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, InvalidBitOneInDataPacketHeaderTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, kEmptyUpperByte, @@ -907,7 +908,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, InvalidBitZeroInDataPacketHeaderTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0{kControlResponseHeader, kEmptyUpperByte, @@ -934,7 +935,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ReceivedPacketInErrorState) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::CLIENT); std::vector<uint8_t> p0; @@ -954,7 +955,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, ReceivedPacketInConnectionClosedStateTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, @@ -992,7 +993,7 @@ TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, TEST_F(CryptAuthBluetoothLowEnergyWeavePacketReceiverTest, MultipleControlPacketTest) { std::unique_ptr<BluetoothLowEnergyWeavePacketReceiver> receiver = - BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance( + base::MakeUnique<BluetoothLowEnergyWeavePacketReceiver>( ReceiverType::SERVER); std::vector<uint8_t> p0{kControlRequestHeader, kEmptyUpperByte, diff --git a/chromium/components/cryptauth/bluetooth_throttler.h b/chromium/components/cryptauth/bluetooth_throttler.h deleted file mode 100644 index 90eed117951..00000000000 --- a/chromium/components/cryptauth/bluetooth_throttler.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2015 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_CRYPTAUTH_BLUETOOTH_THROTTLER_H_ -#define COMPONENTS_CRYPTAUTH_BLUETOOTH_THROTTLER_H_ - -namespace base { -class TimeDelta; -} - -namespace cryptauth { - -class Connection; - -// An interface for throttling repeated connection attempts to the same device. -// This throttling is necessary to prevent a kernel race condition when -// connecting before the previous connection fully closes, putting the -// connection in a corrupted, and unrecoverable state. http://crbug.com/345232 -class BluetoothThrottler { - public: - virtual ~BluetoothThrottler() {} - - // Returns the current delay that must be respected prior to reattempting to - // establish a connection with the remote device. The returned value is 0 if - // no delay is needed. - virtual base::TimeDelta GetDelay() const = 0; - - // Should be called when a connection to the remote device is established. - // Note that the |connection| is passed as a weak reference. The throttler - // will ensure, by registering as an observer, that it never attempts to use - // the connection after it has been destroyed. - virtual void OnConnection(Connection* connection) = 0; -}; - -} // namespace cryptauth - -#endif // COMPONENTS_CRYPTAUTH_BLUETOOTH_THROTTLER_H_ diff --git a/chromium/components/cryptauth/bluetooth_throttler_impl.cc b/chromium/components/cryptauth/bluetooth_throttler_impl.cc deleted file mode 100644 index 74e09192655..00000000000 --- a/chromium/components/cryptauth/bluetooth_throttler_impl.cc +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2015 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/cryptauth/bluetooth_throttler_impl.h" - -#include <utility> - -#include "base/memory/ptr_util.h" -#include "base/stl_util.h" -#include "base/time/default_tick_clock.h" -#include "base/time/tick_clock.h" -#include "components/cryptauth/connection.h" - -namespace cryptauth { -namespace { - -// Time to wait after disconnect before reconnecting. -const int kCooldownTimeSecs = 1; - -} // namespace - -// static -BluetoothThrottlerImpl* BluetoothThrottlerImpl::GetInstance() { - return base::Singleton<BluetoothThrottlerImpl>::get(); -} - -BluetoothThrottlerImpl::BluetoothThrottlerImpl() - : BluetoothThrottlerImpl(base::MakeUnique<base::DefaultTickClock>()) {} - -BluetoothThrottlerImpl::BluetoothThrottlerImpl( - std::unique_ptr<base::TickClock> clock) - : clock_(std::move(clock)) {} - -BluetoothThrottlerImpl::~BluetoothThrottlerImpl() { - for (Connection* connection : connections_) { - connection->RemoveObserver(this); - } -} - -base::TimeDelta BluetoothThrottlerImpl::GetDelay() const { - if (last_disconnect_time_.is_null()) - return base::TimeDelta(); - - base::TimeTicks now = clock_->NowTicks(); - base::TimeTicks throttled_start_time = - last_disconnect_time_ + GetCooldownTimeDelta(); - if (now >= throttled_start_time) - return base::TimeDelta(); - - return throttled_start_time - now; -} - -void BluetoothThrottlerImpl::OnConnection(Connection* connection) { - DCHECK(!base::ContainsKey(connections_, connection)); - connections_.insert(connection); - connection->AddObserver(this); -} - -base::TimeDelta BluetoothThrottlerImpl::GetCooldownTimeDelta() const { - return base::TimeDelta::FromSeconds(kCooldownTimeSecs); -} - -void BluetoothThrottlerImpl::OnConnectionStatusChanged( - Connection* connection, - Connection::Status old_status, - Connection::Status new_status) { - DCHECK(base::ContainsKey(connections_, connection)); - if (new_status == Connection::DISCONNECTED) { - last_disconnect_time_ = clock_->NowTicks(); - connection->RemoveObserver(this); - connections_.erase(connection); - } -} - -} // namespace cryptauth diff --git a/chromium/components/cryptauth/bluetooth_throttler_impl.h b/chromium/components/cryptauth/bluetooth_throttler_impl.h deleted file mode 100644 index 7a9fef028f0..00000000000 --- a/chromium/components/cryptauth/bluetooth_throttler_impl.h +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2015 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_CRYPTAUTH_BLUETOOTH_THROTTLER_IMPL_H_ -#define COMPONENTS_CRYPTAUTH_BLUETOOTH_THROTTLER_IMPL_H_ - -#include <memory> -#include <set> - -#include "base/macros.h" -#include "base/memory/singleton.h" -#include "base/time/time.h" -#include "components/cryptauth/bluetooth_throttler.h" -#include "components/cryptauth/connection_observer.h" - -namespace base { -class TickClock; -} - -namespace cryptauth { - -class Connection; - -// This class throttles repeated connection attempts to the same device. This -// throttling is necessary to prevent a kernel race condition when connecting -// before the previous connection fully closes, putting the connection in a -// corrupted, and unrecoverable state. http://crbug.com/345232 -class BluetoothThrottlerImpl : public BluetoothThrottler, - public ConnectionObserver { - public: - static BluetoothThrottlerImpl* GetInstance(); - ~BluetoothThrottlerImpl() override; - - // BluetoothThrottler: - base::TimeDelta GetDelay() const override; - void OnConnection(Connection* connection) override; - - protected: - // Creates a throttler for connections to a remote device, using the |clock| - // as a time source. - explicit BluetoothThrottlerImpl(std::unique_ptr<base::TickClock> clock); - - // Returns the duration to wait, after disconnecting, before reattempting a - // connection to the remote device. Exposed for testing. - base::TimeDelta GetCooldownTimeDelta() const; - - private: - friend struct base::DefaultSingletonTraits<BluetoothThrottlerImpl>; - - BluetoothThrottlerImpl(); - - // ConnectionObserver: - void OnConnectionStatusChanged(Connection* connection, - Connection::Status old_status, - Connection::Status new_status) override; - - // Tracks the last seen disconnect time for the |remote_device_|. - base::TimeTicks last_disconnect_time_; - - // The time source. - std::unique_ptr<base::TickClock> clock_; - - // The currently connected connections. - // Each connection is stored as a weak reference, which is safe because |this| - // instance is registered as an observer, and will unregister when the - // connection disconnects, which is guaranteed to occur before the connection - // is destroyed. - std::set<Connection*> connections_; - - DISALLOW_COPY_AND_ASSIGN(BluetoothThrottlerImpl); -}; - -} // namespace cryptauth - -#endif // COMPONENTS_CRYPTAUTH_BLUETOOTH_THROTTLER_IMPL_H_ diff --git a/chromium/components/cryptauth/bluetooth_throttler_impl_unittest.cc b/chromium/components/cryptauth/bluetooth_throttler_impl_unittest.cc deleted file mode 100644 index 7a3a028f738..00000000000 --- a/chromium/components/cryptauth/bluetooth_throttler_impl_unittest.cc +++ /dev/null @@ -1,101 +0,0 @@ -// Copyright 2015 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/cryptauth/bluetooth_throttler_impl.h" - -#include <utility> - -#include "base/macros.h" -#include "base/memory/ptr_util.h" -#include "base/test/simple_test_tick_clock.h" -#include "base/time/time.h" -#include "components/cryptauth/fake_connection.h" -#include "components/cryptauth/wire_message.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace cryptauth { -namespace { - -class TestBluetoothThrottler : public BluetoothThrottlerImpl { - public: - explicit TestBluetoothThrottler(std::unique_ptr<base::TickClock> clock) - : BluetoothThrottlerImpl(std::move(clock)) {} - ~TestBluetoothThrottler() override {} - - // Increase visibility for testing. - using BluetoothThrottlerImpl::GetCooldownTimeDelta; - - private: - DISALLOW_COPY_AND_ASSIGN(TestBluetoothThrottler); -}; - -} // namespace - -class CryptAuthBluetoothThrottlerImplTest : public testing::Test { - public: - CryptAuthBluetoothThrottlerImplTest() - : clock_(new base::SimpleTestTickClock), - throttler_(base::WrapUnique(clock_)) { - // The throttler treats null times as special, so start with a non-null - // time. - clock_->Advance(base::TimeDelta::FromSeconds(1)); - } - - void PerformConnectionStateTransition(Connection::Status old_status, - Connection::Status new_status) { - FakeConnection connection((RemoteDevice())); - throttler_.OnConnection(&connection); - static_cast<ConnectionObserver*>(&throttler_) - ->OnConnectionStatusChanged(&connection, old_status, new_status); - } - - protected: - // The clock is owned by the |throttler_|. - base::SimpleTestTickClock* clock_; - TestBluetoothThrottler throttler_; -}; - -TEST_F(CryptAuthBluetoothThrottlerImplTest, - GetDelay_FirstConnectionIsNotThrottled) { - EXPECT_EQ(base::TimeDelta(), throttler_.GetDelay()); -} - -TEST_F(CryptAuthBluetoothThrottlerImplTest, - GetDelay_ConnectionAfterDisconnectIsThrottled) { - // Simulate a connection followed by a disconnection. - PerformConnectionStateTransition(Connection::CONNECTED, - Connection::DISCONNECTED); - EXPECT_GT(throttler_.GetDelay(), base::TimeDelta()); -} - -TEST_F(CryptAuthBluetoothThrottlerImplTest, - GetDelay_ConnectionAfterIsProgressDisconnectIsThrottled) { - // Simulate an attempt to connect (in progress connection) followed by a - // disconnection. - PerformConnectionStateTransition(Connection::IN_PROGRESS, - Connection::DISCONNECTED); - EXPECT_GT(throttler_.GetDelay(), base::TimeDelta()); -} - -TEST_F(CryptAuthBluetoothThrottlerImplTest, - GetDelay_DelayedConnectionAfterDisconnectIsNotThrottled) { - // Simulate a connection followed by a disconnection, then allow the cooldown - // period to elapse. - PerformConnectionStateTransition(Connection::CONNECTED, - Connection::DISCONNECTED); - clock_->Advance(throttler_.GetCooldownTimeDelta()); - EXPECT_EQ(base::TimeDelta(), throttler_.GetDelay()); -} - -TEST_F(CryptAuthBluetoothThrottlerImplTest, - GetDelay_DelayedConnectionAfterInProgressDisconnectIsNotThrottled) { - // Simulate an attempt to connect (in progress connection) followed by a - // disconnection, then allow the cooldown period to elapse. - PerformConnectionStateTransition(Connection::IN_PROGRESS, - Connection::DISCONNECTED); - clock_->Advance(throttler_.GetCooldownTimeDelta()); - EXPECT_EQ(base::TimeDelta(), throttler_.GetDelay()); -} - -} // namespace cryptauth diff --git a/chromium/components/cryptauth/connection.cc b/chromium/components/cryptauth/connection.cc index ac0b2c318b2..a97fd688891 100644 --- a/chromium/components/cryptauth/connection.cc +++ b/chromium/components/cryptauth/connection.cc @@ -4,11 +4,13 @@ #include "components/cryptauth/connection.h" +#include <sstream> #include <utility> #include "base/logging.h" #include "components/cryptauth/connection_observer.h" #include "components/cryptauth/wire_message.h" +#include "components/proximity_auth/logging/logging.h" namespace cryptauth { @@ -25,12 +27,14 @@ bool Connection::IsConnected() const { void Connection::SendMessage(std::unique_ptr<WireMessage> message) { if (!IsConnected()) { - VLOG(1) << "Cannot send message when disconnected."; + PA_LOG(ERROR) << "Not yet connected; cannot send message to " + << GetDeviceInfoLogString(); return; } if (is_sending_message_) { - VLOG(1) << "Another message is currently in progress."; + PA_LOG(ERROR) << "Cannot send message because another message is currently " + << "being sent to " << GetDeviceInfoLogString(); return; } @@ -64,7 +68,8 @@ void Connection::SetStatus(Status status) { void Connection::OnDidSendMessage(const WireMessage& message, bool success) { if (!is_sending_message_) { - VLOG(1) << "Send completed, but no message in progress."; + PA_LOG(ERROR) << "OnDidSendMessage(), but a send was not expected to be in " + << "progress to " << GetDeviceInfoLogString(); return; } @@ -75,7 +80,8 @@ void Connection::OnDidSendMessage(const WireMessage& message, bool success) { void Connection::OnBytesReceived(const std::string& bytes) { if (!IsConnected()) { - VLOG(1) << "Received bytes, but not connected."; + PA_LOG(ERROR) << "OnBytesReceived(), but not connected to " + << GetDeviceInfoLogString(); return; } @@ -102,4 +108,11 @@ std::unique_ptr<WireMessage> Connection::DeserializeWireMessage( return WireMessage::Deserialize(received_bytes_, is_incomplete_message); } +std::string Connection::GetDeviceInfoLogString() { + std::stringstream ss; + ss << "{id: \"" << remote_device().GetTruncatedDeviceIdForLogs() + << "\", addr: \"" << GetDeviceAddress() << "\"}"; + return ss.str(); +} + } // namespace cryptauth diff --git a/chromium/components/cryptauth/connection.h b/chromium/components/cryptauth/connection.h index dbc731d728a..16077c5ef46 100644 --- a/chromium/components/cryptauth/connection.h +++ b/chromium/components/cryptauth/connection.h @@ -92,6 +92,9 @@ class Connection { virtual std::unique_ptr<WireMessage> DeserializeWireMessage( bool* is_incomplete_message); + // Returns a string describing the associated device for logging purposes. + std::string GetDeviceInfoLogString(); + private: // The remote device corresponding to this connection. const RemoteDevice remote_device_; diff --git a/chromium/components/cryptauth/cryptauth_api_call_flow.cc b/chromium/components/cryptauth/cryptauth_api_call_flow.cc index cfacd8992bb..c0de9b4e446 100644 --- a/chromium/components/cryptauth/cryptauth_api_call_flow.cc +++ b/chromium/components/cryptauth/cryptauth_api_call_flow.cc @@ -6,6 +6,7 @@ #include "base/strings/string_number_conversions.h" #include "components/proximity_auth/logging/logging.h" +#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/url_fetcher.h" namespace cryptauth { @@ -80,4 +81,10 @@ void CryptAuthApiCallFlow::ProcessApiCallFailure( error_callback_.Run(error_message); } +net::PartialNetworkTrafficAnnotationTag +CryptAuthApiCallFlow::GetNetworkTrafficAnnotationTag() { + DCHECK(partial_network_annotation_ != nullptr); + return *partial_network_annotation_.get(); +} + } // namespace cryptauth diff --git a/chromium/components/cryptauth/cryptauth_api_call_flow.h b/chromium/components/cryptauth/cryptauth_api_call_flow.h index 07a487edca6..ce5e34d294e 100644 --- a/chromium/components/cryptauth/cryptauth_api_call_flow.h +++ b/chromium/components/cryptauth/cryptauth_api_call_flow.h @@ -40,6 +40,14 @@ class CryptAuthApiCallFlow : public OAuth2ApiCallFlow { const ResultCallback& result_callback, const ErrorCallback& error_callback); + void SetPartialNetworkTrafficAnnotation( + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation) { + partial_network_annotation_.reset( + new net::PartialNetworkTrafficAnnotationTag( + partial_traffic_annotation)); + } + protected: // Reduce the visibility of OAuth2ApiCallFlow::Start() to avoid exposing // overloaded methods. @@ -53,6 +61,8 @@ class CryptAuthApiCallFlow : public OAuth2ApiCallFlow { const std::string& body) override; void ProcessApiCallSuccess(const net::URLFetcher* source) override; void ProcessApiCallFailure(const net::URLFetcher* source) override; + net::PartialNetworkTrafficAnnotationTag GetNetworkTrafficAnnotationTag() + override; private: // The URL of the CryptAuth endpoint serving the request. @@ -68,6 +78,9 @@ class CryptAuthApiCallFlow : public OAuth2ApiCallFlow { // Callback invoked with an error message when the flow fails. ErrorCallback error_callback_; + std::unique_ptr<net::PartialNetworkTrafficAnnotationTag> + partial_network_annotation_; + DISALLOW_COPY_AND_ASSIGN(CryptAuthApiCallFlow); }; diff --git a/chromium/components/cryptauth/cryptauth_api_call_flow_unittest.cc b/chromium/components/cryptauth/cryptauth_api_call_flow_unittest.cc index cbc6701e4f8..2c97320736a 100644 --- a/chromium/components/cryptauth/cryptauth_api_call_flow_unittest.cc +++ b/chromium/components/cryptauth/cryptauth_api_call_flow_unittest.cc @@ -30,7 +30,10 @@ class CryptAuthApiCallFlowTest protected: CryptAuthApiCallFlowTest() : url_request_context_getter_(new net::TestURLRequestContextGetter( - new base::TestSimpleTaskRunner())) {} + new base::TestSimpleTaskRunner())) { + flow_.SetPartialNetworkTrafficAnnotation( + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); + } void SetUp() override { // The TestURLFetcherFactory will override the global URLFetcherFactory for diff --git a/chromium/components/cryptauth/cryptauth_client.h b/chromium/components/cryptauth/cryptauth_client.h index 98c83d417ba..1fe98a91bb6 100644 --- a/chromium/components/cryptauth/cryptauth_client.h +++ b/chromium/components/cryptauth/cryptauth_client.h @@ -10,6 +10,7 @@ #include "base/callback_forward.h" #include "base/macros.h" +#include "net/traffic_annotation/network_traffic_annotation.h" namespace cryptauth { class GetMyDevicesRequest; @@ -24,6 +25,8 @@ class SetupEnrollmentRequest; class SetupEnrollmentResponse; class FinishEnrollmentRequest; class FinishEnrollmentResponse; +class FindEligibleForPromotionRequest; +class FindEligibleForPromotionResponse; } namespace cryptauth { @@ -45,7 +48,9 @@ class CryptAuthClient { GetMyDevicesCallback; virtual void GetMyDevices(const GetMyDevicesRequest& request, const GetMyDevicesCallback& callback, - const ErrorCallback& error_callback) = 0; + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation) = 0; // FindEligibleUnlockDevices typedef base::Callback<void( @@ -56,13 +61,23 @@ class CryptAuthClient { const FindEligibleUnlockDevicesCallback& callback, const ErrorCallback& error_callback) = 0; + // FindEligibleForPromotion + typedef base::Callback<void(const FindEligibleForPromotionResponse&)> + FindEligibleForPromotionCallback; + virtual void FindEligibleForPromotion( + const FindEligibleForPromotionRequest& request, + const FindEligibleForPromotionCallback& callback, + const ErrorCallback& error_callback) = 0; + // SendDeviceSyncTickle typedef base::Callback<void(const SendDeviceSyncTickleResponse&)> SendDeviceSyncTickleCallback; virtual void SendDeviceSyncTickle( const SendDeviceSyncTickleRequest& request, const SendDeviceSyncTickleCallback& callback, - const ErrorCallback& error_callback) = 0; + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation) = 0; // ToggleEasyUnlock typedef base::Callback<void(const ToggleEasyUnlockResponse&)> diff --git a/chromium/components/cryptauth/cryptauth_client_impl.cc b/chromium/components/cryptauth/cryptauth_client_impl.cc index 259838598ef..8fbe82e07a1 100644 --- a/chromium/components/cryptauth/cryptauth_client_impl.cc +++ b/chromium/components/cryptauth/cryptauth_client_impl.cc @@ -26,6 +26,8 @@ const char kCryptAuthPath[] = "cryptauth/v1/"; const char kGetMyDevicesPath[] = "deviceSync/getmydevices"; const char kFindEligibleUnlockDevicesPath[] = "deviceSync/findeligibleunlockdevices"; +const char kFindEligibleForPromotionPath[] = + "deviceSync/findeligibleforpromotion"; const char kSendDeviceSyncTicklePath[] = "deviceSync/senddevicesynctickle"; const char kToggleEasyUnlockPath[] = "deviceSync/toggleeasyunlock"; const char kSetupEnrollmentPath[] = "enrollment/setup"; @@ -66,44 +68,178 @@ CryptAuthClientImpl::~CryptAuthClientImpl() { void CryptAuthClientImpl::GetMyDevices( const GetMyDevicesRequest& request, const GetMyDevicesCallback& callback, - const ErrorCallback& error_callback) { - MakeApiCall(kGetMyDevicesPath, request, callback, error_callback); + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& partial_traffic_annotation) { + MakeApiCall(kGetMyDevicesPath, request, callback, error_callback, + partial_traffic_annotation); } void CryptAuthClientImpl::FindEligibleUnlockDevices( const FindEligibleUnlockDevicesRequest& request, const FindEligibleUnlockDevicesCallback& callback, const ErrorCallback& error_callback) { - MakeApiCall(kFindEligibleUnlockDevicesPath, request, callback, - error_callback); + net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation = + net::DefinePartialNetworkTrafficAnnotation( + "cryptauth_find_eligible_unlock_devices", "oauth2_api_call_flow", + R"( + semantics { + sender: "CryptAuth Device Manager" + description: + "Gets the list of mobile devices that can be used by Smart Lock to " + "unlock the current device." + trigger: + "This request is sent when the user starts the Smart Lock setup flow." + data: "OAuth 2.0 token and the device's public key." + destination: GOOGLE_OWNED_SERVICE + } + policy { + setting: + "This feature cannot be disabled in settings, but the request will " + "only be sent if the user explicitly tries to enable Smart Lock " + "(EasyUnlock), i.e. starts the setup flow." + chrome_policy { + EasyUnlockAllowed { + EasyUnlockAllowed: false + } + } + })"); + MakeApiCall(kFindEligibleUnlockDevicesPath, request, callback, error_callback, + partial_traffic_annotation); +} + +void CryptAuthClientImpl::FindEligibleForPromotion( + const FindEligibleForPromotionRequest& request, + const FindEligibleForPromotionCallback& callback, + const ErrorCallback& error_callback) { + net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation = + net::DefinePartialNetworkTrafficAnnotation( + "cryptauth_find_eligible_for_promotion", "oauth2_api_call_flow", + R"( + semantics { + sender: "Promotion Manager" + description: + "Return whether the current device is eligible for a Smart Lock promotion." + trigger: + "This request is sent when the user starts the Smart Lock setup flow." + data: "OAuth 2.0 token and the device's public key." + destination: GOOGLE_OWNED_SERVICE + } + policy { + setting: + "This feature cannot be disabled in settings" + chrome_policy { + EasyUnlockAllowed { + EasyUnlockAllowed: false + } + } + })"); + MakeApiCall(kFindEligibleForPromotionPath, request, callback, error_callback, + partial_traffic_annotation); } void CryptAuthClientImpl::SendDeviceSyncTickle( const SendDeviceSyncTickleRequest& request, const SendDeviceSyncTickleCallback& callback, - const ErrorCallback& error_callback) { - MakeApiCall(kSendDeviceSyncTicklePath, request, callback, error_callback); + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& partial_traffic_annotation) { + MakeApiCall(kSendDeviceSyncTicklePath, request, callback, error_callback, + partial_traffic_annotation); } void CryptAuthClientImpl::ToggleEasyUnlock( const ToggleEasyUnlockRequest& request, const ToggleEasyUnlockCallback& callback, const ErrorCallback& error_callback) { - MakeApiCall(kToggleEasyUnlockPath, request, callback, error_callback); + net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation = + net::DefinePartialNetworkTrafficAnnotation("cryptauth_toggle_easyunlock", + "oauth2_api_call_flow", R"( + semantics { + sender: "CryptAuth Device Manager" + description: "Enables Smart Lock (EasyUnlock) for the current device." + trigger: + "This request is send after the user goes through the EasyUnlock " + "setup flow." + data: "OAuth 2.0 token and the device public key." + destination: GOOGLE_OWNED_SERVICE + } + policy { + setting: + "This feature cannot be disabled in settings, but the request will " + "only be send if the user explicitly enables Smart Lock " + "(EasyUnlock), i.e. uccessfully complete the setup flow." + chrome_policy { + EasyUnlockAllowed { + EasyUnlockAllowed: false + } + } + })"); + MakeApiCall(kToggleEasyUnlockPath, request, callback, error_callback, + partial_traffic_annotation); } void CryptAuthClientImpl::SetupEnrollment( const SetupEnrollmentRequest& request, const SetupEnrollmentCallback& callback, const ErrorCallback& error_callback) { - MakeApiCall(kSetupEnrollmentPath, request, callback, error_callback); + net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation = + net::DefinePartialNetworkTrafficAnnotation( + "cryptauth_enrollment_flow_setup", "oauth2_api_call_flow", R"( + semantics { + sender: "CryptAuth Device Manager" + description: "Starts the CryptAuth registration flow." + trigger: + "Occurs periodically, at least once a month, because if the device " + "does not re-enroll for more than a specific number of days " + "(currently 45) it will be removed from the server." + data: + "Various device information (public key, bluetooth MAC address, " + "model, OS version, screen size, manufacturer, has screen lock " + "enabled), and OAuth 2.0 token." + destination: GOOGLE_OWNED_SERVICE + } + policy { + setting: + "This feature cannot be disabled by settings. However, this request " + "is made only for signed-in users." + chrome_policy { + SigninAllowed { + SigninAllowed: false + } + } + })"); + MakeApiCall(kSetupEnrollmentPath, request, callback, error_callback, + partial_traffic_annotation); } void CryptAuthClientImpl::FinishEnrollment( const FinishEnrollmentRequest& request, const FinishEnrollmentCallback& callback, const ErrorCallback& error_callback) { - MakeApiCall(kFinishEnrollmentPath, request, callback, error_callback); + net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation = + net::DefinePartialNetworkTrafficAnnotation( + "cryptauth_enrollment_flow_finish", "oauth2_api_call_flow", R"( + semantics { + sender: "CryptAuth Device Manager" + description: "Finishes the CryptAuth registration flow." + trigger: + "Occurs periodically, at least once a month, because if the device " + "does not re-enroll for more than a specific number of days " + "(currently 45) it will be removed from the server." + data: "OAuth 2.0 token." + destination: GOOGLE_OWNED_SERVICE + } + policy { + setting: + "This feature cannot be disabled by settings. However, this request " + "is made only for signed-in users." + chrome_policy { + SigninAllowed { + SigninAllowed: false + } + } + })"); + MakeApiCall(kFinishEnrollmentPath, request, callback, error_callback, + partial_traffic_annotation); } std::string CryptAuthClientImpl::GetAccessTokenUsed() { @@ -115,7 +251,8 @@ void CryptAuthClientImpl::MakeApiCall( const std::string& request_path, const RequestProto& request_proto, const base::Callback<void(const ResponseProto&)>& response_callback, - const ErrorCallback& error_callback) { + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& partial_traffic_annotation) { if (has_call_started_) { error_callback.Run( "Client has been used for another request. Do not reuse."); @@ -123,6 +260,9 @@ void CryptAuthClientImpl::MakeApiCall( } has_call_started_ = true; + api_call_flow_->SetPartialNetworkTrafficAnnotation( + partial_traffic_annotation); + // The |device_classifier| field must be present for all CryptAuth requests. RequestProto request_copy(request_proto); request_copy.mutable_device_classifier()->CopyFrom(device_classifier_); diff --git a/chromium/components/cryptauth/cryptauth_client_impl.h b/chromium/components/cryptauth/cryptauth_client_impl.h index d880a118e24..3ccea85148d 100644 --- a/chromium/components/cryptauth/cryptauth_client_impl.h +++ b/chromium/components/cryptauth/cryptauth_client_impl.h @@ -11,6 +11,7 @@ #include "components/cryptauth/cryptauth_api_call_flow.h" #include "components/cryptauth/cryptauth_client.h" #include "components/cryptauth/proto/cryptauth_api.pb.h" +#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/url_request_context_getter.h" class OAuth2TokenService; @@ -39,15 +40,22 @@ class CryptAuthClientImpl : public CryptAuthClient { // CryptAuthClient: void GetMyDevices(const GetMyDevicesRequest& request, const GetMyDevicesCallback& callback, - const ErrorCallback& error_callback) override; + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation) override; void FindEligibleUnlockDevices( const FindEligibleUnlockDevicesRequest& request, const FindEligibleUnlockDevicesCallback& callback, const ErrorCallback& error_callback) override; - void SendDeviceSyncTickle( - const SendDeviceSyncTickleRequest& request, - const SendDeviceSyncTickleCallback& callback, + void FindEligibleForPromotion( + const FindEligibleForPromotionRequest& request, + const FindEligibleForPromotionCallback& callback, const ErrorCallback& error_callback) override; + void SendDeviceSyncTickle(const SendDeviceSyncTickleRequest& request, + const SendDeviceSyncTickleCallback& callback, + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation) override; void ToggleEasyUnlock(const ToggleEasyUnlockRequest& request, const ToggleEasyUnlockCallback& callback, const ErrorCallback& error_callback) override; @@ -68,7 +76,9 @@ class CryptAuthClientImpl : public CryptAuthClient { const std::string& request_path, const RequestProto& request_proto, const base::Callback<void(const ResponseProto&)>& response_callback, - const ErrorCallback& error_callback); + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation); // Called when the access token is obtained so the API request can be made. template <class ResponseProto> diff --git a/chromium/components/cryptauth/cryptauth_client_impl_unittest.cc b/chromium/components/cryptauth/cryptauth_client_impl_unittest.cc index 20224c00295..bcf4be213ed 100644 --- a/chromium/components/cryptauth/cryptauth_client_impl_unittest.cc +++ b/chromium/components/cryptauth/cryptauth_client_impl_unittest.cc @@ -13,6 +13,7 @@ #include "components/cryptauth/proto/cryptauth_api.pb.h" #include "components/cryptauth/switches.h" #include "google_apis/gaia/fake_oauth2_token_service.h" +#include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_request_test_util.h" #include "testing/gmock/include/gmock/gmock.h" @@ -63,7 +64,9 @@ class FakeCryptAuthAccessTokenFetcher : public CryptAuthAccessTokenFetcher { // Mock CryptAuthApiCallFlow, which handles the HTTP requests to CryptAuth. class MockCryptAuthApiCallFlow : public CryptAuthApiCallFlow { public: - MockCryptAuthApiCallFlow() : CryptAuthApiCallFlow() {} + MockCryptAuthApiCallFlow() : CryptAuthApiCallFlow() { + SetPartialNetworkTrafficAnnotation(PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); + } virtual ~MockCryptAuthApiCallFlow() {} MOCK_METHOD6(Start, @@ -166,7 +169,8 @@ TEST_F(CryptAuthClientTest, GetMyDevicesSuccess) { client_->GetMyDevices( request_proto, base::Bind(&SaveResult<GetMyDevicesResponse>, &result_proto), - base::Bind(&NotCalled<std::string>)); + base::Bind(&NotCalled<std::string>), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); GetMyDevicesRequest expected_request; EXPECT_TRUE(expected_request.ParseFromString(serialized_request_)); @@ -203,7 +207,8 @@ TEST_F(CryptAuthClientTest, GetMyDevicesFailure) { std::string error_message; client_->GetMyDevices(GetMyDevicesRequest(), base::Bind(&NotCalled<GetMyDevicesResponse>), - base::Bind(&SaveResult<std::string>, &error_message)); + base::Bind(&SaveResult<std::string>, &error_message), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); std::string kStatus500Error("HTTP status: 500"); FailApiCallFlow(kStatus500Error); @@ -271,6 +276,24 @@ TEST_F(CryptAuthClientTest, FindEligibleUnlockDevicesFailure) { EXPECT_EQ(kStatus403Error, error_message); } +TEST_F(CryptAuthClientTest, FindEligibleForPromotionSuccess) { + ExpectRequest( + "https://www.testgoogleapis.com/cryptauth/v1/deviceSync/" + "findeligibleforpromotion?alt=proto"); + + FindEligibleForPromotionResponse result_proto; + client_->FindEligibleForPromotion( + FindEligibleForPromotionRequest(), + base::Bind(&SaveResult<FindEligibleForPromotionResponse>, &result_proto), + base::Bind(&NotCalled<std::string>)); + + FindEligibleForPromotionRequest expected_request; + EXPECT_TRUE(expected_request.ParseFromString(serialized_request_)); + + FindEligibleForPromotionResponse response_proto; + FinishApiCallFlow(&response_proto); +} + TEST_F(CryptAuthClientTest, SendDeviceSyncTickleSuccess) { ExpectRequest( "https://www.testgoogleapis.com/cryptauth/v1/deviceSync/" @@ -279,9 +302,9 @@ TEST_F(CryptAuthClientTest, SendDeviceSyncTickleSuccess) { SendDeviceSyncTickleResponse result_proto; client_->SendDeviceSyncTickle( SendDeviceSyncTickleRequest(), - base::Bind(&SaveResult<SendDeviceSyncTickleResponse>, - &result_proto), - base::Bind(&NotCalled<std::string>)); + base::Bind(&SaveResult<SendDeviceSyncTickleResponse>, &result_proto), + base::Bind(&NotCalled<std::string>), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); SendDeviceSyncTickleRequest expected_request; EXPECT_TRUE(expected_request.ParseFromString(serialized_request_)); @@ -401,7 +424,8 @@ TEST_F(CryptAuthClientTest, FetchAccessTokenFailure) { std::string error_message; client_->GetMyDevices(GetMyDevicesRequest(), base::Bind(&NotCalled<GetMyDevicesResponse>), - base::Bind(&SaveResult<std::string>, &error_message)); + base::Bind(&SaveResult<std::string>, &error_message), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); EXPECT_EQ("Failed to get a valid access token.", error_message); } @@ -414,7 +438,8 @@ TEST_F(CryptAuthClientTest, ParseResponseProtoFailure) { std::string error_message; client_->GetMyDevices(GetMyDevicesRequest(), base::Bind(&NotCalled<GetMyDevicesResponse>), - base::Bind(&SaveResult<std::string>, &error_message)); + base::Bind(&SaveResult<std::string>, &error_message), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); flow_result_callback_.Run("Not a valid serialized response message."); EXPECT_EQ("Failed to parse response proto.", error_message); @@ -431,7 +456,8 @@ TEST_F(CryptAuthClientTest, client_->GetMyDevices( GetMyDevicesRequest(), base::Bind(&SaveResult<GetMyDevicesResponse>, &result_proto), - base::Bind(&NotCalled<std::string>)); + base::Bind(&NotCalled<std::string>), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); // With request pending, make second request. { @@ -466,7 +492,8 @@ TEST_F(CryptAuthClientTest, std::string error_message; client_->GetMyDevices(GetMyDevicesRequest(), base::Bind(&NotCalled<GetMyDevicesResponse>), - base::Bind(&SaveResult<std::string>, &error_message)); + base::Bind(&SaveResult<std::string>, &error_message), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); // With request pending, make second request. { @@ -496,7 +523,8 @@ TEST_F(CryptAuthClientTest, client_->GetMyDevices( GetMyDevicesRequest(), base::Bind(&SaveResult<GetMyDevicesResponse>, &result_proto), - base::Bind(&NotCalled<std::string>)); + base::Bind(&NotCalled<std::string>), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); GetMyDevicesResponse response_proto; response_proto.add_devices(); @@ -529,7 +557,8 @@ TEST_F(CryptAuthClientTest, DeviceClassifierIsSet) { client_->GetMyDevices( request_proto, base::Bind(&SaveResult<GetMyDevicesResponse>, &result_proto), - base::Bind(&NotCalled<std::string>)); + base::Bind(&NotCalled<std::string>), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); GetMyDevicesRequest expected_request; EXPECT_TRUE(expected_request.ParseFromString(serialized_request_)); @@ -555,7 +584,8 @@ TEST_F(CryptAuthClientTest, GetAccessTokenUsed) { client_->GetMyDevices( request_proto, base::Bind(&SaveResult<GetMyDevicesResponse>, &result_proto), - base::Bind(&NotCalled<std::string>)); + base::Bind(&NotCalled<std::string>), + PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS); EXPECT_EQ(kAccessToken, client_->GetAccessTokenUsed()); } diff --git a/chromium/components/cryptauth/cryptauth_device_manager.cc b/chromium/components/cryptauth/cryptauth_device_manager.cc index 3c7a3cf5789..76d651d3473 100644 --- a/chromium/components/cryptauth/cryptauth_device_manager.cc +++ b/chromium/components/cryptauth/cryptauth_device_manager.cc @@ -18,6 +18,7 @@ #include "components/prefs/pref_service.h" #include "components/prefs/scoped_user_pref_update.h" #include "components/proximity_auth/logging/logging.h" +#include "net/traffic_annotation/network_traffic_annotation.h" namespace cryptauth { @@ -45,6 +46,8 @@ const char kExternalDeviceKeyMobileHotspotSupported[] = "mobile_hotspot_supported"; const char kExternalDeviceKeyDeviceType[] = "device_type"; const char kExternalDeviceKeyBeaconSeeds[] = "beacon_seeds"; +const char kExternalDeviceKeyArcPlusPlus[] = "arc_plus_plus"; +const char kExternalDeviceKeyPixelPhone[] = "pixel_phone"; const char kExternalDeviceKeyBeaconSeedData[] = "beacon_seed_data"; const char kExternalDeviceKeyBeaconSeedStartMs[] = "beacon_seed_start_ms"; const char kExternalDeviceKeyBeaconSeedEndMs[] = "beacon_seed_end_ms"; @@ -155,6 +158,15 @@ std::unique_ptr<base::DictionaryValue> UnlockKeyToDictionary( device.device_type()); } + if (device.has_arc_plus_plus()) { + dictionary->SetBoolean(kExternalDeviceKeyArcPlusPlus, + device.arc_plus_plus()); + } + + if (device.has_pixel_phone()) { + dictionary->SetBoolean(kExternalDeviceKeyPixelPhone, device.pixel_phone()); + } + std::unique_ptr<base::ListValue> beacon_seed_list = BeaconSeedsToListValue(device.beacon_seeds()); dictionary->Set(kExternalDeviceKeyBeaconSeeds, std::move(beacon_seed_list)); @@ -253,14 +265,12 @@ bool DictionaryToUnlockKey(const base::DictionaryValue& dictionary, } bool unlock_key; - if (dictionary.GetBoolean(kExternalDeviceKeyUnlockKey, &unlock_key)) { + if (dictionary.GetBoolean(kExternalDeviceKeyUnlockKey, &unlock_key)) external_device->set_unlock_key(unlock_key); - } bool unlockable; - if (dictionary.GetBoolean(kExternalDeviceKeyUnlockable, &unlockable)) { + if (dictionary.GetBoolean(kExternalDeviceKeyUnlockable, &unlockable)) external_device->set_unlockable(unlockable); - } std::string last_update_time_millis_str; if (dictionary.GetString( @@ -290,13 +300,28 @@ bool DictionaryToUnlockKey(const base::DictionaryValue& dictionary, const base::ListValue* beacon_seeds = nullptr; dictionary.GetList(kExternalDeviceKeyBeaconSeeds, &beacon_seeds); - if (beacon_seeds) { + if (beacon_seeds) AddBeaconSeedsToExternalDevice(*beacon_seeds, *external_device); - } + + bool arc_plus_plus; + if (dictionary.GetBoolean(kExternalDeviceKeyArcPlusPlus, &arc_plus_plus)) + external_device->set_arc_plus_plus(arc_plus_plus); + + bool pixel_phone; + if (dictionary.GetBoolean(kExternalDeviceKeyPixelPhone, &pixel_phone)) + external_device->set_pixel_phone(pixel_phone); return true; } +std::unique_ptr<SyncSchedulerImpl> CreateSyncScheduler( + SyncScheduler::Delegate* delegate) { + return base::MakeUnique<SyncSchedulerImpl>( + delegate, base::TimeDelta::FromHours(kRefreshPeriodHours), + base::TimeDelta::FromMinutes(kDeviceSyncBaseRecoveryPeriodMinutes), + kDeviceSyncMaxJitterRatio, "CryptAuth DeviceSync"); +} + } // namespace CryptAuthDeviceManager::CryptAuthDeviceManager( @@ -308,6 +333,7 @@ CryptAuthDeviceManager::CryptAuthDeviceManager( client_factory_(std::move(client_factory)), gcm_manager_(gcm_manager), pref_service_(pref_service), + scheduler_(CreateSyncScheduler(this)), weak_ptr_factory_(this) { UpdateUnlockKeysFromPrefs(); } @@ -349,7 +375,6 @@ void CryptAuthDeviceManager::Start() { prefs::kCryptAuthDeviceSyncIsRecoveringFromFailure) || last_successful_sync.is_null(); - scheduler_ = CreateSyncScheduler(); scheduler_->Start(elapsed_time_since_last_sync, is_recovering_from_failure ? SyncScheduler::Strategy::AGGRESSIVE_RECOVERY @@ -405,6 +430,17 @@ std::vector<ExternalDeviceInfo> CryptAuthDeviceManager::GetUnlockKeys() const { return unlock_keys; } +std::vector<ExternalDeviceInfo> CryptAuthDeviceManager::GetPixelUnlockKeys() + const { + std::vector<ExternalDeviceInfo> unlock_keys; + for (const auto& device : synced_devices_) { + if (device.unlock_key() && device.pixel_phone()) { + unlock_keys.push_back(device); + } + } + return unlock_keys; +} + std::vector<ExternalDeviceInfo> CryptAuthDeviceManager::GetTetherHosts() const { std::vector<ExternalDeviceInfo> tether_hosts; for (const auto& device : synced_devices_) { @@ -415,14 +451,37 @@ std::vector<ExternalDeviceInfo> CryptAuthDeviceManager::GetTetherHosts() const { return tether_hosts; } +std::vector<ExternalDeviceInfo> CryptAuthDeviceManager::GetPixelTetherHosts() + const { + std::vector<ExternalDeviceInfo> tether_hosts; + for (const auto& device : synced_devices_) { + if (device.mobile_hotspot_supported() && device.pixel_phone()) { + tether_hosts.push_back(device); + } + } + return tether_hosts; +} + void CryptAuthDeviceManager::OnGetMyDevicesSuccess( const GetMyDevicesResponse& response) { // Update the synced devices stored in the user's prefs. std::unique_ptr<base::ListValue> devices_as_list(new base::ListValue()); + + if (!response.devices().empty()) + PA_LOG(INFO) << "Devices were successfully synced."; + for (const auto& device : response.devices()) { - devices_as_list->Append(UnlockKeyToDictionary(device)); + std::unique_ptr<base::DictionaryValue> device_dictionary = + UnlockKeyToDictionary(device); + + const std::string& device_name = device.has_friendly_device_name() + ? device.friendly_device_name() + : "[unknown]"; + PA_LOG(INFO) << "Synced device '" << device_name + << "': " << *device_dictionary; + + devices_as_list->Append(std::move(device_dictionary)); } - PA_LOG(INFO) << "Devices Synced:\n" << *devices_as_list; bool unlock_keys_changed = !devices_as_list->Equals( pref_service_->GetList(prefs::kCryptAuthDeviceSyncUnlockKeys)); @@ -462,13 +521,6 @@ void CryptAuthDeviceManager::OnGetMyDevicesFailure(const std::string& error) { observer.OnSyncFinished(SyncResult::FAILURE, DeviceChangeResult::UNCHANGED); } -std::unique_ptr<SyncScheduler> CryptAuthDeviceManager::CreateSyncScheduler() { - return base::MakeUnique<SyncSchedulerImpl>( - this, base::TimeDelta::FromHours(kRefreshPeriodHours), - base::TimeDelta::FromMinutes(kDeviceSyncBaseRecoveryPeriodMinutes), - kDeviceSyncMaxJitterRatio, "CryptAuth DeviceSync"); -} - void CryptAuthDeviceManager::OnResyncMessage() { ForceSyncNow(INVOCATION_REASON_SERVER_INITIATED); } @@ -529,11 +581,38 @@ void CryptAuthDeviceManager::OnSyncRequested( GetMyDevicesRequest request; request.set_invocation_reason(invocation_reason); request.set_allow_stale_read(is_sync_speculative); + net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation = + net::DefinePartialNetworkTrafficAnnotation("cryptauth_get_my_devices", + "oauth2_api_call_flow", R"( + semantics { + sender: "CryptAuth Device Manager" + description: + "Gets a list of the devices registered (for the same user) on " + "CryptAuth." + trigger: + "Once every day, or by API request. Periodic calls happen because " + "devides that do not re-enrolled for more than X days (currently 45) " + "are automatically removed from the server." + data: "OAuth 2.0 token." + destination: GOOGLE_OWNED_SERVICE + } + policy { + setting: + "This feature cannot be disabled in settings. However, this request " + "is made only for signed-in users." + chrome_policy { + SigninAllowed { + SigninAllowed: false + } + } + })"); cryptauth_client_->GetMyDevices( - request, base::Bind(&CryptAuthDeviceManager::OnGetMyDevicesSuccess, - weak_ptr_factory_.GetWeakPtr()), + request, + base::Bind(&CryptAuthDeviceManager::OnGetMyDevicesSuccess, + weak_ptr_factory_.GetWeakPtr()), base::Bind(&CryptAuthDeviceManager::OnGetMyDevicesFailure, - weak_ptr_factory_.GetWeakPtr())); + weak_ptr_factory_.GetWeakPtr()), + partial_traffic_annotation); } } // namespace cryptauth diff --git a/chromium/components/cryptauth/cryptauth_device_manager.h b/chromium/components/cryptauth/cryptauth_device_manager.h index a728f247abf..6994e54759b 100644 --- a/chromium/components/cryptauth/cryptauth_device_manager.h +++ b/chromium/components/cryptauth/cryptauth_device_manager.h @@ -110,17 +110,22 @@ class CryptAuthDeviceManager : public SyncScheduler::Delegate, // Returns a list of remote devices that can unlock the user's other devices. virtual std::vector<ExternalDeviceInfo> GetUnlockKeys() const; + // Like GetUnlockKeys(), but only returns Pixel devices. + virtual std::vector<ExternalDeviceInfo> GetPixelUnlockKeys() const; // Returns a list of remote devices that can host tether hotspots. virtual std::vector<ExternalDeviceInfo> GetTetherHosts() const; + // Like GetTetherHosts(), but only returns Pixel devices. + virtual std::vector<ExternalDeviceInfo> GetPixelTetherHosts() const; protected: // Empty constructor, to be used by tests to mock the device manager. Do not // use this constructor outside of tests. CryptAuthDeviceManager(); - // Creates a new SyncScheduler instance. Exposed for testing. - virtual std::unique_ptr<SyncScheduler> CreateSyncScheduler(); + void SetSyncSchedulerForTest(std::unique_ptr<SyncScheduler> sync_scheduler) { + scheduler_ = std::move(sync_scheduler); + } private: // CryptAuthGCMManager::Observer: diff --git a/chromium/components/cryptauth/cryptauth_device_manager_unittest.cc b/chromium/components/cryptauth/cryptauth_device_manager_unittest.cc index 7dcd284349e..ec960e085f1 100644 --- a/chromium/components/cryptauth/cryptauth_device_manager_unittest.cc +++ b/chromium/components/cryptauth/cryptauth_device_manager_unittest.cc @@ -21,6 +21,7 @@ #include "components/cryptauth/pref_names.h" #include "components/prefs/scoped_user_pref_update.h" #include "components/prefs/testing_pref_service.h" +#include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -52,7 +53,7 @@ const bool kStoredMobileHotspotSupported = true; // ExternalDeviceInfo fields for the synced unlock key. const char kPublicKey1[] = "GOOG"; -const char kDeviceName1[] = "Nexus 5"; +const char kDeviceName1[] = "Pixel XL"; const char kBluetoothAddress1[] = "aa:bb:cc:ee:dd:ff"; const bool kUnlockKey1 = true; const bool kUnlockable1 = false; @@ -63,6 +64,8 @@ const int64_t kBeaconSeed1EndTime = 123457; const char kBeaconSeed2Data[] = "beaconSeed2Data"; const int64_t kBeaconSeed2StartTime = 234567; const int64_t kBeaconSeed2EndTime = 234568; +const bool kArcPlusPlus1 = true; +const bool kPixelPhone1 = true; // ExternalDeviceInfo fields for a non-synced unlockable device. const char kPublicKey2[] = "MSFT"; @@ -76,6 +79,8 @@ const int64_t kBeaconSeed3EndTime = 123457; const char kBeaconSeed4Data[] = "beaconSeed4Data"; const int64_t kBeaconSeed4StartTime = 234567; const int64_t kBeaconSeed4EndTime = 234568; +const bool kArcPlusPlus2 = false; +const bool kPixelPhone2 = false; // Validates that |devices| is equal to |expected_devices|. void ExpectSyncedDevicesAreEqual( @@ -297,23 +302,20 @@ class TestCryptAuthDeviceManager : public CryptAuthDeviceManager { gcm_manager, pref_service), scoped_sync_scheduler_(new NiceMock<MockSyncScheduler>()), - weak_sync_scheduler_factory_(scoped_sync_scheduler_.get()) {} + weak_sync_scheduler_factory_(scoped_sync_scheduler_) { + SetSyncSchedulerForTest(base::WrapUnique(scoped_sync_scheduler_)); + } ~TestCryptAuthDeviceManager() override {} - std::unique_ptr<SyncScheduler> CreateSyncScheduler() override { - EXPECT_TRUE(scoped_sync_scheduler_); - return std::move(scoped_sync_scheduler_); - } - base::WeakPtr<MockSyncScheduler> GetSyncScheduler() { return weak_sync_scheduler_factory_.GetWeakPtr(); } private: // Ownership is passed to |CryptAuthDeviceManager| super class when - // |CreateSyncScheduler()| is called. - std::unique_ptr<MockSyncScheduler> scoped_sync_scheduler_; + // SetSyncSchedulerForTest() is called. + MockSyncScheduler* scoped_sync_scheduler_; // Stores the pointer of |scoped_sync_scheduler_| after ownership is passed to // the super class. @@ -353,6 +355,8 @@ class CryptAuthDeviceManagerTest seed2->set_data(kBeaconSeed2Data); seed2->set_start_time_millis(kBeaconSeed2StartTime); seed2->set_end_time_millis(kBeaconSeed2EndTime); + unlock_key.set_arc_plus_plus(kArcPlusPlus1); + unlock_key.set_pixel_phone(kPixelPhone1); devices_in_response_.push_back(unlock_key); ExternalDeviceInfo unlockable_device; @@ -369,6 +373,8 @@ class CryptAuthDeviceManagerTest seed4->set_data(kBeaconSeed4Data); seed4->set_start_time_millis(kBeaconSeed4StartTime); seed4->set_end_time_millis(kBeaconSeed4EndTime); + unlockable_device.set_arc_plus_plus(kArcPlusPlus2); + unlockable_device.set_pixel_phone(kPixelPhone2); devices_in_response_.push_back(unlockable_device); } @@ -411,8 +417,7 @@ class CryptAuthDeviceManagerTest bluetooth_address_b64); device_dictionary->SetBoolean("unlock_key", kStoredUnlockKey); device_dictionary->SetBoolean("unlockable", kStoredUnlockable); - device_dictionary->Set("beacon_seeds", - base::WrapUnique(new base::ListValue())); + device_dictionary->Set("beacon_seeds", base::MakeUnique<base::ListValue>()); device_dictionary->SetBoolean("mobile_hotspot_supported", kStoredMobileHotspotSupported); { @@ -473,7 +478,7 @@ class CryptAuthDeviceManagerTest // MockCryptAuthClientFactory::Observer: void OnCryptAuthClientCreated(MockCryptAuthClient* client) override { - EXPECT_CALL(*client, GetMyDevices(_, _, _)) + EXPECT_CALL(*client, GetMyDevices(_, _, _, _)) .WillOnce(DoAll(SaveArg<0>(&get_my_devices_request_), SaveArg<1>(&success_callback_), SaveArg<2>(&error_callback_))); diff --git a/chromium/components/cryptauth/cryptauth_enrollment_manager.cc b/chromium/components/cryptauth/cryptauth_enrollment_manager.cc index 8381de24ccf..53303471fe9 100644 --- a/chromium/components/cryptauth/cryptauth_enrollment_manager.cc +++ b/chromium/components/cryptauth/cryptauth_enrollment_manager.cc @@ -42,6 +42,14 @@ const double kEnrollmentMaxJitterRatio = 0.2; // registration. const char kDeviceSoftwarePackage[] = "com.google.chrome.cryptauth"; +std::unique_ptr<SyncScheduler> CreateSyncScheduler( + SyncScheduler::Delegate* delegate) { + return base::MakeUnique<SyncSchedulerImpl>( + delegate, base::TimeDelta::FromDays(kEnrollmentRefreshPeriodDays), + base::TimeDelta::FromMinutes(kEnrollmentBaseRecoveryPeriodMinutes), + kEnrollmentMaxJitterRatio, "CryptAuth Enrollment"); +} + } // namespace CryptAuthEnrollmentManager::CryptAuthEnrollmentManager( @@ -57,6 +65,7 @@ CryptAuthEnrollmentManager::CryptAuthEnrollmentManager( device_info_(device_info), gcm_manager_(gcm_manager), pref_service_(pref_service), + scheduler_(CreateSyncScheduler(this /* delegate */)), weak_ptr_factory_(this) {} CryptAuthEnrollmentManager::~CryptAuthEnrollmentManager() { @@ -89,7 +98,6 @@ void CryptAuthEnrollmentManager::Start() { base::TimeDelta elapsed_time_since_last_sync = clock_->Now() - last_successful_enrollment; - scheduler_ = CreateSyncScheduler(); scheduler_->Start(elapsed_time_since_last_sync, is_recovering_from_failure ? SyncScheduler::Strategy::AGGRESSIVE_RECOVERY @@ -159,14 +167,6 @@ void CryptAuthEnrollmentManager::OnEnrollmentFinished(bool success) { observer.OnEnrollmentFinished(success); } -std::unique_ptr<SyncScheduler> -CryptAuthEnrollmentManager::CreateSyncScheduler() { - return base::MakeUnique<SyncSchedulerImpl>( - this, base::TimeDelta::FromDays(kEnrollmentRefreshPeriodDays), - base::TimeDelta::FromMinutes(kEnrollmentBaseRecoveryPeriodMinutes), - kEnrollmentMaxJitterRatio, "CryptAuth Enrollment"); -} - std::string CryptAuthEnrollmentManager::GetUserPublicKey() const { std::string public_key; if (!base::Base64UrlDecode( @@ -189,6 +189,11 @@ std::string CryptAuthEnrollmentManager::GetUserPrivateKey() const { return private_key; } +void CryptAuthEnrollmentManager::SetSyncSchedulerForTest( + std::unique_ptr<SyncScheduler> sync_scheduler) { + scheduler_ = std::move(sync_scheduler); +} + void CryptAuthEnrollmentManager::OnGCMRegistrationResult(bool success) { if (!sync_request_) return; diff --git a/chromium/components/cryptauth/cryptauth_enrollment_manager.h b/chromium/components/cryptauth/cryptauth_enrollment_manager.h index ce4c9da774d..8bd187a5691 100644 --- a/chromium/components/cryptauth/cryptauth_enrollment_manager.h +++ b/chromium/components/cryptauth/cryptauth_enrollment_manager.h @@ -116,8 +116,7 @@ class CryptAuthEnrollmentManager : public SyncScheduler::Delegate, virtual std::string GetUserPrivateKey() const; protected: - // Creates a new SyncScheduler instance. Exposed for testing. - virtual std::unique_ptr<SyncScheduler> CreateSyncScheduler(); + void SetSyncSchedulerForTest(std::unique_ptr<SyncScheduler> sync_scheduler); private: // CryptAuthGCMManager::Observer: diff --git a/chromium/components/cryptauth/cryptauth_enrollment_manager_unittest.cc b/chromium/components/cryptauth/cryptauth_enrollment_manager_unittest.cc index 7a98f86d8e4..d0a49203e6b 100644 --- a/chromium/components/cryptauth/cryptauth_enrollment_manager_unittest.cc +++ b/chromium/components/cryptauth/cryptauth_enrollment_manager_unittest.cc @@ -112,15 +112,12 @@ class TestCryptAuthEnrollmentManager : public CryptAuthEnrollmentManager { gcm_manager, pref_service), scoped_sync_scheduler_(new NiceMock<MockSyncScheduler>()), - weak_sync_scheduler_factory_(scoped_sync_scheduler_.get()) {} + weak_sync_scheduler_factory_(scoped_sync_scheduler_) { + SetSyncSchedulerForTest(base::WrapUnique(scoped_sync_scheduler_)); + } ~TestCryptAuthEnrollmentManager() override {} - std::unique_ptr<SyncScheduler> CreateSyncScheduler() override { - EXPECT_TRUE(scoped_sync_scheduler_); - return std::move(scoped_sync_scheduler_); - } - base::WeakPtr<MockSyncScheduler> GetSyncScheduler() { return weak_sync_scheduler_factory_.GetWeakPtr(); } @@ -128,7 +125,7 @@ class TestCryptAuthEnrollmentManager : public CryptAuthEnrollmentManager { private: // Ownership is passed to |CryptAuthEnrollmentManager| super class when // |CreateSyncScheduler()| is called. - std::unique_ptr<MockSyncScheduler> scoped_sync_scheduler_; + NiceMock<MockSyncScheduler>* scoped_sync_scheduler_; // Stores the pointer of |scoped_sync_scheduler_| after ownership is passed to // the super class. diff --git a/chromium/components/cryptauth/device_to_device_authenticator.cc b/chromium/components/cryptauth/device_to_device_authenticator.cc index 75e19f2f304..a8a648b390d 100644 --- a/chromium/components/cryptauth/device_to_device_authenticator.cc +++ b/chromium/components/cryptauth/device_to_device_authenticator.cc @@ -11,7 +11,6 @@ #include "base/timer/timer.h" #include "components/cryptauth/authenticator.h" #include "components/cryptauth/connection.h" -#include "components/cryptauth/device_to_device_initiator_operations.h" #include "components/cryptauth/device_to_device_secure_context.h" #include "components/cryptauth/secure_context.h" #include "components/cryptauth/secure_message_delegate.h" @@ -69,6 +68,7 @@ DeviceToDeviceAuthenticator::DeviceToDeviceAuthenticator( : connection_(connection), account_id_(account_id), secure_message_delegate_(std::move(secure_message_delegate)), + helper_(base::MakeUnique<DeviceToDeviceInitiatorHelper>()), state_(State::NOT_STARTED), weak_ptr_factory_(this) { DCHECK(connection_); @@ -112,9 +112,9 @@ void DeviceToDeviceAuthenticator::OnKeyPairGenerated( } local_session_private_key_ = private_key; - // Create the [Hello] message to send to the remote device. + // Create the [Initiator Hello] message to send to the remote device. state_ = State::SENDING_HELLO; - DeviceToDeviceInitiatorOperations::CreateHelloMessage( + helper_->CreateHelloMessage( public_key, connection_->remote_device().persistent_symmetric_key, secure_message_delegate_.get(), base::Bind(&DeviceToDeviceAuthenticator::OnHelloMessageCreated, @@ -129,10 +129,12 @@ void DeviceToDeviceAuthenticator::OnHelloMessageCreated( const std::string& message) { DCHECK(state_ == State::SENDING_HELLO); if (message.empty()) { - Fail("Failed to create [Hello]"); + Fail("Failed to create [Initiator Hello]"); return; } + PA_LOG(INFO) << "Sending [Initiator Hello] message."; + // Add a timeout for receiving the [Responder Auth] message as a guard. timer_ = CreateTimer(); timer_->Start( @@ -140,7 +142,7 @@ void DeviceToDeviceAuthenticator::OnHelloMessageCreated( base::Bind(&DeviceToDeviceAuthenticator::OnResponderAuthTimedOut, weak_ptr_factory_.GetWeakPtr())); - // Send the [Hello] message to the remote device. + // Send the [Initiator Hello] message to the remote device. state_ = State::SENT_HELLO; hello_message_ = message; connection_->SendMessage(base::MakeUnique<WireMessage>( @@ -166,7 +168,7 @@ void DeviceToDeviceAuthenticator::OnResponderAuthValidated( session_keys_ = session_keys; // Create the [Initiator Auth] message to send to the remote device. - DeviceToDeviceInitiatorOperations::CreateInitiatorAuthMessage( + helper_->CreateInitiatorAuthMessage( session_keys_, connection_->remote_device().persistent_symmetric_key, responder_auth_message_, secure_message_delegate_.get(), base::Bind(&DeviceToDeviceAuthenticator::OnInitiatorAuthCreated, @@ -241,7 +243,7 @@ void DeviceToDeviceAuthenticator::OnMessageReceived( // Attempt to validate the [Responder Auth] message received from the remote // device. std::string responder_public_key = connection.remote_device().public_key; - DeviceToDeviceInitiatorOperations::ValidateResponderAuthMessage( + helper_->ValidateResponderAuthMessage( responder_auth_message_, responder_public_key, connection_->remote_device().persistent_symmetric_key, local_session_private_key_, hello_message_, @@ -264,7 +266,7 @@ void DeviceToDeviceAuthenticator::OnSendCompleted( Fail("Failed to send [Initiator Auth]"); } else if (!success && state_ == State::SENT_HELLO) { DCHECK(message.payload() == hello_message_); - Fail("Failed to send [Hello]"); + Fail("Failed to send [Initiator Hello]"); } } diff --git a/chromium/components/cryptauth/device_to_device_authenticator.h b/chromium/components/cryptauth/device_to_device_authenticator.h index 13ffa6c2226..ac34a9c7e15 100644 --- a/chromium/components/cryptauth/device_to_device_authenticator.h +++ b/chromium/components/cryptauth/device_to_device_authenticator.h @@ -13,6 +13,7 @@ #include "components/cryptauth/authenticator.h" #include "components/cryptauth/connection.h" #include "components/cryptauth/connection_observer.h" +#include "components/cryptauth/device_to_device_initiator_helper.h" #include "components/cryptauth/session_keys.h" namespace base { @@ -153,6 +154,9 @@ class DeviceToDeviceAuthenticator : public Authenticator, // Handles SecureMessage crypto operations. std::unique_ptr<SecureMessageDelegate> secure_message_delegate_; + // Performs authentication handshake. + std::unique_ptr<DeviceToDeviceInitiatorHelper> helper_; + // The current state in the authentication flow. State state_; diff --git a/chromium/components/cryptauth/device_to_device_authenticator_unittest.cc b/chromium/components/cryptauth/device_to_device_authenticator_unittest.cc index 362cd8ea205..851df649c62 100644 --- a/chromium/components/cryptauth/device_to_device_authenticator_unittest.cc +++ b/chromium/components/cryptauth/device_to_device_authenticator_unittest.cc @@ -143,15 +143,15 @@ class DeviceToDeviceAuthenticatorForTest : public DeviceToDeviceAuthenticator { } // namespace -class ProximityAuthDeviceToDeviceAuthenticatorTest : public testing::Test { +class CryptAuthDeviceToDeviceAuthenticatorTest : public testing::Test { public: - ProximityAuthDeviceToDeviceAuthenticatorTest() + CryptAuthDeviceToDeviceAuthenticatorTest() : remote_device_(CreateClassicRemoteDeviceForTest()), connection_(remote_device_), secure_message_delegate_(new FakeSecureMessageDelegate), authenticator_(&connection_, base::WrapUnique(secure_message_delegate_)) {} - ~ProximityAuthDeviceToDeviceAuthenticatorTest() override {} + ~CryptAuthDeviceToDeviceAuthenticatorTest() override {} void SetUp() override { // Set up the session asymmetric keys for both the local and remote devices. @@ -179,7 +179,7 @@ class ProximityAuthDeviceToDeviceAuthenticatorTest : public testing::Test { // device to the remote device. std::string BeginAuthentication() { authenticator_.Authenticate(base::Bind( - &ProximityAuthDeviceToDeviceAuthenticatorTest::OnAuthenticationResult, + &CryptAuthDeviceToDeviceAuthenticatorTest::OnAuthenticationResult, base::Unretained(this))); EXPECT_EQ(1u, connection_.message_buffer().size()); @@ -251,10 +251,10 @@ class ProximityAuthDeviceToDeviceAuthenticatorTest : public testing::Test { // Stores the SecureContext returned after authentication succeeds. std::unique_ptr<SecureContext> secure_context_; - DISALLOW_COPY_AND_ASSIGN(ProximityAuthDeviceToDeviceAuthenticatorTest); + DISALLOW_COPY_AND_ASSIGN(CryptAuthDeviceToDeviceAuthenticatorTest); }; -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, AuthenticateSucceeds) { +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, AuthenticateSucceeds) { // Starts the authentication protocol and grab [Hello] message. std::string hello_message = BeginAuthentication(); @@ -277,7 +277,7 @@ TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, AuthenticateSucceeds) { ASSERT_TRUE(initiator_auth_validated); } -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, ResponderRejectsHello) { +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, ResponderRejectsHello) { std::string hello_message = BeginAuthentication(); // If the responder could not validate the [Hello message], it essentially @@ -290,7 +290,7 @@ TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, ResponderRejectsHello) { EXPECT_FALSE(secure_context_); } -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, ResponderAuthTimesOut) { +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, ResponderAuthTimesOut) { // Starts the authentication protocol and grab [Hello] message. std::string hello_message = BeginAuthentication(); ASSERT_TRUE(authenticator_.timer()); @@ -300,7 +300,7 @@ TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, ResponderAuthTimesOut) { EXPECT_FALSE(secure_context_); } -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, DisconnectsWaitingForResponderAuth) { std::string hello_message = BeginAuthentication(); EXPECT_CALL(*this, @@ -309,27 +309,27 @@ TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, EXPECT_FALSE(secure_context_); } -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, NotConnectedInitially) { +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, NotConnectedInitially) { connection_.Disconnect(); EXPECT_CALL(*this, OnAuthenticationResultProxy(Authenticator::Result::DISCONNECTED)); authenticator_.Authenticate(base::Bind( - &ProximityAuthDeviceToDeviceAuthenticatorTest::OnAuthenticationResult, + &CryptAuthDeviceToDeviceAuthenticatorTest::OnAuthenticationResult, base::Unretained(this))); EXPECT_FALSE(secure_context_); } -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, FailToSendHello) { +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, FailToSendHello) { connection_.set_connection_blocked(true); EXPECT_CALL(*this, OnAuthenticationResultProxy(Authenticator::Result::FAILURE)); authenticator_.Authenticate(base::Bind( - &ProximityAuthDeviceToDeviceAuthenticatorTest::OnAuthenticationResult, + &CryptAuthDeviceToDeviceAuthenticatorTest::OnAuthenticationResult, base::Unretained(this))); EXPECT_FALSE(secure_context_); } -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, FailToSendInitiatorAuth) { +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, FailToSendInitiatorAuth) { std::string hello_message = BeginAuthentication(); connection_.set_connection_blocked(true); @@ -339,7 +339,7 @@ TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, FailToSendInitiatorAuth) { EXPECT_FALSE(secure_context_); } -TEST_F(ProximityAuthDeviceToDeviceAuthenticatorTest, +TEST_F(CryptAuthDeviceToDeviceAuthenticatorTest, SendMessagesAfterAuthenticationSuccess) { std::string hello_message = BeginAuthentication(); EXPECT_CALL(*this, diff --git a/chromium/components/cryptauth/device_to_device_initiator_operations.cc b/chromium/components/cryptauth/device_to_device_initiator_helper.cc index 9df030f4888..bf09eed0832 100644 --- a/chromium/components/cryptauth/device_to_device_initiator_operations.cc +++ b/chromium/components/cryptauth/device_to_device_initiator_helper.cc @@ -2,15 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "components/cryptauth/device_to_device_initiator_operations.h" +#include "components/cryptauth/device_to_device_initiator_helper.h" #include "base/bind.h" #include "base/callback.h" #include "base/memory/ptr_util.h" #include "components/cryptauth/proto/cryptauth_api.pb.h" -#include "components/cryptauth/proto/securemessage.pb.h" #include "components/cryptauth/secure_message_delegate.h" -#include "components/cryptauth/session_keys.h" #include "components/proximity_auth/logging/logging.h" namespace cryptauth { @@ -29,12 +27,141 @@ const int kGcmMetadataVersion = 1; // The D2D protocol version. const int kD2DProtocolVersion = 1; -// Callback for DeviceToDeviceInitiatorOperations::CreateInitiatorAuthMessage(), -// after the inner message is created. -void OnInnerMessageCreatedForInitiatorAuth( +} // namespace + +DeviceToDeviceInitiatorHelper::DeviceToDeviceInitiatorHelper() + : weak_ptr_factory_(this) {} + +DeviceToDeviceInitiatorHelper::~DeviceToDeviceInitiatorHelper() {} + +void DeviceToDeviceInitiatorHelper::CreateHelloMessage( + const std::string& session_public_key, + const std::string& persistent_symmetric_key, + SecureMessageDelegate* secure_message_delegate, + const MessageCallback& callback) { + // Decode public key into the |initator_hello| proto. + securemessage::InitiatorHello initiator_hello; + if (!initiator_hello.mutable_public_dh_key()->ParseFromString( + session_public_key)) { + PA_LOG(ERROR) << "Unable to parse user's public key"; + callback.Run(std::string()); + return; + } + initiator_hello.set_protocol_version(kD2DProtocolVersion); + + // The [Hello] message has the structure: + // { + // header: <session_public_key>, + // Sig(<session_public_key>, persistent_symmetric_key) + // payload: "" + // } + SecureMessageDelegate::CreateOptions create_options; + create_options.encryption_scheme = securemessage::NONE; + create_options.signature_scheme = securemessage::HMAC_SHA256; + initiator_hello.SerializeToString(&create_options.public_metadata); + secure_message_delegate->CreateSecureMessage( + kPayloadFiller, persistent_symmetric_key, create_options, callback); +} + +void DeviceToDeviceInitiatorHelper::ValidateResponderAuthMessage( + const std::string& responder_auth_message, + const std::string& persistent_responder_public_key, + const std::string& persistent_symmetric_key, + const std::string& session_private_key, + const std::string& hello_message, + SecureMessageDelegate* secure_message_delegate, + const ValidateResponderAuthCallback& callback) { + // The [Responder Auth] message has the structure: + // { + // header: <responder_public_key>, + // Sig(<responder_public_key> + payload1, + // session_symmetric_key), + // payload1: Enc({ + // header: Sig(payload2 + <hello_message>, persistent_symmetric_key), + // payload2: { + // sequence_number: 1, + // body: Enc({ + // header: Sig(payload3 + <hello_message>, + // persistent_responder_public_key), + // payload3: "" + // }, persistent_symmetric_key) + // } + // }, session_symmetric_key), + // } + ValidateResponderAuthMessageContext context( + responder_auth_message, persistent_responder_public_key, + persistent_symmetric_key, session_private_key, hello_message, + secure_message_delegate, callback); + BeginResponderAuthValidation(context); +} + +void DeviceToDeviceInitiatorHelper::CreateInitiatorAuthMessage( + const SessionKeys& session_keys, + const std::string& persistent_symmetric_key, + const std::string& responder_auth_message, + SecureMessageDelegate* secure_message_delegate, + const MessageCallback& callback) { + // The [Initiator Auth] message has the structure: + // { + // header: Sig(payload1, session_symmetric_key) + // payload1: Enc({ + // sequence_number: 2, + // body: { + // header: Sig(payload2 + responder_auth_message, + // persistent_symmetric_key) + // payload2: "" + // } + // }, session_symmetric_key) + // } + SecureMessageDelegate::CreateOptions create_options; + create_options.encryption_scheme = securemessage::AES_256_CBC; + create_options.signature_scheme = securemessage::HMAC_SHA256; + create_options.associated_data = responder_auth_message; + secure_message_delegate->CreateSecureMessage( + kPayloadFiller, persistent_symmetric_key, create_options, + base::Bind( + &DeviceToDeviceInitiatorHelper::OnInnerMessageCreatedForInitiatorAuth, + weak_ptr_factory_.GetWeakPtr(), session_keys, secure_message_delegate, + callback)); +} + +DeviceToDeviceInitiatorHelper::ValidateResponderAuthMessageContext :: + ValidateResponderAuthMessageContext( + const std::string& responder_auth_message, + const std::string& persistent_responder_public_key, + const std::string& persistent_symmetric_key, + const std::string& session_private_key, + const std::string& hello_message, + SecureMessageDelegate* secure_message_delegate, + const ValidateResponderAuthCallback& callback) + : responder_auth_message(responder_auth_message), + persistent_responder_public_key(persistent_responder_public_key), + persistent_symmetric_key(persistent_symmetric_key), + session_private_key(session_private_key), + hello_message(hello_message), + secure_message_delegate(secure_message_delegate), + callback(callback) {} + +DeviceToDeviceInitiatorHelper::ValidateResponderAuthMessageContext :: + ValidateResponderAuthMessageContext( + const ValidateResponderAuthMessageContext& other) + : responder_auth_message(other.responder_auth_message), + persistent_responder_public_key(other.persistent_responder_public_key), + persistent_symmetric_key(other.persistent_symmetric_key), + session_private_key(other.session_private_key), + hello_message(other.hello_message), + secure_message_delegate(other.secure_message_delegate), + callback(other.callback), + responder_session_public_key(other.responder_session_public_key), + session_symmetric_key(other.session_symmetric_key) {} + +DeviceToDeviceInitiatorHelper::ValidateResponderAuthMessageContext :: + ~ValidateResponderAuthMessageContext() {} + +void DeviceToDeviceInitiatorHelper::OnInnerMessageCreatedForInitiatorAuth( const SessionKeys& session_keys, SecureMessageDelegate* secure_message_delegate, - const DeviceToDeviceInitiatorOperations::MessageCallback& callback, + const DeviceToDeviceInitiatorHelper::MessageCallback& callback, const std::string& inner_message) { if (inner_message.empty()) { PA_LOG(INFO) << "Failed to create inner message for [Initiator Auth]."; @@ -61,44 +188,8 @@ void OnInnerMessageCreatedForInitiatorAuth( session_keys.initiator_encode_key(), create_options, callback); } -// Helper struct containing all the context needed to validate the -// [Responder Auth] message. -struct ValidateResponderAuthMessageContext { - std::string responder_auth_message; - std::string persistent_responder_public_key; - std::string persistent_symmetric_key; - std::string session_private_key; - std::string hello_message; - SecureMessageDelegate* secure_message_delegate; - DeviceToDeviceInitiatorOperations::ValidateResponderAuthCallback callback; - std::string responder_session_public_key; - std::string session_symmetric_key; -}; - -// Forward declarations of functions used in the [Responder Auth] validation -// flow. These functions are declared in order in which they are called during -// the flow. -void BeginResponderAuthValidation(ValidateResponderAuthMessageContext context); -void OnSessionSymmetricKeyDerived(ValidateResponderAuthMessageContext context, - const std::string& session_symmetric_key); -void OnOuterMessageUnwrappedForResponderAuth( - const ValidateResponderAuthMessageContext& context, - bool verified, - const std::string& payload, - const securemessage::Header& header); -void OnMiddleMessageUnwrappedForResponderAuth( - const ValidateResponderAuthMessageContext& context, - bool verified, - const std::string& payload, - const securemessage::Header& header); -void OnInnerMessageUnwrappedForResponderAuth( - const ValidateResponderAuthMessageContext& context, - bool verified, - const std::string& payload, - const securemessage::Header& header); - -// Begins the [Responder Auth] validation flow by validating the header. -void BeginResponderAuthValidation(ValidateResponderAuthMessageContext context) { +void DeviceToDeviceInitiatorHelper::BeginResponderAuthValidation( + ValidateResponderAuthMessageContext context) { // Parse the encrypted SecureMessage so we can get plaintext data from the // header. Note that the payload will be encrypted. securemessage::SecureMessage encrypted_message; @@ -114,8 +205,7 @@ void BeginResponderAuthValidation(ValidateResponderAuthMessageContext context) { securemessage::Header header = header_and_body.header(); GcmMetadata gcm_metadata; if (!gcm_metadata.ParseFromString(header.public_metadata()) || - gcm_metadata.type() != - DEVICE_TO_DEVICE_RESPONDER_HELLO_PAYLOAD || + gcm_metadata.type() != DEVICE_TO_DEVICE_RESPONDER_HELLO_PAYLOAD || gcm_metadata.version() != kGcmMetadataVersion) { PA_LOG(WARNING) << "Failed to validate GcmMetadata in " << "[Responder Auth] header."; @@ -137,13 +227,13 @@ void BeginResponderAuthValidation(ValidateResponderAuthMessageContext context) { // Perform a Diffie-Helmann key exchange to get the session symmetric key. context.secure_message_delegate->DeriveKey( context.session_private_key, context.responder_session_public_key, - base::Bind(&OnSessionSymmetricKeyDerived, context)); + base::Bind(&DeviceToDeviceInitiatorHelper::OnSessionSymmetricKeyDerived, + weak_ptr_factory_.GetWeakPtr(), context)); } -// Called after the session symmetric key is derived, so now we can unwrap the -// outer message of [Responder Auth]. -void OnSessionSymmetricKeyDerived(ValidateResponderAuthMessageContext context, - const std::string& session_symmetric_key) { +void DeviceToDeviceInitiatorHelper::OnSessionSymmetricKeyDerived( + ValidateResponderAuthMessageContext context, + const std::string& session_symmetric_key) { context.session_symmetric_key = session_symmetric_key; // Unwrap the outer message, using symmetric key encryption and signature. @@ -153,11 +243,12 @@ void OnSessionSymmetricKeyDerived(ValidateResponderAuthMessageContext context, context.secure_message_delegate->UnwrapSecureMessage( context.responder_auth_message, SessionKeys(session_symmetric_key).responder_encode_key(), unwrap_options, - base::Bind(&OnOuterMessageUnwrappedForResponderAuth, context)); + base::Bind(&DeviceToDeviceInitiatorHelper:: + OnOuterMessageUnwrappedForResponderAuth, + weak_ptr_factory_.GetWeakPtr(), context)); } -// Called after the outer-most layer of [Responder Auth] is unwrapped. -void OnOuterMessageUnwrappedForResponderAuth( +void DeviceToDeviceInitiatorHelper::OnOuterMessageUnwrappedForResponderAuth( const ValidateResponderAuthMessageContext& context, bool verified, const std::string& payload, @@ -186,11 +277,12 @@ void OnOuterMessageUnwrappedForResponderAuth( context.secure_message_delegate->UnwrapSecureMessage( device_to_device_message.message(), context.persistent_symmetric_key, unwrap_options, - base::Bind(&OnMiddleMessageUnwrappedForResponderAuth, context)); + base::Bind(&DeviceToDeviceInitiatorHelper:: + OnMiddleMessageUnwrappedForResponderAuth, + weak_ptr_factory_.GetWeakPtr(), context)); } -// Called after the middle layer of [Responder Auth] is unwrapped. -void OnMiddleMessageUnwrappedForResponderAuth( +void DeviceToDeviceInitiatorHelper::OnMiddleMessageUnwrappedForResponderAuth( const ValidateResponderAuthMessageContext& context, bool verified, const std::string& payload, @@ -209,11 +301,13 @@ void OnMiddleMessageUnwrappedForResponderAuth( unwrap_options.associated_data = context.hello_message; context.secure_message_delegate->UnwrapSecureMessage( payload, context.persistent_responder_public_key, unwrap_options, - base::Bind(&OnInnerMessageUnwrappedForResponderAuth, context)); + base::Bind(&DeviceToDeviceInitiatorHelper:: + OnInnerMessageUnwrappedForResponderAuth, + weak_ptr_factory_.GetWeakPtr(), context)); } // Called after the inner-most layer of [Responder Auth] is unwrapped. -void OnInnerMessageUnwrappedForResponderAuth( +void DeviceToDeviceInitiatorHelper::OnInnerMessageUnwrappedForResponderAuth( const ValidateResponderAuthMessageContext& context, bool verified, const std::string& payload, @@ -235,102 +329,4 @@ void OnInnerMessageUnwrappedForResponderAuth( context.callback.Run(verified, SessionKeys(context.session_symmetric_key)); } -} // namespace - -// static -void DeviceToDeviceInitiatorOperations::CreateHelloMessage( - const std::string& session_public_key, - const std::string& persistent_symmetric_key, - SecureMessageDelegate* secure_message_delegate, - const MessageCallback& callback) { - // Decode public key into the |initator_hello| proto. - securemessage::InitiatorHello initiator_hello; - if (!initiator_hello.mutable_public_dh_key()->ParseFromString( - session_public_key)) { - PA_LOG(ERROR) << "Unable to parse user's public key"; - callback.Run(std::string()); - return; - } - initiator_hello.set_protocol_version(kD2DProtocolVersion); - - // The [Hello] message has the structure: - // { - // header: <session_public_key>, - // Sig(<session_public_key>, persistent_symmetric_key) - // payload: "" - // } - SecureMessageDelegate::CreateOptions create_options; - create_options.encryption_scheme = securemessage::NONE; - create_options.signature_scheme = securemessage::HMAC_SHA256; - initiator_hello.SerializeToString(&create_options.public_metadata); - secure_message_delegate->CreateSecureMessage( - kPayloadFiller, persistent_symmetric_key, create_options, callback); -} - -// static -void DeviceToDeviceInitiatorOperations::ValidateResponderAuthMessage( - const std::string& responder_auth_message, - const std::string& persistent_responder_public_key, - const std::string& persistent_symmetric_key, - const std::string& session_private_key, - const std::string& hello_message, - SecureMessageDelegate* secure_message_delegate, - const ValidateResponderAuthCallback& callback) { - // The [Responder Auth] message has the structure: - // { - // header: <responder_public_key>, - // Sig(<responder_public_key> + payload1, - // session_symmetric_key), - // payload1: Enc({ - // header: Sig(payload2 + <hello_message>, persistent_symmetric_key), - // payload2: { - // sequence_number: 1, - // body: Enc({ - // header: Sig(payload3 + <hello_message>, - // persistent_responder_public_key), - // payload3: "" - // }, persistent_symmetric_key) - // } - // }, session_symmetric_key), - // } - struct ValidateResponderAuthMessageContext context = { - responder_auth_message, - persistent_responder_public_key, - persistent_symmetric_key, - session_private_key, - hello_message, - secure_message_delegate, - callback}; - BeginResponderAuthValidation(context); -} - -// static -void DeviceToDeviceInitiatorOperations::CreateInitiatorAuthMessage( - const SessionKeys& session_keys, - const std::string& persistent_symmetric_key, - const std::string& responder_auth_message, - SecureMessageDelegate* secure_message_delegate, - const MessageCallback& callback) { - // The [Initiator Auth] message has the structure: - // { - // header: Sig(payload1, session_symmetric_key) - // payload1: Enc({ - // sequence_number: 2, - // body: { - // header: Sig(payload2 + responder_auth_message, - // persistent_symmetric_key) - // payload2: "" - // } - // }, session_symmetric_key) - // } - SecureMessageDelegate::CreateOptions create_options; - create_options.encryption_scheme = securemessage::AES_256_CBC; - create_options.signature_scheme = securemessage::HMAC_SHA256; - create_options.associated_data = responder_auth_message; - secure_message_delegate->CreateSecureMessage( - kPayloadFiller, persistent_symmetric_key, create_options, - base::Bind(&OnInnerMessageCreatedForInitiatorAuth, session_keys, - secure_message_delegate, callback)); -} - -} // cryptauth +} // namespace cryptauth diff --git a/chromium/components/cryptauth/device_to_device_initiator_operations.h b/chromium/components/cryptauth/device_to_device_initiator_helper.h index cdef8bc8040..c9627f93399 100644 --- a/chromium/components/cryptauth/device_to_device_initiator_operations.h +++ b/chromium/components/cryptauth/device_to_device_initiator_helper.h @@ -2,23 +2,26 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef COMPONENTS_CRYPTAUTH_DEVICE_TO_DEVICE_INITIATOR_OPERATIONS_H_ -#define COMPONENTS_CRYPTAUTH_DEVICE_TO_DEVICE_INITIATOR_OPERATIONS_H_ +#ifndef COMPONENTS_CRYPTAUTH_DEVICE_TO_DEVICE_INITIATOR_HELPER_H_ +#define COMPONENTS_CRYPTAUTH_DEVICE_TO_DEVICE_INITIATOR_HELPER_H_ #include <memory> #include <string> -#include "base/callback_forward.h" +#include "base/callback.h" #include "base/macros.h" +#include "base/memory/weak_ptr.h" +#include "components/cryptauth/proto/securemessage.pb.h" #include "components/cryptauth/session_keys.h" namespace cryptauth { class SecureMessageDelegate; -// Utility class containing operations in the DeviceToDevice protocol that the -// initiator needs to perform. For Smart Lock, in which a phone unlocks a -// laptop, the initiator is laptop. +// Class containing operations in the DeviceToDevice protocol that the initiator +// needs to perform. This class is instantiable rather than being a utility +// class because it relies on a WeakPtrFactory to prevent referencing deleted +// memory. // // All operations are asynchronous because we use the SecureMessageDelegate for // crypto operations, whose implementation may be asynchronous. @@ -35,7 +38,7 @@ class SecureMessageDelegate; // 3. Send [Initiator Auth] Message // After receiving the responder's session public key, the initiator crafts // and sends this message so the responder can authenticate the initiator. -class DeviceToDeviceInitiatorOperations { +class DeviceToDeviceInitiatorHelper { public: // Callback for operations that create a message. Invoked with the serialized // SecureMessage upon success or the empty string upon failure. @@ -48,6 +51,9 @@ class DeviceToDeviceInitiatorOperations { typedef base::Callback<void(bool, const SessionKeys&)> ValidateResponderAuthCallback; + DeviceToDeviceInitiatorHelper(); + virtual ~DeviceToDeviceInitiatorHelper(); + // Creates the [Hello] message, which is the first message that is sent: // |session_public_key|: This session public key will be stored in plaintext // (but signed) so the responder can parse it. @@ -57,11 +63,10 @@ class DeviceToDeviceInitiatorOperations { // instance is not owned, and must live until after |callback| is invoked. // |callback|: Invoked upon operation completion with the serialized message // or an empty string. - static void CreateHelloMessage( - const std::string& session_public_key, - const std::string& persistent_symmetric_key, - SecureMessageDelegate* secure_message_delegate, - const MessageCallback& callback); + void CreateHelloMessage(const std::string& session_public_key, + const std::string& persistent_symmetric_key, + SecureMessageDelegate* secure_message_delegate, + const MessageCallback& callback); // Validates that the [Responder Auth] message, received from the responder, // is properly signed and encrypted. @@ -80,7 +85,7 @@ class DeviceToDeviceInitiatorOperations { // instance is not owned, and must live until after |callback| is invoked. // |callback|: Invoked upon operation completion with whether // |responder_auth_message| is validated successfully. - static void ValidateResponderAuthMessage( + void ValidateResponderAuthMessage( const std::string& responder_auth_message, const std::string& persistent_responder_public_key, const std::string& persistent_symmetric_key, @@ -100,7 +105,7 @@ class DeviceToDeviceInitiatorOperations { // instance is not owned, and must live until after |callback| is invoked. // |callback|: Invoked upon operation completion with the serialized message // or an empty string. - static void CreateInitiatorAuthMessage( + void CreateInitiatorAuthMessage( const SessionKeys& session_keys, const std::string& persistent_symmetric_key, const std::string& responder_auth_message, @@ -108,9 +113,75 @@ class DeviceToDeviceInitiatorOperations { const MessageCallback& callback); private: - DISALLOW_IMPLICIT_CONSTRUCTORS(DeviceToDeviceInitiatorOperations); + // Helper struct containing all the context needed to validate the + // [Responder Auth] message. + struct ValidateResponderAuthMessageContext { + ValidateResponderAuthMessageContext( + const std::string& responder_auth_message, + const std::string& persistent_responder_public_key, + const std::string& persistent_symmetric_key, + const std::string& session_private_key, + const std::string& hello_message, + SecureMessageDelegate* secure_message_delegate, + const ValidateResponderAuthCallback& callback); + ValidateResponderAuthMessageContext( + const ValidateResponderAuthMessageContext& other); + ~ValidateResponderAuthMessageContext(); + + std::string responder_auth_message; + std::string persistent_responder_public_key; + std::string persistent_symmetric_key; + std::string session_private_key; + std::string hello_message; + SecureMessageDelegate* secure_message_delegate; + ValidateResponderAuthCallback callback; + std::string responder_session_public_key; + std::string session_symmetric_key; + }; + + // Begins the [Responder Auth] validation flow by validating the header. + void BeginResponderAuthValidation( + ValidateResponderAuthMessageContext context); + + // Called after the session symmetric key is derived, so now we can unwrap the + // outer message of [Responder Auth]. + void OnSessionSymmetricKeyDerived(ValidateResponderAuthMessageContext context, + const std::string& session_symmetric_key); + + // Called after the outer-most layer of [Responder Auth] is unwrapped. + void OnOuterMessageUnwrappedForResponderAuth( + const ValidateResponderAuthMessageContext& context, + bool verified, + const std::string& payload, + const securemessage::Header& header); + + // Called after the middle layer of [Responder Auth] is unwrapped. + void OnMiddleMessageUnwrappedForResponderAuth( + const ValidateResponderAuthMessageContext& context, + bool verified, + const std::string& payload, + const securemessage::Header& header); + + // Called after inner message is created. + void OnInnerMessageCreatedForInitiatorAuth( + const SessionKeys& session_keys, + SecureMessageDelegate* secure_message_delegate, + const DeviceToDeviceInitiatorHelper::MessageCallback& callback, + const std::string& inner_message); + + // Callback for CreateInitiatorAuthMessage(), after the inner message is + // created. + void OnInnerMessageUnwrappedForResponderAuth( + const ValidateResponderAuthMessageContext& context, + bool verified, + const std::string& payload, + const securemessage::Header& header); + + base::WeakPtrFactory<DeviceToDeviceInitiatorHelper> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(DeviceToDeviceInitiatorHelper); }; -} // cryptauth +} // namespace cryptauth -#endif // COMPONENTS_CRYPTAUTH_DEVICE_TO_DEVICE_INITIATOR_OPERATIONS_H_ +#endif // COMPONENTS_CRYPTAUTH_DEVICE_TO_DEVICE_INITIATOR_HELPER_H_ diff --git a/chromium/components/cryptauth/device_to_device_operations_unittest.cc b/chromium/components/cryptauth/device_to_device_operations_unittest.cc index 8cffab2c410..31bba05a155 100644 --- a/chromium/components/cryptauth/device_to_device_operations_unittest.cc +++ b/chromium/components/cryptauth/device_to_device_operations_unittest.cc @@ -5,7 +5,8 @@ #include "base/base64url.h" #include "base/bind.h" #include "base/macros.h" -#include "components/cryptauth/device_to_device_initiator_operations.h" +#include "base/memory/ptr_util.h" +#include "components/cryptauth/device_to_device_initiator_helper.h" #include "components/cryptauth/device_to_device_responder_operations.h" #include "components/cryptauth/fake_secure_message_delegate.h" #include "components/cryptauth/session_keys.h" @@ -63,10 +64,10 @@ void SaveValidationResultWithSessionKeys(bool* out_success, } // namespace -class ProximityAuthDeviceToDeviceOperationsTest : public testing::Test { +class CryptAuthDeviceToDeviceOperationsTest : public testing::Test { protected: - ProximityAuthDeviceToDeviceOperationsTest() {} - ~ProximityAuthDeviceToDeviceOperationsTest() override {} + CryptAuthDeviceToDeviceOperationsTest() {} + ~CryptAuthDeviceToDeviceOperationsTest() override {} void SetUp() override { ASSERT_TRUE( @@ -92,15 +93,17 @@ class ProximityAuthDeviceToDeviceOperationsTest : public testing::Test { session_keys_ = SessionKeys(session_symmetric_key_); persistent_symmetric_key_ = "persistent symmetric key"; + + helper_ = base::MakeUnique<DeviceToDeviceInitiatorHelper>(); } // Creates the initator's [Hello] message. std::string CreateHelloMessage() { std::string hello_message; - DeviceToDeviceInitiatorOperations::CreateHelloMessage( - local_session_public_key_, persistent_symmetric_key_, - &secure_message_delegate_, - base::Bind(&SaveMessageResult, &hello_message)); + helper_->CreateHelloMessage(local_session_public_key_, + persistent_symmetric_key_, + &secure_message_delegate_, + base::Bind(&SaveMessageResult, &hello_message)); EXPECT_FALSE(hello_message.empty()); return hello_message; } @@ -125,7 +128,7 @@ class ProximityAuthDeviceToDeviceOperationsTest : public testing::Test { std::string CreateInitiatorAuthMessage( const std::string& remote_auth_message) { std::string local_auth_message; - DeviceToDeviceInitiatorOperations::CreateInitiatorAuthMessage( + helper_->CreateInitiatorAuthMessage( session_keys_, persistent_symmetric_key_, remote_auth_message, &secure_message_delegate_, base::Bind(&SaveMessageResult, &local_auth_message)); @@ -143,11 +146,12 @@ class ProximityAuthDeviceToDeviceOperationsTest : public testing::Test { std::string session_symmetric_key_; SessionKeys session_keys_; - DISALLOW_COPY_AND_ASSIGN(ProximityAuthDeviceToDeviceOperationsTest); + std::unique_ptr<DeviceToDeviceInitiatorHelper> helper_; + + DISALLOW_COPY_AND_ASSIGN(CryptAuthDeviceToDeviceOperationsTest); }; -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, - ValidateHelloMessage_Success) { +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateHelloMessage_Success) { bool validation_success = false; std::string hello_public_key; DeviceToDeviceResponderOperations::ValidateHelloMessage( @@ -160,8 +164,7 @@ TEST_F(ProximityAuthDeviceToDeviceOperationsTest, EXPECT_EQ(local_session_public_key_, hello_public_key); } -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, - ValidateHelloMessage_Failure) { +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateHelloMessage_Failure) { bool validation_success = true; std::string hello_public_key = "non-empty string"; DeviceToDeviceResponderOperations::ValidateHelloMessage( @@ -174,14 +177,14 @@ TEST_F(ProximityAuthDeviceToDeviceOperationsTest, EXPECT_TRUE(hello_public_key.empty()); } -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateResponderAuthMessage_Success) { std::string hello_message = CreateHelloMessage(); std::string remote_auth_message = CreateResponderAuthMessage(hello_message); bool validation_success = false; SessionKeys session_keys; - DeviceToDeviceInitiatorOperations::ValidateResponderAuthMessage( + helper_->ValidateResponderAuthMessage( remote_auth_message, kResponderPersistentPublicKey, persistent_symmetric_key_, local_session_private_key_, hello_message, &secure_message_delegate_, @@ -195,14 +198,14 @@ TEST_F(ProximityAuthDeviceToDeviceOperationsTest, session_keys.responder_encode_key()); } -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateResponderAuthMessage_InvalidHelloMessage) { std::string hello_message = CreateHelloMessage(); std::string remote_auth_message = CreateResponderAuthMessage(hello_message); bool validation_success = true; SessionKeys session_keys("non empty"); - DeviceToDeviceInitiatorOperations::ValidateResponderAuthMessage( + helper_->ValidateResponderAuthMessage( remote_auth_message, kResponderPersistentPublicKey, persistent_symmetric_key_, local_session_private_key_, "invalid hello message", &secure_message_delegate_, @@ -214,14 +217,14 @@ TEST_F(ProximityAuthDeviceToDeviceOperationsTest, EXPECT_TRUE(session_keys.responder_encode_key().empty()); } -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateResponderAuthMessage_InvalidPSK) { std::string hello_message = CreateHelloMessage(); std::string remote_auth_message = CreateResponderAuthMessage(hello_message); bool validation_success = true; SessionKeys session_keys("non empty"); - DeviceToDeviceInitiatorOperations::ValidateResponderAuthMessage( + helper_->ValidateResponderAuthMessage( remote_auth_message, kResponderPersistentPublicKey, "invalid persistent symmetric key", local_session_private_key_, hello_message, &secure_message_delegate_, @@ -233,7 +236,7 @@ TEST_F(ProximityAuthDeviceToDeviceOperationsTest, EXPECT_TRUE(session_keys.responder_encode_key().empty()); } -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateInitiatorAuthMessage_Success) { std::string hello_message = CreateHelloMessage(); std::string remote_auth_message = CreateResponderAuthMessage(hello_message); @@ -249,7 +252,7 @@ TEST_F(ProximityAuthDeviceToDeviceOperationsTest, EXPECT_TRUE(validation_success); } -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateInitiatorAuthMessage_InvalidRemoteAuth) { std::string hello_message = CreateHelloMessage(); std::string remote_auth_message = CreateResponderAuthMessage(hello_message); @@ -265,7 +268,7 @@ TEST_F(ProximityAuthDeviceToDeviceOperationsTest, EXPECT_FALSE(validation_success); } -TEST_F(ProximityAuthDeviceToDeviceOperationsTest, +TEST_F(CryptAuthDeviceToDeviceOperationsTest, ValidateInitiatorAuthMessage_InvalidPSK) { std::string hello_message = CreateHelloMessage(); std::string remote_auth_message = CreateResponderAuthMessage(hello_message); diff --git a/chromium/components/cryptauth/fake_secure_channel.cc b/chromium/components/cryptauth/fake_secure_channel.cc index 03956aadc48..77db2d948e4 100644 --- a/chromium/components/cryptauth/fake_secure_channel.cc +++ b/chromium/components/cryptauth/fake_secure_channel.cc @@ -38,11 +38,21 @@ void FakeSecureChannel::ReceiveMessage(const std::string& feature, } } +void FakeSecureChannel::CompleteSendingMessage(int sequence_number) { + DCHECK(next_sequence_number_ > sequence_number); + // Copy to prevent channel from being removed during handler. + std::vector<Observer*> observers_copy = observers_; + for (auto* observer : observers_copy) { + observer->OnMessageSent(this, sequence_number); + } +} + void FakeSecureChannel::Initialize() {} -void FakeSecureChannel::SendMessage(const std::string& feature, - const std::string& payload) { +int FakeSecureChannel::SendMessage(const std::string& feature, + const std::string& payload) { sent_messages_.push_back(SentMessage(feature, payload)); + return next_sequence_number_++; } void FakeSecureChannel::Disconnect() { diff --git a/chromium/components/cryptauth/fake_secure_channel.h b/chromium/components/cryptauth/fake_secure_channel.h index 4a202d9a771..716b595e926 100644 --- a/chromium/components/cryptauth/fake_secure_channel.h +++ b/chromium/components/cryptauth/fake_secure_channel.h @@ -28,6 +28,7 @@ class FakeSecureChannel : public SecureChannel { void ChangeStatus(const Status& new_status); void ReceiveMessage(const std::string& feature, const std::string& payload); + void CompleteSendingMessage(int sequence_number); std::vector<Observer*> observers() { return observers_; } @@ -35,13 +36,14 @@ class FakeSecureChannel : public SecureChannel { // SecureChannel: void Initialize() override; - void SendMessage(const std::string& feature, - const std::string& payload) override; + int SendMessage(const std::string& feature, + const std::string& payload) override; void Disconnect() override; void AddObserver(Observer* observer) override; void RemoveObserver(Observer* observer) override; private: + int next_sequence_number_ = 0; std::vector<Observer*> observers_; std::vector<SentMessage> sent_messages_; diff --git a/chromium/components/cryptauth/foreground_eid_generator.cc b/chromium/components/cryptauth/foreground_eid_generator.cc index 01e269e717c..5c2b742ff48 100644 --- a/chromium/components/cryptauth/foreground_eid_generator.cc +++ b/chromium/components/cryptauth/foreground_eid_generator.cc @@ -19,14 +19,12 @@ namespace cryptauth { namespace { -const int64_t kNoTimestamp = 0; -const int64_t kMaxPositiveInt64TValue = 0x7FFFFFFF; +constexpr int64_t kNoTimestamp = 0; +constexpr int64_t kMaxPositiveInt64TValue = 0x7FFFFFFF; +constexpr base::TimeDelta kEidPeriod = base::TimeDelta::FromHours(8); +constexpr base::TimeDelta kBeginningOfEidPeriod = base::TimeDelta::FromHours(2); } // namespace -const int64_t ForegroundEidGenerator::kNumMsInEidPeriod = - base::TimeDelta::FromHours(8).InMilliseconds(); -const int64_t ForegroundEidGenerator::kNumMsInBeginningOfEidPeriod = - base::TimeDelta::FromHours(2).InMilliseconds(); const int8_t ForegroundEidGenerator::kBluetooth4Flag = 0x01; ForegroundEidGenerator::EidData::EidData( @@ -271,8 +269,9 @@ ForegroundEidGenerator::GetEidPeriodTimestamps( // current EID period must be determined. for (int64_t start_of_eid_period_ms = current_seed->start_time_millis(); start_of_eid_period_ms <= current_seed->end_time_millis(); - start_of_eid_period_ms += kNumMsInEidPeriod) { - int64_t end_of_eid_period_ms = start_of_eid_period_ms + kNumMsInEidPeriod; + start_of_eid_period_ms += kEidPeriod.InMilliseconds()) { + int64_t end_of_eid_period_ms = + start_of_eid_period_ms + kEidPeriod.InMilliseconds(); if (start_of_eid_period_ms <= current_time_ms && current_time_ms < end_of_eid_period_ms) { int64_t start_of_adjacent_period_ms; @@ -282,12 +281,13 @@ ForegroundEidGenerator::GetEidPeriodTimestamps( // If the current time is at the beginning of the period, the "adjacent // period" is the period before this one. start_of_adjacent_period_ms = - start_of_eid_period_ms - kNumMsInEidPeriod; + start_of_eid_period_ms - kEidPeriod.InMilliseconds(); end_of_adjacent_period_ms = start_of_eid_period_ms; } else { // Otherwise, the "adjacent period" is the one after this one. start_of_adjacent_period_ms = end_of_eid_period_ms; - end_of_adjacent_period_ms = end_of_eid_period_ms + kNumMsInEidPeriod; + end_of_adjacent_period_ms = + end_of_eid_period_ms + kEidPeriod.InMilliseconds(); } return base::WrapUnique(new EidPeriodTimestamps{ @@ -312,7 +312,7 @@ ForegroundEidGenerator::GetBeaconSeedForCurrentPeriod( for (auto seed : scanning_device_beacon_seeds) { int64_t seed_period_length_ms = seed.end_time_millis() - seed.start_time_millis(); - if (seed_period_length_ms % kNumMsInEidPeriod != 0) { + if (seed_period_length_ms % kEidPeriod.InMilliseconds() != 0) { PA_LOG(WARNING) << "Seed has period length which is not an multiple of " << "the rotation length."; continue; @@ -338,7 +338,7 @@ ForegroundEidGenerator::GetClosestPeriod( for (auto seed : scanning_device_beacon_seeds) { int64_t seed_period_length_ms = seed.end_time_millis() - seed.start_time_millis(); - if (seed_period_length_ms % kNumMsInEidPeriod != 0) { + if (seed_period_length_ms % kEidPeriod.InMilliseconds() != 0) { PA_LOG(WARNING) << "Seed has period length which is not an multiple of " << "the rotation length."; continue; @@ -353,7 +353,7 @@ ForegroundEidGenerator::GetClosestPeriod( smallest_diff_so_far_ms = diff; start_of_period_timestamp_ms = seed.start_time_millis(); end_of_period_timestamp_ms = - seed.start_time_millis() + kNumMsInEidPeriod; + seed.start_time_millis() + kEidPeriod.InMilliseconds(); } } else if (seed.end_time_millis() <= current_time_ms) { int64_t diff = current_time_ms - seed.end_time_millis(); @@ -361,13 +361,13 @@ ForegroundEidGenerator::GetClosestPeriod( smallest_diff_so_far_ms = diff; end_of_period_timestamp_ms = seed.end_time_millis(); start_of_period_timestamp_ms = - end_of_period_timestamp_ms - kNumMsInEidPeriod; + end_of_period_timestamp_ms - kEidPeriod.InMilliseconds(); } } } if (smallest_diff_so_far_ms < kMaxPositiveInt64TValue && - smallest_diff_so_far_ms > kNumMsInEidPeriod) { + smallest_diff_so_far_ms > kEidPeriod.InMilliseconds()) { return nullptr; } @@ -385,7 +385,7 @@ bool ForegroundEidGenerator::IsCurrentTimeAtStartOfEidPeriod( DCHECK(current_timestamp_ms < end_of_period_timestamp_ms); return current_timestamp_ms < - start_of_period_timestamp_ms + kNumMsInBeginningOfEidPeriod; + start_of_period_timestamp_ms + kBeginningOfEidPeriod.InMilliseconds(); } } // namespace cryptauth diff --git a/chromium/components/cryptauth/foreground_eid_generator.h b/chromium/components/cryptauth/foreground_eid_generator.h index ec68de89920..8a7250cf3d5 100644 --- a/chromium/components/cryptauth/foreground_eid_generator.h +++ b/chromium/components/cryptauth/foreground_eid_generator.h @@ -104,9 +104,6 @@ class ForegroundEidGenerator { int64_t adjacent_period_end_timestamp_ms; }; - static const int64_t kNumMsInEidPeriod; - static const int64_t kNumMsInBeginningOfEidPeriod; - ForegroundEidGenerator(std::unique_ptr<RawEidGenerator> raw_eid_generator, std::unique_ptr<base::Clock> clock); diff --git a/chromium/components/cryptauth/foreground_eid_generator_unittest.cc b/chromium/components/cryptauth/foreground_eid_generator_unittest.cc index 97e90179685..a964f494349 100644 --- a/chromium/components/cryptauth/foreground_eid_generator_unittest.cc +++ b/chromium/components/cryptauth/foreground_eid_generator_unittest.cc @@ -27,13 +27,17 @@ using testing::SaveArg; namespace cryptauth { namespace { -const int64_t kNumMsInEidPeriod = - base::TimeDelta::FromHours(8).InMilliseconds(); -const int64_t kNumMsInBeginningOfEidPeriod = - base::TimeDelta::FromHours(2).InMilliseconds(); + +// These constants could be made as integer amounts of milliseconds by calling +// .InMilliseconds(), but this would create a static initializer because there +// is no constexpr implementation of TimeDelta::InMilliseconds() yet. Static +// initializers are not a big problem in tests, but it is preferable to avoid +// them here for consistensy with similar definitions going into release +// binaries. +constexpr base::TimeDelta kEidPeriod = base::TimeDelta::FromHours(8); +constexpr base::TimeDelta kEidSeedPeriod = base::TimeDelta::FromDays(14); + const int32_t kNumBytesInEidValue = 2; -const int64_t kNumMsInEidSeedPeriod = - base::TimeDelta::FromDays(14).InMilliseconds(); // Midnight on 1/1/2020. const int64_t kDefaultCurrentPeriodStart = 1577836800000L; @@ -101,17 +105,20 @@ class CryptAuthForegroundEidGeneratorTest : public testing::Test { protected: CryptAuthForegroundEidGeneratorTest() { scanning_device_beacon_seeds_.push_back(CreateBeaconSeed( - kFirstSeed, kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod, + kFirstSeed, + kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds(), kDefaultCurrentPeriodStart)); - scanning_device_beacon_seeds_.push_back( - CreateBeaconSeed(kSecondSeed, kDefaultCurrentPeriodStart, - kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod)); scanning_device_beacon_seeds_.push_back(CreateBeaconSeed( - kThirdSeed, kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod, - kDefaultCurrentPeriodStart + 2 * kNumMsInEidSeedPeriod)); + kSecondSeed, kDefaultCurrentPeriodStart, + kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds())); + scanning_device_beacon_seeds_.push_back(CreateBeaconSeed( + kThirdSeed, + kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds(), + kDefaultCurrentPeriodStart + 2 * kEidSeedPeriod.InMilliseconds())); scanning_device_beacon_seeds_.push_back(CreateBeaconSeed( - kFourthSeed, kDefaultCurrentPeriodStart + 2 * kNumMsInEidSeedPeriod, - kDefaultCurrentPeriodStart + 3 * kNumMsInEidSeedPeriod)); + kFourthSeed, + kDefaultCurrentPeriodStart + 2 * kEidSeedPeriod.InMilliseconds(), + kDefaultCurrentPeriodStart + 3 * kEidSeedPeriod.InMilliseconds())); } class TestRawEidGenerator : public RawEidGenerator { @@ -162,25 +169,26 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, ForegroundEidGenerator::EidData::AdjacentDataType::PAST); EXPECT_EQ(kDefaultCurrentPeriodStart, data->current_data.start_timestamp_ms); - EXPECT_EQ(kDefaultCurrentPeriodStart + kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidPeriod.InMilliseconds(), data->current_data.end_timestamp_ms); EXPECT_EQ( GenerateFakeEidData(kSecondSeed, kDefaultCurrentPeriodStart, nullptr), data->current_data.data); ASSERT_TRUE(data->adjacent_data); - EXPECT_EQ(kDefaultCurrentPeriodStart - kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart - kEidPeriod.InMilliseconds(), data->adjacent_data->start_timestamp_ms); EXPECT_EQ(kDefaultCurrentPeriodStart, data->adjacent_data->end_timestamp_ms); EXPECT_EQ( GenerateFakeEidData( - kFirstSeed, kDefaultCurrentPeriodStart - kNumMsInEidPeriod, nullptr), + kFirstSeed, kDefaultCurrentPeriodStart - kEidPeriod.InMilliseconds(), + nullptr), data->adjacent_data->data); } TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateBackgroundScanFilter_StartOfPeriod_NoSeedBefore) { - SetTestTime(kDefaultCurrentTime - kNumMsInEidSeedPeriod); + SetTestTime(kDefaultCurrentTime - kEidSeedPeriod.InMilliseconds()); std::unique_ptr<ForegroundEidGenerator::EidData> data = eid_generator_->GenerateBackgroundScanFilter( @@ -189,13 +197,14 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ(data->GetAdjacentDataType(), ForegroundEidGenerator::EidData::AdjacentDataType::NONE); - EXPECT_EQ(kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds(), data->current_data.start_timestamp_ms); - EXPECT_EQ( - kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod + kNumMsInEidPeriod, - data->current_data.end_timestamp_ms); + EXPECT_EQ(kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds() + + kEidPeriod.InMilliseconds(), + data->current_data.end_timestamp_ms); EXPECT_EQ(GenerateFakeEidData( - kFirstSeed, kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod, + kFirstSeed, + kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds(), nullptr), data->current_data.data); @@ -215,26 +224,27 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, ForegroundEidGenerator::EidData::AdjacentDataType::FUTURE); EXPECT_EQ(kDefaultCurrentPeriodStart, data->current_data.start_timestamp_ms); - EXPECT_EQ(kDefaultCurrentPeriodStart + kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidPeriod.InMilliseconds(), data->current_data.end_timestamp_ms); EXPECT_EQ( GenerateFakeEidData(kSecondSeed, kDefaultCurrentPeriodStart, nullptr), data->current_data.data); ASSERT_TRUE(data->adjacent_data); - EXPECT_EQ(kDefaultCurrentPeriodStart + kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidPeriod.InMilliseconds(), data->adjacent_data->start_timestamp_ms); - EXPECT_EQ(kDefaultCurrentPeriodStart + 2 * kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + 2 * kEidPeriod.InMilliseconds(), data->adjacent_data->end_timestamp_ms); EXPECT_EQ( GenerateFakeEidData( - kSecondSeed, kDefaultCurrentPeriodStart + kNumMsInEidPeriod, nullptr), + kSecondSeed, kDefaultCurrentPeriodStart + kEidPeriod.InMilliseconds(), + nullptr), data->adjacent_data->data); } TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateBackgroundScanFilter_EndOfPeriod) { - SetTestTime(kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod - 1); + SetTestTime(kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds() - 1); std::unique_ptr<ForegroundEidGenerator::EidData> data = eid_generator_->GenerateBackgroundScanFilter( @@ -243,32 +253,35 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ(data->GetAdjacentDataType(), ForegroundEidGenerator::EidData::AdjacentDataType::FUTURE); - EXPECT_EQ( - kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod - kNumMsInEidPeriod, - data->current_data.start_timestamp_ms); - EXPECT_EQ(kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds() - + kEidPeriod.InMilliseconds(), + data->current_data.start_timestamp_ms); + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds(), data->current_data.end_timestamp_ms); EXPECT_EQ(GenerateFakeEidData(kSecondSeed, kDefaultCurrentPeriodStart + - kNumMsInEidSeedPeriod - kNumMsInEidPeriod, + kEidSeedPeriod.InMilliseconds() - + kEidPeriod.InMilliseconds(), nullptr), data->current_data.data); ASSERT_TRUE(data->adjacent_data); - EXPECT_EQ(kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds(), data->adjacent_data->start_timestamp_ms); - EXPECT_EQ( - kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod + kNumMsInEidPeriod, - data->adjacent_data->end_timestamp_ms); + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds() + + kEidPeriod.InMilliseconds(), + data->adjacent_data->end_timestamp_ms); EXPECT_EQ(GenerateFakeEidData( - kThirdSeed, kDefaultCurrentPeriodStart + kNumMsInEidSeedPeriod, + kThirdSeed, + kDefaultCurrentPeriodStart + kEidSeedPeriod.InMilliseconds(), nullptr), data->adjacent_data->data); } TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateBackgroundScanFilter_EndOfPeriod_NoSeedAfter) { - SetTestTime(kDefaultCurrentPeriodStart + 3 * kNumMsInEidSeedPeriod - 1); + SetTestTime(kDefaultCurrentPeriodStart + 3 * kEidSeedPeriod.InMilliseconds() - + 1); std::unique_ptr<ForegroundEidGenerator::EidData> data = eid_generator_->GenerateBackgroundScanFilter( @@ -277,24 +290,25 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ(data->GetAdjacentDataType(), ForegroundEidGenerator::EidData::AdjacentDataType::NONE); - EXPECT_EQ(kDefaultCurrentPeriodStart + 3 * kNumMsInEidSeedPeriod - - kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + 3 * kEidSeedPeriod.InMilliseconds() - + kEidPeriod.InMilliseconds(), data->current_data.start_timestamp_ms); - EXPECT_EQ(kDefaultCurrentPeriodStart + 3 * kNumMsInEidSeedPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + 3 * kEidSeedPeriod.InMilliseconds(), data->current_data.end_timestamp_ms); - EXPECT_EQ( - GenerateFakeEidData(kFourthSeed, - kDefaultCurrentPeriodStart + - 3 * kNumMsInEidSeedPeriod - kNumMsInEidPeriod, - nullptr), - data->current_data.data); + EXPECT_EQ(GenerateFakeEidData(kFourthSeed, + kDefaultCurrentPeriodStart + + 3 * kEidSeedPeriod.InMilliseconds() - + kEidPeriod.InMilliseconds(), + nullptr), + data->current_data.data); EXPECT_FALSE(data->adjacent_data); } TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateBackgroundScanFilter_NoCurrentPeriodSeed) { - SetTestTime(kDefaultCurrentPeriodStart + 4 * kNumMsInEidSeedPeriod - 1); + SetTestTime(kDefaultCurrentPeriodStart + 4 * kEidSeedPeriod.InMilliseconds() - + 1); std::unique_ptr<ForegroundEidGenerator::EidData> data = eid_generator_->GenerateBackgroundScanFilter( @@ -341,14 +355,14 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, ForegroundEidGenerator::EidData::AdjacentDataType::PAST); EXPECT_EQ(kDefaultCurrentPeriodStart, data->current_data.start_timestamp_ms); - EXPECT_EQ(kDefaultCurrentPeriodStart + kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidPeriod.InMilliseconds(), data->current_data.end_timestamp_ms); // Since this uses the real RawEidGenerator, just make sure the data // exists and has the proper length. EXPECT_EQ((size_t)kNumBytesInEidValue, data->current_data.data.length()); ASSERT_TRUE(data->adjacent_data); - EXPECT_EQ(kDefaultCurrentPeriodStart - kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart - kEidPeriod.InMilliseconds(), data->adjacent_data->start_timestamp_ms); EXPECT_EQ(kDefaultCurrentPeriodStart, data->adjacent_data->end_timestamp_ms); // Since this uses the real RawEidGenerator, just make sure the data @@ -365,7 +379,7 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateAdvertisementData) { ASSERT_TRUE(data); EXPECT_EQ(kDefaultCurrentPeriodStart, data->start_timestamp_ms); - EXPECT_EQ(kDefaultCurrentPeriodStart + kNumMsInEidPeriod, + EXPECT_EQ(kDefaultCurrentPeriodStart + kEidPeriod.InMilliseconds(), data->end_timestamp_ms); EXPECT_EQ(GenerateFakeAdvertisement(kSecondSeed, kDefaultCurrentPeriodStart, kDefaultAdvertisingDevicePublicKey), @@ -374,7 +388,7 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateAdvertisementData) { TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateAdvertisementData_NoSeedForPeriod) { - SetTestTime(kDefaultCurrentTime + 4 * kNumMsInEidSeedPeriod); + SetTestTime(kDefaultCurrentTime + 4 * kEidSeedPeriod.InMilliseconds()); std::unique_ptr<DataWithTimestamp> data = eid_generator_->GenerateAdvertisement(kDefaultAdvertisingDevicePublicKey, @@ -384,7 +398,7 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, TEST_F(CryptAuthForegroundEidGeneratorTest, GenerateAdvertisementData_EmptySeeds) { - SetTestTime(kDefaultCurrentTime + 4 * kNumMsInEidSeedPeriod); + SetTestTime(kDefaultCurrentTime + 4 * kEidSeedPeriod.InMilliseconds()); std::vector<BeaconSeed> empty; std::unique_ptr<DataWithTimestamp> data = @@ -405,10 +419,11 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ(GenerateFakeAdvertisement(kSecondSeed, kDefaultCurrentPeriodStart, kDefaultAdvertisingDevicePublicKey), possible_advertisements[0]); - EXPECT_EQ(GenerateFakeAdvertisement( - kFirstSeed, kDefaultCurrentPeriodStart - kNumMsInEidPeriod, - kDefaultAdvertisingDevicePublicKey), - possible_advertisements[1]); + EXPECT_EQ( + GenerateFakeAdvertisement( + kFirstSeed, kDefaultCurrentPeriodStart - kEidPeriod.InMilliseconds(), + kDefaultAdvertisingDevicePublicKey), + possible_advertisements[1]); } TEST_F(CryptAuthForegroundEidGeneratorTest, @@ -424,15 +439,16 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ(GenerateFakeAdvertisement(kSecondSeed, kDefaultCurrentPeriodStart, kDefaultAdvertisingDevicePublicKey), possible_advertisements[0]); - EXPECT_EQ(GenerateFakeAdvertisement( - kSecondSeed, kDefaultCurrentPeriodStart + kNumMsInEidPeriod, - kDefaultAdvertisingDevicePublicKey), - possible_advertisements[1]); + EXPECT_EQ( + GenerateFakeAdvertisement( + kSecondSeed, kDefaultCurrentPeriodStart + kEidPeriod.InMilliseconds(), + kDefaultAdvertisingDevicePublicKey), + possible_advertisements[1]); } TEST_F(CryptAuthForegroundEidGeneratorTest, GeneratePossibleAdvertisements_OnlyCurrentPeriod) { - SetTestTime(kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod); + SetTestTime(kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds()); std::vector<std::string> possible_advertisements = eid_generator_->GeneratePossibleAdvertisements( @@ -440,15 +456,16 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ((size_t)1, possible_advertisements.size()); EXPECT_EQ(GenerateFakeAdvertisement( - kFirstSeed, kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod, + kFirstSeed, + kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds(), kDefaultAdvertisingDevicePublicKey), possible_advertisements[0]); } TEST_F(CryptAuthForegroundEidGeneratorTest, GeneratePossibleAdvertisements_OnlyFuturePeriod) { - SetTestTime(kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod - - kNumMsInEidPeriod); + SetTestTime(kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds() - + kEidPeriod.InMilliseconds()); std::vector<std::string> possible_advertisements = eid_generator_->GeneratePossibleAdvertisements( @@ -456,15 +473,16 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ((size_t)1, possible_advertisements.size()); EXPECT_EQ(GenerateFakeAdvertisement( - kFirstSeed, kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod, + kFirstSeed, + kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds(), kDefaultAdvertisingDevicePublicKey), possible_advertisements[0]); } TEST_F(CryptAuthForegroundEidGeneratorTest, GeneratePossibleAdvertisements_NoAdvertisements_SeedsTooFarInFuture) { - SetTestTime(kDefaultCurrentPeriodStart - kNumMsInEidSeedPeriod - - kNumMsInEidPeriod - 1); + SetTestTime(kDefaultCurrentPeriodStart - kEidSeedPeriod.InMilliseconds() - + kEidPeriod.InMilliseconds() - 1); std::vector<std::string> possible_advertisements = eid_generator_->GeneratePossibleAdvertisements( @@ -474,8 +492,8 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, TEST_F(CryptAuthForegroundEidGeneratorTest, GeneratePossibleAdvertisements_OnlyPastPeriod) { - SetTestTime(kDefaultCurrentPeriodStart + 3 * kNumMsInEidSeedPeriod + - kNumMsInEidPeriod); + SetTestTime(kDefaultCurrentPeriodStart + 3 * kEidSeedPeriod.InMilliseconds() + + kEidPeriod.InMilliseconds()); std::vector<std::string> possible_advertisements = eid_generator_->GeneratePossibleAdvertisements( @@ -484,16 +502,16 @@ TEST_F(CryptAuthForegroundEidGeneratorTest, EXPECT_EQ((size_t)1, possible_advertisements.size()); EXPECT_EQ(GenerateFakeAdvertisement(kFourthSeed, kDefaultCurrentPeriodStart + - 3 * kNumMsInEidSeedPeriod - - kNumMsInEidPeriod, + 3 * kEidSeedPeriod.InMilliseconds() - + kEidPeriod.InMilliseconds(), kDefaultAdvertisingDevicePublicKey), possible_advertisements[0]); } TEST_F(CryptAuthForegroundEidGeneratorTest, GeneratePossibleAdvertisements_NoAdvertisements_SeedsTooFarInPast) { - SetTestTime(kDefaultCurrentPeriodStart + 3 * kNumMsInEidSeedPeriod + - kNumMsInEidPeriod + 1); + SetTestTime(kDefaultCurrentPeriodStart + 3 * kEidSeedPeriod.InMilliseconds() + + kEidPeriod.InMilliseconds() + 1); std::vector<std::string> possible_advertisements = eid_generator_->GeneratePossibleAdvertisements( diff --git a/chromium/components/cryptauth/local_device_data_provider.cc b/chromium/components/cryptauth/local_device_data_provider.cc new file mode 100644 index 00000000000..f98cad8160f --- /dev/null +++ b/chromium/components/cryptauth/local_device_data_provider.cc @@ -0,0 +1,54 @@ +// Copyright 2015 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/cryptauth/local_device_data_provider.h" + +#include "components/cryptauth/cryptauth_device_manager.h" +#include "components/cryptauth/cryptauth_enrollment_manager.h" +#include "components/cryptauth/cryptauth_service.h" +#include "components/cryptauth/proto/cryptauth_api.pb.h" + +namespace cryptauth { + +LocalDeviceDataProvider::LocalDeviceDataProvider( + CryptAuthService* cryptauth_service) + : cryptauth_service_(cryptauth_service) {} + +LocalDeviceDataProvider::~LocalDeviceDataProvider() {} + +bool LocalDeviceDataProvider::GetLocalDeviceData( + std::string* public_key_out, + std::vector<BeaconSeed>* beacon_seeds_out) const { + DCHECK(public_key_out || beacon_seeds_out); + + std::string public_key = + cryptauth_service_->GetCryptAuthEnrollmentManager()->GetUserPublicKey(); + if (public_key.empty()) { + return false; + } + + std::vector<ExternalDeviceInfo> synced_devices = + cryptauth_service_->GetCryptAuthDeviceManager()->GetSyncedDevices(); + for (const auto& device : synced_devices) { + if (device.has_public_key() && device.public_key() == public_key && + device.beacon_seeds_size() > 0) { + if (public_key_out) { + public_key_out->assign(public_key); + } + + if (beacon_seeds_out) { + beacon_seeds_out->clear(); + for (int i = 0; i < device.beacon_seeds_size(); i++) { + beacon_seeds_out->push_back(device.beacon_seeds(i)); + } + } + + return true; + } + } + + return false; +} + +} // namespace cryptauth diff --git a/chromium/components/cryptauth/local_device_data_provider.h b/chromium/components/cryptauth/local_device_data_provider.h new file mode 100644 index 00000000000..927179d0dca --- /dev/null +++ b/chromium/components/cryptauth/local_device_data_provider.h @@ -0,0 +1,44 @@ +// Copyright 2016 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_CRYPTAUTH_LOCAL_DEVICE_DATA_PROVIDER_H_ +#define COMPONENTS_CRYPTAUTH_LOCAL_DEVICE_DATA_PROVIDER_H_ + +#include <memory> +#include <string> +#include <vector> + +#include "base/macros.h" +#include "base/memory/ptr_util.h" + +namespace cryptauth { + +class BeaconSeed; +class CryptAuthService; + +// Fetches CryptAuth data about the local device (i.e., the device on which this +// code is running) for the current user (i.e., the one which is logged-in). +class LocalDeviceDataProvider { + public: + explicit LocalDeviceDataProvider(CryptAuthService* cryptauth_service); + virtual ~LocalDeviceDataProvider(); + + // Fetches the public key and/or the beacon seeds for the local device. + // Returns whether the operation succeeded. If |nullptr| is passed as a + // parameter, the associated data will not be fetched. + virtual bool GetLocalDeviceData( + std::string* public_key_out, + std::vector<BeaconSeed>* beacon_seeds_out) const; + + private: + friend class LocalDeviceDataProviderTest; + + CryptAuthService* cryptauth_service_; + + DISALLOW_COPY_AND_ASSIGN(LocalDeviceDataProvider); +}; + +} // namespace cryptauth + +#endif // COMPONENTS_CRYPTAUTH_LOCAL_DEVICE_DATA_PROVIDER_H_ diff --git a/chromium/components/cryptauth/local_device_data_provider_unittest.cc b/chromium/components/cryptauth/local_device_data_provider_unittest.cc new file mode 100644 index 00000000000..9259c5d0c54 --- /dev/null +++ b/chromium/components/cryptauth/local_device_data_provider_unittest.cc @@ -0,0 +1,236 @@ +// Copyright 2016 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/cryptauth/local_device_data_provider.h" + +#include <string> +#include <vector> + +#include "base/logging.h" +#include "base/memory/ptr_util.h" +#include "components/cryptauth/cryptauth_device_manager.h" +#include "components/cryptauth/cryptauth_enroller.h" +#include "components/cryptauth/cryptauth_enrollment_manager.h" +#include "components/cryptauth/fake_cryptauth_gcm_manager.h" +#include "components/cryptauth/fake_cryptauth_service.h" +#include "components/cryptauth/proto/cryptauth_api.pb.h" +#include "components/cryptauth/secure_message_delegate.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::NiceMock; +using testing::Return; + +namespace cryptauth { + +namespace { + +const char kDefaultPublicKey[] = "publicKey"; + +const char kBeaconSeed1Data[] = "beaconSeed1Data"; +const int64_t kBeaconSeed1StartMs = 1000L; +const int64_t kBeaconSeed1EndMs = 2000L; + +const char kBeaconSeed2Data[] = "beaconSeed2Data"; +const int64_t kBeaconSeed2StartMs = 2000L; +const int64_t kBeaconSeed2EndMs = 3000L; + +class MockCryptAuthDeviceManager : public CryptAuthDeviceManager { + public: + MockCryptAuthDeviceManager() {} + ~MockCryptAuthDeviceManager() override {} + + MOCK_CONST_METHOD0(GetSyncedDevices, std::vector<ExternalDeviceInfo>()); +}; + +class MockCryptAuthEnrollmentManager : public CryptAuthEnrollmentManager { + public: + explicit MockCryptAuthEnrollmentManager( + FakeCryptAuthGCMManager* fake_cryptauth_gcm_manager) + : CryptAuthEnrollmentManager(nullptr /* clock */, + nullptr /* enroller_factory */, + nullptr /* secure_message_delegate */, + GcmDeviceInfo(), + fake_cryptauth_gcm_manager, + nullptr /* pref_service */) {} + ~MockCryptAuthEnrollmentManager() override {} + + MOCK_CONST_METHOD0(GetUserPublicKey, std::string()); +}; + +BeaconSeed CreateBeaconSeed(const std::string& data, + int64_t start_ms, + int64_t end_ms) { + BeaconSeed seed; + seed.set_data(data); + seed.set_start_time_millis(start_ms); + seed.set_end_time_millis(end_ms); + return seed; +} + +} // namespace + +class LocalDeviceDataProviderTest : public testing::Test { + protected: + LocalDeviceDataProviderTest() { + fake_beacon_seeds_.push_back(CreateBeaconSeed( + kBeaconSeed1Data, kBeaconSeed1StartMs, kBeaconSeed1EndMs)); + fake_beacon_seeds_.push_back(CreateBeaconSeed( + kBeaconSeed2Data, kBeaconSeed2StartMs, kBeaconSeed2EndMs)); + + // Has no public key and no BeaconSeeds. + ExternalDeviceInfo synced_device1; + fake_synced_devices_.push_back(synced_device1); + + // Has no public key and some BeaconSeeds. + ExternalDeviceInfo synced_device2; + synced_device2.add_beacon_seeds()->CopyFrom(CreateBeaconSeed( + kBeaconSeed1Data, kBeaconSeed1StartMs, kBeaconSeed1EndMs)); + synced_device2.add_beacon_seeds()->CopyFrom(CreateBeaconSeed( + kBeaconSeed2Data, kBeaconSeed2StartMs, kBeaconSeed2EndMs)); + fake_synced_devices_.push_back(synced_device2); + + // Has another different public key and no BeaconSeeds. + ExternalDeviceInfo synced_device3; + synced_device3.set_public_key("anotherPublicKey"); + fake_synced_devices_.push_back(synced_device3); + + // Has different public key and BeaconSeeds. + ExternalDeviceInfo synced_device4; + synced_device4.set_public_key("otherPublicKey"); + synced_device4.add_beacon_seeds()->CopyFrom(CreateBeaconSeed( + kBeaconSeed1Data, kBeaconSeed1StartMs, kBeaconSeed1EndMs)); + synced_device4.add_beacon_seeds()->CopyFrom(CreateBeaconSeed( + kBeaconSeed2Data, kBeaconSeed2StartMs, kBeaconSeed2EndMs)); + fake_synced_devices_.push_back(synced_device4); + + // Has public key but no BeaconSeeds. + ExternalDeviceInfo synced_device5; + synced_device5.set_public_key(kDefaultPublicKey); + fake_synced_devices_.push_back(synced_device5); + + // Has public key and BeaconSeeds. + ExternalDeviceInfo synced_device6; + synced_device6.set_public_key(kDefaultPublicKey); + synced_device6.add_beacon_seeds()->CopyFrom(CreateBeaconSeed( + kBeaconSeed1Data, kBeaconSeed1StartMs, kBeaconSeed1EndMs)); + synced_device6.add_beacon_seeds()->CopyFrom(CreateBeaconSeed( + kBeaconSeed2Data, kBeaconSeed2StartMs, kBeaconSeed2EndMs)); + fake_synced_devices_.push_back(synced_device6); + } + + void SetUp() override { + mock_device_manager_ = + base::WrapUnique(new NiceMock<MockCryptAuthDeviceManager>()); + fake_cryptauth_gcm_manager_ = + base::MakeUnique<FakeCryptAuthGCMManager>("registrationId"); + mock_enrollment_manager_ = + base::WrapUnique(new NiceMock<MockCryptAuthEnrollmentManager>( + fake_cryptauth_gcm_manager_.get())); + + fake_cryptauth_service_ = base::MakeUnique<FakeCryptAuthService>(); + fake_cryptauth_service_->set_cryptauth_device_manager( + mock_device_manager_.get()); + fake_cryptauth_service_->set_cryptauth_enrollment_manager( + mock_enrollment_manager_.get()); + + provider_ = base::WrapUnique( + new LocalDeviceDataProvider(fake_cryptauth_service_.get())); + } + + std::vector<BeaconSeed> fake_beacon_seeds_; + std::vector<ExternalDeviceInfo> fake_synced_devices_; + + std::unique_ptr<FakeCryptAuthGCMManager> fake_cryptauth_gcm_manager_; + std::unique_ptr<NiceMock<MockCryptAuthDeviceManager>> mock_device_manager_; + std::unique_ptr<NiceMock<MockCryptAuthEnrollmentManager>> + mock_enrollment_manager_; + std::unique_ptr<FakeCryptAuthService> fake_cryptauth_service_; + + std::unique_ptr<LocalDeviceDataProvider> provider_; + + private: + DISALLOW_COPY_AND_ASSIGN(LocalDeviceDataProviderTest); +}; + +TEST_F(LocalDeviceDataProviderTest, TestGetLocalDeviceData_NoPublicKey) { + ON_CALL(*mock_enrollment_manager_, GetUserPublicKey()) + .WillByDefault(Return(std::string())); + ON_CALL(*mock_device_manager_, GetSyncedDevices()) + .WillByDefault(Return(fake_synced_devices_)); + + std::string public_key; + std::vector<BeaconSeed> beacon_seeds; + + EXPECT_FALSE(provider_->GetLocalDeviceData(&public_key, &beacon_seeds)); +} + +TEST_F(LocalDeviceDataProviderTest, TestGetLocalDeviceData_NoSyncedDevices) { + ON_CALL(*mock_enrollment_manager_, GetUserPublicKey()) + .WillByDefault(Return(kDefaultPublicKey)); + ON_CALL(*mock_device_manager_, GetSyncedDevices()) + .WillByDefault(Return(std::vector<ExternalDeviceInfo>())); + + std::string public_key; + std::vector<BeaconSeed> beacon_seeds; + + EXPECT_FALSE(provider_->GetLocalDeviceData(&public_key, &beacon_seeds)); +} + +TEST_F(LocalDeviceDataProviderTest, + TestGetLocalDeviceData_NoSyncedDeviceMatchingPublicKey) { + ON_CALL(*mock_enrollment_manager_, GetUserPublicKey()) + .WillByDefault(Return(kDefaultPublicKey)); + ON_CALL(*mock_device_manager_, GetSyncedDevices()) + .WillByDefault(Return(std::vector<ExternalDeviceInfo>{ + fake_synced_devices_[0], fake_synced_devices_[1], + fake_synced_devices_[2], fake_synced_devices_[3]})); + + std::string public_key; + std::vector<BeaconSeed> beacon_seeds; + + EXPECT_FALSE(provider_->GetLocalDeviceData(&public_key, &beacon_seeds)); +} + +TEST_F(LocalDeviceDataProviderTest, + TestGetLocalDeviceData_SyncedDeviceIncludesPublicKeyButNoBeaconSeeds) { + ON_CALL(*mock_enrollment_manager_, GetUserPublicKey()) + .WillByDefault(Return(kDefaultPublicKey)); + ON_CALL(*mock_device_manager_, GetSyncedDevices()) + .WillByDefault(Return(std::vector<ExternalDeviceInfo>{ + fake_synced_devices_[4], + })); + + std::string public_key; + std::vector<BeaconSeed> beacon_seeds; + + EXPECT_FALSE(provider_->GetLocalDeviceData(&public_key, &beacon_seeds)); +} + +TEST_F(LocalDeviceDataProviderTest, TestGetLocalDeviceData_Success) { + ON_CALL(*mock_enrollment_manager_, GetUserPublicKey()) + .WillByDefault(Return(kDefaultPublicKey)); + ON_CALL(*mock_device_manager_, GetSyncedDevices()) + .WillByDefault(Return(fake_synced_devices_)); + + std::string public_key; + std::vector<BeaconSeed> beacon_seeds; + + EXPECT_TRUE(provider_->GetLocalDeviceData(&public_key, &beacon_seeds)); + + EXPECT_EQ(kDefaultPublicKey, public_key); + + ASSERT_EQ(fake_beacon_seeds_.size(), beacon_seeds.size()); + for (size_t i = 0; i < fake_beacon_seeds_.size(); i++) { + // Note: google::protobuf::util::MessageDifferencer can only be used to diff + // Message, but BeaconSeed derives from the incompatible MessageLite class. + BeaconSeed expected = fake_beacon_seeds_[i]; + BeaconSeed actual = beacon_seeds[i]; + EXPECT_EQ(expected.data(), actual.data()); + EXPECT_EQ(expected.start_time_millis(), actual.start_time_millis()); + EXPECT_EQ(expected.end_time_millis(), actual.end_time_millis()); + } +} + +} // namespace cryptauth diff --git a/chromium/components/cryptauth/mock_cryptauth_client.h b/chromium/components/cryptauth/mock_cryptauth_client.h index ddc40e90537..d4df464043f 100644 --- a/chromium/components/cryptauth/mock_cryptauth_client.h +++ b/chromium/components/cryptauth/mock_cryptauth_client.h @@ -19,18 +19,26 @@ class MockCryptAuthClient : public CryptAuthClient { ~MockCryptAuthClient() override; // CryptAuthClient: - MOCK_METHOD3(GetMyDevices, + MOCK_METHOD4(GetMyDevices, void(const GetMyDevicesRequest& request, const GetMyDevicesCallback& callback, - const ErrorCallback& error_callback)); + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation)); MOCK_METHOD3(FindEligibleUnlockDevices, void(const FindEligibleUnlockDevicesRequest& request, const FindEligibleUnlockDevicesCallback& callback, const ErrorCallback& error_callback)); - MOCK_METHOD3(SendDeviceSyncTickle, + MOCK_METHOD3(FindEligibleForPromotion, + void(const FindEligibleForPromotionRequest& request, + const FindEligibleForPromotionCallback& callback, + const ErrorCallback& error_callback)); + MOCK_METHOD4(SendDeviceSyncTickle, void(const SendDeviceSyncTickleRequest& request, const SendDeviceSyncTickleCallback& callback, - const ErrorCallback& error_callback)); + const ErrorCallback& error_callback, + const net::PartialNetworkTrafficAnnotationTag& + partial_traffic_annotation)); MOCK_METHOD3(ToggleEasyUnlock, void(const ToggleEasyUnlockRequest& request, const ToggleEasyUnlockCallback& callback, diff --git a/chromium/components/cryptauth/mock_local_device_data_provider.cc b/chromium/components/cryptauth/mock_local_device_data_provider.cc new file mode 100644 index 00000000000..be56d0b9ad4 --- /dev/null +++ b/chromium/components/cryptauth/mock_local_device_data_provider.cc @@ -0,0 +1,54 @@ +// Copyright 2015 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/cryptauth/mock_local_device_data_provider.h" + +#include "components/cryptauth/cryptauth_device_manager.h" +#include "components/cryptauth/cryptauth_enrollment_manager.h" +#include "components/cryptauth/proto/cryptauth_api.pb.h" + +namespace cryptauth { + +MockLocalDeviceDataProvider::MockLocalDeviceDataProvider() + : LocalDeviceDataProvider(nullptr /* cryptauth_service */) {} + +MockLocalDeviceDataProvider::~MockLocalDeviceDataProvider() {} + +void MockLocalDeviceDataProvider::SetPublicKey( + std::unique_ptr<std::string> public_key) { + if (public_key) { + public_key_ = std::move(public_key); + } else { + public_key_.reset(); + } +} + +void MockLocalDeviceDataProvider::SetBeaconSeeds( + std::unique_ptr<std::vector<BeaconSeed>> beacon_seeds) { + if (beacon_seeds) { + beacon_seeds_ = std::move(beacon_seeds); + } else { + beacon_seeds_.reset(); + } +} + +bool MockLocalDeviceDataProvider::GetLocalDeviceData( + std::string* public_key_out, + std::vector<BeaconSeed>* beacon_seeds_out) const { + bool success = false; + + if (public_key_ && public_key_out) { + *public_key_out = *public_key_; + success = true; + } + + if (beacon_seeds_ && beacon_seeds_out) { + *beacon_seeds_out = *beacon_seeds_; + success = true; + } + + return success; +} + +} // namespace cryptauth diff --git a/chromium/components/cryptauth/mock_local_device_data_provider.h b/chromium/components/cryptauth/mock_local_device_data_provider.h new file mode 100644 index 00000000000..563e1e866b3 --- /dev/null +++ b/chromium/components/cryptauth/mock_local_device_data_provider.h @@ -0,0 +1,43 @@ +// Copyright 2016 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_CRYPTAUTH_MOCK_LOCAL_DEVICE_DATA_PROVIDER_H_ +#define COMPONENTS_CRYPTAUTH_MOCK_LOCAL_DEVICE_DATA_PROVIDER_H_ + +#include <memory> +#include <string> +#include <vector> + +#include "base/macros.h" +#include "base/memory/ptr_util.h" +#include "components/cryptauth/local_device_data_provider.h" + +namespace cryptauth { + +class BeaconSeed; + +// Test double for LocalDeviceDataProvider. +class MockLocalDeviceDataProvider : public LocalDeviceDataProvider { + public: + MockLocalDeviceDataProvider(); + ~MockLocalDeviceDataProvider() override; + + void SetPublicKey(std::unique_ptr<std::string> public_key); + void SetBeaconSeeds(std::unique_ptr<std::vector<BeaconSeed>> beacon_seeds); + + // LocalDeviceDataProvider: + bool GetLocalDeviceData( + std::string* public_key_out, + std::vector<BeaconSeed>* beacon_seeds_out) const override; + + private: + std::unique_ptr<std::string> public_key_; + std::unique_ptr<std::vector<BeaconSeed>> beacon_seeds_; + + DISALLOW_COPY_AND_ASSIGN(MockLocalDeviceDataProvider); +}; + +} // namespace cryptauth + +#endif // COMPONENTS_CRYPTAUTH_MOCK_LOCAL_DEVICE_DATA_PROVIDER_H_ diff --git a/chromium/components/cryptauth/proto/cryptauth_api.proto b/chromium/components/cryptauth/proto/cryptauth_api.proto index 6e2be7e2956..5672f239674 100644 --- a/chromium/components/cryptauth/proto/cryptauth_api.proto +++ b/chromium/components/cryptauth/proto/cryptauth_api.proto @@ -81,6 +81,12 @@ message ExternalDeviceInfo { // A list of seeds for EID BLE advertisements targeting this device. repeated BeaconSeed beacon_seeds = 9; + + // Whether this device is an ARC++ device enrolled via gmscore. + optional bool arc_plus_plus = 10 [default = false]; + + // Whether this is a "Pixel Experience" phone. + optional bool pixel_phone = 11 [default = false]; } // Determine if the calling device is allowed to promote the SmartLock diff --git a/chromium/components/cryptauth/remote_device.cc b/chromium/components/cryptauth/remote_device.cc index e3d26d0b646..b9556956e57 100644 --- a/chromium/components/cryptauth/remote_device.cc +++ b/chromium/components/cryptauth/remote_device.cc @@ -45,8 +45,7 @@ RemoteDevice::RemoteDevice(const std::string& user_id, public_key(public_key), bluetooth_address(bluetooth_address), persistent_symmetric_key(persistent_symmetric_key), - sign_in_challenge(sign_in_challenge), - are_beacon_seeds_loaded(false) {} + sign_in_challenge(sign_in_challenge) {} RemoteDevice::RemoteDevice(const RemoteDevice& other) = default; diff --git a/chromium/components/cryptauth/remote_device.h b/chromium/components/cryptauth/remote_device.h index cd4085f8d21..7662b1a38c7 100644 --- a/chromium/components/cryptauth/remote_device.h +++ b/chromium/components/cryptauth/remote_device.h @@ -23,7 +23,7 @@ struct RemoteDevice { // Note: To save space, the BeaconSeeds may not necessarily be included in // this object. - bool are_beacon_seeds_loaded; + bool are_beacon_seeds_loaded = false; std::vector<BeaconSeed> beacon_seeds; RemoteDevice(); diff --git a/chromium/components/cryptauth/secure_channel.cc b/chromium/components/cryptauth/secure_channel.cc index 9048f4d4f8b..068bdf02014 100644 --- a/chromium/components/cryptauth/secure_channel.cc +++ b/chromium/components/cryptauth/secure_channel.cc @@ -56,11 +56,10 @@ std::string SecureChannel::StatusToString(const Status& status) { } } -SecureChannel::PendingMessage::PendingMessage() {} - -SecureChannel::PendingMessage::PendingMessage( - const std::string& feature, const std::string& payload) - : feature(feature), payload(payload) {} +SecureChannel::PendingMessage::PendingMessage(const std::string& feature, + const std::string& payload, + int sequence_number) + : feature(feature), payload(payload), sequence_number(sequence_number) {} SecureChannel::PendingMessage::~PendingMessage() {} @@ -70,11 +69,6 @@ SecureChannel::SecureChannel(std::unique_ptr<Connection> connection, connection_(std::move(connection)), cryptauth_service_(cryptauth_service), weak_ptr_factory_(this) { - DCHECK(connection_); - DCHECK(!connection_->IsConnected()); - DCHECK(!connection_->remote_device().user_id.empty()); - DCHECK(cryptauth_service); - connection_->AddObserver(this); } @@ -88,22 +82,27 @@ void SecureChannel::Initialize() { TransitionToStatus(Status::CONNECTING); } -void SecureChannel::SendMessage( - const std::string& feature, const std::string& payload) { +int SecureChannel::SendMessage(const std::string& feature, + const std::string& payload) { DCHECK(status_ == Status::AUTHENTICATED); - PA_LOG(INFO) << "Queuing new message to send: {" - << "feature: \"" << feature << "\", " - << "payload: \"" << payload << "\"" - << "}"; + int sequence_number = next_sequence_number_; + next_sequence_number_++; - queued_messages_.push_back(PendingMessage(feature, payload)); + queued_messages_.emplace( + base::MakeUnique<PendingMessage>(feature, payload, sequence_number)); ProcessMessageQueue(); + + return sequence_number; } void SecureChannel::Disconnect() { if (connection_->IsConnected()) { + // If |connection_| is active, calling Disconnect() will eventually cause + // its status to transition to DISCONNECTED, which will in turn cause this + // class to transition to DISCONNECTED. connection_->Disconnect(); + return; } TransitionToStatus(Status::DISCONNECTED); @@ -148,6 +147,13 @@ void SecureChannel::OnMessageReceived(const Connection& connection, return; } + if (!secure_context_) { + PA_LOG(WARNING) << "Received unexpected message before authentication " + << "was complete. Feature: " << wire_message.feature() + << ", Payload: " << wire_message.payload(); + return; + } + secure_context_->Decode(wire_message.payload(), base::Bind(&SecureChannel::OnMessageDecoded, weak_ptr_factory_.GetWeakPtr(), @@ -157,11 +163,40 @@ void SecureChannel::OnMessageReceived(const Connection& connection, void SecureChannel::OnSendCompleted(const cryptauth::Connection& connection, const cryptauth::WireMessage& wire_message, bool success) { - DCHECK(pending_message_->feature == wire_message.feature()); + if (wire_message.feature() == Authenticator::kAuthenticationFeature) { + // No need to process authentication messages; these are handled by + // |authenticator_|. + return; + } + + if (!pending_message_) { + PA_LOG(ERROR) << "OnSendCompleted(), but a send was not expected to be in " + << "progress. Disconnecting from " + << connection_->GetDeviceAddress(); + Disconnect(); + return; + } if (success && status_ != Status::DISCONNECTED) { pending_message_.reset(); - ProcessMessageQueue(); + + // Create a WeakPtr to |this| before invoking observer callbacks. It is + // possible that an Observer will respond to the OnMessageSent() call by + // destroying the connection (e.g., if the client only wanted to send one + // message and destroyed the connection after the message was sent). + base::WeakPtr<SecureChannel> weak_this = weak_ptr_factory_.GetWeakPtr(); + + if (wire_message.sequence_number() != -1) { + for (auto& observer : observer_list_) + observer.OnMessageSent(this, wire_message.sequence_number()); + } + + // Process the next message if possible. Note that if the SecureChannel was + // deleted by the OnMessageSent() callback, this will be a no-op since + // |weak_this| will have been invalidated in that case. + if (weak_this.get()) + weak_this->ProcessMessageQueue(); + return; } @@ -183,16 +218,11 @@ void SecureChannel::TransitionToStatus(const Status& new_status) { return; } - PA_LOG(INFO) << "Connection status changed: " - << StatusToString(status_) - << " => " << StatusToString(new_status); - Status old_status = status_; status_ = new_status; - for (auto& observer : observer_list_) { + for (auto& observer : observer_list_) observer.OnSecureChannelStatusChanged(this, old_status, status_); - } } void SecureChannel::Authenticate() { @@ -216,36 +246,39 @@ void SecureChannel::ProcessMessageQueue() { DCHECK(!connection_->is_sending_message()); - pending_message_.reset(new PendingMessage(queued_messages_.front())); - queued_messages_.pop_front(); + pending_message_ = std::move(queued_messages_.front()); + queued_messages_.pop(); - PA_LOG(INFO) << "Sending message: {" + PA_LOG(INFO) << "Sending message to " << connection_->GetDeviceAddress() + << ": {" << "feature: \"" << pending_message_->feature << "\", " << "payload: \"" << pending_message_->payload << "\"" << "}"; - secure_context_->Encode(pending_message_->payload, - base::Bind(&SecureChannel::OnMessageEncoded, - weak_ptr_factory_.GetWeakPtr(), - pending_message_->feature)); + secure_context_->Encode( + pending_message_->payload, + base::Bind(&SecureChannel::OnMessageEncoded, + weak_ptr_factory_.GetWeakPtr(), pending_message_->feature, + pending_message_->sequence_number)); } -void SecureChannel::OnMessageEncoded( - const std::string& feature, const std::string& encoded_message) { +void SecureChannel::OnMessageEncoded(const std::string& feature, + int sequence_number, + const std::string& encoded_message) { connection_->SendMessage(base::MakeUnique<cryptauth::WireMessage>( - encoded_message, feature)); + encoded_message, feature, sequence_number)); } void SecureChannel::OnMessageDecoded( const std::string& feature, const std::string& decoded_message) { - PA_LOG(INFO) << "Received message: {" + PA_LOG(INFO) << "Received message from " << connection_->GetDeviceAddress() + << ": {" << "feature: \"" << feature << "\", " << "payload: \"" << decoded_message << "\"" << "}"; - for (auto& observer : observer_list_) { + for (auto& observer : observer_list_) observer.OnMessageReceived(this, feature, decoded_message); - } } void SecureChannel::OnAuthenticationResult( diff --git a/chromium/components/cryptauth/secure_channel.h b/chromium/components/cryptauth/secure_channel.h index e0d83b2f853..289500bd75c 100644 --- a/chromium/components/cryptauth/secure_channel.h +++ b/chromium/components/cryptauth/secure_channel.h @@ -5,7 +5,7 @@ #ifndef COMPONENTS_CRYPTAUTH_SECURE_CHANNEL_H_ #define COMPONENTS_CRYPTAUTH_SECURE_CHANNEL_H_ -#include <deque> +#include <queue> #include "base/macros.h" #include "base/memory/weak_ptr.h" @@ -59,6 +59,11 @@ class SecureChannel : public ConnectionObserver { SecureChannel* secure_channel, const std::string& feature, const std::string& payload) = 0; + + // Called when a message has been sent successfully; |sequence_number| + // corresponds to the value returned by an earlier call to SendMessage(). + virtual void OnMessageSent(SecureChannel* secure_channel, + int sequence_number) {} }; class Factory { @@ -82,8 +87,11 @@ class SecureChannel : public ConnectionObserver { virtual void Initialize(); - virtual void SendMessage(const std::string& feature, - const std::string& payload); + // Sends a message over the connection and returns a sequence number. If the + // message is successfully sent, observers will be notified that the message + // has been sent and will be provided this sequence number. + virtual int SendMessage(const std::string& feature, + const std::string& payload); virtual void Disconnect(); @@ -111,23 +119,28 @@ class SecureChannel : public ConnectionObserver { Status status_; private: + friend class CryptAuthSecureChannelTest; + // Message waiting to be sent. Note that this is *not* the message that will // end up being sent over the wire; before that can be done, the payload must // be encrypted. struct PendingMessage { - PendingMessage(); - PendingMessage(const std::string& feature, const std::string& payload); + PendingMessage(const std::string& feature, + const std::string& payload, + int sequence_number); virtual ~PendingMessage(); const std::string feature; const std::string payload; + const int sequence_number; }; void TransitionToStatus(const Status& new_status); void Authenticate(); void ProcessMessageQueue(); - void OnMessageEncoded( - const std::string& feature, const std::string& encoded_message); + void OnMessageEncoded(const std::string& feature, + int sequence_number, + const std::string& encoded_message); void OnMessageDecoded( const std::string& feature, const std::string& decoded_message); void OnAuthenticationResult( @@ -138,8 +151,9 @@ class SecureChannel : public ConnectionObserver { CryptAuthService* cryptauth_service_; // Outlives this instance. std::unique_ptr<Authenticator> authenticator_; std::unique_ptr<SecureContext> secure_context_; - std::deque<PendingMessage> queued_messages_; + std::queue<std::unique_ptr<PendingMessage>> queued_messages_; std::unique_ptr<PendingMessage> pending_message_; + int next_sequence_number_ = 0; base::ObserverList<Observer> observer_list_; base::WeakPtrFactory<SecureChannel> weak_ptr_factory_; diff --git a/chromium/components/cryptauth/secure_channel_unittest.cc b/chromium/components/cryptauth/secure_channel_unittest.cc index f894110238c..97156a48374 100644 --- a/chromium/components/cryptauth/secure_channel_unittest.cc +++ b/chromium/components/cryptauth/secure_channel_unittest.cc @@ -64,18 +64,57 @@ class TestObserver : public SecureChannel::Observer { received_messages_.push_back(ReceivedMessage(feature, payload)); } - std::vector<SecureChannelStatusChange>& connection_status_changes() { + void OnMessageSent(SecureChannel* secure_channel, + int sequence_number) override { + DCHECK(secure_channel == secure_channel_); + sent_sequence_numbers_.push_back(sequence_number); + } + + const std::vector<SecureChannelStatusChange>& connection_status_changes() { return connection_status_changes_; } - std::vector<ReceivedMessage>& received_messages() { + const std::vector<ReceivedMessage>& received_messages() { return received_messages_; } + const std::vector<int>& sent_sequence_numbers() { + return sent_sequence_numbers_; + } + private: SecureChannel* secure_channel_; std::vector<SecureChannelStatusChange> connection_status_changes_; std::vector<ReceivedMessage> received_messages_; + std::vector<int> sent_sequence_numbers_; +}; + +// Observer used in the ObserverDeletesChannel test. This Observer deletes the +// SecureChannel when it receives an OnMessageSent() call. +class DeletingObserver : public SecureChannel::Observer { + public: + DeletingObserver(std::unique_ptr<SecureChannel>* secure_channel) + : secure_channel_(secure_channel) {} + + // SecureChannel::Observer: + void OnSecureChannelStatusChanged( + SecureChannel* secure_channel, + const SecureChannel::Status& old_status, + const SecureChannel::Status& new_status) override {} + + void OnMessageReceived(SecureChannel* secure_channel, + const std::string& feature, + const std::string& payload) override {} + + void OnMessageSent(SecureChannel* secure_channel, + int sequence_number) override { + DCHECK(secure_channel == secure_channel_->get()); + // Delete the channel when an OnMessageSent() call occurs. + secure_channel_->reset(); + } + + private: + std::unique_ptr<SecureChannel>* secure_channel_; }; class TestAuthenticatorFactory : public DeviceToDeviceAuthenticator::Factory { @@ -105,13 +144,6 @@ RemoteDevice CreateTestRemoteDevice() { return remote_device; } -class TestSecureChannel : public SecureChannel { - public: - TestSecureChannel(std::unique_ptr<Connection> connection, - CryptAuthService* cryptauth_service) - : SecureChannel(std::move(connection), cryptauth_service) {} -}; - } // namespace class CryptAuthSecureChannelTest : public testing::Test { @@ -133,8 +165,8 @@ class CryptAuthSecureChannelTest : public testing::Test { new FakeConnection(test_device_, /* should_auto_connect */ false); EXPECT_FALSE(fake_connection_->observers().size()); - secure_channel_ = base::MakeUnique<TestSecureChannel>( - base::WrapUnique(fake_connection_), fake_cryptauth_service_.get()); + secure_channel_ = base::WrapUnique(new SecureChannel( + base::WrapUnique(fake_connection_), fake_cryptauth_service_.get())); EXPECT_EQ(static_cast<size_t>(1), fake_connection_->observers().size()); EXPECT_EQ(secure_channel_.get(), fake_connection_->observers()[0]); @@ -151,7 +183,8 @@ class CryptAuthSecureChannelTest : public testing::Test { VerifyReceivedMessages(std::vector<ReceivedMessage>()); // Same with messages being sent. - VerifyNoMessageBeingSent(); + if (secure_channel_) + VerifyNoMessageBeingSent(); } void VerifyConnectionStateChanges( @@ -243,16 +276,33 @@ class CryptAuthSecureChannelTest : public testing::Test { }); } - void StartSendingMessage( - const std::string& feature, const std::string& payload) { - secure_channel_->SendMessage(feature, payload); + // Starts sending the message and returns the sequence number. + int StartSendingMessage(const std::string& feature, + const std::string& payload) { + int sequence_number = secure_channel_->SendMessage(feature, payload); VerifyMessageBeingSent(feature, payload); + return sequence_number; + } + + void FinishSendingMessage(int sequence_number, bool success) { + std::vector<int> sent_sequence_numbers_before_send = + test_observer_->sent_sequence_numbers(); + + fake_connection_->FinishSendingMessageWithSuccess(success); + + if (success) { + std::vector<int> sent_sequence_numbers_after_send = + test_observer_->sent_sequence_numbers(); + EXPECT_EQ(sent_sequence_numbers_before_send.size() + 1u, + sent_sequence_numbers_after_send.size()); + EXPECT_EQ(sequence_number, sent_sequence_numbers_after_send.back()); + } } void StartAndFinishSendingMessage( const std::string& feature, const std::string& payload, bool success) { - StartSendingMessage(feature, payload); - fake_connection_->FinishSendingMessageWithSuccess(success); + int sequence_number = StartSendingMessage(feature, payload); + FinishSendingMessage(sequence_number, success); } void VerifyNoMessageBeingSent() { @@ -402,9 +452,33 @@ TEST_F(CryptAuthSecureChannelTest, AuthenticationFails_Failure) { }); } +// Regression test for crbug.com/765810. This test ensures that a crash does not +// occur if an unexpected message is received before authentication is complete. +TEST_F(CryptAuthSecureChannelTest, ReceiveMessageBeforeAuth) { + secure_channel_->Initialize(); + VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>{ + {SecureChannel::Status::DISCONNECTED, + SecureChannel::Status::CONNECTING}}); + + fake_connection_->CompleteInProgressConnection(/* success */ true); + VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>{ + {SecureChannel::Status::CONNECTING, SecureChannel::Status::CONNECTED}, + {SecureChannel::Status::CONNECTED, + SecureChannel::Status::AUTHENTICATING}}); + + // Receive an unexpected message (i.e., a non-auth message). + fake_connection_->ReceiveMessage("feature", "payload, but encoded"); + + // Still should be able to finish authentication. + AuthenticateSuccessfully(); + VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>{ + {SecureChannel::Status::AUTHENTICATING, + SecureChannel::Status::AUTHENTICATED}}); +} + TEST_F(CryptAuthSecureChannelTest, SendMessage_DisconnectWhileSending) { ConnectAndAuthenticate(); - StartSendingMessage("feature", "payload"); + int sequence_number = StartSendingMessage("feature", "payload"); fake_connection_->Disconnect(); VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange> { @@ -414,7 +488,7 @@ TEST_F(CryptAuthSecureChannelTest, SendMessage_DisconnectWhileSending) { } }); - fake_connection_->FinishSendingMessageWithSuccess(false); + FinishSendingMessage(sequence_number, false); // No further state change should have occurred. VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>()); } @@ -461,32 +535,32 @@ TEST_F(CryptAuthSecureChannelTest, SendMessage_MultipleMessages_Success) { ConnectAndAuthenticate(); // Send a second message before the first has completed. - secure_channel_->SendMessage("feature1", "payload1"); - secure_channel_->SendMessage("feature2", "payload2"); + int sequence_number1 = secure_channel_->SendMessage("feature1", "payload1"); + int sequence_number2 = secure_channel_->SendMessage("feature2", "payload2"); // The first message should still be sending. VerifyMessageBeingSent("feature1", "payload1"); // Send the first message. - fake_connection_->FinishSendingMessageWithSuccess(true); + FinishSendingMessage(sequence_number1, true); // Now, the second message should be sending. VerifyMessageBeingSent("feature2", "payload2"); - fake_connection_->FinishSendingMessageWithSuccess(true); + FinishSendingMessage(sequence_number2, true); } TEST_F(CryptAuthSecureChannelTest, SendMessage_MultipleMessages_FirstFails) { ConnectAndAuthenticate(); // Send a second message before the first has completed. - secure_channel_->SendMessage("feature1", "payload1"); + int sequence_number1 = secure_channel_->SendMessage("feature1", "payload1"); secure_channel_->SendMessage("feature2", "payload2"); // The first message should still be sending. VerifyMessageBeingSent("feature1", "payload1"); // Fail sending the first message. - fake_connection_->FinishSendingMessageWithSuccess(false); + FinishSendingMessage(sequence_number1, false); // The connection should have become disconnected. VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange> { @@ -531,4 +605,20 @@ TEST_F(CryptAuthSecureChannelTest, SendAndReceiveMessages) { }); } +TEST_F(CryptAuthSecureChannelTest, ObserverDeletesChannel) { + // Add a special Observer which deletes |secure_channel_| once it receives an + // OnMessageSent() call. + std::unique_ptr<DeletingObserver> deleting_observer = + base::WrapUnique(new DeletingObserver(&secure_channel_)); + secure_channel_->AddObserver(deleting_observer.get()); + + ConnectAndAuthenticate(); + + // Send a message successfully; this triggers an OnMessageSent() call which + // deletes the channel. Note that this would have caused a crash before the + // fix for crbug.com/751884. + StartAndFinishSendingMessage("feature", "request1", /* success */ true); + EXPECT_FALSE(secure_channel_); +} + } // namespace cryptauth diff --git a/chromium/components/cryptauth/wire_message.cc b/chromium/components/cryptauth/wire_message.cc index 0e29872e58d..ae52678dcc8 100644 --- a/chromium/components/cryptauth/wire_message.cc +++ b/chromium/components/cryptauth/wire_message.cc @@ -181,9 +181,20 @@ std::string WireMessage::Serialize() const { return header_string + json_body; } +WireMessage::WireMessage(const std::string& payload, + const std::string& feature, + int sequence_number) + : payload_(payload), feature_(feature), sequence_number_(sequence_number) {} + WireMessage::WireMessage(const std::string& payload, const std::string& feature) : payload_(payload), feature_(feature) {} WireMessage::WireMessage(const std::string& body) : body_(body) {} +WireMessage::WireMessage(const WireMessage& other) + : payload_(other.payload_), + feature_(other.feature_), + body_(other.body_), + sequence_number_(other.sequence_number_) {} + } // namespace cryptauth diff --git a/chromium/components/cryptauth/wire_message.h b/chromium/components/cryptauth/wire_message.h index 5aed96d970c..d0fe80ca69d 100644 --- a/chromium/components/cryptauth/wire_message.h +++ b/chromium/components/cryptauth/wire_message.h @@ -14,8 +14,16 @@ namespace cryptauth { class WireMessage { public: + // Creates a WireMessage containing |payload| for feature |feature| and + // sequence number |sequence_number|. + WireMessage(const std::string& payload, + const std::string& feature, + int sequence_number); + // Creates a WireMessage containing |payload| for feature |feature|. - explicit WireMessage(const std::string& payload, const std::string& feature); + WireMessage(const std::string& payload, const std::string& feature); + + WireMessage(const WireMessage& other); // Creates a WireMessage containing |body| (a serialized JSON) as the message // body. @@ -38,6 +46,8 @@ class WireMessage { const std::string& payload() const { return payload_; } const std::string& feature() const { return feature_; } const std::string& body() const { return body_; } + // Will return -1 if no sequence number was passed to the constructor. + int sequence_number() const { return sequence_number_; } private: // The message payload. @@ -51,7 +61,8 @@ class WireMessage { // vice-versa. const std::string body_; - DISALLOW_COPY_AND_ASSIGN(WireMessage); + // Sequence number for this message; set to -1 if no sequence number if set. + const int sequence_number_ = -1; }; } // namespace cryptauth |