Switch to using v5, class M QR codes.

This configuration allows for more data in the QR code, which we'll need
in order to put a public key in it.

BUG=1002262

Change-Id: If70c8c67a61ce9fdd3ce1ef87851c5e6cfdcc076
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107425
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Travis Skare <skare@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#751537}

chrome/browser/ui/views/webauthn/authenticator_qr_sheet_view.cc

@@ -37,11 +37,11 @@ class QRView : public views::View {
   static constexpr int kDinoY =
       kMid - (dino_image::kDinoHeight * kDinoTilePixels) / 2;
 
-  explicit QRView(const uint8_t qr_data[QRCode::kInputBytes])
+  explicit QRView(base::span<const uint8_t> qr_data)
       : qr_tiles_(qr_.Generate(qr_data)) {}
-  ~QRView() override {}
+  ~QRView() override = default;
 
-  void RefreshQRCode(const uint8_t new_qr_data[QRCode::kInputBytes]) {
+  void RefreshQRCode(base::span<const uint8_t> new_qr_data) {
     state_ = (state_ + 1) % 6;
     qr_tiles_ = qr_.Generate(new_qr_data);
     SchedulePaint();
@@ -164,12 +164,13 @@ constexpr size_t Base64EncodedSize(size_t input_length) {
   return ((input_length * 4) + 2) / 3;
 }
 
-// QRDataForCurrentTime writes a URL suitable for encoding as a QR to
-// |out_qr_data|. The URL is generated based on |qr_generator_key| and the
-// current time such that the caBLE discovery code can recognise the URL as
-// valid.
-void QRDataForCurrentTime(uint8_t out_qr_data[QRCode::kInputBytes],
-                          base::span<const uint8_t, 32> qr_generator_key) {
+// QRDataForCurrentTime writes a URL suitable for encoding as a QR to |out_buf|
+// and returns a span pointing into that buffer. The URL is generated based on
+// |qr_generator_key| and the current time such that the caBLE discovery code
+// can recognise the URL as valid.
+base::span<uint8_t> QRDataForCurrentTime(
+    uint8_t out_buf[QRCode::kInputBytes],
+    base::span<const uint8_t, 32> qr_generator_key) {
   const int64_t current_tick = device::CableDiscoveryData::CurrentTimeTick();
   auto qr_secret = device::CableDiscoveryData::DeriveQRSecret(qr_generator_key,
                                                               current_tick);
@@ -186,19 +187,19 @@ void QRDataForCurrentTime(uint8_t out_qr_data[QRCode::kInputBytes],
   static constexpr char kPrefix[] = "fido://c1/";
   static constexpr size_t kPrefixLength = sizeof(kPrefix) - 1;
 
-  static_assert(QRCode::kInputBytes == kPrefixLength + kEncodedSecretLength,
+  static_assert(QRCode::kInputBytes >= kPrefixLength + kEncodedSecretLength,
                 "unexpected QR input length");
-  memcpy(out_qr_data, kPrefix, kPrefixLength);
-  memcpy(&out_qr_data[kPrefixLength], base64_qr_secret.data(),
+  memcpy(out_buf, kPrefix, kPrefixLength);
+  memcpy(&out_buf[kPrefixLength], base64_qr_secret.data(),
          kEncodedSecretLength);
+  return base::span<uint8_t>(out_buf, kPrefixLength + kEncodedSecretLength);
 }
 
 }  // anonymous namespace
 
 class AuthenticatorQRViewCentered : public views::View {
  public:
-  explicit AuthenticatorQRViewCentered(
-      const uint8_t qr_data[QRCode::kInputBytes]) {
+  explicit AuthenticatorQRViewCentered(base::span<const uint8_t> qr_data) {
     views::BoxLayout* layout =
         SetLayoutManager(std::make_unique<views::BoxLayout>(
             views::BoxLayout::Orientation::kHorizontal));
@@ -210,7 +211,7 @@ class AuthenticatorQRViewCentered : public views::View {
     AddChildView(qr_view_);
   }
 
-  void RefreshQRCode(const uint8_t new_qr_data[QRCode::kInputBytes]) {
+  void RefreshQRCode(base::span<const uint8_t> new_qr_data) {
     qr_view_->RefreshQRCode(new_qr_data);
   }
 
@@ -228,9 +229,9 @@ AuthenticatorQRSheetView::~AuthenticatorQRSheetView() = default;
 
 std::unique_ptr<views::View>
 AuthenticatorQRSheetView::BuildStepSpecificContent() {
-  uint8_t qr_data[QRCode::kInputBytes];
-  QRDataForCurrentTime(qr_data, qr_generator_key_);
-  auto qr_view = std::make_unique<AuthenticatorQRViewCentered>(qr_data);
+  uint8_t qr_data_buf[QRCode::kInputBytes];
+  auto qr_view = std::make_unique<AuthenticatorQRViewCentered>(
+      QRDataForCurrentTime(qr_data_buf, qr_generator_key_));
   qr_view_ = qr_view.get();
 
   timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(600), this,
@@ -239,7 +240,6 @@ AuthenticatorQRSheetView::BuildStepSpecificContent() {
 }
 
 void AuthenticatorQRSheetView::Update() {
-  uint8_t qr_data[QRCode::kInputBytes];
-  QRDataForCurrentTime(qr_data, qr_generator_key_);
-  qr_view_->RefreshQRCode(qr_data);
+  uint8_t qr_data_buf[QRCode::kInputBytes];
+  qr_view_->RefreshQRCode(QRDataForCurrentTime(qr_data_buf, qr_generator_key_));
 }

chrome/browser/ui/views/webauthn/authenticator_qr_sheet_view.h

@@ -23,7 +23,7 @@ class AuthenticatorQRSheetView : public AuthenticatorRequestSheetView {
   ~AuthenticatorQRSheetView() override;
 
   // RefreshQRCode causes a fresh QR code to be painted.
-  void RefreshQRCode(const uint8_t new_qr_data[24]);
+  void RefreshQRCode(base::span<const uint8_t> new_qr_data);
 
  private:
   // AuthenticatorRequestSheetView:

chrome/common/qr_code_generator/BUILD.gn

@@ -2,6 +2,11 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+declare_args() {
+  # Enables building a development / debugging binary.
+  enable_qr_print = false
+}
+
 source_set("qr_code_generator") {
   sources = [
     "dino_image.h",
@@ -13,3 +18,10 @@ source_set("qr_code_generator") {
 
   public_deps = [ "//base" ]
 }
+
+if (enable_qr_print) {
+  executable("qr_print") {
+    sources = [ "qr_print.cc" ]
+    deps = [ ":qr_code_generator" ]
+  }
+}

chrome/common/qr_code_generator/qr_code_generator.cc

@@ -14,11 +14,13 @@ static_assert(QRCodeGenerator::kNumSegments != 0 &&
                       0,
               "invalid configuration");
 
-// Generate generates a QR code containing the given data and returns a
-// pointer to an array of kSize×kSize bytes where the least-significant bit of
-// each byte is set if that tile should be "black".
-base::span<const uint8_t, QRCodeGenerator::kTotalSize>
-QRCodeGenerator::Generate(const uint8_t in[kInputBytes]) {
+base::span<uint8_t, QRCodeGenerator::kTotalSize> QRCodeGenerator::Generate(
+    base::span<const uint8_t> in) {
+  // Can't pass a constexpr to CHECK_LE.
+  if (in.size() > kInputBytes) {
+    CHECK(false) << "input too long";
+  }
+
   memset(d_, 0, sizeof(d_));
   PutVerticalTiming(6);
   PutHorizontalTiming(6);
@@ -26,29 +28,42 @@ QRCodeGenerator::Generate(const uint8_t in[kInputBytes]) {
   PutFinder(3, kSize - 4);
   PutFinder(kSize - 4, 3);
   // See table E.1 for the location of alignment symbols.
-  PutAlignment(22, 22);
+  PutAlignment(30, 30);
 
   // kFormatInformation is the encoded formatting word for the QR code that
-  // this code generates:
-  //                  11 011
+  // this code generates. See tables 10 and 12.
+  //                  00 011
   //                  --|---
-  // error correction Q | Mask pattern 3
+  // error correction M | Mask pattern 3
   //
   // It's translated into the following, 15-bit value using the table on page
   // 80.
-  constexpr uint16_t kFormatInformation = 0x3a06;
+  constexpr uint16_t kFormatInformation = 0x5b4b;
   PutFormatBits(kFormatInformation);
 
   // QR codes require some framing of the data which requires 12 bits of
   // overhead. Since 12 is not a multiple of eight, a frame-shift of all
   // subsequent bytes is required.
   uint8_t prefixed_data[kDataBytes];
-  prefixed_data[0] = 0x42;
-  prefixed_data[1] = 0x00 | (in[0] >> 4);
-  for (size_t i = 0; i < kInputBytes - 1; i++) {
+  static_assert(kInputBytes < 256, "in too large for 8-bit length");
+  const uint8_t len8 = static_cast<uint8_t>(in.size());
+  prefixed_data[0] = 0x40 | (len8 >> 4);
+  prefixed_data[1] = (len8 << 4);
+  if (!in.empty()) {
+    prefixed_data[1] |= (in[0] >> 4);
+  }
+  for (size_t i = 0; i < in.size() - 1; i++) {
     prefixed_data[i + 2] = (in[i] << 4) | (in[i + 1] >> 4);
   }
-  prefixed_data[kDataBytes - 1] = in[kInputBytes - 1] << 4;
+  if (!in.empty()) {
+    prefixed_data[in.size() + 1] = in[in.size() - 1] << 4;
+  }
+
+  // The QR code looks a little odd with fixed padding. Thus replicate the
+  // message to fill the input.
+  for (size_t i = in.size() + 2; i < kInputBytes; i++) {
+    prefixed_data[i] = prefixed_data[i % (in.size() + 2)];
+  }
 
   // Each segment of input data is expanded with error correcting
   // information and then interleaved.
@@ -329,13 +344,13 @@ void QRCodeGenerator::AddErrorCorrection(uint8_t out[kSegmentBytes],
   //        acc *= (z - x^i)
   //    return acc
   //
-  // gen = generatorPoly(18)
+  // gen = generatorPoly(24)
   // coeffs = list(gen)
   // gen = [toByte(x) for x in coeffs]
   // print 'uint8_t kGenerator[' + str(len(gen)) + '] = {' + str(gen) + '}'
   static constexpr uint8_t kGenerator[kSegmentECBytes + 1] = {
-      146, 217, 67,  32,  75,  173, 82,  73,  220, 240,
-      215, 199, 175, 149, 113, 183, 251, 239, 1,
+      117, 144, 217, 127, 247, 237, 1,   206, 43,  61,  72,  130, 73,
+      229, 150, 115, 102, 216, 237, 178, 70,  169, 118, 122, 1,
   };
 
   // The error-correction bytes are the remainder of dividing |in| * x^k by

chrome/common/qr_code_generator/qr_code_generator.h

@@ -10,23 +10,25 @@
 
 #include "base/containers/span.h"
 
-// QRCodeGenerator generates version three, class Q QR codes that carry 32
+// QRCodeGenerator generates version five, class M QR codes that carry 84
 // bytes of raw data. References in the following comments refer to ISO 18004
 // (3rd edition).
 class QRCodeGenerator {
  public:
-  // kSize is the number of "tiles" in each dimension for a v3 QR code. See
+  // kSize is the number of "tiles" in each dimension for a v5 QR code. See
   // table 1. (The colored squares in in QR codes are called tiles in the
   // spec.)
-  static constexpr int kSize = 29;
-  // kTotalSize is the total number of tiles for a v3 QR code, in both
+  static constexpr int kSize = 37;
+  // kTotalSize is the total number of tiles for a v5 QR code, in both
   // directions.
   static constexpr int kTotalSize = kSize * kSize;
-  // These values are taken from table 9 (page 38) for a version three, class Q
-  // QR code.
-  static constexpr size_t kTotalBytes = 70;
+  // These values are taken from table 9 (page 38) for a version five, class M
+  // QR code. (That table is very badly formatted for version five, the rows in
+  // the last column don't line up correctly. The correct value for 5M is
+  // (67,43,12).)
+  static constexpr size_t kTotalBytes = 134;
   static constexpr size_t kNumSegments = 2;
-  static constexpr size_t kSegmentDataBytes = 17;
+  static constexpr size_t kSegmentDataBytes = 43;
 
   static constexpr size_t kSegmentBytes = kTotalBytes / kNumSegments;
   static constexpr size_t kSegmentECBytes = kSegmentBytes - kSegmentDataBytes;
@@ -36,8 +38,9 @@ class QRCodeGenerator {
 
   // Generate generates a QR code containing the given data and returns a
   // pointer to an array of kTotalSize bytes where the least-significant bit of
-  // each byte is set if that tile should be "black".
-  base::span<const uint8_t, kTotalSize> Generate(const uint8_t in[kInputBytes]);
+  // each byte is set if that tile should be "black". The length of |in| must be
+  // less than, or equal to, |kInputBytes|.
+  base::span<uint8_t, kTotalSize> Generate(base::span<const uint8_t> in);
 
  private:
   // MaskFunction3 implements one of the data-masking functions. See figure 21.

chrome/common/qr_code_generator/qr_print.cc

+// Copyright 2020 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.
+
+// This helper binary can be compiled to aid in development / debugging of the
+// QR generation code. It prints a QR code to the console and thus allows much
+// faster iteration. It is not built by default, see the BUILD.gn in this
+// directory.
+
+#include <stdio.h>
+
+#include <utility>
+
+#include "base/containers/span.h"
+#include "chrome/common/qr_code_generator/qr_code_generator.h"
+
+// kTerminalBackgroundIsBright controls the output polarity. Many QR scanners
+// will cope with inverted bright / dark but, if you have a bright terminal
+// background, you may need to change this.
+constexpr bool kTerminalBackgroundIsBright = false;
+
+// kPaint is a pair of UTF-8 encoded code points for U+2588 ("FULL BLOCK").
+static constexpr char kPaint[] = "\xe2\x96\x88\xe2\x96\x88";
+static constexpr char kNoPaint[] = "  ";
+
+static void PrintHorizontalLine(const char* white) {
+  for (size_t x = 0; x < QRCodeGenerator::kSize + 2; x++) {
+    fputs(white, stdout);
+  }
+  fputs("\n", stdout);
+}
+
+int main(int argc, char** argv) {
+  // Presubmits don't allow fprintf to a variable called |stderr|.
+  FILE* const STDERR = stderr;
+
+  if (argc != 2) {
+    fprintf(STDERR, "Usage: %s <input string>\n", argv[0]);
+    return 1;
+  }
+
+  const char* const input = argv[1];
+  const size_t input_len = strlen(input);
+  if (input_len > QRCodeGenerator::kInputBytes) {
+    fprintf(STDERR,
+            "Input string too long. Have %u bytes, but max is %u bytes.\n",
+            static_cast<unsigned>(input_len),
+            static_cast<unsigned>(QRCodeGenerator::kInputBytes));
+    return 2;
+  }
+
+  const char* black = kNoPaint;
+  const char* white = kPaint;
+  if (kTerminalBackgroundIsBright) {
+    std::swap(black, white);
+  }
+
+  QRCodeGenerator generator;
+  base::span<const uint8_t, QRCodeGenerator::kTotalSize> code =
+      generator.Generate(base::span<const uint8_t>(
+          reinterpret_cast<const uint8_t*>(input), input_len));
+
+  PrintHorizontalLine(white);
+
+  size_t i = 0;
+  for (size_t y = 0; y < QRCodeGenerator::kSize; y++) {
+    fputs(white, stdout);
+    for (size_t x = 0; x < QRCodeGenerator::kSize; x++) {
+      fputs((code[i++] & 1) ? black : white, stdout);
+    }
+    fputs(white, stdout);
+    fputs("\n", stdout);
+  }
+
+  PrintHorizontalLine(white);
+
+  return 0;
+}

chrome/services/qrcode_generator/qrcode_generator_service_impl.cc

@@ -25,10 +25,6 @@ static const int kDinoTileSizePixels = 5;
 // Size of a QR locator, in modules.
 static const int kLocatorSizeModules = 7;
 
-// Maximum supported data length.
-// Will likely change with higher-supported versions.
-constexpr int kMaxInputLength = 512;
-
 QRCodeGeneratorServiceImpl::QRCodeGeneratorServiceImpl(
     mojo::PendingReceiver<mojom::QRCodeGeneratorService> receiver)
     : receiver_(this, std::move(receiver)) {
@@ -230,25 +226,29 @@ void QRCodeGeneratorServiceImpl::GenerateQRCode(
   mojom::GenerateQRCodeResponsePtr response =
       mojom::GenerateQRCodeResponse::New();
 
-  if (request->data.length() > kMaxInputLength) {
+  // TODO(skare): Use a higher QR code version to allow for more data.
+  // TODO(skare): cap string length with message in the UI.
+  if (request->data.length() > QRCodeGenerator::kInputBytes) {
     response->error_code = mojom::QRCodeGeneratorError::INPUT_TOO_LONG;
     std::move(callback).Run(std::move(response));
     return;
   }
 
-  // TODO(skare): Use a higher QR code vdersion. Until then, use kSize=29x29
-  // from the common encoder.
-  const gfx::Size qr_output_data_size = {29, 29};
-  // TODO(skare): cap string length with message in the UI.
-  uint8_t input[kMaxInputLength + 1] = {0};
-  base::strlcpy(reinterpret_cast<char*>(input), request->data.c_str(),
-                kMaxInputLength);
   QRCodeGenerator qr;
-  auto qr_data_span = qr.Generate(input);
-
-  for (uint8_t i : qr_data_span) {
-    response->data.push_back(i);
+  auto qr_data_span = qr.Generate(base::span<const uint8_t>(
+      reinterpret_cast<const uint8_t*>(request->data.data()),
+      request->data.size()));
+
+  // The least significant bit of each byte in |qr_data_span| is set if the tile
+  // should be black.
+  for (size_t i = 0; i < qr_data_span.size(); i++) {
+    qr_data_span[i] &= 1;
   }
+
+  const gfx::Size qr_output_data_size = {QRCodeGenerator::kSize,
+                                         QRCodeGenerator::kSize};
+  response->data.insert(response->data.begin(), qr_data_span.begin(),
+                        qr_data_span.end());
   response->data_size = qr_output_data_size;
   response->error_code = mojom::QRCodeGeneratorError::NONE;
   RenderBitmap(qr_data_span.data(), qr_output_data_size, request, &response);