Skip to content

GitLab

T
tangled
Commit b4ac4494 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot
Browse files
Options

[WebLayer] Use ChromeSSLHostStateDelegate 

With this CL, WebLayer now shares the logic of when to allow users to
proceed past security interstitials with Chrome rather than using its
previous custom implementation. I verified manually that proceeding
past an SSL interstitial still works as expected in WebLayer on Android
and Linux. There are potential behavioral differences, as the logic of
the previous custom implementation was simpler than that of
ChromeSSLHostStateDelegate. However, any such behavioral differences
should simply be bringing WebLayer's behavior in line with Chrome's
behavior here, and hence should be a Good Thing from a security UX
POV.

This CL also opens the possibility of tracking metrics for recurrent
errors in WebLayer, as ChromeSSLHostStateDelegate exposes the
necessary functionality. However, this CL does not make any changes
there. Making those changes would be a matter of porting the
//chrome-level callsites of
ChromeSSLHostStateDelegate::HasSeenRecurrentErrors() to //weblayer
(or figuring out how to share them).

Bug: 1030692
Change-Id: I21fcc417bbbaf7574fabbf27d64ebf07dbceae9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2064393
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarCarlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743897}
parent e4827359
Show whitespace changes
Inline Side-by-side
Showing with 2 additions and 196 deletions
weblayer/BUILD.gn
View file @ b4ac4494
......@@ -137,8 +137,6 @@ jumbo_static_library("weblayer_lib") {
"browser/profile_impl.h",
"browser/ssl_error_controller_client.cc",
"browser/ssl_error_controller_client.h",
"browser/ssl_host_state_delegate_impl.cc",
"browser/ssl_host_state_delegate_impl.h",
"browser/system_network_context_manager.cc",
"browser/system_network_context_manager.h",
"browser/tab_impl.cc",
......
weblayer/browser/browser_context_impl.cc
View file @ b4ac4494
......@@ -18,6 +18,7 @@
#include "content/public/browser/device_service.h"
#include "content/public/browser/download_request_utils.h"
#include "content/public/browser/resource_context.h"
#include "weblayer/browser/chrome_ssl_host_state_delegate_factory.h"
#include "weblayer/browser/fake_permission_controller_delegate.h"
#include "weblayer/public/common/switches.h"
......@@ -140,7 +141,7 @@ BrowserContextImpl::GetStorageNotificationService() {
}
content::SSLHostStateDelegate* BrowserContextImpl::GetSSLHostStateDelegate() {
return &ssl_host_state_delegate_;
return ChromeSSLHostStateDelegateFactory::GetForBrowserContext(this);
}
content::PermissionControllerDelegate*
......
weblayer/browser/browser_context_impl.h
View file @ b4ac4494
......@@ -10,7 +10,6 @@
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "weblayer/browser/download_manager_delegate_impl.h"
#include "weblayer/browser/ssl_host_state_delegate_impl.h"
#include "weblayer/public/profile.h"
namespace user_prefs {
......@@ -81,7 +80,6 @@ class BrowserContextImpl : public content::BrowserContext {
std::unique_ptr<ResourceContextImpl, content::BrowserThread::DeleteOnIOThread>
resource_context_;
DownloadManagerDelegateImpl download_delegate_;
SSLHostStateDelegateImpl ssl_host_state_delegate_;
std::unique_ptr<PrefService> user_pref_service_;
std::unique_ptr<content::PermissionControllerDelegate>
permission_controller_delegate_;
......
weblayer/browser/ssl_host_state_delegate_impl.cc deleted 100644 → 0
View file @ e4827359
// Copyright 2019 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 "weblayer/browser/ssl_host_state_delegate_impl.h"
#include "base/callback.h"
#include "net/base/hash_value.h"
using content::SSLHostStateDelegate;
namespace weblayer {
namespace internal {
CertPolicy::CertPolicy() = default;
CertPolicy::~CertPolicy() = default;
// For an allowance, we consider a given |cert| to be a match to a saved
// allowed cert if the |error| is an exact match to or subset of the errors
// in the saved CertStatus.
bool CertPolicy::Check(const net::X509Certificate& cert, int error) const {
net::SHA256HashValue fingerprint = cert.CalculateChainFingerprint256();
auto allowed_iter = allowed_.find(fingerprint);
return ((allowed_iter != allowed_.end()) && (allowed_iter->second & error) &&
((allowed_iter->second & error) == error));
}
void CertPolicy::Allow(const net::X509Certificate& cert, int error) {
// If this same cert had already been saved with a different error status,
// this will replace it with the new error status.
net::SHA256HashValue fingerprint = cert.CalculateChainFingerprint256();
allowed_[fingerprint] = error;
}
} // namespace internal
SSLHostStateDelegateImpl::SSLHostStateDelegateImpl() = default;
SSLHostStateDelegateImpl::~SSLHostStateDelegateImpl() = default;
void SSLHostStateDelegateImpl::HostRanInsecureContent(
const std::string& host,
int child_id,
InsecureContentType content_type) {
// Intentional no-op.
}
bool SSLHostStateDelegateImpl::DidHostRunInsecureContent(
const std::string& host,
int child_id,
InsecureContentType content_type) {
// Intentional no-op.
return false;
}
void SSLHostStateDelegateImpl::AllowCert(const std::string& host,
const net::X509Certificate& cert,
int error,
content::WebContents* web_contents) {
cert_policy_for_host_[host].Allow(cert, error);
}
void SSLHostStateDelegateImpl::Clear(
base::RepeatingCallback<bool(const std::string&)> host_filter) {
if (host_filter.is_null()) {
cert_policy_for_host_.clear();
return;
}
for (auto it = cert_policy_for_host_.begin();
it != cert_policy_for_host_.end();) {
auto next_it = std::next(it);
if (host_filter.Run(it->first))
cert_policy_for_host_.erase(it);
it = next_it;
}
}
SSLHostStateDelegate::CertJudgment SSLHostStateDelegateImpl::QueryPolicy(
const std::string& host,
const net::X509Certificate& cert,
int error,
content::WebContents* web_contents) {
return cert_policy_for_host_[host].Check(cert, error)
? SSLHostStateDelegate::ALLOWED
: SSLHostStateDelegate::DENIED;
}
void SSLHostStateDelegateImpl::RevokeUserAllowExceptions(
const std::string& host) {
cert_policy_for_host_.erase(host);
}
bool SSLHostStateDelegateImpl::HasAllowException(
const std::string& host,
content::WebContents* web_contents) {
auto policy_iterator = cert_policy_for_host_.find(host);
return policy_iterator != cert_policy_for_host_.end() &&
policy_iterator->second.HasAllowException();
}
} // namespace weblayer
weblayer/browser/ssl_host_state_delegate_impl.h deleted 100644 → 0
View file @ e4827359
// Copyright 2019 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 WEBLAYER_BROWSER_SSL_HOST_STATE_DELEGATE_IMPL_H_
#define WEBLAYER_BROWSER_SSL_HOST_STATE_DELEGATE_IMPL_H_
#include <map>
#include <string>
#include "base/macros.h"
#include "content/public/browser/ssl_host_state_delegate.h"
#include "net/base/hash_value.h"
#include "net/cert/x509_certificate.h"
namespace weblayer {
namespace internal {
// This class maintains the policy for storing actions on certificate errors.
class CertPolicy {
public:
CertPolicy();
~CertPolicy();
// Returns true if the user has decided to proceed through the ssl error
// before. For a certificate to be allowed, it must not have any
// *additional* errors from when it was allowed.
bool Check(const net::X509Certificate& cert, int error) const;
// Causes the policy to allow this certificate for a given |error|. And
// remember the user's choice.
void Allow(const net::X509Certificate& cert, int error);
// Returns true if and only if there exists a user allow exception for some
// certificate.
bool HasAllowException() const { return allowed_.size() > 0; }
private:
// The set of fingerprints of allowed certificates.
std::map<net::SHA256HashValue, int> allowed_;
};
} // namespace internal
// This class is a copy of AwSSLHostStateDelegate. It saves cert decisions in
// memory, and doesn't perpetuate across application restarts.
class SSLHostStateDelegateImpl : public content::SSLHostStateDelegate {
public:
SSLHostStateDelegateImpl();
~SSLHostStateDelegateImpl() override;
// Records that |cert| is permitted to be used for |host| in the future, for
// a specified |error| type.
void AllowCert(const std::string& host,
const net::X509Certificate& cert,
int error,
content::WebContents* web_contents) override;
void Clear(
base::RepeatingCallback<bool(const std::string&)> host_filter) override;
// content::SSLHostStateDelegate:
content::SSLHostStateDelegate::CertJudgment QueryPolicy(
const std::string& host,
const net::X509Certificate& cert,
int error,
content::WebContents* web_contents) override;
void HostRanInsecureContent(const std::string& host,
int child_id,
InsecureContentType content_type) override;
bool DidHostRunInsecureContent(const std::string& host,
int child_id,
InsecureContentType content_type) override;
void RevokeUserAllowExceptions(const std::string& host) override;
bool HasAllowException(const std::string& host,
content::WebContents* web_contents) override;
private:
// Certificate policies for each host.
std::map<std::string, internal::CertPolicy> cert_policy_for_host_;
DISALLOW_COPY_AND_ASSIGN(SSLHostStateDelegateImpl);
};
} // namespace weblayer
#endif // WEBLAYER_BROWSER_SSL_HOST_STATE_DELEGATE_IMPL_H_
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment