Certificate Transparency: Notify STH Observers of known STHs

net/cert/sth_distributor_unittest.cc

+// 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.
+
+#include "net/cert/sth_distributor.h"
+
+#include <map>
+#include <string>
+
+#include "base/test/histogram_tester.h"
+#include "crypto/sha2.h"
+#include "net/cert/signed_tree_head.h"
+#include "net/cert/sth_observer.h"
+#include "net/test/ct_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace ct {
+
+namespace {
+
+// An STHObserver implementation that simply stores all
+// observed STHs, keyed by log ID.
+class StoringSTHObserver : public STHObserver {
+ public:
+  void NewSTHObserved(const SignedTreeHead& sth) override {
+    sths[sth.log_id] = sth;
+  }
+
+  std::map<std::string, SignedTreeHead> sths;
+};
+
+class STHDistributorTest : public ::testing::Test {
+ public:
+  STHDistributorTest() {}
+
+  void SetUp() override {
+    ASSERT_TRUE(GetSampleSignedTreeHead(&sample_sth_));
+    sample_sth_.log_id = GetTestPublicKeyId();
+  }
+
+ protected:
+  STHDistributor distributor_;
+  SignedTreeHead sample_sth_;
+};
+
+TEST_F(STHDistributorTest, NotifiesOfExistingSTHs) {
+  const std::string other_log = "another log";
+  SignedTreeHead second_sth(sample_sth_);
+  second_sth.log_id = other_log;
+
+  distributor_.NewSTHObserved(sample_sth_);
+  distributor_.NewSTHObserved(second_sth);
+
+  StoringSTHObserver observer;
+  distributor_.RegisterObserver(&observer);
+
+  // Check that two STHs from different logs received prior to observer
+  // registration were reported to the observer once registered.
+  EXPECT_EQ(2u, observer.sths.size());
+  EXPECT_EQ(1u, observer.sths.count(other_log));
+}
+
+TEST_F(STHDistributorTest, LogsUMAForPilotSTH) {
+  const char kPilotSTHAgeHistogram[] =
+      "Net.CertificateTransparency.PilotSTHAge";
+  base::HistogramTester histograms;
+  histograms.ExpectTotalCount(kPilotSTHAgeHistogram, 0);
+
+  const uint8_t kPilotLogID[] = {
+      0xa4, 0xb9, 0x09, 0x90, 0xb4, 0x18, 0x58, 0x14, 0x87, 0xbb, 0x13,
+      0xa2, 0xcc, 0x67, 0x70, 0x0a, 0x3c, 0x35, 0x98, 0x04, 0xf9, 0x1b,
+      0xdf, 0xb8, 0xe3, 0x77, 0xcd, 0x0e, 0xc8, 0x0d, 0xdc, 0x10};
+  sample_sth_.log_id = std::string(reinterpret_cast<const char*>(kPilotLogID),
+                                   crypto::kSHA256Length);
+
+  distributor_.NewSTHObserved(sample_sth_);
+  histograms.ExpectTotalCount(kPilotSTHAgeHistogram, 1);
+}
+
+TEST_F(STHDistributorTest, UpdatesObservedSTHData) {
+  distributor_.NewSTHObserved(sample_sth_);
+  sample_sth_.tree_size = 23u;

[Ryan Sleevi, 2016/05/17 16:52:51]

DESIGN: You should set a value for .tree_size before this test, otherwise this
test is tightly coupled to GetSampleSignedTreeHead, and I don't see any API
contract that it's setting tree_size

e.g.

sample_sth.tree_size = 22u;
NewSTHObserved(...)
// Increase the tree size, but not the timestamp, and ensure that the newer STH
is processed.
sample_sth.tree_size = 23u;
NewSTHObserved(...)

Seems you should also write separate tests for the other conditions - that is,
you have [same timestamp+new tree size], [newer timestamp+same tree size],
[older timestamp + same tree size], [older timestamp + larger tree size - e.g.
the log is naughty], [older timestamp + smaller tree size]

This suggests that it may be useful to factor into a test that covers a set of
test data permutations - the old/new treesize and the old/new timestamp - and
goes through each to make sure that the appropriate thing happens.


Alternatively, if this logic is not part of the STHDistributor, then making sure
that in all cases it just takes the 'latest' (last observed) STH is also fine.
But as it stands, changing the tree size and doing the test is unclear about the
value, and seems to imply a semantic relationship when there is none - either we
test that relationship, or we make it explicit that the relationship is
irrelevant (by doing the 'worst thing possible' - that violates all the
preconditions - and seeing that it still updates to that STH)

[Eran Messeri, 2016/05/17 17:04:26]

What I've done is stored the value of sample_sth_.tree_size, increased it by one
after calling NewSTHObserved once and checked (in EXPECT_EQ) that the tree size
of the observed STH is the original sample_sth_.tree_size + 1.

As the STHDistributor does not have any logic for dropping updates of smaller /
older STHs there's no benefit in testing it here.

+  distributor_.NewSTHObserved(sample_sth_);
+
+  StoringSTHObserver observer;
+  distributor_.RegisterObserver(&observer);
+  EXPECT_EQ(1u, observer.sths.size());
+  EXPECT_EQ(23u, observer.sths[GetTestPublicKeyId()].tree_size);
+}
+
+}  // namespace
+
+}  // namespace ct
+
+}  // namespace net