Add CertIssuerSourceAia: authorityInfoAccess fetching for CertPathBuilder.

net/cert/internal/cert_issuer_source_aia.cc

Index: net/cert/internal/cert_issuer_source_aia.cc
diff --git a/net/cert/internal/cert_issuer_source_aia.cc b/net/cert/internal/cert_issuer_source_aia.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ee40205ba31ad17dea3738df447b684dc1bc5f1e
--- /dev/null
+++ b/net/cert/internal/cert_issuer_source_aia.cc
@@ -0,0 +1,125 @@
+// 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/internal/cert_issuer_source_aia.h"
+
+#include "base/bind.h"
+#include "net/cert/internal/parsed_certificate.h"
+#include "url/gurl.h"
+
+namespace net {
+
+namespace {
+
+// TODO(mattm): These are arbitrary choices. Re-evaluate.
+const int kTimeoutMilliseconds = 10000;
+const int kMaxResponseBytes = 65536;
+
+}  // namespace
+
+CertIssuerSourceAia::CertIssuerSourceAia(CertNetFetcher* cert_fetcher)
+    : cert_fetcher_(cert_fetcher) {}
+
+CertIssuerSourceAia::~CertIssuerSourceAia() = default;
+
+void CertIssuerSourceAia::SyncGetIssuersOf(
+    const ParsedCertificate* cert,
+    std::vector<scoped_refptr<ParsedCertificate>>* issuers) {
+  // CertIssuerSourceAia never returns synchronous results.
+}
+
+void CertIssuerSourceAia::AsyncGetIssuersOf(
+    const ParsedCertificate* cert,
+    const IssuerCallback& issuers_callback,
+    std::unique_ptr<Request>* out_req) {
+  if (!cert->has_authority_info_access())
+    return;
+
+  // RFC 5280 section 4.2.2.1:
+  //
+  //    An authorityInfoAccess extension may include multiple instances of
+  //    the id-ad-caIssuers accessMethod.  The different instances may
+  //    specify different methods for accessing the same information or may
+  //    point to different information.
+
+  std::vector<GURL> urls;
+  for (const auto& uri : cert->ca_issuers_uris()) {
+    GURL url(uri);
+    // XXX should HTTPS be disallowed?

[eroman, 2016/06/03 16:28:52]

XXX --> TODO(...)

[mattm, 2016/06/03 21:27:26]

Acknowledged.

+    // XXX should FTP be allowed?
+    if (url.is_valid() && url.SchemeIsHTTPOrHTTPS())

[eroman, 2016/06/03 16:28:52]

We only support http:// for the fetching of AIA [1]

Shouldn't encode that fetching policy in here, but rather leave it to the
CertNetFetcher implementation.
Possible solutions:
  * Simply pass all URLs to the fetcher and let it fail them if it doesn't like
  * Add a synchronous failure mode for FetchCaIssuers()
  * Add a method to CertNetFetcher's interface similar to
"SupportsFetchingUrl(GURL)"

Of those I suggest the first option for this CL.
The second option might be reasonable, but as a separate change.
And the third option I don't much care fore.

[1]
https://cs.chromium.org/chromium/src/net/cert_net/cert_net_fetcher_impl.cc?sq...

[mattm, 2016/06/03 21:27:26]

Agreed. I've done #1 for this CL.

+      urls.push_back(url);
+  }
+  if (urls.empty())

[eroman, 2016/06/03 16:28:52]

I think we also should have a failsafe that bounds how many AIA's we will
consider for a single certificate.

We can run some numbers to see what is reasonable, but I would imagine 5-10 is
already more than will be used in practice

[mattm, 2016/06/03 21:27:26]

Done.

+    return;
+
+  std::unique_ptr<AiaRequest> aia_request(new AiaRequest(issuers_callback));
+
+  for (const auto& url : urls) {
+    aia_request->AddCertFetcherRequest(cert_fetcher_->FetchCaIssuers(
+        url, kTimeoutMilliseconds, kMaxResponseBytes,
+        base::Bind(&AiaRequest::FetchCallback,
+                   base::Unretained(aia_request.get()))));
+  }
+
+  *out_req = std::move(aia_request);
+}
+
+CertIssuerSourceAia::AiaRequest::AiaRequest(
+    const IssuerCallback& issuers_callback)
+    : issuers_callback_(issuers_callback) {}
+
+CertIssuerSourceAia::AiaRequest::~AiaRequest() = default;
+
+CompletionStatus CertIssuerSourceAia::AiaRequest::GetNext(
+    scoped_refptr<ParsedCertificate>* out_cert) {
+  if (current_result_ < results_.size()) {
+    *out_cert = std::move(results_[current_result_++]);
+    return CompletionStatus::SYNC;
+  }
+  *out_cert = nullptr;
+  if (pending_requests_)
+    return CompletionStatus::ASYNC;
+  return CompletionStatus::SYNC;
+}
+
+void CertIssuerSourceAia::AiaRequest::AddCertFetcherRequest(
+    std::unique_ptr<CertNetFetcher::Request> cert_fetcher_request) {
+  cert_fetcher_requests_.push_back(std::move(cert_fetcher_request));
+  pending_requests_++;
+}
+
+void CertIssuerSourceAia::AiaRequest::FetchCallback(
+    Error error,
+    const std::vector<uint8_t>& fetched_bytes) {
+  DCHECK(pending_requests_ > 0);
+  pending_requests_--;
+  bool client_waiting_for_callback = current_result_ >= results_.size();
+  if (error != OK) {
+    LOG(ERROR) << "AiaRequest::FetchCallback got error " << error;

[eroman, 2016/06/03 16:28:52]

Please add a TODO here. Rather than swallowing the error, we will want to
propagate this through.

[mattm, 2016/06/03 21:27:26]

Done.

+  } else {
+    // RFC 5280 section 4.2.2.1:
+    //
+    //    Conforming applications that support HTTP or FTP for accessing
+    //    certificates MUST be able to accept individual DER encoded
+    //    certificates and SHOULD be able to accept "certs-only" CMS messages.
+    //
+    // TODO(mattm): Is supporting CMS message format important?
+    if (!ParsedCertificate::CreateAndAddToVector(
+            fetched_bytes.data(), fetched_bytes.size(),
+            ParsedCertificate::DataSource::INTERNAL_COPY, &results_)) {

[eroman, 2016/06/03 16:28:52]

Can you add a TODO to avoid copying the bytes?

(We can change the CertNetFetcher interface to pass ownership of the bytes
instead, and also enable passing into ParsedCertificate).

[mattm, 2016/06/03 21:27:26]

Done.

+      LOG(ERROR) << "Error parsing AIA data";

[eroman, 2016/06/03 16:28:52]

Also add a TODO here about the error propagation.

[mattm, 2016/06/03 21:27:26]

Done.

+    }
+  }
+  // If the client is waiting for results, need to run callback if:
+  //  * Some are available now.
+  //  * The last fetch finished, even with no results. (Client needs to know to
+  //    stop waiting.)
+  if (client_waiting_for_callback &&
+      (current_result_ < results_.size() || pending_requests_ == 0)) {
+    issuers_callback_.Run(this);
+  }
+}
+
+}  // namespace net