Certificate Transparency: Code for unpacking EV cert hashes whitelist

net/cert/ct_ev_whitelist.cc

+// Copyright 2014 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.
+
+#include "net/cert/ct_ev_whitelist.h"
+
+#include <set>
+
+#include "base/big_endian.h"
+#include "base/file_util.h"
+#include "base/lazy_instance.h"
+#include "base/logging.h"
+
+namespace net {
+
+namespace ct {
+
+namespace {
+base::LazyInstance<std::set<std::string> >::Leaky g_current_ev_whitelist =
+    LAZY_INSTANCE_INITIALIZER;
+
+const uint8 kCertHashLengthBits = 64;     // 8 bytes
+const uint64 kGolombMParameterBits = 47;  // 2^47
+}  // namespace
+
+namespace internal {
+
+BitStreamReader::BitStreamReader(const char* source, size_t length)
+    : source_(source), length_(length), curr_byte_(0), curr_bit_(7) {
+}
+
+bool BitStreamReader::ReadUnaryEncoding(uint64* out) {
+  uint64 res(0);

[wtc, 2014/08/20 02:52:07]

Nit: for this line and other similar lines, I'd prefer the conventional C-style
initialization:

  uint64 res = 0;

[Eran Messeri, 2014/09/02 12:20:22]

Done, throughout the file.

+  if (!HasMoreBits()) {
+    return false;
+  }

[wtc, 2014/08/20 02:52:07]

Nit: omit curly braces.

[Eran Messeri, 2014/09/02 12:20:22]

Done, throughout the file.

+
+  uint8 b = ReadBit();
+  while (HasMoreBits() && b) {
+    res += 1;
+    b = ReadBit();
+  }
+
+  *out = res;
+  return true;
+}
+
+bool BitStreamReader::ReadBits(uint8 num_bits, uint64* out) {
+  if (num_bits > 64)
+    return false;
+
+  if (BitsLeft() < num_bits)
+    return false;
+
+  uint64 res(0);
+  for (uint8 i = 0; i < num_bits; ++i) {
+    res |= (static_cast<uint64>(ReadBit()) << (num_bits - (i + 1)));
+  }

[wtc, 2014/08/20 02:52:07]

Nit: omit curly braces.

[Eran Messeri, 2014/09/02 12:20:22]

Done.

+
+  *out = res;
+  return true;
+}
+
+uint64 BitStreamReader::BitsLeft() const {
+  if (curr_byte_ == length_)
+    return 0;
+  return (length_ - (curr_byte_ + 1)) * 8 + curr_bit_ + 1;
+}
+
+bool BitStreamReader::HasMoreBits() const {

[wtc, 2014/08/20 02:52:07]

Nit: this function doesn't seem very useful. I would probably just call
BitsLeft() directly.

[Eran Messeri, 2014/09/02 12:20:22]

Done. Personally I find it more readable not having to read a condition every
time, but that's a very simple condition so I don't mind inlining it.

+  return BitsLeft() > 0;
+}
+
+uint8 BitStreamReader::ReadBit() {
+  DCHECK(HasMoreBits());
+  DCHECK(curr_bit_ < 8 && curr_bit_ >= 0);
+  uint8 res = (source_[curr_byte_] & (1 << curr_bit_)) >> curr_bit_;
+  curr_bit_--;
+  if (curr_bit_ < 0) {
+    curr_byte_ += 1;
+    curr_bit_ = 7;
+  }
+
+  return res;
+}
+
+void SetEVWhitelistData(std::set<std::string> ev_whitelist) {

[wtc, 2014/08/20 02:52:07]

Nit: in the .h file this function is declared after UncompressEVWhitelist. So in
thie file it should be defined after UncompressEVWhitelist.

[Eran Messeri, 2014/09/02 12:20:22]

Done.

+  g_current_ev_whitelist.Get().swap(ev_whitelist);
+}
+
+bool UncompressEVWhitelist(const std::string& compressed_whitelist,
+                           std::set<std::string>* uncompressed_list) {
+  BitStreamReader reader(compressed_whitelist.data(),
+                         compressed_whitelist.size());
+  std::set<std::string> result;
+
+  VLOG(1) << "Uncompressing EV whitelist of size "
+          << compressed_whitelist.size();
+  uint64 curr_hash(0);
+  if (!reader.ReadBits(kCertHashLengthBits, &curr_hash)) {
+    VLOG(1) << "Failed reading first hash.";
+    return false;
+  }
+  char hash_bytes[8];
+  base::WriteBigEndian(hash_bytes, curr_hash);
+  result.insert(std::string(hash_bytes, 8));
+  uint64 M = static_cast<uint64>(1) << kGolombMParameterBits;

[wtc, 2014/08/20 02:52:07]

Nit: this can be defined as a compile-time constant.

[Eran Messeri, 2014/09/02 12:20:22]

Acknowledged, I prefer to leave it defined here though as the name (M) only
makes sense in this context. 

+
+  while (reader.BitsLeft() > kGolombMParameterBits) {
+    uint64 read_prefix(0);
+    if (!reader.ReadUnaryEncoding(&read_prefix)) {
+      VLOG(1) << "Failed reading unary-encoded prefix.";
+      return false;
+    }
+
+    uint64 r(0);
+    if (!reader.ReadBits(kGolombMParameterBits, &r)) {
+      VLOG(1) << "Failed reading " << kGolombMParameterBits << " bits.";
+      return false;
+    }
+
+    uint64 curr_diff = read_prefix * M + r;
+    uint64 next_hash = curr_hash + curr_diff;

[wtc, 2014/08/20 02:52:07]

Nit: we don't seem to need the |next_hash| variable. I think we can just do

  curr_hash += curr_diff;

here.

[Eran Messeri, 2014/09/02 12:20:22]

Done.

+
+    base::WriteBigEndian(hash_bytes, next_hash);
+    result.insert(std::string(hash_bytes, 8));
+    curr_hash = next_hash;
+  }
+
+  uncompressed_list->swap(result);
+  return true;
+}
+
+}  // namespace internal
+
+void SetEVWhitelistFromFile(const base::FilePath& compressed_whitelist_file) {
+  VLOG(1) << "Setting EV whitelist from file: "
+          << compressed_whitelist_file.value();
+  std::string compressed_list;
+  if (!base::ReadFileToString(compressed_whitelist_file, &compressed_list)) {
+    VLOG(1) << "Failed reading from " << compressed_whitelist_file.value();
+    return;
+  }
+
+  std::set<std::string> uncompressed_list;
+  if (!internal::UncompressEVWhitelist(compressed_list, &uncompressed_list)) {
+    VLOG(1) << "Failed uncompressing.";
+    return;
+  }
+  VLOG(1) << "Uncompressing succeeded, hashes: " << uncompressed_list.size();
+
+  internal::SetEVWhitelistData(uncompressed_list);
+}
+
+bool IsCertificateHashInWhitelist(const std::string& certificate_hash) {
+  return g_current_ev_whitelist.Get().find(certificate_hash) !=
+         g_current_ev_whitelist.Get().end();

[wtc, 2014/08/20 02:52:07]

Nit: store the return value of g_current_ev_whitelist.Get() in a local variable
so that it is called only once.

[Eran Messeri, 2014/09/02 12:20:22]

Done.

+}
+
+}  // namespace ct
+
+}  // namespace net