[fido] Improve CtapDeviceResponseCode handling

Improve handling conversion between a byte and a CtapDeviceResponseCode:
* Fix undefined behaviour from static_cast'ing a byte before verifying
  it's in the CtapDeviceResponseCode enum.
* Log the byte we actually received when we get an error, instead of
  converting the response code back into a byte. This way, we log codes
  we don't know about and save me some emabarrassment when reporting
  bugs to authenticator vendors.
* Convert the list of codes into a shiny new constexpr flat_set. Now we
  don't have to manually increase the array size to avoid the compiler
  yelling at us.
* Add some missing CTAP codes.
* Add a logging for bad GetInfo responses.

Fixed: 1148479
Change-Id: Ic8966f4c881d4a6c7872ed8dc6cbf1a5fd05377a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562973
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Auto-Submit: Nina Satragno <nsatragno@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#832121}

device/fido/ctap2_device_operation.h

@@ -122,7 +122,7 @@ class Ctap2DeviceOperation : public DeviceOperation<Request, Response> {
     // TODO: it would be nice to see which device each response is coming from,
     // but that breaks every mock test because they aren't expecting a call to
     // GetId().
-    if (!device_response) {
+    if (!device_response || device_response->empty()) {
       FIDO_LOG(ERROR) << "-> (error reading)";
       std::move(this->callback())
           .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
@@ -131,8 +131,8 @@ class Ctap2DeviceOperation : public DeviceOperation<Request, Response> {
 
     auto response_code = GetResponseCode(*device_response);
     if (response_code != CtapDeviceResponseCode::kSuccess) {
-      FIDO_LOG(DEBUG) << "-> (CTAP2 error code "
-                      << static_cast<int>(response_code) << ")";
+      FIDO_LOG(DEBUG) << "-> (CTAP2 error code " << +device_response->at(0)
+                      << ")";
       std::move(this->callback()).Run(response_code, base::nullopt);
       return;
     }

device/fido/device_response_converter.cc

@@ -58,9 +58,8 @@ CtapDeviceResponseCode GetResponseCode(base::span<const uint8_t> buffer) {
   if (buffer.empty())
     return CtapDeviceResponseCode::kCtap2ErrInvalidCBOR;
 
-  auto code = static_cast<CtapDeviceResponseCode>(buffer[0]);
-  return base::Contains(kCtapResponseCodeList, code)
-             ? code
+  return kCtapResponseCodeList.contains(buffer[0])
+             ? static_cast<CtapDeviceResponseCode>(buffer[0])
              : CtapDeviceResponseCode::kCtap2ErrInvalidCBOR;
 }
 
@@ -196,9 +195,15 @@ base::Optional<AuthenticatorGetAssertionResponse> ReadCTAPGetAssertionResponse(
 
 base::Optional<AuthenticatorGetInfoResponse> ReadCTAPGetInfoResponse(
     base::span<const uint8_t> buffer) {
-  if (buffer.size() <= kResponseCodeLength ||
-      GetResponseCode(buffer) != CtapDeviceResponseCode::kSuccess)
+  if (buffer.size() <= kResponseCodeLength) {
+    FIDO_LOG(ERROR) << "-> (GetInfo response too short: " << buffer.size()
+                    << " bytes)";
     return base::nullopt;
+  }
+  if (GetResponseCode(buffer) != CtapDeviceResponseCode::kSuccess) {
+    FIDO_LOG(ERROR) << "-> (GetInfo CTAP2 error code " << +buffer[0] << ")";
+    return base::nullopt;
+  }
 
   cbor::Reader::DecoderError error;
   base::Optional<CBOR> decoded_response =

device/fido/fido_constants.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/component_export.h"
+#include "base/containers/fixed_flat_set.h"
 #include "base/time/time.h"
 #include "device/fido/fido_types.h"
 
@@ -117,9 +118,13 @@ enum class CtapDeviceResponseCode : uint8_t {
   kCtap2ErrPinPolicyViolation = 0x37,
   kCtap2ErrPinTokenExpired = 0x38,
   kCtap2ErrRequestTooLarge = 0x39,
+  kCtap2ErrActionTimeout = 0x3A,
+  kCtap2ErrUpRequired = 0x3B,
   kCtap2ErrUvBlocked = 0x3C,
   kCtap2ErrIntegrityFailure = 0x3D,
+  kCtap2ErrInvalidSubcommand = 0x3E,
   kCtap2ErrUvInvalid = 0x3F,
+  kCtap2ErrUnauthorizedPermission = 0x40,
   kCtap2ErrOther = 0x7F,
   kCtap2ErrSpecLast = 0xDF,
   kCtap2ErrExtensionFirst = 0xE0,
@@ -128,59 +133,64 @@ enum class CtapDeviceResponseCode : uint8_t {
   kCtap2ErrVendorLast = 0xFF
 };
 
-constexpr std::array<CtapDeviceResponseCode, 51> kCtapResponseCodeList{
-    CtapDeviceResponseCode::kSuccess,
-    CtapDeviceResponseCode::kCtap1ErrInvalidCommand,
-    CtapDeviceResponseCode::kCtap1ErrInvalidParameter,
-    CtapDeviceResponseCode::kCtap1ErrInvalidLength,
-    CtapDeviceResponseCode::kCtap1ErrInvalidSeq,
-    CtapDeviceResponseCode::kCtap1ErrTimeout,
-    CtapDeviceResponseCode::kCtap1ErrChannelBusy,
-    CtapDeviceResponseCode::kCtap1ErrLockRequired,
-    CtapDeviceResponseCode::kCtap1ErrInvalidChannel,
-    CtapDeviceResponseCode::kCtap2ErrCBORUnexpectedType,
-    CtapDeviceResponseCode::kCtap2ErrInvalidCBOR,
-    CtapDeviceResponseCode::kCtap2ErrMissingParameter,
-    CtapDeviceResponseCode::kCtap2ErrLimitExceeded,
-    CtapDeviceResponseCode::kCtap2ErrUnsupportedExtension,
-    CtapDeviceResponseCode::kCtap2ErrTooManyElements,
-    CtapDeviceResponseCode::kCtap2ErrLargeBlobStorageFull,
-    CtapDeviceResponseCode::kCtap2ErrCredentialExcluded,
-    CtapDeviceResponseCode::kCtap2ErrProcesssing,
-    CtapDeviceResponseCode::kCtap2ErrInvalidCredential,
-    CtapDeviceResponseCode::kCtap2ErrUserActionPending,
-    CtapDeviceResponseCode::kCtap2ErrOperationPending,
-    CtapDeviceResponseCode::kCtap2ErrNoOperations,
-    CtapDeviceResponseCode::kCtap2ErrUnsupportedAlgorithm,
-    CtapDeviceResponseCode::kCtap2ErrOperationDenied,
-    CtapDeviceResponseCode::kCtap2ErrKeyStoreFull,
-    CtapDeviceResponseCode::kCtap2ErrNotBusy,
-    CtapDeviceResponseCode::kCtap2ErrNoOperationPending,
-    CtapDeviceResponseCode::kCtap2ErrUnsupportedOption,
-    CtapDeviceResponseCode::kCtap2ErrInvalidOption,
-    CtapDeviceResponseCode::kCtap2ErrKeepAliveCancel,
-    CtapDeviceResponseCode::kCtap2ErrNoCredentials,
-    CtapDeviceResponseCode::kCtap2ErrUserActionTimeout,
-    CtapDeviceResponseCode::kCtap2ErrNotAllowed,
-    CtapDeviceResponseCode::kCtap2ErrPinInvalid,
-    CtapDeviceResponseCode::kCtap2ErrPinBlocked,
-    CtapDeviceResponseCode::kCtap2ErrPinAuthInvalid,
-    CtapDeviceResponseCode::kCtap2ErrPinAuthBlocked,
-    CtapDeviceResponseCode::kCtap2ErrPinNotSet,
-    CtapDeviceResponseCode::kCtap2ErrPinRequired,
-    CtapDeviceResponseCode::kCtap2ErrPinPolicyViolation,
-    CtapDeviceResponseCode::kCtap2ErrPinTokenExpired,
-    CtapDeviceResponseCode::kCtap2ErrRequestTooLarge,
-    CtapDeviceResponseCode::kCtap2ErrUvBlocked,
-    CtapDeviceResponseCode::kCtap2ErrIntegrityFailure,
-    CtapDeviceResponseCode::kCtap2ErrUvInvalid,
-    CtapDeviceResponseCode::kCtap2ErrOther,
-    CtapDeviceResponseCode::kCtap2ErrSpecLast,
-    CtapDeviceResponseCode::kCtap2ErrExtensionFirst,
-    CtapDeviceResponseCode::kCtap2ErrExtensionLast,
-    CtapDeviceResponseCode::kCtap2ErrVendorFirst,
-    CtapDeviceResponseCode::kCtap2ErrVendorLast,
-};
+constexpr auto kCtapResponseCodeList = base::MakeFixedFlatSet<uint8_t>({
+    static_cast<uint8_t>(CtapDeviceResponseCode::kSuccess),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrInvalidCommand),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrInvalidParameter),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrInvalidLength),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrInvalidSeq),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrTimeout),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrChannelBusy),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrLockRequired),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap1ErrInvalidChannel),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrCBORUnexpectedType),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrInvalidCBOR),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrMissingParameter),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrLimitExceeded),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUnsupportedExtension),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrTooManyElements),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrLargeBlobStorageFull),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrCredentialExcluded),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrProcesssing),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrInvalidCredential),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUserActionPending),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrOperationPending),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrNoOperations),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUnsupportedAlgorithm),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrOperationDenied),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrKeyStoreFull),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrNotBusy),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrNoOperationPending),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUnsupportedOption),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrInvalidOption),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrKeepAliveCancel),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrNoCredentials),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUserActionTimeout),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrNotAllowed),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinInvalid),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinBlocked),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinAuthInvalid),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinAuthBlocked),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinNotSet),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinRequired),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinPolicyViolation),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrPinTokenExpired),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrRequestTooLarge),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrActionTimeout),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUpRequired),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUvBlocked),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrIntegrityFailure),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrInvalidSubcommand),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrUvInvalid),
+    static_cast<uint8_t>(
+        CtapDeviceResponseCode::kCtap2ErrUnauthorizedPermission),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrOther),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrSpecLast),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrExtensionFirst),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrExtensionLast),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrVendorFirst),
+    static_cast<uint8_t>(CtapDeviceResponseCode::kCtap2ErrVendorLast),
+});
 
 // Commands supported by CTAPHID device as specified in
 // https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#ctaphid-commands