Certificate Transparency: Fetching consistency proofs.

components/certificate_transparency/log_proof_fetcher_unittest.cc

Index: components/certificate_transparency/log_proof_fetcher_unittest.cc
diff --git a/components/certificate_transparency/log_proof_fetcher_unittest.cc b/components/certificate_transparency/log_proof_fetcher_unittest.cc
index 7782f7f0d8372f0f6703a708cad60fb540b98297..29d76f824157385daaedfffd8ba3a265fc3553bc 100644
--- a/components/certificate_transparency/log_proof_fetcher_unittest.cc
+++ b/components/certificate_transparency/log_proof_fetcher_unittest.cc
@@ -7,7 +7,9 @@
 #include <string>
 #include <utility>
 
+#include "base/format_macros.h"
 #include "base/macros.h"
+#include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "components/safe_json/testing_json_parser.h"
 #include "net/base/net_errors.h"
@@ -27,11 +29,11 @@ namespace certificate_transparency {
 
 namespace {
 
-const char kGetSTHHeaders[] =
+const char kGetResponseHeaders[] =
     "HTTP/1.1 200 OK\n"
     "Content-Type: application/json; charset=ISO-8859-1\n";
 
-const char kGetSTHNotFoundHeaders[] =
+const char kGetResponseNotFoundHeaders[] =
     "HTTP/1.1 404 Not Found\n"
     "Content-Type: text/html; charset=iso-8859-1\n";
 
@@ -40,23 +42,30 @@ const char kLogHost[] = "ct.log.example.com";
 const char kLogPathPrefix[] = "somelog";
 const char kLogID[] = "some_id";
 
-class FetchSTHTestJob : public net::URLRequestTestJob {
+// Node returned will be chr(node_id) * 32.
+std::string GetDummyConsistencyProofNode(uint64_t node_id) {
+  return std::string(32, static_cast<char>(node_id));
+}

[Ryan Sleevi, 2016/01/23 00:34:23]

This comment/function is not really clear. Why the static_cast to char? What
makes that safe? Should it just be the low 8 bits, which at least is easier to
reason about/

// Gets a dummy consistency proof for the given |node_id|.
std::string ... {
  // Take the low 8 bits and repeat them as a string. This
  // has no special meaning, other than making it easier to
  // debug which consistency proof was used.
  return std::string(32, static_cast<char>(node_id & 0xFF));
}

By documenting as you have on line 45, it's unclear whether or not that's an API
contract you're intending or is just incidental (the Dummy implies it's
incidental)

[Eran Messeri, 2016/01/25 17:07:38]

Done.

+
+const size_t kDummyConsistencyProofLength = 4;

[Ryan Sleevi, 2016/01/23 00:34:23]

This needs documentation. In looking how it's used, it makes me wonder whether
the documentation on 45 should be expanded to note the distinction between node
and proof here, since until you see how it's used (proof is kDummy...Length
nodes), it's ... non-obvious?

[Eran Messeri, 2016/01/25 17:07:38]

I agree it's non-obvious what this value is for in particular, until you see how
it's used.
Renamed to kDummyConsistencyProofNumNodes - is it clearer?

+
+class LogFetchTestJob : public net::URLRequestTestJob {
  public:
-  FetchSTHTestJob(const std::string& get_sth_data,
-                  const std::string& get_sth_headers,
+  LogFetchTestJob(const std::string& get_log_data,
+                  const std::string& get_log_headers,
                   net::URLRequest* request,
                   net::NetworkDelegate* network_delegate)
       : URLRequestTestJob(request,
                           network_delegate,
-                          get_sth_headers,
-                          get_sth_data,
+                          get_log_headers,
+                          get_log_data,
                           true),
         async_io_(false) {}
 
   void set_async_io(bool async_io) { async_io_ = async_io; }
 
  private:
-  ~FetchSTHTestJob() override {}
+  ~LogFetchTestJob() override {}
 
   bool NextReadAsync() override {
     // Response with indication of async IO only once, otherwise the final
@@ -75,26 +84,24 @@ class FetchSTHTestJob : public net::URLRequestTestJob {
 
   bool async_io_;
 
-  DISALLOW_COPY_AND_ASSIGN(FetchSTHTestJob);
+  DISALLOW_COPY_AND_ASSIGN(LogFetchTestJob);
 };
 
-class GetSTHResponseHandler : public net::URLRequestInterceptor {
+class LogGetResponseHandler : public net::URLRequestInterceptor {
  public:
-  GetSTHResponseHandler()
+  LogGetResponseHandler()
       : async_io_(false),
-        response_body_(""),
         response_headers_(
-            std::string(kGetSTHHeaders, arraysize(kGetSTHHeaders))) {}
-  ~GetSTHResponseHandler() override {}
+            std::string(kGetResponseHeaders, arraysize(kGetResponseHeaders))) {}
+  ~LogGetResponseHandler() override {}
 
   // URLRequestInterceptor implementation:
   net::URLRequestJob* MaybeInterceptRequest(
       net::URLRequest* request,
       net::NetworkDelegate* network_delegate) const override {
-    std::string expected_url = base::StringPrintf(
-        "%s://%s/%s/ct/v1/get-sth", kLogSchema, kLogHost, kLogPathPrefix);
-    EXPECT_EQ(GURL(expected_url), request->url());
-    FetchSTHTestJob* job = new FetchSTHTestJob(
+    EXPECT_EQ(expected_url_, request->url());
+
+    LogFetchTestJob* job = new LogFetchTestJob(
         response_body_, response_headers_, request, network_delegate);
     job->set_async_io(async_io_);
     return job;
@@ -110,68 +117,98 @@ class GetSTHResponseHandler : public net::URLRequestInterceptor {
 
   void set_async_io(bool async_io) { async_io_ = async_io; }
 
+  void set_expected_url(const GURL& url) { expected_url_ = url; }
+
  private:
   bool async_io_;
   std::string response_body_;
   std::string response_headers_;
 
-  DISALLOW_COPY_AND_ASSIGN(GetSTHResponseHandler);
+  // Stored for test body to assert on
+  GURL expected_url_;
+
+  DISALLOW_COPY_AND_ASSIGN(LogGetResponseHandler);
+};
+
+enum InterceptedResultType {
+  NOTHING,
+  FAILURE,
+  STH_FETCH,
+  CONSISTENCY_PROOF_FETCH
 };
 
 class RecordFetchCallbackInvocations {
  public:
   RecordFetchCallbackInvocations(bool expect_success)
       : expect_success_(expect_success),
-        invoked_(false),
         net_error_(net::OK),
-        http_response_code_(-1) {}
+        http_response_code_(-1),
+        request_type_(NOTHING) {}
 
-  void STHFetched(const std::string& log_id,
+  void STHFetched(base::Closure quit_closure,
+                  const std::string& log_id,
                   const net::ct::SignedTreeHead& sth) {
     ASSERT_TRUE(expect_success_);
-    ASSERT_FALSE(invoked_);
-    invoked_ = true;
-    // If expected to succeed, expecting the known_good STH.
-    net::ct::SignedTreeHead expected_sth;
-    net::ct::GetSampleSignedTreeHead(&expected_sth);
+    ASSERT_EQ(NOTHING, request_type_);
+    request_type_ = STH_FETCH;
+    sth_ = sth;
+    log_id_ = log_id;
+    quit_closure.Run();
+  }
 
-    EXPECT_EQ(kLogID, log_id);
-    EXPECT_EQ(expected_sth.version, sth.version);
-    EXPECT_EQ(expected_sth.timestamp, sth.timestamp);
-    EXPECT_EQ(expected_sth.tree_size, sth.tree_size);
-    EXPECT_STREQ(expected_sth.sha256_root_hash, sth.sha256_root_hash);
-    EXPECT_EQ(expected_sth.signature.hash_algorithm,
-              sth.signature.hash_algorithm);
-    EXPECT_EQ(expected_sth.signature.signature_algorithm,
-              sth.signature.signature_algorithm);
-    EXPECT_EQ(expected_sth.signature.signature_data,
-              sth.signature.signature_data);
+  void ConsistencyProofFetched(
+      base::Closure quit_closure,
+      const std::string& log_id,
+      const std::vector<std::string>& consistency_proof) {
+    ASSERT_TRUE(expect_success_);
+    ASSERT_EQ(NOTHING, request_type_);
+    request_type_ = CONSISTENCY_PROOF_FETCH;
+    consistency_proof_.assign(consistency_proof.begin(),
+                              consistency_proof.end());
+    log_id_ = log_id;
+    quit_closure.Run();
   }
 
-  void FetchingFailed(const std::string& log_id,
+  void FetchingFailed(base::Closure quit_closure,
+                      const std::string& log_id,
                       int net_error,
                       int http_response_code) {
     ASSERT_FALSE(expect_success_);
-    ASSERT_FALSE(invoked_);
-    invoked_ = true;
+    ASSERT_EQ(NOTHING, request_type_);
+    request_type_ = FAILURE;
     net_error_ = net_error;
     http_response_code_ = http_response_code;
     if (net_error_ == net::OK) {
       EXPECT_NE(net::HTTP_OK, http_response_code_);
     }
+
+    quit_closure.Run();
   }
 
-  bool invoked() const { return invoked_; }
+  InterceptedResultType intercepted_result_type() const {
+    return request_type_;
+  }
 
   int net_error() const { return net_error_; }
 
   int http_response_code() const { return http_response_code_; }
 
+  const net::ct::SignedTreeHead& intercepted_sth() const { return sth_; }
+
+  const std::string& intercepted_log_id() const { return log_id_; }
+
+  const std::vector<std::string>& intercepted_proof() const {
+    return consistency_proof_;
+  }
+
  private:
   const bool expect_success_;
-  bool invoked_;
   int net_error_;
   int http_response_code_;
+  InterceptedResultType request_type_;
+  net::ct::SignedTreeHead sth_;
+  std::string log_id_;
+  std::vector<std::string> consistency_proof_;
 };
 
 class LogProofFetcherTest : public ::testing::Test {
@@ -181,7 +218,7 @@ class LogProofFetcherTest : public ::testing::Test {
                                     kLogSchema,
                                     kLogHost,
                                     kLogPathPrefix)) {
-    scoped_ptr<GetSTHResponseHandler> handler(new GetSTHResponseHandler());
+    scoped_ptr<LogGetResponseHandler> handler(new LogGetResponseHandler());
     handler_ = handler.get();
 
     net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
@@ -199,24 +236,74 @@ class LogProofFetcherTest : public ::testing::Test {
   void SetValidSTHJSONResponse() {
     std::string sth_json_reply_data = net::ct::GetSampleSTHAsJson();
     handler_->set_response_body(sth_json_reply_data);
+    handler_->set_expected_url(log_url_.Resolve("ct/v1/get-sth"));
   }
 
   void RunFetcherWithCallback(RecordFetchCallbackInvocations* callback) {
     fetcher_->FetchSignedTreeHead(
         log_url_, kLogID,
         base::Bind(&RecordFetchCallbackInvocations::STHFetched,
-                   base::Unretained(callback)),
+                   base::Unretained(callback), run_loop_.QuitClosure()),
+        base::Bind(&RecordFetchCallbackInvocations::FetchingFailed,
+                   base::Unretained(callback), run_loop_.QuitClosure()));
+    run_loop_.Run();
+  }
+
+  void RunGetConsistencyFetcherWithCallback(
+      RecordFetchCallbackInvocations* callback) {
+    const uint64_t kOldTree = 5;
+    const uint64_t kNewTree = 8;
+    handler_->set_expected_url(log_url_.Resolve(base::StringPrintf(
+        "ct/v1/get-sth-consistency?first=%" PRIu64 "&second=%" PRIu64, kOldTree,
+        kNewTree)));
+    fetcher_->FetchConsistencyProof(
+        log_url_, kLogID, kOldTree, kNewTree,
+        base::Bind(&RecordFetchCallbackInvocations::ConsistencyProofFetched,
+                   base::Unretained(callback), run_loop_.QuitClosure()),
         base::Bind(&RecordFetchCallbackInvocations::FetchingFailed,
-                   base::Unretained(callback)));
-    message_loop_.RunUntilIdle();
+                   base::Unretained(callback), run_loop_.QuitClosure()));
+    run_loop_.Run();
   }
 
+  void VerifyReceivedSTH(const std::string& log_id,
+                         const net::ct::SignedTreeHead& sth) {
+    net::ct::SignedTreeHead expected_sth;
+    net::ct::GetSampleSignedTreeHead(&expected_sth);
+
+    EXPECT_EQ(kLogID, log_id);
+    EXPECT_EQ(expected_sth.version, sth.version);
+    EXPECT_EQ(expected_sth.timestamp, sth.timestamp);
+    EXPECT_EQ(expected_sth.tree_size, sth.tree_size);
+    EXPECT_STREQ(expected_sth.sha256_root_hash, sth.sha256_root_hash);
+    EXPECT_EQ(expected_sth.signature.hash_algorithm,
+              sth.signature.hash_algorithm);
+    EXPECT_EQ(expected_sth.signature.signature_algorithm,
+              sth.signature.signature_algorithm);
+    EXPECT_EQ(expected_sth.signature.signature_data,
+              sth.signature.signature_data);
+  }
+
+  void VerifyConsistencyProof(
+      const std::string& log_id,
+      const std::vector<std::string>& consistency_proof) {
+    EXPECT_EQ(kLogID, log_id);
+    EXPECT_EQ(kDummyConsistencyProofLength, consistency_proof.size());
+    for (uint64_t i = 0; i < kDummyConsistencyProofLength; ++i) {
+      EXPECT_EQ(GetDummyConsistencyProofNode(i), consistency_proof[i])
+          << " node: " << i;
+    }
+  }
+
+  // The |message_loop_|, while seemingly unused, is necessary
+  // for URL request interception. That is the message loop that
+  // will be used by the RunLoop.
   base::MessageLoopForIO message_loop_;
+  base::RunLoop run_loop_;
   net::TestURLRequestContext context_;
   safe_json::TestingJsonParser::ScopedFactoryOverride factory_override_;
   scoped_ptr<LogProofFetcher> fetcher_;
   const GURL log_url_;
-  GetSTHResponseHandler* handler_;
+  LogGetResponseHandler* handler_;
 };
 
 TEST_F(LogProofFetcherTest, TestValidGetReply) {
@@ -226,7 +313,8 @@ TEST_F(LogProofFetcherTest, TestValidGetReply) {
 
   RunFetcherWithCallback(&callback);
 
-  ASSERT_TRUE(callback.invoked());
+  ASSERT_EQ(STH_FETCH, callback.intercepted_result_type());
+  VerifyReceivedSTH(callback.intercepted_log_id(), callback.intercepted_sth());
 }
 
 TEST_F(LogProofFetcherTest, TestValidGetReplyAsyncIO) {
@@ -236,7 +324,8 @@ TEST_F(LogProofFetcherTest, TestValidGetReplyAsyncIO) {
   RecordFetchCallbackInvocations callback(true);
   RunFetcherWithCallback(&callback);
 
-  ASSERT_TRUE(callback.invoked());
+  ASSERT_EQ(STH_FETCH, callback.intercepted_result_type());
+  VerifyReceivedSTH(callback.intercepted_log_id(), callback.intercepted_sth());
 }
 
 TEST_F(LogProofFetcherTest, TestInvalidGetReplyIncompleteJSON) {
@@ -244,22 +333,24 @@ TEST_F(LogProofFetcherTest, TestInvalidGetReplyIncompleteJSON) {
       21 /* tree_size */, 123456u /* timestamp */, std::string(),
       std::string());
   handler_->set_response_body(sth_json_reply_data);
+  handler_->set_expected_url(log_url_.Resolve("ct/v1/get-sth"));
 
   RecordFetchCallbackInvocations callback(false);
   RunFetcherWithCallback(&callback);
 
-  ASSERT_TRUE(callback.invoked());
+  ASSERT_EQ(FAILURE, callback.intercepted_result_type());
   EXPECT_EQ(net::ERR_CT_STH_INCOMPLETE, callback.net_error());
 }
 
 TEST_F(LogProofFetcherTest, TestInvalidGetReplyInvalidJSON) {
   std::string sth_json_reply_data = "{\"tree_size\":21,\"timestamp\":}";
   handler_->set_response_body(sth_json_reply_data);
+  handler_->set_expected_url(log_url_.Resolve("ct/v1/get-sth"));
 
   RecordFetchCallbackInvocations callback(false);
   RunFetcherWithCallback(&callback);
 
-  ASSERT_TRUE(callback.invoked());
+  ASSERT_EQ(FAILURE, callback.intercepted_result_type());
   EXPECT_EQ(net::ERR_CT_STH_PARSING_FAILED, callback.net_error());
 }
 
@@ -269,11 +360,12 @@ TEST_F(LogProofFetcherTest, TestLogReplyIsTooLong) {
   sth_json_reply_data.append(
       std::string(LogProofFetcher::kMaxLogResponseSizeInBytes, ' '));
   handler_->set_response_body(sth_json_reply_data);
+  handler_->set_expected_url(log_url_.Resolve("ct/v1/get-sth"));
 
   RecordFetchCallbackInvocations callback(false);
   RunFetcherWithCallback(&callback);
 
-  ASSERT_TRUE(callback.invoked());
+  ASSERT_EQ(FAILURE, callback.intercepted_result_type());
   EXPECT_EQ(net::ERR_FILE_TOO_BIG, callback.net_error());
   EXPECT_EQ(net::HTTP_OK, callback.http_response_code());
 }
@@ -285,25 +377,57 @@ TEST_F(LogProofFetcherTest, TestLogReplyIsExactlyMaxSize) {
       LogProofFetcher::kMaxLogResponseSizeInBytes - sth_json_reply_data.size(),
       ' '));
   handler_->set_response_body(sth_json_reply_data);
+  handler_->set_expected_url(log_url_.Resolve("ct/v1/get-sth"));
 
   RecordFetchCallbackInvocations callback(true);
   RunFetcherWithCallback(&callback);
 
-  ASSERT_TRUE(callback.invoked());
+  ASSERT_EQ(STH_FETCH, callback.intercepted_result_type());
+  VerifyReceivedSTH(callback.intercepted_log_id(), callback.intercepted_sth());
 }
 
 TEST_F(LogProofFetcherTest, TestLogRepliesWithHttpError) {
-  handler_->set_response_headers(
-      std::string(kGetSTHNotFoundHeaders, arraysize(kGetSTHNotFoundHeaders)));
+  handler_->set_response_headers(std::string(
+      kGetResponseNotFoundHeaders, arraysize(kGetResponseNotFoundHeaders)));
+  handler_->set_expected_url(log_url_.Resolve("ct/v1/get-sth"));
 
   RecordFetchCallbackInvocations callback(false);
   RunFetcherWithCallback(&callback);
 
-  ASSERT_TRUE(callback.invoked());
+  ASSERT_EQ(FAILURE, callback.intercepted_result_type());
   EXPECT_EQ(net::OK, callback.net_error());
   EXPECT_EQ(net::HTTP_NOT_FOUND, callback.http_response_code());
 }
 
+TEST_F(LogProofFetcherTest, TestValidGetConsistencyValidReply) {
+  std::vector<std::string> proof;
+  for (uint64_t i = 0; i < kDummyConsistencyProofLength; ++i)
+    proof.push_back(GetDummyConsistencyProofNode(i));
+
+  std::string consistency_proof_reply_data =
+      net::ct::CreateConsistencyProofJsonString(proof);
+  handler_->set_response_body(consistency_proof_reply_data);
+
+  RecordFetchCallbackInvocations callback(true);
+  RunGetConsistencyFetcherWithCallback(&callback);
+
+  ASSERT_EQ(CONSISTENCY_PROOF_FETCH, callback.intercepted_result_type());
+  VerifyConsistencyProof(callback.intercepted_log_id(),
+                         callback.intercepted_proof());
+}
+
+TEST_F(LogProofFetcherTest, TestInvalidGetConsistencyReplyInvalidJSON) {
+  std::string consistency_proof_reply_data = "{\"consistency\": [1,2]}";
+  handler_->set_response_body(consistency_proof_reply_data);
+
+  RecordFetchCallbackInvocations callback(false);
+  RunGetConsistencyFetcherWithCallback(&callback);
+
+  ASSERT_EQ(FAILURE, callback.intercepted_result_type());
+  EXPECT_EQ(net::ERR_CT_CONSISTENCY_PROOF_PARSING_FAILED, callback.net_error());
+  EXPECT_EQ(net::HTTP_OK, callback.http_response_code());
+}
+
 }  // namespace
 
 }  // namespace certificate_transparency