Add FIDO U2F message and packet classes

device/u2f/u2f_message.h

+// 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 DEVICE_U2F_U2F_MESSAGE_H_
+#define DEVICE_U2F_U2F_MESSAGE_H_
+
+#include <list>
+#include <vector>
+#include "base/macros.h"
+#include "device/u2f/u2f_packet.h"
+
+namespace device {
+
+// U2fMessages are defined by the specification at
+// http://fidoalliance.org/specs/u2f-specs-1.0-bt-nfc-id-amendment/fido-u2f-hid-protocol.html
+
+class U2fMessage : public base::RefCountedThreadSafe<U2fMessage> {
+ public:
+  enum class Type : uint8_t {
+    CMD_PING = 0x81,
+    CMD_MSG = 0x83,
+    CMD_INIT = 0x86,
+    CMD_WINK = 0x88,
+    CMD_ERROR = 0xbf,
+  };
+
+  static const size_t kInitPacketHeader = 7;

[juanlang (chromium.org), 2016/12/07 18:29:26]

Nit: this, and the remaining constants, seem like a detail of the
implementation, rather than part of the interface. Perhaps they shouldn't be
public?

[Casey Piper, 2016/12/09 00:15:54]

Done, moved to private and test classes were added as friends.

+  static const size_t kContPacketHeader = 5;

[juanlang (chromium.org), 2016/12/07 18:29:26]

Nit: prefer Continuation to Cont.

[Casey Piper, 2016/12/09 00:15:54]

Done.

+  static const size_t kMaxHidPacketSize = 64;
+  static const size_t kInitPacketDataSize =
+      kMaxHidPacketSize - kInitPacketHeader;
+  static const size_t kContPacketDataSize =
+      kMaxHidPacketSize - kContPacketHeader;
+  // Messages are limited to an init packet and 128 cont packets
+  // Maximum payload length therefore is 64-7 + 128 * (64-5) = 7609 bytes
+  static const size_t kMaxMessageSize = 7609;
+
+  static scoped_refptr<U2fMessage> Create(
+      const uint8_t channel_id[U2fPacket::kChannelIdSize],

[juanlang (chromium.org), 2016/12/07 18:29:26]

Passing arrays as parameters in C is a little dicey. While you're signalling to
the caller that a 4-byte array is expected, there's no guarantee the caller will
do so. I'd suggest you pass the channel_id as a uint32_t instead.

[Casey Piper, 2016/12/09 00:15:54]

Done.

+      const Type type,
+      const std::vector<uint8_t>& data);
+  static scoped_refptr<U2fMessage> Create(

[juanlang (chromium.org), 2016/12/07 18:29:26]

This method name makes it a little hard to differentiate from the previous one.
In what circumstances should the caller call Create with a single parameter, vs.
calling Create with 3 parameters? I'd suggest a name that signals a little more
clearly what this method is for.

Also, I believe it's the norm to add method documentation on public methods.

[Casey Piper, 2016/12/09 00:15:54]

Create method names changed for clarity.

+      scoped_refptr<net::IOBufferWithSize> buf);
+  scoped_refptr<net::IOBufferWithSize> PopNextPacket();
+  bool AddContPacket(scoped_refptr<net::IOBufferWithSize> packet_buf);
+  size_t NumPackets();
+  const std::vector<uint8_t> GetData();
+  const uint8_t* GetChannelId();
+  bool MessageComplete();
+  std::list<scoped_refptr<U2fPacket>>::const_iterator begin();
+  std::list<scoped_refptr<U2fPacket>>::const_iterator end();
+
+ protected:
+  virtual ~U2fMessage();
+
+ private:
+  friend class base::RefCountedThreadSafe<U2fMessage>;
+
+  U2fMessage(const uint8_t channel_id[U2fPacket::kChannelIdSize],
+             const Type type,
+             const std::vector<uint8_t>& data);
+  U2fMessage(scoped_refptr<net::IOBufferWithSize> buf);
+
+  std::list<scoped_refptr<U2fPacket>> packets_;
+  size_t remaining_size_;
+  uint8_t channel_id_[U2fPacket::kChannelIdSize];
+};
+}  // namespace device
+
+#endif  // DEVICE_U2F_U2F_MESSAGE_H_