Change AAGUID of AttestedCredentialData to array

Since AAGUID always has fixed size(16), change |aaguid_| of
AttestedCredentialData from std::vector<uint8_t> to std::array<uint8_t,
16>.

Bug: 780078
Change-Id: If30dd55b6e88e38f2a6ab53365a6724cf29892a5
Reviewed-on: https://chromium-review.googlesource.com/981682
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547321}

device/fido/attested_credential_data.cc

@@ -4,8 +4,7 @@
 
 #include "device/fido/attested_credential_data.h"
 
-#include <stddef.h>
-
+#include <algorithm>
 #include <utility>
 
 #include "base/numerics/safe_math.h"
@@ -15,15 +14,6 @@
 
 namespace device {
 
-namespace {
-
-constexpr size_t kAaguidLength = 16;
-
-// Number of bytes used to represent length of credential ID.
-constexpr size_t kCredentialIdLengthLength = 2;
-
-}  // namespace
-
 // static
 base::Optional<AttestedCredentialData>
 AttestedCredentialData::DecodeFromCtapResponse(
@@ -31,16 +21,20 @@ AttestedCredentialData::DecodeFromCtapResponse(
   if (buffer.size() < kAaguidLength + kCredentialIdLengthLength)
     return base::nullopt;
 
-  auto aaguid = u2f_parsing_utils::Extract(buffer, 0, kAaguidLength);
-  auto credential_id_length_span = u2f_parsing_utils::ExtractSpan(
-      buffer, kAaguidLength, kCredentialIdLengthLength);
+  std::array<uint8_t, kAaguidLength> aaguid;
+  if (!u2f_parsing_utils::ExtractArray(buffer, 0, &aaguid))
+    return base::nullopt;
 
-  if (aaguid.empty() || credential_id_length_span.empty())
+  std::array<uint8_t, kCredentialIdLengthLength> credential_id_length_array;
+  if (!u2f_parsing_utils::ExtractArray(buffer, kAaguidLength,
+                                       &credential_id_length_array)) {
     return base::nullopt;
+  }
 
   static_assert(kCredentialIdLengthLength == 2u, "L must be 2 bytes");
   const size_t credential_id_length =
-      credential_id_length_span[0] << 8 | credential_id_length_span[1];
+      (base::strict_cast<size_t>(credential_id_length_array[0]) << 8) |
+      base::strict_cast<size_t>(credential_id_length_array[1]);
 
   auto credential_id = u2f_parsing_utils::Extract(
       buffer, kAaguidLength + kCredentialIdLengthLength, credential_id_length);
@@ -54,9 +48,7 @@ AttestedCredentialData::DecodeFromCtapResponse(
           kAaguidLength + kCredentialIdLengthLength + credential_id_length));
 
   return AttestedCredentialData(
-      std::move(aaguid),
-      std::vector<uint8_t>(credential_id_length_span.begin(),
-                           credential_id_length_span.end()),
+      std::move(aaguid), std::move(credential_id_length_array),
       std::move(credential_id), std::move(credential_public_key_data));
 }
 
@@ -64,7 +56,6 @@ AttestedCredentialData::DecodeFromCtapResponse(
 base::Optional<AttestedCredentialData>
 AttestedCredentialData::CreateFromU2fRegisterResponse(
     base::span<const uint8_t> u2f_data,
-    std::vector<uint8_t> aaguid,
     std::unique_ptr<PublicKey> public_key) {
   // TODO(crbug/799075): Introduce a CredentialID class to do this extraction.
   // Extract the length of the credential (i.e. of the U2FResponse key
@@ -76,8 +67,13 @@ AttestedCredentialData::CreateFromU2fRegisterResponse(
     return base::nullopt;
   }
 
+  // For U2F register request, device AAGUID is set to zeros.
+  std::array<uint8_t, kAaguidLength> aaguid;
+  aaguid.fill(0);
+
   // Note that U2F responses only use one byte for length.
-  std::vector<uint8_t> credential_id_length = {0, extracted_length[0]};
+  std::array<uint8_t, kCredentialIdLengthLength> credential_id_length = {
+      0, extracted_length[0]};
 
   // Extract the credential id (i.e. key handle).
   std::vector<uint8_t> credential_id = u2f_parsing_utils::Extract(
@@ -93,16 +89,6 @@ AttestedCredentialData::CreateFromU2fRegisterResponse(
       std::move(credential_id), std::move(public_key));
 }
 
-AttestedCredentialData::AttestedCredentialData(
-    std::vector<uint8_t> aaguid,
-    std::vector<uint8_t> length,
-    std::vector<uint8_t> credential_id,
-    std::unique_ptr<PublicKey> public_key)
-    : aaguid_(std::move(aaguid)),
-      credential_id_length_(std::move(length)),
-      credential_id_(std::move(credential_id)),
-      public_key_(std::move(public_key)) {}
-
 AttestedCredentialData::AttestedCredentialData(AttestedCredentialData&& other) =
     default;
 
@@ -112,16 +98,29 @@ AttestedCredentialData& AttestedCredentialData::operator=(
 AttestedCredentialData::~AttestedCredentialData() = default;
 
 void AttestedCredentialData::DeleteAaguid() {
-  aaguid_ = std::vector<uint8_t>(kAaguidLength, 0);
+  std::fill(aaguid_.begin(), aaguid_.end(), 0);
 }
 
 std::vector<uint8_t> AttestedCredentialData::SerializeAsBytes() const {
   std::vector<uint8_t> attestation_data;
-  u2f_parsing_utils::Append(&attestation_data, aaguid_);
-  u2f_parsing_utils::Append(&attestation_data, credential_id_length_);
+  u2f_parsing_utils::Append(&attestation_data,
+                            base::make_span(aaguid_.data(), kAaguidLength));
+  u2f_parsing_utils::Append(
+      &attestation_data,
+      base::make_span(credential_id_length_.data(), kCredentialIdLengthLength));
   u2f_parsing_utils::Append(&attestation_data, credential_id_);
   u2f_parsing_utils::Append(&attestation_data, public_key_->EncodeAsCOSEKey());
   return attestation_data;
 }
 
+AttestedCredentialData::AttestedCredentialData(
+    std::array<uint8_t, kAaguidLength> aaguid,
+    std::array<uint8_t, kCredentialIdLengthLength> credential_id_length,
+    std::vector<uint8_t> credential_id,
+    std::unique_ptr<PublicKey> public_key)
+    : aaguid_(std::move(aaguid)),
+      credential_id_length_(std::move(credential_id_length)),
+      credential_id_(std::move(credential_id)),
+      public_key_(std::move(public_key)) {}
+
 }  // namespace device

device/fido/attested_credential_data.h

@@ -5,6 +5,7 @@
 #ifndef DEVICE_FIDO_ATTESTED_CREDENTIAL_DATA_H_
 #define DEVICE_FIDO_ATTESTED_CREDENTIAL_DATA_H_
 
+#include <stddef.h>
 #include <stdint.h>
 #include <memory>
 #include <vector>
@@ -26,14 +27,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AttestedCredentialData {
 
   static base::Optional<AttestedCredentialData> CreateFromU2fRegisterResponse(
       base::span<const uint8_t> u2f_data,
-      std::vector<uint8_t> aaguid,
       std::unique_ptr<PublicKey> public_key);
 
-  AttestedCredentialData(std::vector<uint8_t> aaguid,
-                         std::vector<uint8_t> length,
-                         std::vector<uint8_t> credential_id,
-                         std::unique_ptr<PublicKey> public_key);
-
   // Moveable.
   AttestedCredentialData(AttestedCredentialData&& other);
   AttestedCredentialData& operator=(AttestedCredentialData&& other);
@@ -54,11 +49,22 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AttestedCredentialData {
   std::vector<uint8_t> SerializeAsBytes() const;
 
  private:
+  static constexpr size_t kAaguidLength = 16;
+  // Number of bytes used to represent length of credential ID.
+  static constexpr size_t kCredentialIdLengthLength = 2;
+
+  AttestedCredentialData(
+      std::array<uint8_t, kAaguidLength> aaguid,
+      std::array<uint8_t, kCredentialIdLengthLength> credential_id_length,
+      std::vector<uint8_t> credential_id,
+      std::unique_ptr<PublicKey> public_key);
+
   // The 16-byte AAGUID of the authenticator.
-  std::vector<uint8_t> aaguid_;
+  std::array<uint8_t, kAaguidLength> aaguid_;
 
   // Big-endian length of the credential (i.e. key handle).
-  std::vector<uint8_t> credential_id_length_;
+  std::array<uint8_t, kCredentialIdLengthLength> credential_id_length_;
+
   std::vector<uint8_t> credential_id_;
   std::unique_ptr<PublicKey> public_key_;
 

device/fido/authenticator_make_credential_response.cc

@@ -25,12 +25,9 @@ AuthenticatorMakeCredentialResponse::CreateFromU2fRegisterResponse(
   if (!public_key)
     return base::nullopt;
 
-  // AAGUID is zeroed out for U2F responses.
-  std::vector<uint8_t> aaguid(16u);
-
   auto attested_credential_data =
       AttestedCredentialData::CreateFromU2fRegisterResponse(
-          u2f_data, std::move(aaguid), std::move(public_key));
+          u2f_data, std::move(public_key));
 
   if (!attested_credential_data)
     return base::nullopt;

device/fido/u2f_parsing_utils.h

@@ -7,6 +7,8 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <algorithm>
+#include <array>
 #include <vector>
 
 #include "base/component_export.h"
@@ -59,6 +61,19 @@ COMPONENT_EXPORT(DEVICE_FIDO)
 base::span<const uint8_t> ExtractSuffixSpan(base::span<const uint8_t> span,
                                             size_t pos);
 
+template <size_t N>
+COMPONENT_EXPORT(DEVICE_FIDO)
+bool ExtractArray(base::span<const uint8_t> span,
+                  size_t pos,
+                  std::array<uint8_t, N>* array) {
+  const auto extracted_span = ExtractSpan(span, pos, N);
+  if (extracted_span.size() != N)
+    return false;
+
+  std::copy(extracted_span.begin(), extracted_span.end(), array->begin());
+  return true;
+}
+
 }  // namespace u2f_parsing_utils
 }  // namespace device
 

device/fido/u2f_parsing_utils_unittest.cc

@@ -117,5 +117,21 @@ TEST(U2fParsingUtils, ExtractSuffixOutOfBounds) {
   EXPECT_THAT(ExtractSuffix(kOneTwoThree, 4), ::testing::IsEmpty());
 }
 
+TEST(U2fParsingUtils, ExtractArray) {
+  const std::vector<uint8_t> empty;
+  std::array<uint8_t, 0> array_empty;
+  EXPECT_TRUE(ExtractArray(empty, 0, &array_empty));
+
+  std::array<uint8_t, 2> array_two_three;
+  EXPECT_TRUE(ExtractArray(kTwoThree, 0, &array_two_three));
+  EXPECT_THAT(array_two_three, ::testing::ElementsAreArray(kTwoThree));
+
+  EXPECT_FALSE(ExtractArray(kOneTwoThree, 2, &array_two_three));
+
+  std::array<uint8_t, 1> array_three;
+  EXPECT_TRUE(ExtractArray(kOneTwoThree, 2, &array_three));
+  EXPECT_THAT(array_three, ::testing::ElementsAreArray(kThree));
+}
+
 }  // namespace u2f_parsing_utils
 }  // namespace device

device/fido/u2f_register_unittest.cc

@@ -729,8 +729,7 @@ TEST_F(U2fRegisterTest, TestAttestedCredentialData) {
           u2f_parsing_utils::kEs256, GetTestRegisterResponse());
   base::Optional<AttestedCredentialData> attested_data =
       AttestedCredentialData::CreateFromU2fRegisterResponse(
-          GetTestRegisterResponse(), std::vector<uint8_t>(16) /* aaguid */,
-          std::move(public_key));
+          GetTestRegisterResponse(), std::move(public_key));
 
   EXPECT_EQ(GetTestAttestedCredentialDataBytes(),
             attested_data->SerializeAsBytes());
@@ -743,8 +742,7 @@ TEST_F(U2fRegisterTest, TestAuthenticatorData) {
           u2f_parsing_utils::kEs256, GetTestRegisterResponse());
   base::Optional<AttestedCredentialData> attested_data =
       AttestedCredentialData::CreateFromU2fRegisterResponse(
-          GetTestRegisterResponse(), std::vector<uint8_t>(16) /* aaguid */,
-          std::move(public_key));
+          GetTestRegisterResponse(), std::move(public_key));
 
   constexpr uint8_t flags =
       static_cast<uint8_t>(AuthenticatorData::Flag::kTestOfUserPresence) |
@@ -766,8 +764,7 @@ TEST_F(U2fRegisterTest, TestU2fAttestationObject) {
           u2f_parsing_utils::kEs256, GetTestRegisterResponse());
   base::Optional<AttestedCredentialData> attested_data =
       AttestedCredentialData::CreateFromU2fRegisterResponse(
-          GetTestRegisterResponse(), std::vector<uint8_t>(16) /* aaguid */,
-          std::move(public_key));
+          GetTestRegisterResponse(), std::move(public_key));
 
   constexpr uint8_t flags =
       static_cast<uint8_t>(AuthenticatorData::Flag::kTestOfUserPresence) |