From e66e7ff038eacc0c31ef442987c2bc433e8dd5cc Mon Sep 17 00:00:00 2001 From: fahl Date: Thu, 23 Apr 2015 12:38:30 -0700 Subject: [PATCH] Add enterprise policy for SSL error overriding This new feature allows enterprises to set a policy such that users will not be allowed to override any SSL errors. BUG=440949 Review URL: https://codereview.chromium.org/1060833003 Cr-Commit-Position: refs/heads/master@{#326607} --- .../security_interstitial_page_test_utils.cc | 5 +- .../security_interstitial_page_test_utils.h | 5 +- .../configuration_policy_handler_list_factory.cc | 3 + chrome/browser/policy/policy_browsertest.cc | 67 ++++++++++++++++++++++ chrome/browser/profiles/profile.cc | 4 ++ chrome/browser/ssl/ssl_blocking_page.cc | 14 +++-- chrome/browser/ssl/ssl_blocking_page.h | 6 +- chrome/browser/ssl/ssl_error_handler.cc | 16 ++++-- chrome/common/pref_names.cc | 4 ++ chrome/common/pref_names.h | 1 + chrome/test/data/policy/policy_test_cases.json | 10 +++- components/policy/resources/policy_templates.json | 17 +++++- tools/metrics/histograms/histograms.xml | 1 + 13 files changed, 135 insertions(+), 18 deletions(-) diff --git a/chrome/browser/interstitials/security_interstitial_page_test_utils.cc b/chrome/browser/interstitials/security_interstitial_page_test_utils.cc index 6e41b1d1c77d..5c224c3e27c7 100644 --- a/chrome/browser/interstitials/security_interstitial_page_test_utils.cc +++ b/chrome/browser/interstitials/security_interstitial_page_test_utils.cc @@ -21,8 +21,9 @@ namespace chrome_browser_interstitials { -bool IsInterstitialDisplayingText(content::InterstitialPage* interstitial, - const std::string& text) { +bool IsInterstitialDisplayingText( + const content::InterstitialPage* const interstitial, + const std::string& text) { // It's valid for |text| to contain "\'", but simply look for "'" instead // since this function is used for searching host names and a predefined // string. diff --git a/chrome/browser/interstitials/security_interstitial_page_test_utils.h b/chrome/browser/interstitials/security_interstitial_page_test_utils.h index 34c613a7be87..65a82f366ddd 100644 --- a/chrome/browser/interstitials/security_interstitial_page_test_utils.h +++ b/chrome/browser/interstitials/security_interstitial_page_test_utils.h @@ -20,8 +20,9 @@ class SecurityInterstitialPage; namespace chrome_browser_interstitials { -bool IsInterstitialDisplayingText(content::InterstitialPage* interstitial, - const std::string& text); +bool IsInterstitialDisplayingText( + const content::InterstitialPage* const interstitial, + const std::string& text); // This class is used for testing the display of IDN names in security // interstitials. diff --git a/chrome/browser/policy/configuration_policy_handler_list_factory.cc b/chrome/browser/policy/configuration_policy_handler_list_factory.cc index 0991c67abef8..da1fba23dab7 100644 --- a/chrome/browser/policy/configuration_policy_handler_list_factory.cc +++ b/chrome/browser/policy/configuration_policy_handler_list_factory.cc @@ -325,6 +325,9 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = { { key::kSafeBrowsingExtendedReportingOptInAllowed, prefs::kSafeBrowsingExtendedReportingOptInAllowed, base::Value::TYPE_BOOLEAN }, + { key::kSSLErrorOverrideAllowed, + prefs::kSSLErrorOverrideAllowed, + base::Value::TYPE_BOOLEAN }, #if defined(ENABLE_SPELLCHECK) { key::kSpellCheckServiceEnabled, diff --git a/chrome/browser/policy/policy_browsertest.cc b/chrome/browser/policy/policy_browsertest.cc index ceb95e014a86..6fdb0005bd78 100644 --- a/chrome/browser/policy/policy_browsertest.cc +++ b/chrome/browser/policy/policy_browsertest.cc @@ -45,6 +45,7 @@ #include "chrome/browser/extensions/updater/extension_updater.h" #include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/interstitials/security_interstitial_page.h" +#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h" #include "chrome/browser/media/media_capture_devices_dispatcher.h" #include "chrome/browser/media/media_stream_devices_controller.h" #include "chrome/browser/metrics/variations/variations_service.h" @@ -3620,6 +3621,72 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, SafeBrowsingExtendedReportingOptInAllowed) { EXPECT_EQ(SecurityInterstitialPage::CMD_TEXT_NOT_FOUND, result); } +// Test that when SSL error overriding is allowed by policy (default), the +// proceed link appears on SSL blocking pages. +IN_PROC_BROWSER_TEST_F(PolicyTest, SSLErrorOverridingAllowed) { + net::SpawnedTestServer https_server_expired( + net::SpawnedTestServer::TYPE_HTTPS, + net::SpawnedTestServer::SSLOptions( + net::SpawnedTestServer::SSLOptions::CERT_EXPIRED), + base::FilePath(FILE_PATH_LITERAL("chrome/test/data"))); + ASSERT_TRUE(https_server_expired.Start()); + + const PrefService* const prefs = browser()->profile()->GetPrefs(); + + // Policy should allow overriding by default. + EXPECT_TRUE(prefs->GetBoolean(prefs::kSSLErrorOverrideAllowed)); + + // Policy allows overriding - navigate to an SSL error page and expect the + // proceed link. + ui_test_utils::NavigateToURL(browser(), https_server_expired.GetURL("/")); + + const content::InterstitialPage* const interstitial = + content::InterstitialPage::GetInterstitialPage( + browser()->tab_strip_model()->GetActiveWebContents()); + ASSERT_TRUE(interstitial); + EXPECT_TRUE(content::WaitForRenderFrameReady(interstitial->GetMainFrame())); + + // The interstitial should display the proceed link. + EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( + interstitial, "proceed-link")); +} + +// Test that when SSL error overriding is disallowed by policy, the +// proceed link does not appear on SSL blocking pages. +IN_PROC_BROWSER_TEST_F(PolicyTest, SSLErrorOverridingDisallowed) { + net::SpawnedTestServer https_server_expired( + net::SpawnedTestServer::TYPE_HTTPS, + net::SpawnedTestServer::SSLOptions( + net::SpawnedTestServer::SSLOptions::CERT_EXPIRED), + base::FilePath(FILE_PATH_LITERAL("chrome/test/data"))); + ASSERT_TRUE(https_server_expired.Start()); + + const PrefService* const prefs = browser()->profile()->GetPrefs(); + EXPECT_TRUE(prefs->GetBoolean(prefs::kSSLErrorOverrideAllowed)); + + // Disallowing the proceed link by setting the policy to |false|. + PolicyMap policies; + policies.Set(key::kSSLErrorOverrideAllowed, POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, new base::FundamentalValue(false), NULL); + UpdateProviderPolicy(policies); + + // Policy should not allow overriding anymore. + EXPECT_FALSE(prefs->GetBoolean(prefs::kSSLErrorOverrideAllowed)); + + // Policy disallows overriding - navigate to an SSL error page and expect no + // proceed link. + ui_test_utils::NavigateToURL(browser(), https_server_expired.GetURL("/")); + const content::InterstitialPage* const interstitial = + content::InterstitialPage::GetInterstitialPage( + browser()->tab_strip_model()->GetActiveWebContents()); + ASSERT_TRUE(interstitial); + EXPECT_TRUE(content::WaitForRenderFrameReady(interstitial->GetMainFrame())); + + // The interstitial should not display the proceed link. + EXPECT_FALSE(chrome_browser_interstitials::IsInterstitialDisplayingText( + interstitial, "proceed-link")); +} + #if !defined(OS_CHROMEOS) // Similar to PolicyTest but sets the proper policy before the browser is // started. diff --git a/chrome/browser/profiles/profile.cc b/chrome/browser/profiles/profile.cc index 4f65b32b2477..c28a980093f6 100644 --- a/chrome/browser/profiles/profile.cc +++ b/chrome/browser/profiles/profile.cc @@ -104,6 +104,10 @@ void Profile::RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { prefs::kSafeBrowsingProceedAnywayDisabled, false, user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); + registry->RegisterBooleanPref( + prefs::kSSLErrorOverrideAllowed, + true, + user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); registry->RegisterDictionaryPref( prefs::kSafeBrowsingIncidentsSent, user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); diff --git a/chrome/browser/ssl/ssl_blocking_page.cc b/chrome/browser/ssl/ssl_blocking_page.cc index 717d13847171..529cbcab3b6d 100644 --- a/chrome/browser/ssl/ssl_blocking_page.cc +++ b/chrome/browser/ssl/ssl_blocking_page.cc @@ -248,7 +248,9 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents, callback_(callback), cert_error_(cert_error), ssl_info_(ssl_info), - overridable_(IsOptionsOverridable(options_mask)), + overridable_(IsOverridable( + options_mask, + Profile::FromBrowserContext(web_contents->GetBrowserContext()))), danger_overridable_(true), strict_enforcement_((options_mask & STRICT_ENFORCEMENT) != 0), expired_but_previously_allowed_( @@ -697,7 +699,11 @@ bool SSLBlockingPage::ShouldReportCertificateError() { } // static -bool SSLBlockingPage::IsOptionsOverridable(int options_mask) { - return (options_mask & SSLBlockingPage::OVERRIDABLE) && - !(options_mask & SSLBlockingPage::STRICT_ENFORCEMENT); +bool SSLBlockingPage::IsOverridable(int options_mask, + const Profile* const profile) { + const bool is_overridable = + (options_mask & SSLBlockingPage::OVERRIDABLE) && + !(options_mask & SSLBlockingPage::STRICT_ENFORCEMENT) && + profile->GetPrefs()->GetBoolean(prefs::kSSLErrorOverrideAllowed); + return is_overridable; } diff --git a/chrome/browser/ssl/ssl_blocking_page.h b/chrome/browser/ssl/ssl_blocking_page.h index ce694f801a0a..36026b50a8d4 100644 --- a/chrome/browser/ssl/ssl_blocking_page.h +++ b/chrome/browser/ssl/ssl_blocking_page.h @@ -13,6 +13,7 @@ #include "base/task/cancelable_task_tracker.h" #include "base/time/time.h" #include "chrome/browser/interstitials/security_interstitial_page.h" +#include "chrome/browser/profiles/profile.h" #include "chrome/browser/ssl/ssl_cert_reporter.h" #include "net/ssl/ssl_info.h" #include "url/gurl.h" @@ -71,8 +72,9 @@ class SSLBlockingPage : public SecurityInterstitialPage { // InterstitialPageDelegate method: InterstitialPageDelegate::TypeID GetTypeForTesting() const override; - // Returns true if |options_mask| refers to an overridable SSL error. - static bool IsOptionsOverridable(int options_mask); + // Returns true if |options_mask| refers to an overridable SSL error and + // if SSL error overriding is allowed by policy. + static bool IsOverridable(int options_mask, const Profile* const profile); void SetSSLCertReporterForTesting( scoped_ptr ssl_cert_reporter); diff --git a/chrome/browser/ssl/ssl_error_handler.cc b/chrome/browser/ssl/ssl_error_handler.cc index 25e4b6aef02c..72da17f2f0ba 100644 --- a/chrome/browser/ssl/ssl_error_handler.cc +++ b/chrome/browser/ssl/ssl_error_handler.cc @@ -187,9 +187,11 @@ void SSLErrorHandler::CheckForCaptivePortal() { void SSLErrorHandler::ShowCaptivePortalInterstitial(const GURL& landing_url) { #if defined(ENABLE_CAPTIVE_PORTAL_DETECTION) // Show captive portal blocking page. The interstitial owns the blocking page. - RecordUMA(SSLBlockingPage::IsOptionsOverridable(options_mask_) ? - SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE : - SHOW_CAPTIVE_PORTAL_INTERSTITIAL_NONOVERRIDABLE); + const Profile* const profile = + Profile::FromBrowserContext(web_contents_->GetBrowserContext()); + RecordUMA(SSLBlockingPage::IsOverridable(options_mask_, profile) + ? SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE + : SHOW_CAPTIVE_PORTAL_INTERSTITIAL_NONOVERRIDABLE); (new CaptivePortalBlockingPage(web_contents_, request_url_, landing_url, callback_))->Show(); // Once an interstitial is displayed, no need to keep the handler around. @@ -202,9 +204,11 @@ void SSLErrorHandler::ShowCaptivePortalInterstitial(const GURL& landing_url) { void SSLErrorHandler::ShowSSLInterstitial() { // Show SSL blocking page. The interstitial owns the blocking page. - RecordUMA(SSLBlockingPage::IsOptionsOverridable(options_mask_) ? - SHOW_SSL_INTERSTITIAL_OVERRIDABLE : - SHOW_SSL_INTERSTITIAL_NONOVERRIDABLE); + const Profile* const profile = + Profile::FromBrowserContext(web_contents_->GetBrowserContext()); + RecordUMA(SSLBlockingPage::IsOverridable(options_mask_, profile) + ? SHOW_SSL_INTERSTITIAL_OVERRIDABLE + : SHOW_SSL_INTERSTITIAL_NONOVERRIDABLE); (new SSLBlockingPage(web_contents_, cert_error_, ssl_info_, request_url_, options_mask_, base::Time::NowFromSystemTime(), ssl_cert_reporter_.Pass(), callback_))->Show(); diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index eca2d7036426..0e93b1f44fad 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -350,6 +350,10 @@ const char kSafeBrowsingIncidentsSent[] = "safebrowsing.incidents_sent"; const char kSafeBrowsingExtendedReportingOptInAllowed[] = "safebrowsing.extended_reporting_opt_in_allowed"; +// Boolean that is true when the SSL interstitial should allow users to +// proceed anyway. Otherwise, proceeding is not possible. +const char kSSLErrorOverrideAllowed[] = "ssl.error_override_allowed"; + // Enum that specifies whether Incognito mode is: // 0 - Enabled. Default behaviour. Default mode is available on demand. // 1 - Disabled. Used cannot browse pages in Incognito mode. diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 634421147b21..a6e36baafd1b 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -145,6 +145,7 @@ extern const char kSafeBrowsingExtendedReportingEnabled[]; extern const char kSafeBrowsingProceedAnywayDisabled[]; extern const char kSafeBrowsingIncidentsSent[]; extern const char kSafeBrowsingExtendedReportingOptInAllowed[]; +extern const char kSSLErrorOverrideAllowed[]; extern const char kIncognitoModeAvailability[]; extern const char kSearchSuggestEnabled[]; #if defined(OS_ANDROID) diff --git a/chrome/test/data/policy/policy_test_cases.json b/chrome/test/data/policy/policy_test_cases.json index cb38eb4ff43c..f9344ce7725c 100644 --- a/chrome/test/data/policy/policy_test_cases.json +++ b/chrome/test/data/policy/policy_test_cases.json @@ -2221,7 +2221,7 @@ "os": ["chromeos"], "can_be_recommended": true }, - + "ForceMaximizeOnFirstRun": { "os": ["chromeos"], "test_policy": { "ForceMaximizeOnFirstRun": true }, @@ -2230,6 +2230,14 @@ ] }, + "SSLErrorOverrideAllowed": { + "os": ["win", "linux", "mac", "chromeos"], + "test_policy": { "SSLErrorOverrideAllowed": true }, + "pref_mappings": [ + { "pref": "ssl.error_override_allowed" } + ] + }, + "----- Chrome OS device policies ---------------------------------------": {}, "DevicePolicyRefreshRate": { diff --git a/components/policy/resources/policy_templates.json b/components/policy/resources/policy_templates.json index 82b162c71aec..eb45ccb4fab0 100644 --- a/components/policy/resources/policy_templates.json +++ b/components/policy/resources/policy_templates.json @@ -123,7 +123,7 @@ # persistent IDs for all fields (but not for groups!) are needed. These are # specified by the 'id' keys of each policy. NEVER CHANGE EXISTING IDs, # because doing so would break the deployed wire format! -# For your editing convenience: highest ID currently used: 299 +# For your editing convenience: highest ID currently used: 300 # # Placeholders: # The following placeholder strings are automatically substituted: @@ -7335,6 +7335,21 @@ 'desc': '''If this policy is set to true, $1Google Chrome will unconditionally maximize the the first window shown on first run. If this policy is set to false or not configured, a heuristic will decide whether to maximize the first window shown, based on the screen size.''', }, + { + 'name': 'SSLErrorOverrideAllowed', + 'type': 'main', + 'schema': { 'type': 'boolean' }, + 'supported_on': ['chrome.*:44-', 'chrome_os:44-', 'android:44-'], + 'features': { + 'dynamic_refresh': True, + 'per_profile': True, + }, + 'example_value': True, + 'id': 300, + 'caption': '''Allow proceeding from the SSL warning page''', + 'desc': '''Chrome shows a warning page when users navigate to sites that have SSL errors. By default or when this policy is set to true, users are allowed to click through these warning pages. + Setting this policy to false disallows users to click through any warning page.''', + }, ], 'messages': { # Messages that are not associated to any policies. diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index f3d7bf01c851..2143fbfa0ae2 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -49953,6 +49953,7 @@ Therefore, the affected-histogram name has to have at least one dot in it. + -- 2.11.4.GIT