Commit 6473c592 authored by estark's avatar estark Committed by Commit bot

Add interstitial info to certificate reports

This change adds additional information about the SSL interstitial (what
type it was, whether the user clicked through, and whether it was
overridable) to SSL certificate reports.

BUG=462713,461588

Review URL: https://codereview.chromium.org/1076273002

Cr-Commit-Position: refs/heads/master@{#330605}
parent a2295b7f
...@@ -20,6 +20,27 @@ syntax = "proto2"; ...@@ -20,6 +20,27 @@ syntax = "proto2";
option optimize_for = LITE_RUNTIME; option optimize_for = LITE_RUNTIME;
// Protocol types // Protocol types
message CertLoggerInterstitialInfo {
// The different reasons that an SSL warning interstitial could be shown to
// a user.
enum InterstitialReason {
// A standard SSL interstitial
INTERSTITIAL_SSL = 1;
// An interstitial alerting the user that they are in a captive portal
INTERSTITIAL_CAPTIVE_PORTAL = 2;
// An interstitial telling the user to update their system clock
INTERSTITIAL_CLOCK = 3;
}
// The type of interstitial that was shown
optional InterstitialReason interstitial_reason = 1;
// True if the user clicked through to the offending website
optional bool user_proceeded = 2;
// True if the user was shown an option to click through
optional bool overridable = 3;
}
message CertLoggerRequest { message CertLoggerRequest {
// The hostname being accessed (required as the cert could be valid for // The hostname being accessed (required as the cert could be valid for
// multiple hosts, e.g. a wildcard or a SubjectAltName. // multiple hosts, e.g. a wildcard or a SubjectAltName.
...@@ -56,4 +77,8 @@ message CertLoggerRequest { ...@@ -56,4 +77,8 @@ message CertLoggerRequest {
// Certificate errors encountered (if any) when validating this // Certificate errors encountered (if any) when validating this
// certificate chain. // certificate chain.
repeated CertError cert_error = 6; repeated CertError cert_error = 6;
// Information about the interstitial that was shown to the user for
// this certificate error.
optional CertLoggerInterstitialInfo interstitial_info = 7;
}; };
\ No newline at end of file
...@@ -87,6 +87,32 @@ bool CertificateErrorReport::Serialize(std::string* output) const { ...@@ -87,6 +87,32 @@ bool CertificateErrorReport::Serialize(std::string* output) const {
return cert_report_->SerializeToString(output); return cert_report_->SerializeToString(output);
} }
void CertificateErrorReport::SetInterstitialInfo(
const InterstitialReason& interstitial_reason,
const ProceedDecision& proceed_decision,
const Overridable& overridable) {
CertLoggerInterstitialInfo* interstitial_info =
cert_report_->mutable_interstitial_info();
switch (interstitial_reason) {
case INTERSTITIAL_SSL:
interstitial_info->set_interstitial_reason(
CertLoggerInterstitialInfo::INTERSTITIAL_SSL);
break;
case INTERSTITIAL_CAPTIVE_PORTAL:
interstitial_info->set_interstitial_reason(
CertLoggerInterstitialInfo::INTERSTITIAL_CAPTIVE_PORTAL);
break;
case INTERSTITIAL_CLOCK:
interstitial_info->set_interstitial_reason(
CertLoggerInterstitialInfo::INTERSTITIAL_CLOCK);
break;
}
interstitial_info->set_user_proceeded(proceed_decision == USER_PROCEEDED);
interstitial_info->set_overridable(overridable == INTERSTITIAL_OVERRIDABLE);
}
const std::string& CertificateErrorReport::hostname() const { const std::string& CertificateErrorReport::hostname() const {
return cert_report_->hostname(); return cert_report_->hostname();
} }
...@@ -20,6 +20,22 @@ class CertLoggerRequest; ...@@ -20,6 +20,22 @@ class CertLoggerRequest;
// chrome_browser_net::CertificateErrorReporter. // chrome_browser_net::CertificateErrorReporter.
class CertificateErrorReport { class CertificateErrorReport {
public: public:
// Describes the type of interstitial that the user was shown for the
// error that this report represents. Gets mapped to
// |CertLoggerInterstitialInfo::InterstitialReason|.
enum InterstitialReason {
INTERSTITIAL_SSL,
INTERSTITIAL_CAPTIVE_PORTAL,
INTERSTITIAL_CLOCK
};
// Whether the user clicked through the interstitial or not.
enum ProceedDecision { USER_PROCEEDED, USER_DID_NOT_PROCEED };
// Whether the user was shown an option to click through the
// interstitial.
enum Overridable { INTERSTITIAL_OVERRIDABLE, INTERSTITIAL_NOT_OVERRIDABLE };
// Constructs an empty report. // Constructs an empty report.
CertificateErrorReport(); CertificateErrorReport();
...@@ -41,6 +57,10 @@ class CertificateErrorReport { ...@@ -41,6 +57,10 @@ class CertificateErrorReport {
// successful and false otherwise. // successful and false otherwise.
bool Serialize(std::string* output) const; bool Serialize(std::string* output) const;
void SetInterstitialInfo(const InterstitialReason& interstitial_reason,
const ProceedDecision& proceed_decision,
const Overridable& overridable);
// Gets the hostname to which this report corresponds. // Gets the hostname to which this report corresponds.
const std::string& hostname() const; const std::string& hostname() const;
......
...@@ -80,6 +80,46 @@ TEST(CertificateErrorReportTest, SerializedReportAsProtobuf) { ...@@ -80,6 +80,46 @@ TEST(CertificateErrorReportTest, SerializedReportAsProtobuf) {
EXPECT_EQ(1u, reported_errors.count(kSecondReportedCertError)); EXPECT_EQ(1u, reported_errors.count(kSecondReportedCertError));
} }
TEST(CertificateErrorReportTest,
SerializedReportAsProtobufWithInterstitialInfo) {
SSLInfo ssl_info = GetTestSSLInfo();
std::string serialized_report;
CertificateErrorReport report(kDummyHostname, ssl_info);
const CertificateErrorReport::InterstitialReason interstitial_reason =
CertificateErrorReport::INTERSTITIAL_CLOCK;
const CertificateErrorReport::ProceedDecision proceeded =
CertificateErrorReport::USER_PROCEEDED;
const CertificateErrorReport::Overridable overridable =
CertificateErrorReport::INTERSTITIAL_OVERRIDABLE;
report.SetInterstitialInfo(interstitial_reason, proceeded, overridable);
report.Serialize(&serialized_report);
CertLoggerRequest deserialized_report;
ASSERT_TRUE(deserialized_report.ParseFromString(serialized_report));
EXPECT_EQ(kDummyHostname, deserialized_report.hostname());
EXPECT_EQ(GetPEMEncodedChain(), deserialized_report.cert_chain());
EXPECT_EQ(1, deserialized_report.pin().size());
EXPECT_EQ(kDummyFailureLog, deserialized_report.pin().Get(0));
EXPECT_EQ(CertLoggerInterstitialInfo::INTERSTITIAL_CLOCK,
deserialized_report.interstitial_info().interstitial_reason());
EXPECT_EQ(true, deserialized_report.interstitial_info().user_proceeded());
EXPECT_EQ(true, deserialized_report.interstitial_info().overridable());
std::set<CertLoggerRequest::CertError> reported_errors;
reported_errors.insert(static_cast<CertLoggerRequest::CertError>(
deserialized_report.cert_error().Get(0)));
reported_errors.insert(static_cast<CertLoggerRequest::CertError>(
deserialized_report.cert_error().Get(1)));
EXPECT_EQ(kNumCertErrors, reported_errors.size());
EXPECT_EQ(1u, reported_errors.count(kFirstReportedCertError));
EXPECT_EQ(1u, reported_errors.count(kSecondReportedCertError));
}
// Test that a serialized report can be parsed. // Test that a serialized report can be parsed.
TEST(CertificateErrorReportTest, ParseSerializedReport) { TEST(CertificateErrorReportTest, ParseSerializedReport) {
SSLInfo ssl_info = GetTestSSLInfo(); SSLInfo ssl_info = GetTestSSLInfo();
......
...@@ -588,7 +588,7 @@ void SSLBlockingPage::OnProceed() { ...@@ -588,7 +588,7 @@ void SSLBlockingPage::OnProceed() {
// Finish collecting information about invalid certificates, if the // Finish collecting information about invalid certificates, if the
// user opted in to. // user opted in to.
FinishCertCollection(); FinishCertCollection(CertificateErrorReport::USER_PROCEEDED);
RecordSSLExpirationPageEventState( RecordSSLExpirationPageEventState(
expired_but_previously_allowed_, true, overridable_); expired_but_previously_allowed_, true, overridable_);
...@@ -602,7 +602,7 @@ void SSLBlockingPage::OnDontProceed() { ...@@ -602,7 +602,7 @@ void SSLBlockingPage::OnDontProceed() {
// Finish collecting information about invalid certificates, if the // Finish collecting information about invalid certificates, if the
// user opted in to. // user opted in to.
FinishCertCollection(); FinishCertCollection(CertificateErrorReport::USER_DID_NOT_PROCEED);
RecordSSLExpirationPageEventState( RecordSSLExpirationPageEventState(
expired_but_previously_allowed_, false, overridable_); expired_but_previously_allowed_, false, overridable_);
...@@ -651,7 +651,8 @@ std::string SSLBlockingPage::GetSamplingEventName() const { ...@@ -651,7 +651,8 @@ std::string SSLBlockingPage::GetSamplingEventName() const {
return event_name; return event_name;
} }
void SSLBlockingPage::FinishCertCollection() { void SSLBlockingPage::FinishCertCollection(
CertificateErrorReport::ProceedDecision user_proceeded) {
if (!ShouldShowCertificateReporterCheckbox()) if (!ShouldShowCertificateReporterCheckbox())
return; return;
...@@ -668,6 +669,21 @@ void SSLBlockingPage::FinishCertCollection() { ...@@ -668,6 +669,21 @@ void SSLBlockingPage::FinishCertCollection() {
std::string serialized_report; std::string serialized_report;
CertificateErrorReport report(request_url().host(), ssl_info_); CertificateErrorReport report(request_url().host(), ssl_info_);
CertificateErrorReport::InterstitialReason report_interstitial_reason;
switch (interstitial_reason_) {
case SSL_REASON_SSL:
report_interstitial_reason = CertificateErrorReport::INTERSTITIAL_SSL;
break;
case SSL_REASON_BAD_CLOCK:
report_interstitial_reason = CertificateErrorReport::INTERSTITIAL_CLOCK;
break;
}
report.SetInterstitialInfo(
report_interstitial_reason, user_proceeded,
overridable_ ? CertificateErrorReport::INTERSTITIAL_OVERRIDABLE
: CertificateErrorReport::INTERSTITIAL_NOT_OVERRIDABLE);
if (!report.Serialize(&serialized_report)) { if (!report.Serialize(&serialized_report)) {
LOG(ERROR) << "Failed to serialize certificate report."; LOG(ERROR) << "Failed to serialize certificate report.";
return; return;
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/interstitials/security_interstitial_page.h" #include "chrome/browser/interstitials/security_interstitial_page.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ssl/certificate_error_report.h"
#include "chrome/browser/ssl/ssl_cert_reporter.h" #include "chrome/browser/ssl/ssl_cert_reporter.h"
#include "net/ssl/ssl_info.h" #include "net/ssl/ssl_info.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -109,8 +110,11 @@ class SSLBlockingPage : public SecurityInterstitialPage { ...@@ -109,8 +110,11 @@ class SSLBlockingPage : public SecurityInterstitialPage {
std::string GetUmaHistogramPrefix() const; std::string GetUmaHistogramPrefix() const;
std::string GetSamplingEventName() const; std::string GetSamplingEventName() const;
// Send a report about an invalid certificate to the server. // Send a report about an invalid certificate to the
void FinishCertCollection(); // server. |user_proceeded| indicates whether the user clicked through
// the interstitial or not, and will be included in the report.
void FinishCertCollection(
CertificateErrorReport::ProceedDecision user_proceeded);
// Check whether a checkbox should be shown on the page that allows // Check whether a checkbox should be shown on the page that allows
// the user to opt in to Safe Browsing extended reporting. // the user to opt in to Safe Browsing extended reporting.
......
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