Certificate Transparency: Fetching of Signed Tree Heads (DRAFT)

components/certificate_transparency/log_proofs_fetcher.h

+// 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.
+
+#ifndef COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_PROOFS_FETCHER_H_
+#define COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_PROOFS_FETCHER_H_
+
+#include <map>
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "net/base/io_buffer.h"
+#include "net/url_request/url_request.h"
+
+namespace base {
+class Value;
+}
+
+namespace net {
+class URLRequestContext;
+class CTLogVerifier;
+
+namespace ct {
+struct SignedTreeHead;
+}  // namespace ct
+
+}  // namespace net
+
+namespace certificate_transparency {
+

[davidben, 2015/05/07 21:59:38]

Comment describing what this class does.

+class LogProofsFetcher : public net::URLRequest::Delegate {
+ public:

[davidben, 2015/05/07 21:59:38]

Document that the LogProofsFetcher cannot outlive request_context.

+  explicit LogProofsFetcher(net::URLRequestContext* request_context);
+
+  ~LogProofsFetcher() override;
+

[davidben, 2015/05/07 21:59:37]

Comment for what this function does.

+  void FetchSTH(net::CTLogVerifier* verifier);

[davidben, 2015/05/07 21:59:38]

Nit: STH -> SignedTreeHead? Most APIs seem to use the full name.

+
+  // net::URLRequest::Delegate
+  void OnResponseStarted(net::URLRequest* request) override;
+  void OnReadCompleted(net::URLRequest* request, int bytes_read) override;
+
+ private:
+  scoped_ptr<net::URLRequest> CreateURLRequest(GURL fetch_sth_url);
+
+  // Performs post-report cleanup.
+  void RequestComplete(net::URLRequest* request, int bytes_read);
+  void RequestCleanup(net::URLRequest* request);
+
+  struct NET_EXPORT_PRIVATE FetchParams {

[davidben, 2015/05/07 21:59:38]

Why is this NET_EXPORT_PRIVATE? It's not in net and doesn't need to be exported
at all.

[Eran Messeri, 2015/06/18 15:18:43]

Done.

+    explicit FetchParams(net::CTLogVerifier* verifier);
+    ~FetchParams();
+
+    net::CTLogVerifier* verifier;
+    scoped_refptr<net::IOBufferWithSize> sth_buffer;
+    int read_bytes;
+  };
+
+  typedef std::map<net::URLRequest*, FetchParams*> RequestsMap;
+
+  net::URLRequestContext* const request_context_;
+
+  // Owns the contained requests.
+  RequestsMap inflight_requests_;
+
+  DISALLOW_COPY_AND_ASSIGN(LogProofsFetcher);
+};
+
+class CTLogResponseParser {

[davidben, 2015/05/07 21:59:38]

We don't usually create empty classes for naming. Either use a namespace or have
the function sit on its own. (There's only one, so maybe just have it sit on its
own.)

Actually... does this function not already exist as
net/cert/ct_log_response_parser.h?

+ public:
+  // Extracts STH from the json struct

[davidben, 2015/05/07 21:59:38]

Period at the end. Mention that it returns false on parse error.

+  static bool FillSignedTreeHead(const base::StringPiece& json_sth,
+                                 net::ct::SignedTreeHead* sth);
+};

[davidben, 2015/05/07 21:59:38]

Probably wants to be in a separate header / source file as it's not really part
of the LogProofsFetcher.

+
+}  // namespace certificate_transparency
+
+#endif  // COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_PROOFS_FETCHER_H_