Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(422)

Unified Diff: net/cert/internal/cert_issuer_source_aia.cc

Issue 2036033002: Add CertIssuerSourceAia: authorityInfoAccess fetching for CertPathBuilder. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@cert-parsing-path-building
Patch Set: review changes Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..ebcfdb88d24d04246fab0a878e871ef992e7917f
--- /dev/null
+++ b/net/cert/internal/cert_issuer_source_aia.cc
@@ -0,0 +1,144 @@
+// 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;
+const int kMaxFetchesPerCert = 5;
+
+} // 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);
+ if (url.is_valid()) {
+ // TODO(mattm): do the kMaxFetchesPerCert check only on the number of
+ // supported URL schemes, not all the URLs.
+ if (urls.size() < kMaxFetchesPerCert) {
+ urls.push_back(url);
+ } else {
+ // TODO(mattm): propagate error info.
+ LOG(ERROR) << "kMaxFetchesPerCert exceeded, skipping";
+ }
+ } else {
+ // TODO(mattm): propagate error info.
+ LOG(ERROR) << "invalid AIA URL: " << uri;
+ }
+ }
+ if (urls.empty())
+ return;
+
+ std::unique_ptr<AiaRequest> aia_request(new AiaRequest(issuers_callback));
+
+ for (const auto& url : urls) {
+ // TODO(mattm): add synchronous failure mode to FetchCaIssuers interface so
+ // that this doesn't need to wait for async callback just to tell that an
+ // URL has an unsupported scheme?
+ 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));
eroman 2016/06/04 01:08:47 Should this DCHECK that cert_fetcher_request != nu
mattm 2016/06/04 03:24:08 Done.
+ pending_requests_++;
+}
+
+void CertIssuerSourceAia::AiaRequest::FetchCallback(
eroman 2016/06/04 01:08:47 optional nit: Might I suggest OnFetchCompleted() o
mattm 2016/06/04 03:24:08 Done.
+ Error error,
+ const std::vector<uint8_t>& fetched_bytes) {
+ DCHECK(pending_requests_ > 0);
eroman 2016/06/04 01:08:47 nit: DCHECK_GT
mattm 2016/06/04 03:24:08 Done.
+ pending_requests_--;
+ bool client_waiting_for_callback = current_result_ >= results_.size();
+ if (error != OK) {
+ // TODO(mattm): propagate error info.
+ LOG(ERROR) << "AiaRequest::FetchCallback got error " << error;
+ } 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?
eroman 2016/06/04 01:08:47 My expectation is no, but TODO sgtm until we can d
mattm 2016/06/04 03:24:08 Acknowledged.
+ //
+ // TODO(mattm): Avoid copying bytes. Change the CertNetFetcher and
+ // ParsedCertificate interface to allow passing through ownership of the
+ // bytes.
+ if (!ParsedCertificate::CreateAndAddToVector(
+ fetched_bytes.data(), fetched_bytes.size(),
+ ParsedCertificate::DataSource::INTERNAL_COPY, &results_)) {
+ // TODO(mattm): propagate error info.
+ LOG(ERROR) << "Error parsing AIA data";
+ }
+ }
+ // 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)) {
eroman 2016/06/04 01:08:47 I suggest extracting this to a helper. Maybe somet
mattm 2016/06/04 03:24:08 Done.
+ issuers_callback_.Run(this);
+ }
+}
+
+} // namespace net

Powered by Google App Engine
This is Rietveld 408576698