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

Unified Diff: chrome/browser/ssl/ssl_error_handler.cc

Issue 2949003003: Implement a skeleton of the Superfish interstitial (Closed)
Patch Set: Created 3 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: chrome/browser/ssl/ssl_error_handler.cc
diff --git a/chrome/browser/ssl/ssl_error_handler.cc b/chrome/browser/ssl/ssl_error_handler.cc
index 5353a1c3ebfd0c039021a37358837ba3d765b824..d502eddbe4e5413a2285f2866b413d0a30cce39b 100644
--- a/chrome/browser/ssl/ssl_error_handler.cc
+++ b/chrome/browser/ssl/ssl_error_handler.cc
@@ -24,6 +24,7 @@
#include "chrome/browser/ssl/ssl_blocking_page.h"
#include "chrome/browser/ssl/ssl_cert_reporter.h"
#include "chrome/browser/ssl/ssl_error_assistant.pb.h"
+#include "chrome/browser/ssl/superfish_blocking_page.h"
#include "chrome/common/features.h"
#include "chrome/grit/browser_resources.h"
#include "components/network_time/network_time_tracker.h"
@@ -59,6 +60,9 @@ const base::Feature kCaptivePortalCertificateList{
const base::Feature kSSLCommonNameMismatchHandling{
"SSLCommonNameMismatchHandling", base::FEATURE_ENABLED_BY_DEFAULT};
+const base::Feature kSuperfishInterstitial{"SuperfishInterstitial",
+ base::FEATURE_DISABLED_BY_DEFAULT};
+
// Default delay in milliseconds before displaying the SSL interstitial.
// This can be changed in tests.
// - If there is a name mismatch and a suggested URL available result arrives
@@ -70,26 +74,34 @@ const int64_t kInterstitialDelayInMilliseconds = 3000;
const char kHistogram[] = "interstitial.ssl_error_handler";
-// Records an UMA histogram for whether the Superfish SPKI was present in the
-// certificate chain.
-void RecordSuperfishUMA(const net::HashValueVector& public_key_hashes) {
- // This is the SPKI hash of the well-known Superfish certificate at
- // https://pastebin.com/WcXv8QcG.
- const char kSuperfishSPKI[] =
- "sha256/S7jzW6HhJvjd4bDEIGJe2G3OYae92tveqauleP8TFF4=";
- net::HashValue superfish_spki_hash_value;
- if (!superfish_spki_hash_value.FromString(kSuperfishSPKI)) {
- NOTREACHED();
- }
- bool found_superfish = false;
- for (const auto& hash : public_key_hashes) {
- if (hash == superfish_spki_hash_value) {
- found_superfish = true;
- break;
+bool IsSuperfish(const scoped_refptr<net::X509Certificate>& cert) {
+ // This is the fingerprint of the well-known Superfish certificate at
+ // https://pastebin.com/WcXv8QcG. Superfish is identified by certificate
+ // fingerprint rather than SPKI because net::SSLInfo does not guarantee
+ // |public_key_hashes| (the SPKIs) to be populated if the certificate doesn't
+ // verify successfully. It so happens that Superfish uses the same certificate
+ // universally (not just the same public key), and calculating the fingerprint
+ // is more convenient here than calculating the SPKI.
+ const net::SHA256HashValue kSuperfishFingerprint{
+ {0xB6, 0xFE, 0x91, 0x51, 0x40, 0x2B, 0xAD, 0x1C, 0x06, 0xD7, 0xE6,
+ 0x6D, 0xB6, 0x7A, 0x26, 0xAA, 0x73, 0x56, 0xF2, 0xE6, 0xC6, 0x44,
+ 0xDB, 0xCF, 0x9F, 0x98, 0x96, 0x8F, 0xF6, 0x32, 0xE1, 0xB7}};
+ for (const net::X509Certificate::OSCertHandle& intermediate :
+ cert->GetIntermediateCertificates()) {
+ net::SHA256HashValue hash =
+ net::X509Certificate::CalculateFingerprint256(intermediate);
+ if (hash == kSuperfishFingerprint) {
+ return true;
}
}
+ return false;
+}
+
+// Records an UMA histogram for whether the Superfish certificate was present in
+// the certificate chain.
+void RecordSuperfishUMA(const scoped_refptr<net::X509Certificate>& cert) {
UMA_HISTOGRAM_BOOLEAN("interstitial.ssl_error_handler.superfish",
- found_superfish);
+ IsSuperfish(cert));
}
// Adds a message to console after navigation commits and then, deletes itself.
@@ -447,10 +459,20 @@ void SSLErrorHandlerDelegateImpl::ShowCaptivePortalInterstitial(
void SSLErrorHandlerDelegateImpl::ShowSSLInterstitial() {
// Show SSL blocking page. The interstitial owns the blocking page.
- (SSLBlockingPage::Create(web_contents_, cert_error_, ssl_info_, request_url_,
- options_mask_, base::Time::NowFromSystemTime(),
- std::move(ssl_cert_reporter_), callback_))
- ->Show();
+ if (base::FeatureList::IsEnabled(kSuperfishInterstitial) &&
+ IsSuperfish(ssl_info_.cert)) {
+ (SuperfishBlockingPage::Create(web_contents_, cert_error_, ssl_info_,
+ request_url_, options_mask_,
+ base::Time::NowFromSystemTime(),
+ std::move(ssl_cert_reporter_), callback_))
+ ->Show();
meacer 2017/06/22 19:06:18 nit: return early here, and remove the else to get
estark 2017/06/22 22:01:05 n/a
+ } else {
+ (SSLBlockingPage::Create(web_contents_, cert_error_, ssl_info_,
+ request_url_, options_mask_,
+ base::Time::NowFromSystemTime(),
+ std::move(ssl_cert_reporter_), callback_))
+ ->Show();
+ }
}
void SSLErrorHandlerDelegateImpl::ShowBadClockInterstitial(
@@ -563,7 +585,7 @@ SSLErrorHandler::~SSLErrorHandler() {
void SSLErrorHandler::StartHandlingError() {
RecordUMA(HANDLE_ALL);
- RecordSuperfishUMA(ssl_info_.public_key_hashes);
+ RecordSuperfishUMA(ssl_info_.cert);
if (ssl_errors::ErrorInfo::NetErrorToErrorType(cert_error_) ==
ssl_errors::ErrorInfo::CERT_DATE_INVALID) {

Powered by Google App Engine
This is Rietveld 408576698