Certificate Transparency: Fetching consistency proofs.

components/certificate_transparency/log_proof_fetcher.cc

Index: components/certificate_transparency/log_proof_fetcher.cc
diff --git a/components/certificate_transparency/log_proof_fetcher.cc b/components/certificate_transparency/log_proof_fetcher.cc
index 7e51858708b0acb5d60dbd550cba646ff6ac5ab6..858000c5eda2ebcbbf08e3a83d51e03daa19981a 100644
--- a/components/certificate_transparency/log_proof_fetcher.cc
+++ b/components/certificate_transparency/log_proof_fetcher.cc
@@ -6,9 +6,11 @@
 
 #include <iterator>
 
+#include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "components/safe_json/safe_json_parser.h"
 #include "net/base/io_buffer.h"
@@ -40,28 +42,113 @@ int GetNetErrorFromURLRequestStatus(const net::URLRequestStatus& status) {
   }
 }
 
+class LogRequest {
+ public:
+  LogRequest(const LogProofFetcher::FetchFailedCallback& failure_callback);

[mmenke, 2015/12/01 16:23:49]

explicit

[Eran Messeri, 2015/12/04 11:41:44]

N/A anymore.

+  virtual ~LogRequest();
+
+  virtual void HandleParsedJson(const std::string& log_id,
+                                const base::Value& parsed_json) = 0;
+
+  virtual void HandleFailure(const std::string& log_id,
+                             int net_error,
+                             int http_response_code);
+
+  virtual const std::string& RequestSuffix() = 0;
+
+ protected:
+  LogProofFetcher::FetchFailedCallback failure_callback_;

[mmenke, 2015/12/01 16:23:49]

const?

[Eran Messeri, 2015/12/04 11:41:44]

Done.

+};
+
+LogRequest::LogRequest(
+    const LogProofFetcher::FetchFailedCallback& failure_callback)
+    : failure_callback_(failure_callback) {
+  DCHECK(!failure_callback_.is_null());
+}
+
+LogRequest::~LogRequest() {}
+
+void LogRequest::HandleFailure(const std::string& log_id,
+                               int net_error,
+                               int http_response_code) {
+  failure_callback_.Run(log_id, net_error, http_response_code);
+}
+
+class GetSTHLogRequest : public LogRequest {
+ public:
+  GetSTHLogRequest(
+      const LogProofFetcher::SignedTreeHeadFetchedCallback& sth_fetch_callback,
+      const LogProofFetcher::FetchFailedCallback& failure_callback)
+      : LogRequest(failure_callback), sth_fetched_(sth_fetch_callback) {}
+
+  void HandleParsedJson(const std::string& log_id,
+                        const base::Value& parsed_json) override {
+    net::ct::SignedTreeHead signed_tree_head;
+    if (net::ct::FillSignedTreeHead(parsed_json, &signed_tree_head)) {
+      sth_fetched_.Run(log_id, signed_tree_head);
+    } else {
+      failure_callback_.Run(log_id, net::ERR_CT_STH_INCOMPLETE, net::HTTP_OK);
+    }
+  }
+
+  const std::string& RequestSuffix() override {
+    static const std::string get_sth("ct/v1/get-sth");
+    return get_sth;
+  }
+
+ private:
+  LogProofFetcher::SignedTreeHeadFetchedCallback sth_fetched_;
+};
+
+class GetConsistencyProofLogRequest : public LogRequest {
+ public:
+  GetConsistencyProofLogRequest(
+      uint64_t first_tree_size,
+      uint64_t second_tree_size,
+      const LogProofFetcher::ConsistencyProofFetchedCallback&
+          proof_fetch_callback,
+      const LogProofFetcher::FetchFailedCallback& failure_callback)
+      : LogRequest(failure_callback), proof_fetched_(proof_fetch_callback) {
+    fetch_suffix_ =
+        base::StringPrintf("ct/v1/get-sth-consistency?first=%lu&second=%lu",
+                           first_tree_size, second_tree_size);
+  }
+
+  void HandleParsedJson(const std::string& log_id,
+                        const base::Value& parsed_json) override {
+    std::vector<std::string> consistency_proof;
+    if (net::ct::FillConsistencyProof(parsed_json, &consistency_proof)) {
+      proof_fetched_.Run(log_id, consistency_proof);
+    } else {
+      failure_callback_.Run(
+          log_id, net::ERR_CT_CONSISTENCY_PROOF_PARSING_FAILED, net::HTTP_OK);
+    }
+  }
+
+  const std::string& RequestSuffix() override { return fetch_suffix_; }
+
+ private:
+  LogProofFetcher::ConsistencyProofFetchedCallback proof_fetched_;
+  std::string fetch_suffix_;
+};
+
 }  // namespace
 
 struct LogProofFetcher::FetchState {
-  FetchState(const std::string& log_id,
-             const SignedTreeHeadFetchedCallback& fetched_callback,
-             const FetchFailedCallback& failed_callback);
+  // Takes ownership of the |request_handler|
+  FetchState(const std::string& log_id, LogRequest* request_handler);

[mmenke, 2015/12/01 16:23:49]

Do we really need both a FetchState and a LogRequest?  Seems like the request
itself could store everything.

[Eran Messeri, 2015/12/04 11:41:44]

As discussed offline, I think the separation between handling the interaction
with URLRequest and dealing with the fully-assembled response should be kept for
clarity.
Now LogFetcher (which replaces FetchState) implements URLRequest::Delegate and
handles assembling the response, the LogResponseHandler handles request-specific
parsing.

   ~FetchState();
 
   std::string log_id;
-  SignedTreeHeadFetchedCallback fetched_callback;
-  FetchFailedCallback failed_callback;
+  scoped_ptr<LogRequest> request_handler;
   scoped_refptr<net::IOBufferWithSize> response_buffer;
   std::string assembled_response;
 };
 
-LogProofFetcher::FetchState::FetchState(
-    const std::string& log_id,
-    const SignedTreeHeadFetchedCallback& fetched_callback,
-    const FetchFailedCallback& failed_callback)
+LogProofFetcher::FetchState::FetchState(const std::string& log_id,
+                                        LogRequest* request_handler)
     : log_id(log_id),
-      fetched_callback(fetched_callback),
-      failed_callback(failed_callback),
+      request_handler(request_handler),
       response_buffer(new net::IOBufferWithSize(kMaxLogResponseSizeInBytes)) {}
 
 LogProofFetcher::FetchState::~FetchState() {}
@@ -81,16 +168,39 @@ void LogProofFetcher::FetchSignedTreeHead(
     const std::string& log_id,
     const SignedTreeHeadFetchedCallback& fetched_callback,
     const FetchFailedCallback& failed_callback) {
+  FetchState* fetch_state = new FetchState(
+      log_id, new GetSTHLogRequest(fetched_callback, failed_callback));
+  FetchFromLog(base_log_url, log_id, fetch_state);
+}
+
+void LogProofFetcher::FetchConsistencyProof(
+    const GURL& base_log_url,
+    const std::string& log_id,
+    uint64_t old_tree_size,
+    uint64_t new_tree_size,
+    const ConsistencyProofFetchedCallback& fetched_callback,
+    const FetchFailedCallback& failed_callback) {
+  FetchState* fetch_state = new FetchState(
+      log_id,
+      new GetConsistencyProofLogRequest(old_tree_size, new_tree_size,
+                                        fetched_callback, failed_callback));
+  FetchFromLog(base_log_url, log_id, fetch_state);
+}
+
+void LogProofFetcher::FetchFromLog(const GURL& base_log_url,
+                                   const std::string& log_id,
+                                   FetchState* fetch_state) {
   DCHECK(base_log_url.SchemeIsHTTPOrHTTPS());
-  GURL fetch_url(base_log_url.Resolve("ct/v1/get-sth"));
+
+  GURL request_url =
+      base_log_url.Resolve(fetch_state->request_handler->RequestSuffix());
+
   scoped_ptr<net::URLRequest> request =
-      request_context_->CreateRequest(fetch_url, net::DEFAULT_PRIORITY, this);
+      request_context_->CreateRequest(request_url, net::DEFAULT_PRIORITY, this);
   request->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
                         net::LOAD_DO_NOT_SAVE_COOKIES |
                         net::LOAD_DO_NOT_SEND_AUTH_DATA);
 
-  FetchState* fetch_state =
-      new FetchState(log_id, fetched_callback, failed_callback);
   request->Start();
   inflight_requests_.insert(std::make_pair(request.release(), fetch_state));
 }
@@ -187,10 +297,10 @@ void LogProofFetcher::RequestComplete(net::URLRequest* request) {
   fetch_state->response_buffer = nullptr;
   safe_json::SafeJsonParser::Parse(
       fetch_state->assembled_response,
-      base::Bind(&LogProofFetcher::OnSTHJsonParseSuccess,
+      base::Bind(&LogProofFetcher::OnJsonParseSuccess,
                  weak_factory_.GetWeakPtr(), request),
-      base::Bind(&LogProofFetcher::OnSTHJsonParseError,
-                 weak_factory_.GetWeakPtr(), request));
+      base::Bind(&LogProofFetcher::OnJsonParseError, weak_factory_.GetWeakPtr(),
+                 request));
 }
 
 void LogProofFetcher::CleanupRequest(net::URLRequest* request) {
@@ -212,30 +322,24 @@ void LogProofFetcher::InvokeFailureCallback(net::URLRequest* request,
   auto it = inflight_requests_.find(request);
   FetchState* fetch_state = it->second;
 
-  fetch_state->failed_callback.Run(fetch_state->log_id, net_error,
-                                   http_response_code);
+  fetch_state->request_handler->HandleFailure(fetch_state->log_id, net_error,
+                                              http_response_code);
   CleanupRequest(request);
 }
 
-void LogProofFetcher::OnSTHJsonParseSuccess(
-    net::URLRequest* request,
-    scoped_ptr<base::Value> parsed_json) {
+void LogProofFetcher::OnJsonParseSuccess(net::URLRequest* request,
+                                         scoped_ptr<base::Value> parsed_json) {
   DCHECK(inflight_requests_.count(request));
 
   FetchState* fetch_state = inflight_requests_.find(request)->second;
-  net::ct::SignedTreeHead signed_tree_head;
-  if (net::ct::FillSignedTreeHead(*parsed_json.get(), &signed_tree_head)) {
-    fetch_state->fetched_callback.Run(fetch_state->log_id, signed_tree_head);
-  } else {
-    fetch_state->failed_callback.Run(fetch_state->log_id,
-                                     net::ERR_CT_STH_INCOMPLETE, net::HTTP_OK);
-  }
+  fetch_state->request_handler->HandleParsedJson(fetch_state->log_id,
+                                                 *parsed_json);
 
   CleanupRequest(request);
 }
 
-void LogProofFetcher::OnSTHJsonParseError(net::URLRequest* request,
-                                          const std::string& error) {
+void LogProofFetcher::OnJsonParseError(net::URLRequest* request,
+                                       const std::string& error) {
   InvokeFailureCallback(request, net::ERR_CT_STH_PARSING_FAILED, net::HTTP_OK);
 }