Certificate Transparency: Fetching consistency proofs.

components/certificate_transparency/log_proof_fetcher_unittest.cc

 // Copyright 2015 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 "components/certificate_transparency/log_proof_fetcher.h"
 
 #include <string>
 
 #include "base/strings/stringprintf.h"
 #include "components/safe_json/testing_json_parser.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate.h"
 #include "net/cert/signed_tree_head.h"
 #include "net/http/http_status_code.h"
 #include "net/test/ct_test_util.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_filter.h"
 #include "net/url_request/url_request_interceptor.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_test_job.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 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";
 
 const char kLogSchema[] = "https";
 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(size_t node_id) {
+  return std::string(32, static_cast<char>(node_id));
+}
+
+const size_t kDummyConsistencyProofLength = 4;
+
+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
     // Read would (incorrectly) be classified as async, causing the
     // URLRequestJob to try reading another time and failing on a CHECK
     // that the raw_read_buffer_ is not null.
     // According to mmenke@, this is a bug in the URLRequestTestJob code.
     // TODO(eranm): Once said bug is fixed, switch most tests to using async
     // IO.
     if (async_io_) {
       async_io_ = false;
       return true;
     }
     return false;
   }
 
   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_(""),

[mmenke, 2015/11/18 19:25:16]

nit:  Not needed (Know it was like that before)

[Eran Messeri, 2015/11/24 22:53:38]

Done.

         response_headers_(
-            std::string(kGetSTHHeaders, arraysize(kGetSTHHeaders))) {}
-  ~GetSTHResponseHandler() override {}
+            std::string(kGetResponseHeaders, arraysize(kGetResponseHeaders))),
+        expected_old_tree_size_(0),
+        expected_new_tree_size_(0) {}
+  ~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);
+    std::string base_expected_url = base::StringPrintf(
+        "%s://%s/%s/ct/v1/", kLogSchema, kLogHost, kLogPathPrefix);

[mmenke, 2015/11/18 19:25:16]

I don't think we want logic in here that matches the logic in production code to
deduce the expected URL.  Suggest adding a setter, and explicitly calling it in
tests.

Alternatively, could make an accessor for the URL we end up fetching, and put
the EXPECT in the test body (Or in RunGetConsistencyFetcherWithCallback /
RunFetcherWithCallback) - think that makes for clearer tests.

[Eran Messeri, 2015/11/24 22:53:38]

Done - by adding a setter, as the MaybeInterceptRequest method is const, I can't
change any fields.

[mmenke, 2015/11/25 17:40:28]

You can always use mutable.  Anyhow, fine as-is.

[Eran Messeri, 2015/11/26 22:07:12]

Acknowledged.

+
+    std::string expected_url;
+    if (expected_old_tree_size_ == 0 && expected_new_tree_size_ == 0) {
+      // Expecting get-sth
+      expected_url = base_expected_url + std::string("get-sth");
+    } else {
+      // Expecting get-consistency-proof
+      expected_url = base_expected_url +
+                     base::StringPrintf(
+                         "get-sth-consistency?first=%lu&second=%lu",
+                         static_cast<unsigned long>(expected_old_tree_size_),
+                         static_cast<unsigned long>(expected_new_tree_size_));
+    }
     EXPECT_EQ(GURL(expected_url), request->url());
-    FetchSTHTestJob* job = new FetchSTHTestJob(
+    LogFetchTestJob* job = new LogFetchTestJob(
         response_body_, response_headers_, request, network_delegate);
     job->set_async_io(async_io_);
     return job;
   }
 
   void set_response_body(const std::string& response_body) {
     response_body_ = response_body;
   }
 
   void set_response_headers(const std::string& response_headers) {
     response_headers_ = response_headers;
   }
 
+  void set_expect_get_consistency_proof(size_t expected_old_tree_size,
+                                        size_t expected_new_tree_size) {
+    expected_old_tree_size_ = expected_old_tree_size;
+    expected_new_tree_size_ = expected_new_tree_size;
+  }
+
   void set_async_io(bool async_io) { async_io_ = async_io; }
 
  private:
   bool async_io_;
   std::string response_body_;
   std::string response_headers_;
 
-  DISALLOW_COPY_AND_ASSIGN(GetSTHResponseHandler);
+  size_t expected_old_tree_size_;
+  size_t expected_new_tree_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(LogGetResponseHandler);
 };
 
 class RecordFetchCallbackInvocations {
  public:
   RecordFetchCallbackInvocations(bool expect_success)
       : expect_success_(expect_success),
         invoked_(false),
         net_error_(net::OK),
         http_response_code_(-1) {}
 
@@ skipping 12 matching lines @@
     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(
+      const std::string& log_id,
+      const std::vector<std::string>& consistency_proof) {
+    ASSERT_TRUE(expect_success_);
+    ASSERT_FALSE(invoked_);
+    invoked_ = true;

[mmenke, 2015/11/18 19:25:15]

Record which method was invoked?

[Eran Messeri, 2015/11/24 22:53:38]

Done - that made the invoked_ field obsolete so I've removed it.

+    EXPECT_EQ(kDummyConsistencyProofLength, consistency_proof.size());
+    for (size_t i = 0; i < kDummyConsistencyProofLength; ++i) {
+      EXPECT_EQ(GetDummyConsistencyProofNode(i), consistency_proof[i])
+          << " node: " << i;

[mmenke, 2015/11/18 19:25:16]

Again, think it's cleaner to store these in the test fixture, and then make the
test body check them.

[Eran Messeri, 2015/11/24 22:53:38]

Done - both for the consistency proof and the STH fetching.

+    }
+  }
+
   void FetchingFailed(const std::string& log_id,
                       int net_error,
                       int http_response_code) {
     ASSERT_FALSE(expect_success_);
     ASSERT_FALSE(invoked_);
     invoked_ = true;
     net_error_ = net_error;
     http_response_code_ = http_response_code;
     if (net_error_ == net::OK) {
       EXPECT_NE(net::HTTP_OK, http_response_code_);
@@ skipping 13 matching lines @@
   int http_response_code_;
 };
 
 class LogProofFetcherTest : public ::testing::Test {
  public:
   LogProofFetcherTest()
       : log_url_(base::StringPrintf("%s://%s/%s/",
                                     kLogSchema,
                                     kLogHost,
                                     kLogPathPrefix)) {
-    scoped_ptr<GetSTHResponseHandler> handler(new GetSTHResponseHandler());
+    scoped_ptr<LogGetResponseHandler> handler(new LogGetResponseHandler());
     handler_ = handler.get();
 
     net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
         kLogSchema, kLogHost, handler.Pass());
 
     fetcher_.reset(new LogProofFetcher(&context_));
   }
 
   ~LogProofFetcherTest() override {
     net::URLRequestFilter::GetInstance()->RemoveHostnameHandler(kLogSchema,
                                                                 kLogHost);
   }
 
  protected:
   void SetValidSTHJSONResponse() {
     std::string sth_json_reply_data = net::ct::GetSampleSTHAsJson();
     handler_->set_response_body(sth_json_reply_data);
   }
 
   void RunFetcherWithCallback(RecordFetchCallbackInvocations* callback) {
     fetcher_->FetchSignedTreeHead(
         log_url_, kLogID,
         base::Bind(&RecordFetchCallbackInvocations::STHFetched,
                    base::Unretained(callback)),
         base::Bind(&RecordFetchCallbackInvocations::FetchingFailed,
                    base::Unretained(callback)));
     message_loop_.RunUntilIdle();

[mmenke, 2015/11/18 19:25:16]

optional:  I did sign off on this before...But think it would be cleaner to use
a RunLoop invoked by RecordFetchCallbackInvocations, rather than rely on
everything happening without any intervening idle time.  The disadvantage is on
breakage, the test hangs.  The advantage is it's more flexible, and doesn't
break if someone changes how the test works (Switches to a real test server, or
whatever).

[Eran Messeri, 2015/11/24 22:53:38]

IIRC there was a problem with the async_io case - I'll dig up the bug number
shortly.

   }
 
+  void RunGetConsistencyFetcherWithCallback(
+      RecordFetchCallbackInvocations* callback) {
+    const size_t kOldTree = 5;
+    const size_t kNewTree = 8;
+    handler_->set_expect_get_consistency_proof(kOldTree, kNewTree);
+    fetcher_->FetchConsistencyProof(
+        log_url_, kLogID, kOldTree, kNewTree,
+        base::Bind(&RecordFetchCallbackInvocations::ConsistencyProofFetched,
+                   base::Unretained(callback)),
+        base::Bind(&RecordFetchCallbackInvocations::FetchingFailed,
+                   base::Unretained(callback)));
+    message_loop_.RunUntilIdle();
+  }
+
   base::MessageLoopForIO message_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) {
   SetValidSTHJSONResponse();
 
   RecordFetchCallbackInvocations callback(true);
 
   RunFetcherWithCallback(&callback);
 
   ASSERT_TRUE(callback.invoked());
@@ skipping 56 matching lines @@
       ' '));
   handler_->set_response_body(sth_json_reply_data);
 
   RecordFetchCallbackInvocations callback(true);
   RunFetcherWithCallback(&callback);
 
   ASSERT_TRUE(callback.invoked());
 }
 
 TEST_F(LogProofFetcherTest, TestLogRepliesWithHttpError) {
-  handler_->set_response_headers(
-      std::string(kGetSTHNotFoundHeaders, arraysize(kGetSTHNotFoundHeaders)));
+  handler_->set_response_headers(std::string(
+      kGetResponseNotFoundHeaders, arraysize(kGetResponseNotFoundHeaders)));
 
   RecordFetchCallbackInvocations callback(false);
   RunFetcherWithCallback(&callback);
 
   ASSERT_TRUE(callback.invoked());
   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 (size_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_TRUE(callback.invoked());
+}
+
+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_TRUE(callback.invoked());
+  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