Commit 8e2860a5 authored by Jun Choi's avatar Jun Choi Committed by Commit Bot

Stop request when device errors out on GetInfo

Currently when FidoDevice fails on AuthenticatorGetInfo command, we
assume that the device supports U2F protocol and initiate U2F request.
Prior to initiating request, check whether AuthenticatorGetInfo command
failed due to any device communication error. If the failure was caused
by device error, drop remaining requests on the device. Also, to prevent
similar failures, always check for device state before dispatching
consecutive requests to authenticators.

Bug: 872293
Change-Id: I82d83975eec8177adb447e5e94711126884f5b5f
Reviewed-on: https://chromium-review.googlesource.com/1168505
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585744}
parent 1b68c0dd
...@@ -10,9 +10,11 @@ ...@@ -10,9 +10,11 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/bind.h"
#include "base/callback.h" #include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "device/fido/fido_constants.h" #include "device/fido/fido_constants.h"
#include "device/fido/fido_device.h" #include "device/fido/fido_device.h"
...@@ -39,8 +41,9 @@ class DeviceOperation { ...@@ -39,8 +41,9 @@ class DeviceOperation {
// TODO(hongjunchoi): Refactor so that |command| is never base::nullopt. // TODO(hongjunchoi): Refactor so that |command| is never base::nullopt.
void DispatchDeviceRequest(base::Optional<std::vector<uint8_t>> command, void DispatchDeviceRequest(base::Optional<std::vector<uint8_t>> command,
FidoDevice::DeviceCallback callback) { FidoDevice::DeviceCallback callback) {
if (!command) { if (!command || device_->state() == FidoDevice::State::kDeviceError) {
std::move(callback).Run(base::nullopt); base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), base::nullopt));
return; return;
} }
......
...@@ -42,6 +42,10 @@ bool FidoDevice::SupportedProtocolIsInitialized() { ...@@ -42,6 +42,10 @@ bool FidoDevice::SupportedProtocolIsInitialized() {
void FidoDevice::OnDeviceInfoReceived( void FidoDevice::OnDeviceInfoReceived(
base::OnceClosure done, base::OnceClosure done,
base::Optional<std::vector<uint8_t>> response) { base::Optional<std::vector<uint8_t>> response) {
// TODO(hongjunchoi): Add tests that verify this behavior.
if (state_ == FidoDevice::State::kDeviceError)
return;
state_ = FidoDevice::State::kReady; state_ = FidoDevice::State::kReady;
base::Optional<AuthenticatorGetInfoResponse> get_info_response = base::Optional<AuthenticatorGetInfoResponse> get_info_response =
......
...@@ -217,6 +217,7 @@ void FidoHidDevice::PacketWritten(base::Optional<FidoHidMessage> message, ...@@ -217,6 +217,7 @@ void FidoHidDevice::PacketWritten(base::Optional<FidoHidMessage> message,
void FidoHidDevice::ReadMessage(HidMessageCallback callback) { void FidoHidDevice::ReadMessage(HidMessageCallback callback) {
if (!connection_) { if (!connection_) {
state_ = State::kDeviceError;
std::move(callback).Run(base::nullopt); std::move(callback).Run(base::nullopt);
return; return;
} }
...@@ -230,6 +231,7 @@ void FidoHidDevice::OnRead(HidMessageCallback callback, ...@@ -230,6 +231,7 @@ void FidoHidDevice::OnRead(HidMessageCallback callback,
uint8_t report_id, uint8_t report_id,
const base::Optional<std::vector<uint8_t>>& buf) { const base::Optional<std::vector<uint8_t>>& buf) {
if (!success) { if (!success) {
state_ = State::kDeviceError;
std::move(callback).Run(base::nullopt); std::move(callback).Run(base::nullopt);
return; return;
} }
...@@ -267,6 +269,7 @@ void FidoHidDevice::OnReadContinuation( ...@@ -267,6 +269,7 @@ void FidoHidDevice::OnReadContinuation(
uint8_t report_id, uint8_t report_id,
const base::Optional<std::vector<uint8_t>>& buf) { const base::Optional<std::vector<uint8_t>>& buf) {
if (!success) { if (!success) {
state_ = State::kDeviceError;
std::move(callback).Run(base::nullopt); std::move(callback).Run(base::nullopt);
return; return;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment